lgblgblgb / xemu

Emulations (running on Linux/Unix/Windows/macOS, utilizing SDL2) of some - mainly - 8 bit machines, including the Commodore LCD, Commodore 65, and the MEGA65 as well.
https://github.com/lgblgblgb/xemu/wiki
GNU General Public License v2.0
201 stars 31 forks source link

MEGA65: D018 VIC-II hot register does not update lowercase/uppercase #381

Closed dansanderson closed 1 year ago

dansanderson commented 1 year ago

Describe the bug After I set the MEGA65 to lowercase by pressing Mega+Shift, when I type POKE $D018,36 it returns to uppercase. This propagates to $D069 = 16 via the "hot register" as described in the Compendium page M-18. In Xemu, I can switch to lowercase with Option+Shift, but when I type POKE $D018,36 it remains in lowercase.

I also notice that ?PEEK($D018) returns 37 in both cases, while the MEGA65 is 36 when upper and 38 when lower. $D069 correctly switches between 16 uppercase and 24 lowercase in both MEGA65 and Xemu.

This was noticed because Run/Stop-Restore tries to return to uppercase by resetting the lower VIC-II registers, expecting hot register behavior. Going lowercase then pressing Run/Stop-Restore in MEGA65 returns to uppercase. In Xemu, it stays in lowercase.

Used version of the project Version 20230201214849-master.

To Reproduce

  1. Press Option+Shift to switch to lowercase.
  2. Press End+PgDn to trigger NMI (same as Run/Stop-Restore).

Alternatively, POKE $D018,36 to attempt to return to uppercase.

Expected behavior Should switch back to uppercase.

Computer/Device (please complete the following information):

dansanderson commented 1 year ago

