search

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: CPU speed change/restore fails in Xemu done by ROMs >= v920385? #394

Closed lgblgblgb closed 8 months ago

lgblgblgb commented 8 months ago

Originally posted by @lgblgblgb in https://github.com/MEGA65/mega65-rom-public/issues/107#issuecomment-1872502574 Issue originally discovered/reported by @gurcei

I've checked the game, what I can see by looking at the title bar of Xemu, that with newer ROMs, the CPU clock is downgraded to 1MHz at the problematic parts, while with older ROMs it stays 40.5MHz. Indeed, the diff between 920384 and 920384 ROMs indicate, there was some CPU speed setting code change between these two ROM versions. So I'm 99.99% sure, this must be the problem. If though it works on the hardware, there must be some Xemu bug here which reacts incorrectly on CPU speed settings. The ROM changelog says (also git diff v920384..v920385 in mega65-rom repo to check the difference between the two releases):

Fix: (Requires new core) 80x50 mode no longer switches to the wrong screen memory when CPU
speed changes. This manifested as half a screen of garbage and instability after DOS operations
and the ringing of the bell (Ctrl-G) while 80x50 mode was active.

If this change requires a newer core, I suppose there was some core change which is not implemented by Xemu yet. @dansanderson, do you remember what change in the core this ROM change depends on? Thanks!

I found this: https://github.com/MEGA65/mega65-core/issues/714

Changes between v920384 and v920385 ROMs: https://github.com/MEGA65/mega65-rom/compare/v920384...v920385

However I am not sure how it's connected, it's about changing speed should not trigger hotreg event, but this bug more seems to be a speed change bug not the side-effects. As far as I can see ...

I will try to write a short example which triggers this problem, as it's kinda hard to test with the "Dino game".

As far as I know (knew?) there is no problem in Xemu on setting CPU speeds, so really it must be a (relative ...) new thing, as the ROM changelog also mentions that it requires a new core. Also the mentioned mega65-core issue is carried into Xemu already in https://github.com/lgblgblgb/xemu/issues/382 as the commit https://github.com/lgblgblgb/xemu/commit/f76fd7bbd0c8084e605fc6342a883dcb4968f1be Surely, it is of course possible that I did a mistake somewhere?

dansanderson commented 8 months ago

This relies on a change to the handling of HOTREG so that it can update FAST without tripping hot register propagation (the original 80x50 bug). I don't think older HOTREG behavior would result in the speed not being changed, only in potentially unwanted propagation of hot registers, but maybe I'm not remembering enough detail. Is that enough to explain the difference in Xemu in this specific case?

The intent of the ROM change was to 1) not trigger HOTREG on changes to FAST (the 80x50 bug), and 2) centralize KERNAL logic that relies on changing the speed so that it does it consistently. DOS ought to be more reliable in restoring the original speed after the DOS operation, but if it isn't, we should take a closer look.

(Dunno if this is related: https://github.com/MEGA65/mega65-rom-public/issues/89)

johnwayner commented 8 months ago

I just tried this in xemu with a build off the HEAD of the master ROM branch and the text comes up immediately. I think I might have fixed this in a commit to the ROM that hasn't been released yet: https://github.com/MEGA65/mega65-rom/commit/afb85c59f1cad41ae6d854df7a5df55d84cafb3a

I didn't try it on hardware, but from the reports that it isn't working there, I wonder if it has to do with some difference in the way xemu handles file i/o that resulted in exposing the bug that the commit above fixes.

lgblgblgb commented 8 months ago

Thanks for the comments, guys! I should really try to create a minimal sized code which can reproduce this, so it's easier to see what's the exact problem is, and also easier to test if a fix is sufficient or not (in meantime I would be interested on the result on real MEGA65 as well). But at this point I am really unsure if it's Xemu's bad implementation or something other which can "help" Xemu to show this bug more easily than mega65-core, but the Xemu is mega65-core compliant otherwise in the speed change behaviour's regard ...

dansanderson commented 8 months ago

ROM 920381 is out and it resolves the original issue with Dino. I'm not 100% sure what's Xemu-specific about the scenario we noticed, but it's resolved with a recent ROM change.

lgblgblgb commented 8 months ago

@dansanderson OK, thanks. I still have the feeling that something does on here, just it's timing or whatever dependent to be able to trigger the bug. But since it seems to be OK with newer ROM, I close this now. If anyone has new experience, for sure, just feel free to re-open the issue. Thank you!