B+ access to displayed RAM from code in private 12K at &Axxxx isn't working

ZornsLemma commented 3 years ago

Using the test program from stardot at https://stardot.org.uk/forums/viewtopic.php?p=158387#p158387, pressing SPACE doesn't seem to invert the screen in non-shadow mode 1, whereas it apparently does on a real B+ (there's a YouTube video linked a bit further down the thread).

This is with commit ed65091eb5e09ea6827ad5d5c8c77e6a833c85e2, which is current master.

(I just updated to the latest b-em since I was having some unrelated problems with the private 12K on an older version which are fixed in this commit, so I guess there's been some work on this at some point relatively recently. Incidentally the new b-em is very cool, the SAA5050 mode 7 font was initially a bit over-bright but I'm already getting used to it, and having SD card emulation for MMFS is great.)

SteveFosdick commented 3 years ago

I have pushed a change that seems to fix this, but looking at the code I think we may need a second change, which would another test case, i.e. instead of programming FE34 first (maybe via the OS) and then flipping FE30 to do it the other way round, set FE30 first and then try flpping FE34 but I am not sure if that can be made visual in quite the same way.

SteveFosdick commented 3 years ago

See: https://github.com/stardot/b-em/commit/607c7ff1cca0218cc737566ba46de046fb35f2bd

SteveFosdick commented 3 years ago

Then see https://github.com/stardot/b-em/commit/b672bf0ad441c3c286b17b4efbe604d4fcc6aecf for what I think is the other half of this fix, the one we need the test case for.

ZornsLemma commented 3 years ago

Thanks for taking a look at this so quickly! I can confirm the fix seems to work, but that's hardly a surprise.

I find this sort of thing very hard to think about - I have a lot of respect for the way you and other emulator authors are able to keep it all straight - so take the following with a pinch of salt, but for what it's worth:

Shouldn't write_acccon_bplus() do "RAMbank[0xA] = 0" in the else branch? Or equivalently unconditionally execute a copy of the "RAMbank[0xA] = ..." line from write_romsel()?
Have a go with TEST2 on the attached zipped ssd. I think the key idea is to fiddle with FE34 from code running within the &Axxx region of the private 12K. This shows difference in behaviour with 607c7ff1cca0218cc737566ba46de046fb35f2bd and b672bf0ad441c3c286b17b4efbe604d4fcc6aecf - it may help to run b-em at 10% speed. I think the fact that the red screen never changes at the top left also illustrates the problem I mentioned in the previous point.

I have no idea what this might do on real hardware; if you think this does address the points you've raised we could post it to stardot. Probably a good idea to tweak the delay loop to make it slower in that case, as on real hardware you can't run at 10% speed!

I hope I'm not missing the point; I'm happy to discuss this if you think it will help...

Cheers! b+12k-test-2.zip

SteveFosdick commented 3 years ago

I think you're right about the else. I have pushed that to the issue branch. I can't do any more testing right now as I need to go out but I will see what I can do over the weekend.

SteveFosdick commented 3 years ago

Looking at the code for test2 I think the correct version is that the top left of the screen changes in either mode (shadow and standard) because, with the top bit of ROMSEL (FE30) set, code running in A000-BFFF will write to the bank as the one displayed. It would be good to try it on a real B+.

Here's a version with a longer delay:

   10IF INKEY-256<>&FB THEN PRINT "Sorry, BBC B+ only!":END
   20DIM code% 256
   30FOR opt%=4 TO 7 STEP 3
   40P%=code%:O%=code%
   50[OPT opt%
   60.copy
   70LDA &F4
   80ORA #&80
   90STA &F4
  100STA &FE30
  110LDA #invert_low MOD 256:STA &70:LDA #invert_low DIV 256:STA &71
  120LDA #invert MOD 256:STA &72:LDA #invert DIV 256:STA &73
  130LDY #&7F
  140.copy_loop
  150LDA (&70),Y
  160STA (&72),Y
  170DEY
  180BPL copy_loop
  190LDA &F4
  200AND #&7F
  210STA &F4
  220STA &FE30
  230RTS
  240:
  250.call_invert
  260LDA &F4
  270ORA #&80
  280STA &F4
  290STA &FE30
  300JSR invert
  310LDA &F4
  320AND #&7F
  330STA &F4
  340STA &FE30
  350RTS
  360]
  370:
  380P%=&A000
  390invert_low=O%
  400[OPT opt%
  410.invert
  420LDX #&03
  430.delay_loop1
  440LDY #0
  450.delay_loop2
  460BIT&FF
  470BPL notesc
  480LDA #&7C
  490JMP &FFF4
  500.notesc
  510INC&71
  520BNE delay_loop2
  530DEY
  540BNE delay_loop2
  550DEX
  560BNE delay_loop2
  570LDA &70
  580EOR #&80
  590STA &70
  600STA &FE34
  610LDX #0
  620.saveloop
  630INC &3000,X
  640INX
  650BNE saveloop
  660JMP invert
  670RTS
  680.invert_end
  690]
  700NEXT
  710CALL copy
  720REM Set shadow RAM to solid red and main RAM to solid white
  730MODE 129
  740COLOUR 129:CLS
  750MODE 1
  760COLOUR 135:CLS:COLOUR 128
  770?&70=128
  780CALL call_invert
  790END

ZornsLemma commented 3 years ago

Thanks, having thought through this fresh I agree with your analysis, and it looks to me as though 6eefaf64b297c8ad45b50d147f3f9d76a76667ba has the correct behaviour. I used the disc image I attached earlier and pasted in the variant above and both seem to behave the same (as we'd expect; I'm just saying I did try both).

It might be worth turning the cursor off just for neatness but I don't think it's a big deal.

It would be good to get this tested on a real B+ if we can find a volunteer. Would you like to post to stardot or shall I? Perhaps in the thread which had the other test, to make it easier to find this in the future?

SteveFosdick commented 3 years ago

It looks like we have an answer: https://stardot.org.uk/forums/viewtopic.php?p=313149#p313149

ZornsLemma commented 3 years ago

Thanks for posting that, it looks like the latest version of this fix was correct then!

SteveFosdick commented 3 years ago

This is now merged.

stardot / b-em

B+ access to displayed RAM from code in private 12K at &Axxxx isn't working #138