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

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

Closed
dansanderson opened this issue Jul 31, 2023 · 6 comments
Closed

Comments

@dansanderson
Copy link

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):

  • Device/Platform: Mac
  • OS and its version: macOS Ventura 3.4.1 (c)
@dansanderson
Copy link
Author

dansanderson commented Jul 31, 2023

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

lgblgblgb commented Jul 31, 2023

@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 lgblgblgb self-assigned this Jul 31, 2023
@lgblgblgb
Copy link
Owner

lgblgblgb commented Jul 31, 2023

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 lgblgblgb added the WIP label Jul 31, 2023
@lgblgblgb
Copy link
Owner

lgblgblgb commented Jul 31, 2023

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

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 added a commit that referenced this issue Jul 31, 2023
$D018 behavour was kind of bad. The problems I have tried to fixed:

1. There was a condition to only propogate the change of $D018 if there
   was really a change on the written value compared to the previous.
   This seems to make sense, however this is wrong. As natively writing
   the "precise address" register can change the real address and the
   $D018 write does not aware it was changed thus there is no
   propogation.

   Fix: always propogate, no condition.

2. $D018 readback is based on $D018 value written before. This is also
   wrong, as according to the VHDL, it seems, the actual precise address
   is used to read by reading $D018, at least some bits of it.

   Fix: reading $D018 uses the precise addresses (both for video ptr
   and char ptr).

Note: this may also fix another very similar issue with the video ptr,
as it's the very same case, also in register $D018.
@lgblgblgb
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants