Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flipping the FAST bit of D031 resets SCRNPTR inappropriately #714

Closed
dansanderson opened this issue Aug 1, 2023 · 9 comments
Closed

Flipping the FAST bit of D031 resets SCRNPTR inappropriately #714

dansanderson opened this issue Aug 1, 2023 · 9 comments
Assignees
Labels
bug Confirmed bug.

Comments

@dansanderson
Copy link
Collaborator

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

  • Platform: MEGA65
  • Core Commit: cae3b0b
  • ROM Release: 920384

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: MEGA65/mega65-rom-public#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.

@dansanderson dansanderson added the new New report, not classified yet label Aug 1, 2023
@dansanderson dansanderson changed the title Flipping a single bit of D031 triggers all Hot Register updates Flipping a single bit of D031 resets SCRNPTR inappropriately Aug 1, 2023
@dansanderson dansanderson changed the title Flipping a single bit of D031 resets SCRNPTR inappropriately Flipping the FAST bit of D031 resets SCRNPTR inappropriately Aug 1, 2023
@lydon42
Copy link
Member

lydon42 commented Aug 1, 2023

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
Copy link
Collaborator Author

dansanderson commented Aug 2, 2023

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
Copy link
Member

lydon42 commented Aug 3, 2023

Fix does remove the problem.

@lydon42 lydon42 closed this as completed Aug 3, 2023
@lydon42
Copy link
Member

lydon42 commented Aug 4, 2023

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...

@lydon42 lydon42 reopened this Aug 4, 2023
@dansanderson
Copy link
Collaborator Author

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.

lydon42 added a commit that referenced this issue Aug 4, 2023
lydon42 added a commit that referenced this issue Aug 4, 2023
This allows to clear changes to HOTREGs.

Normally when you set HOTREG to 0 any change to a hot register after
this will set the reclac flag. Reenabling HOTREG by setting it to 1
will then trigger a recalculation.

By setting HOTREG first to 0 and then to 1, you are able to clear the
recalc flag, thus avoiding the recalculation.

This is needed by ROM.
@lgblgblgb
Copy link
Contributor

lgblgblgb commented Aug 4, 2023

@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
Copy link
Member

lydon42 commented Aug 4, 2023

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

  • writing 1 to HOTREG sets HOTREG.
  • writing 0 to HOTREG clears HOTREG and viciv_legacy_mode_registers_touched

lydon42 added a commit that referenced this issue Aug 4, 2023
@lydon42 lydon42 added the staged Ready to be merged with stable master branch. label Dec 23, 2023
@lydon42
Copy link
Member

lydon42 commented Dec 23, 2023

Needs documentation?

@dansanderson
Copy link
Collaborator Author

I believe HOTREG is accurately documented.

@lydon42 lydon42 removed the staged Ready to be merged with stable master branch. label Feb 24, 2024
@lydon42 lydon42 closed this as completed Feb 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug.
Projects
None yet
Development

No branches or pull requests

3 participants