MEGA65 / mega65-core

MEGA65 FPGA core
Other
241 stars 88 forks source link

Flipping the FAST bit of D031 resets SCRNPTR inappropriately #714

Closed dansanderson closed 7 months ago

dansanderson commented 1 year ago

Test Environment (required) You can use MEGA65INFO to retrieve this.

Describe the bug An update to any of the bits in the VIC-III mode register D031 triggers Hot Register updates to VIC-IV registers like SCRNPTR, even if the mode bits related to those registers have not changed. This is a problem for 80x50 mode, which relocates SCRNPTR to 4.0800. Certain immediate mode actions attempt to flip the FAST flag (bit 6) of D031 temporarily using lda #%01000000; trb vic+49, but this causes SCRNPTR to reset to 0.0800, with the effect of showing 80x50 characters at 0.0800: the screen as it appeared before switching modes plus half a screen of junk. Actions that cause this include SPEED 1 (naturally), DOS commands, and ringing the bell (Ctrl-G or PRINT CHR$(7)).

Originally reported for the ROM: https://github.com/MEGA65/mega65-rom-public/issues/43

To Reproduce Steps to reproduce the behavior:

  1. Press ESC then 5 to enter 80x50 mode.
  2. Press Ctrl-G to ring the bell.

Expected behavior There should be a way to flip the FAST flag of D031 without resetting SCRNPTR. Ideally, the method that the ROM is using would not propagate a reset of SCRNPTR. An update to D031 would detect which bits have changed, and only propagate Hot Register changes related to those bits.

If there exists a way to do this please let me know and I'll update the ROM.

lydon42 commented 1 year ago

So essentially only set viciv_legacy_mode_registers_touched at D031 if something except viciii_fast_internal was set? That's a lot of comparisons... perhaps if we can trim it down to the registers that are really used in the hotreg update functions...

Something like this (viciv.vhdl around 2558):

          if reg_h640 & fastio_wdata(6) & viciii_extended_attributes & bitplane_mode & reg_v400 & reg_h1280 & reg_mono & reg_interlace /= fastio_wdata then
            viciv_legacy_mode_registers_touched <= '1';
          end if;
dansanderson commented 1 year ago

As an alternative, we can make the intended workflow be to disable HOTREG (D05D bit 7), update D031, then re-enable HOTREG.

Unfortunately the current implementation does not support this workflow, because re-enabling HOTREG causes all Hot Register changes to propagate instantly. We'd need a core change so that enabling HOTREG only re-enables propagation on future changes to Hot Registers and not itself trigger propagation. Is that feasible? Or desirable?

It seems like Hot Registers were designed to be disabled and left disabled during the course of a MEGA65 VIC-IV-based program. That's cool, but unfortunately it'd be a major project to update the ROM to work with HOTREG disabled.

lydon42 commented 1 year ago

Fix does remove the problem.

lydon42 commented 1 year ago

There was some discussion on discord, that it might be bad to have a 1 bit exception for hotreg triggering and that we should keep the rule simple: writing to a hotreg does trigger the recalculation.

The other solution would be to change how the trigger handling works.

Currently every write to a hotreg sets the flag for recalculation, even when hotregs are disabled. So if you disable hotregs, then write to some trigger registers (setting the recalc flag) and then re-enable hotregs, this will trigger a recalculation immediately.

@lgblgblgb proposed that this behavior is changed instead, and that writing to hotregs, while they are disabled, does not trigger a recalc afterwards, when hotregs is enabled.

There are two ways to do this: 1) Clear the recalc flag when HOTREG bit goes from 0 -> 1. So enabling HOTREG will never trigger a recalc. 2) Clear the recalc flag when HOTREG bit goes from 1 -> 0. So you need to set HOTREG to 0 then to 1 to clear the flag and enable HOTREG again.

Why propose the second method? The first could potentially break programs that intentionally disable HOTREG, change a bunch of things, then enable HOTREG to apply at once. But I honestly don't see that anyone is doing that...

dansanderson commented 1 year ago

Clarification from discussion: option #2 would reset the propagation flag for any write of 0 to HOTREG, so the procedure to change a hot reg without triggering propagation would be:

  1. Set HOTREG to 0.
  2. Update the VIC-II register, e.g. D031 FAST = ... This sets the hot reg propagation flag.
  3. Write a 0 to HOTREG again. This clears the propagation flag.
  4. Set HOTREG to 1. No propagation occurs because the flag is clear.

This preserves the (admittedly obscure) existing use case where setting HOTREG to 0, touching a hot register, then setting HOTREG to 1 triggers propagation. It's an unusual procedure, but it's an unusual use case, and overall I think the design is clean and easy to understand.

lgblgblgb commented 1 year ago

@lydon42 Ok that makes sense. So the hotreg bit transition table now (I think, it would worth to have the manual with something like this to avoid any possible confusion):

HOTREG bit value transitions:

old value new value written effect
1 0 Disable hotregs
0 1 Enable hotregs, any possible trigger (stored during disabled state) is processed then trigger is cleared
0 0 Horegs remain disabled, but clear possible pending trigger, so 0->1 transition won't cause to re-calc
1 1 No effect, hotregs remains enabled

Any write of a hot-register ("hotreg") cause the trigger to be set regardless hotregs are disabled or not, however, when hotregs are disabled, the trigger is "postponed" to cause re-calculation when hotregs are enabled again (unless cleared with 0->0 transition), while when hotregs are enabled, the trigger causes re-calculation immediately. In case of re-calculation (postponed or not) of course the trigger is cleared.

I want to make this 100% clear for myself too, btw, so my long boring comment here ;)

lydon42 commented 1 year ago

The table is somewhat misleading... the current implementation (with the last commit) is:

lydon42 commented 9 months ago

Needs documentation?

dansanderson commented 8 months ago

I believe HOTREG is accurately documented.