Char attr fix only applies when requested via errata register

[dansanderson]

Tested with:

5 BANK 1
10 PRINT CHR$(147);
20 FOR X=0 TO 15:PRINT "ABCDEFGHIJKLMNOP ",X:NEXT
30 FOR X=0 TO 15:FOR Y=0 TO 15
40 POKE 129024+80*X+Y,16*X+Y
50 NEXT Y:NEXT X
60 BANK 128
RUN
CLRBIT $D07A,5
SETBIT $D07A,5

[dansanderson]

This PR now includes the errata level mechanism.

The previous "compatibility mode" flag D07A.5 is honored like so:

• D07A.5: 1=compatibility mode, no fixes, errata level is 0, bug_compat modes are ON
• D07A.5: 0=full fixes, errata level is FF, bug_compat modes are OFF

The new "errata level" register D08F works like so:

• 0=no fixes, bug_compat modes are ON.
• 1=VIC III D016 Delta fix (bug_compat_vic_iii_d016_delta is OFF),
• 2=Delta fix + Char attr fix (bug_compat_char_attr is OFF).

Tested with:

5 BANK 1
10 PRINT CHR$(147);
20 FOR X=0 TO 15:PRINT "ABCDEFGHIJKLMNOP ",X:NEXT
30 FOR X=0 TO 15:FOR Y=0 TO 15
40 POKE 129024+80*X+Y,16*X+Y
50 NEXT Y:NEXT X
60 BANK 128
RUN
CLRBIT $D07A,5:PRINT PEEK($D07A),PEEK($D08F)
SETBIT $D07A,5:PRINT PEEK($D07A),PEEK($D08F)
POKE $D08F,0:PRINT PEEK($D07A),PEEK($D08F)
POKE $D08F,1:PRINT PEEK($D07A),PEEK($D08F)
POKE $D08F,2:PRINT PEEK($D07A),PEEK($D08F)

With D07A bit 5 set or D08F = 2, watch row 5 go from "changing colors" (compatibility mode) to "full blink" (fixed mode).

[dansanderson]

@lydon42 Please consider hiding whitespace in the Github diff view. Click the gear icon:

If trimming trailing whitespace is causing other workflow issues, I'm willing to prepare a whitespace-only change that trims all trailing whitespace from all VHDL files. We can merge that in ahead of this and related PRs. All current branches/forks off of development will need to merge it in. Preserving spurious trailing whitespace causes other workflow problems, so I'd like to solve this with editor/formatter automation if possible.

(I also did a fresh merge from development that cleaned up this PR, sorry I didn't realize it was out of date.)

[dansanderson]

This core appears to break the Freezer: starting it produces garbage display. The latest development build 66 on Jenkins works fine. I'm using the M65 files from build 66. It stays broken regardless of the values of the new registers, so I'm not sure what's causing it. The only thing I can think of that changed recently would have been something brought in with a merge from development, but the Freezer works with the development build.

Broken Freezer with this core can be reproduced with ROM 920388 and 920377. Freezer works with development build 66 with both ROMs. So not a ROM issue.

I am still able to verify the intended fix in this change (errata register; char attr fix is errata level 2).

[dansanderson]

Fixed: D07A.5 was inverted Fixed: Freezing all VIC-IV registers

Screen corruption: I compared all VIC-IV registers set by setup_menu_screen() https://github.com/MEGA65/mega65-freezemenu/blob/bc89fd1264ec03b803716277573f5eb0101a2930/freezer.c#L105 to what was actually in memory with the charattr_compat core and the development core. TEXTXPOS, LINESTEPLSB, SCRNPTRLSB, and CHARPTRMSB match their prescribed values in the working case (development) and contain erroneous values in the broken case (charattr_compat), using the same FREEZER.M65 file. These explain the display corruption.

• $4C: 50 is 52 : TEXTXPOS
• $58: 50 is 28 : LINESTEPLSB
• $61: B8 is 04 : SCRNPTRLSB
• $69: 90 is 10 : CHARPTRMSB

I don't yet know why these are wrong in the new core.

[dansanderson]

Sanity check: I further ruled out my build environment by switching to the development branch and building and testing the core in the same way. A core built from the development branch does not have the Freezer issue. So now I'm searching for offending changes.

I tried a version where the char attr change stays in compatibility mode regardless of register values. No change. Breakage is apparently unrelated to the char attr change. (No reason it would be, especially considering that the current development branch has the errata permanently enabled, but good to rule it out.)

I tried disabling read and writing to D08F, leaving D08F lines floating. I also tried disconnecting the relationship between D07A.5 and D08F. No change. If there's a bad interaction between register handling and entering the Freezer (possibly in the Hyppo freeze process), it's pretty subtle. The last thing I can think to try is to restore the original D07A.5 handling code (enable_bug_compat / disable_bug_compat); maybe it has subtle properties that the new mechanism, which is intended to work similarly, doesn't.

[dansanderson]

I narrowed the Freezer breakage to viciv_legacy_mode_registers_touched being set every time D07A was written to, which was more aggressive than previously. I'm not entirely sure if this should be set at all: none of the errata involve hot registers, and I would hope that hot register updates would propagate without this.

I think one of my more recent changes broke the feature entirely, so I'm identifying a feature-works + Freezer breaks commit (probably 9bb1d0e8), then attempting to remove that line to see if I can get both a working feature and working Freezer. Then I can step forward through recent changes and double-check both the feature and the Freezer. For example, I noticed new issues about the Freezer not un-freezing properly, which could have something to do with the freezer list change.

[dansanderson]

This PR is now fully functional. Freezer enters and exits cleanly, and the test program succeeds. "broke the feature entirely" was me accidentally using an old version of the test program.

I think I saw an issue with the Freezer not un-freezing correctly (blank screen). I messed around with storing and loading a freeze state, and the problem went away on its own. It might be worth mentioning this to testers, in case it's a temporary issue with the change in the freeze state format.

[dansanderson]

Heads up, another bug: the D016 delta fix has regressed, this is not doing it correctly. D07A:5 high or D08F>0 should shift the text screen over by a pixel. Yesterday I noticed that the fix was permanently in the on position because the assignment logic was flipped, so I reversed it, but now it's permanently in the off position. The char attr change still works.

I double checked that the register lines are all working correctly. For example, I can move bug_compat_char_attr <= ... into the to_integer(hw_errata_level) > 0 conditional that also sets bug_compat_vic_iii_d016_delta, and I can see the char attrs change at errata level 1 but can't see the XSCR offset change. bug_compat_vic_iii_d016_delta seems to always be set as if it were in the else branch of that conditional, and I can change that's branch's value and see the difference.

[dansanderson]

Good news, everyone. We determined that the change that causes bug_compat_vic_iii_d016_delta to not take effect when either errata register is updated is caused by the removal of viciv_legacy_mode_registers_touched <= '1', and we're OK with leaving it as is to prevent issues with the Freezer. We will document that the delta will be applied the next time XSCR is updated.

Separately, we determined that a latching behavior that I removed needed to be restored (or at least is a good idea to restore it), so I have repaired that, and also added similar behavior to the errata register itself. In addition to being more stable, this will allow us to do things like set related toggles for future updates.

I'm still suspicious that there might be an intermittent Freezer resume issue with this PR. Otherwise there are once again no known issues.

[lydon42]

merged into development-r4 temp branch