You may need to use ROM 920384 to see the Run/Stop-Restore behavior. This looks like a VIC-II register issue, but the Run/Stop-Restore behavior was only noticed with the most recent ROM. (It was originally filed as a ROM issue: https://github.com/MEGA65/mega65-rom/issues/18 ) This appears to be an issue with the emulation of VIC-II hot registers, so I won't pursue a ROM change for now.

lgblgblgb commented 1 year ago

@dansanderson Thanks for the report. At the first glance, it does not seem to have anything to do with the run/stop + restore directly, but indeed as you've also described, some generic VIC register emulation anomaly instead.

?PEEK($D018):POKE$D018,36:?PEEK($D018)

It upper case mode, I get: 37 and 37. In lower case mode I get 39 and 37.

The interesting part here, that bit 0 (AFAIK!) is not used for anything. That's why it's always got back as set (in Xemu). Maybe MEGA65 handles that unused (?) bit differently though? In any way, I wouldn't say that's the problem here, though for sure it's better to have the same read-back value as on MEGA65. But the core of the issue - I think - it's not this, but how the charset info is updated on that by propagating though the hot-register "theory".

By the way, this looks very interesting for me. ROM v'373 when I set up some logging of writes on $D018 and pressing 'alt' and 'shift' multiple times (to switch between lower/upper case back and forth several times), I see writes of values $24 and $26. However when I do the same on ROM 384, there is no such a pattern. So I guess ROM 384 does the charset switch by native VIC-IV way, while older ROMs used $D018? Of course that's independent still on the run/stop+restore stuff of setting charset (I guess) but the problem maybe that those are intermixed together (surely, still needs to be fixed in Xemu, just I try to dig deeper about the nature of the bug in Xemu).

Also thanks to @gurcei to report the bug originally to mega65-rom though it seems to be an Xemu issue indeed.

lgblgblgb commented 1 year ago

Hmm, that's interesting:

                    data &= 0xFE;   // bit 0 is not used and cannot be set
                    if (vic_registers[0x18] ^ data) {
                            REG_CHARPTR_B2 = 0;
                            REG_CHARPTR_B1 = (data & 14) << 2;
                            REG_CHARPTR_B0 = 0;
                            REG_SCRNPTR_B2 &= 0xF0;
                            reg_d018_screen_addr = (data & 0xF0) >> 4;
                            vic_hotreg_touched = 1;
                    }

This is the code fragment being used (I already moved one line around, but that's not the point here), when register $D018 is written. If I comment out the if part, so it always run,s it seems the run/stop+restore works normally and upper case is restored, thus probably fixing the problem.

The reason for that if: it tires to detect, if there is change on the value. Maybe that's wrong, and the hotreg event should be always carried over, even the value is not changed? It's possible if you intermix native VIC-IV code with this. Look at my previous comment: if you use native VIC-IV way eg to set the charset, $D018 itself is never changed, thus the if would "think" there is no change then if you try write $D018 on nmi/restore thus ignoring it totally.

@dansanderson Please confirm, that you find this theory reasonable. As I noticed with the previous comment - I think - it's makes sense the problem here, that there should be no condition to "filter out" the "same value written" but always execute the stuff, which is needed when native VIC-IV and $D018 writes are "mixed" in time, thus the condition has the false assumption sometimes, that there is no change to deal with. Also I would make bit 0 being read as 0 as 1 as it seems to way how MEGA65 handles this, though I am not sure, as it seems to put character_set_address(13 downto 10) (from the VHDL) onto the lower 4 bits, so bit 0 is not fixed and not totally unused like it would be on a C64, though it seems it cannot be written at least via $D018. So kind of strange in my opinion ... I am not even sure how bit 0 should be done 100% correct on reading compared to MEGA65 then. Again, it seems writing bit 0 though via $D018 has still no effect. But this whole bit 0 thing is only a side-track of the issue, and not the core one. Though should be nice to be emulated correctly, still.

Looking the VHDL, I have the suspect that something very odd is going on, writing charptr via VIC-IV native way ("precise register") would also alter the result what $D018 will read (surely, only some bits of it). Maybe also true for screen address. I feel kind of unsecure about this whole logic though ...

By the way: this problem can be manifest as a bug in the other similar case as well, when screen address is modified natively with VIC-IV native register then writing $D018 to modify it, again when the same value is written to $D018 as before, the current Xemu code would think there is no change, since it cannot see the native VIC-IV write as change. So probably this proposed fix of mine would fix that case too.

Thus, my supposed fix so far, what I need to test further:

--- a/targets/mega65/vic4.c
+++ b/targets/mega65/vic4.c
@@ -603,15 +603,16 @@ void vic_write_reg ( unsigned int addr, Uint8 data )
                        // Reads are mapped to extended registers.
                        // So we just store the D018 Legacy Screen Address to be referenced elsewhere.
                        //
-                       if (vic_registers[0x18] ^ data) {
+                       data &= 0xFE;   // bit 0 is not used and cannot be set
+                       //if (vic_registers[0x18] ^ data) {
                                REG_CHARPTR_B2 = 0;
                                REG_CHARPTR_B1 = (data & 14) << 2;
                                REG_CHARPTR_B0 = 0;
                                REG_SCRNPTR_B2 &= 0xF0;
                                reg_d018_screen_addr = (data & 0xF0) >> 4;
                                vic_hotreg_touched = 1;
-                       }
-                       data &= 0xFE;
+                       //}
+                       DEBUGPRINT("D018 is set to $%02X @ PC=$%04X" NL, data, cpu65.pc);
                        break;
                CASE_VIC_ALL(0x19):
                        interrupt_status = interrupt_status & (~data) & 0xF;
@@ -848,7 +849,6 @@ Uint8 vic_read_reg ( int unsigned addr )
                CASE_VIC_ALL(0x17):     // sprite-Y expansion
                        break;
                CASE_VIC_ALL(0x18):     // memory pointers
-                       result |= 1;
                        // Always mapped to VIC-IV extended "precise" registers
                        // result = ((REG_SCRNPTR_B1 & 60) << 2) | ((REG_CHARPTR_B1 & 60) >> 2);
                        // DEBUGPRINT("READ 0x81: $%02x" NL, result);
lgblgblgb commented 1 year ago

Hmm, thinking again on the topic of reading back D018 should be MEGA65-accurate. Maybe I should read back the "precise" registers of VIC-IV through reading $D018, at least according to VHDL, that is what MEGA65 does, surely only the 4-4 bits of each at the right position. That would also solve the issue to be compatible show MEGA65 does the reading back business (including the "unused" bit 0 problem). But it feels still odd for me, that writing native VIC-IV registers have a "partial" effect on reading a legacy VIC-II register (ie some bits of the native ones only).

dansanderson commented 1 year ago

Possibly of interest: I notice that POKE'ing 16 or 24 to $D069 also back-propagates a change to $D018 36 or 38.

I also notice that if I poke $d05d,peek($d05d) and 127, which supposedly disables hot registers (unless I did it wrong?), this behavior does not change. (I was hoping to test if writing the same value to D018 twice caused changes, but I can't test that if changes update both ways.) So I wonder if this is not technically a hot register behavior and is instead wired directly. Unfortunately I'm not well versed enough in the core code to dig out the signal behavior, so I'll leave that to someone else.

lgblgblgb commented 1 year ago

@dansanderson Oh, I forgot to react: thanks for your comment. Hopefully this problem has been fixed for now. If you still think there is some anomaly, please just re-open the issue, but I close it for now.