MiSTer-devel / C64_MiSTer

111 stars 56 forks source link

new exciting Transwarp crashes with DolphinDos on cores after 20210530 #109

Closed FRauANtjeK closed 2 years ago

FRauANtjeK commented 2 years ago

Hi,

since core 20210607 "Transwarp" crashes with DolphinDos Kernal and parallel cable enabled.

Kernal 20210530 works, so maybe some code for the parallel cable has been changed between those two kernals?

sorgelig commented 2 years ago

are you talking about this? https://www.c64-wiki.com/wiki/Transwarp or there is something other called Transwarp? It seems too different from normal disk. There can be other factors as PAL/NTSC, enabled external IEC or other features. Generally speaking with DolphinDOS ROM you don't need any other additional fastloaders on top.

KrillPlush commented 2 years ago

It's a compatibility issue that the original system with DolphinDOS doesn't exhibit.

The problem seems to be related to JMP ($DD00). The parallel port at $DD01 is set to all-output and initialised to $E0, such that the indirect jump would go via a table at $E0xy, with xy being the value read from $DD00. But it appears to end up jumping to $00xy instead, implying reading $00 from $DD01.

There is code to check the value at $DD01 for reading the expected $E0 prior to performing the actual indirect jumps later. This doesn't trigger, however, indicating that $DD01 reads the expected $E0 first, but then something would cause the parallel port to read $00 some time later after it has initially read $E0.

As it works with Release 20210530 but doesn't with 20210607 and later, the breaking commit is perhaps https://github.com/MiSTer-devel/C64_MiSTer/commit/d65db29e81044bbe59c01f66b82f912e4e65d48e - specifically, the newly-introduced disk_access timeout (half a second) looks somewhat suspicious in this context.

(Note that Transwarp is 100% software running on an unmodified stock system, and the file encoding is compatible with .d64, as in plain standard low-level format, too. Whether or not it makes sense to run it on a system sporting DolphinDOS isn't quite relevant, and Transwarp loads faster than DolphinDOS.)

sorgelig commented 2 years ago

That commit is very large, so it doesn't point to specific problem. I suggest to turn off Parallel port for disk access as it's not required since Transwap is used instead of DolphinDOS. Basically parallel access is tuned to DolphinDOS specifically to make it transparent for system sharing parallel port between disks and joysticks/UART.

KrillPlush commented 2 years ago

Turning off the parallel port (either by software via @XP- or by removing the cable) works as a work-around, but this is not required on an original system.

Transwarp falls back to using the original DOS for standard-format files, so you can load Transwarp files at one point and standard-format files with DolphinDOS/parallel cable at another, in the same running program. Switching the parallel cable on and off manually makes this somewhat impractical.

sorgelig commented 2 years ago

I understand inconvenience... Since you are the developer of Transwap, may be you can check more detailed why it fails. Actually i wanted some fast and free loader for 1581. similar to DolphinDOS on 1541. Basically loaders in RAM isn't practical as they are vanished after loading the game. So would be good to have it integrated. I also think about shadow ROM which can be automatically activated, so original Kernal/Disk ROMs won't need much modifications and will retain all functions. It can be transwap if it gets the same speed as Dolphin, although i don't understand how serial connection can be faster than parallel one.

KrillPlush commented 2 years ago

I have no consistent theory at this point, but i'm going by the assumption that the breaking change was a commit between the two mentioned releases.

I currently suspect the disk_access timeout due to the apparently delayed fashion in which the parallel port access fails.

However, the C-64 port $DD01 is set to all-output, so any external signals on that port should be ignored (unless they can be overridden by a stronger driver external to the C-64, like on the real system).

It appears that the activity check is not fully qualified:

    if(((c64_iec_clk_old != c64_iec_clk) || (drive_iec_clk_old != drive_iec_clk)) || 
        (disk_parport && ((drive_stb_i_old != drive_stb_i) || (drive_stb_o_old != drive_stb_o))))
    begin
        disk_access <= 1;

It does not check for ATN nor DATA changes from the C-64 side, nor DATA changes from the drive side, which may cause the JMP ($DD00) reading something unexpected from $DD01 (despite there not being actual parallel communication at this point) after a while of activity on ATN/DATA but not CLK.

KrillPlush commented 2 years ago

As for the other points, Transwarp is designed as transient software living in the drive for the duration of loading one file. The C-64 side code is planned to become a cartridge speeder, uploading the drive code for every loaded file, but returning drive-side control to drive ROM afterwards. It will likely never become a drive-side DOS replacement.

Serial communication is of course slower than parallel, but drive-host communication is just a part of the total effort, the others being more efficient disk access and quicker-to-decode data encoding on the disk.

sorgelig commented 2 years ago

Since 0.5s is quite long timeout it's supposed to have some communication between C64 and disk which obviously involves C64 generating CLK signal.

if you change couple lines bellow:

    else disk_access <= 0;

to

    else disk_access <= 1;

will transwap work then? Basically you will enable parport forever.

KrillPlush commented 2 years ago

I'd have investigated more into it than just looking hard at the source, but alas, no MiSTer development kit here for now. :)

sorgelig commented 2 years ago

can you provide me test files (disk?) for testing considering i know nothing about transwap?

KrillPlush commented 2 years ago

The current release version comes with a bunch of showcase disk images in two zip archives: https://csdb.dk/release/?id=209982 There is also a repository with some more current test images: https://bitbucket.org/KrillPlush/transwarp-test/ Some more information about the MiSTer vs DolphinDOS problem can be found here: https://www.lemon64.com/forum/viewtopic.php?t=78558 The older release at https://csdb.dk/release/?id=197710 (different showcase images linked) probably triggers the same problem, but i have no information about this.

KrillPlush commented 2 years ago

What stumps me is that the problem doesn't appear when either disabling the parallel connection in hardware (MiSTer configuration) or disabling parallel communication in software (@XP-).

The latter hints that there are some software registers to set correctly, but the drive-side parallel port should be set to all-input already, and the C-64 side to all-output. So there should be no reason for the JMP ($DD00) with $DD01 set to $E0 to end up anywhere else than $E0xy, regardless of a parallel cable or not, and yet it seems to do that. But not on the original system equipped with DolphinDOS, which reportedly runs this just fine without any modification.

sorgelig commented 2 years ago

I've tried to enable parallel port always enabled - it didn't fix the problem. JMP ($DD00) - is it on drive side or C64?

KrillPlush commented 2 years ago

C-64 side. Note that $DD00 is the serial port (mostly, with bit 2 going out to parallel cable), while $DD01 is the parallel port.

The idea is to react to incoming serial signals quicker (with less jitter) than with a polling loop (loop BIT $DD00 : B?? loop), and $DD01 is expected to read $E0 when doing JMP ($DD00). Hence it is set to all-outputs ($DD03 <= $ff) before being initialised ($DD01 <= $E0).

sorgelig commented 2 years ago

ok, at first core implements the bus with open collector, so any party driving signal low will win. If no one drives low, then it will be 1. I need to check further if anyone drives it low unexpectedly..

sorgelig commented 2 years ago

i cannot see any abnormality.. here what drive set for parallel port. https://github.com/MiSTer-devel/C64_MiSTer/blob/315cc291e2cf60b8f740d1b1617082129c748ac2/rtl/iec_drive/c1541_logic.sv#L138 So if you set all bits in DDRA of VIA to 0(input) then output on parport from disk will be FF and thus on C64 side you should read whatever you output to parport..

May be you will write a small PRG file which will set parport on both C64 and disk sides what you want and will keep it. Then give me to test it on FPGA side.

sorgelig commented 2 years ago

with parport forcibly always enabled i've used integrated DolphinDOS monitor to see DD00 and DD01. If i set DD03 -> FF (all output) then i read from DD01 whatever i write there. Probably something on drive side drives VIA's output to 00. But have no idea what.

KrillPlush commented 2 years ago

There seems to be a timing component to this, as the compatibility check code in Transwarp reads back the value it has written to $DD01 ($E0) and does not bail out, but later the JMP ($DD00) apparently stumbles over $DD01 = 0.

To avoid going after red herrings, can you confirm that the problem appeared with one of the commits between the two mentioned releases, and if yes, which one?

sorgelig commented 2 years ago

i'm going to sleep now. Will walk through commits tomorrow.

sorgelig commented 2 years ago

After spending half day to narrow down the area causing problem i found the "fix" if i replace on line: https://github.com/MiSTer-devel/C64_MiSTer/blob/315cc291e2cf60b8f740d1b1617082129c748ac2/c64.sv#L1516 disk_access by |drive_led.

this brings a question if transwap code has temporal bug. drive_led is activated much later by code in drive than disk_access signal reacting on bus access. So your code start to use DD01 much earlier than it gets prepared from drive side and some time in the beginning it may contain wrong value driven by disk drive. Are you sure you fully tested transwap on C64 equipped by DolphinDOS with parallel port connection?

sorgelig commented 2 years ago

This also explains why permanently enabled parallel port for disk breaks transwap too from my experiment above.

KrillPlush commented 2 years ago

It has been reported to work without problems on a C-64 with DolphinDOS. (Can't test it myself because no DolphinDOS, but i trust the reporters.)

When trying out things before, did you set the disk_parport side or the disk_access side of the check to true, or maybe both? (Tying the decision to the drive LED is obviously wrong, but i guess that's why it was changed in the first place.)

I'm rather sure there is no temporal bug in the C-64 code.

And there is no parallel communication to begin with (with Transwarp itself), nor a need for the drive code to set up things. The C-64 side sets $DD01 to all-output with value $E0, and external signals (e.g., by a parallel cable that happens to be connected) should not be able to change that value. (With some TTL driver on the real C-64's user port, external signals can override the output setting, but that's what the initial compatibility check is guarding against.)

Furthermore, the drive side has the parallel port on all-inputs at the point of crashing, with or without DolphinDOS.

There is one interesting thing i noticed in the meanwhile: Between the compatibility check and the crash-failure, the drive code is uploaded, via DolphinDOS and parallel connection. Now, DolphinDOS is well-behaved, buffering and restoring $DD01 and $DD03:

.C:f1b8  AD 03 DD    LDA $DD03
.C:f1bb  8D 96 02    STA $0296
.C:f1be  A9 FF       LDA #$FF
.C:f1c0  8D 03 DD    STA $DD03
.C:f1c3  AD 01 DD    LDA $DD01
.C:f1c6  8D 95 02    STA $0295

.C:f95f  AD 0C DC    LDA $DC0C
.C:f962  29 02       AND #$02
.C:f964  8D 0C DC    STA $DC0C
.C:f967  AD 96 02    LDA $0296
.C:f96a  8D 03 DD    STA $DD03
.C:f96d  AD 95 02    LDA $0295
.C:f970  8D 01 DD    STA $DD01

It might be that, for some reason (could be a long-standing bug in another component, triggered now), a wrong value is read from $DD01 when buffering, and then later written back.

sorgelig commented 2 years ago

So basically you came to conclusion i've wrote above, right? Drive side just set some output on parallel port (due to loading initial code through parport) and it drives the bus till your code switches it back to input mode. On real HW it's possible that C64 and Drive compete on bus driving it from both sides with active 1 driver (not pull up resistor) and C64 wins because you read from C64 side. On FPGA it's impossible to implement that 2 modules will drive the signal into opposite levels. It's simply impossible by design. So bi-directional bus is usually implemented as AND of both modules, so the party driving 0 will win. This is equal to open collector driver with pullup resistor. This is the only explanation i can find. Although i've thought IEC bus is open-collector bus with pullups.

So i didn't get what you wanted to tell by assembler code. Is it patch for DolphinDOS ROM? Or you already fixed it in your loader?

KrillPlush commented 2 years ago

I don't think i've come to any conclusion yet. :) Still puzzling out what is going wrong, and where.

But one assumption is that the drive is not driving the parallel port at the time of the fatal JMP ($DD00), and in fact, pretty much never between the compatibility check (which says all good, $DD01 reads $E0) and that indirect jump.

The question remains why $DD01 would read something else ($00) at some point, and in emulation.

With an open-collector bus, something external would have to wire-AND the bits to 0. But you ruled that out. It's all still mysterious. :)

The assembly bits are the relevant buffer/restore parts in the DolphinDOS KERNAL, which might or might not play a role here.

sorgelig commented 2 years ago

parallel port is shared between disk and Joystick/UART. Will it be shared with joystick or UART is set by Hardware->Expansion OSD option. So use Joystick. They are joysticks 3 and 4. So if no buttons pressed on that joysticks then port will read FF. If you share with UART then parport may read different values. Besides that nothing connected to the port..

Remember above i've experimented with connection of parport to disks permanently? And Transwap failed in that experiment. So it's exactly disk drive who is driving parport to 00. Transwap start to work only if i use drive_led signal to allow parport to be connected to disk. So i'm pretty sure something happening on drive side to let it drive parport to 00. But i'm not good in C64/drive ROMs, so i cannot help with debugging it. I thing you are much more professional in these ROMs than me.

sorgelig commented 2 years ago

Just an idea for more robust communication in Transwarp parts: After you transferred initial loader, wait for a command/reply before start to use the loop/procedure using JMP ($DD00). So that "ready" sequence will insure both sides C64 and Drive were taken over and fully controlled by your loader and partports on both sides are set to correct values.

KrillPlush commented 2 years ago

Such a communication sync point is in place already.

But good point about the previous drive LED gate, which might have shielded off an emulated-drive-side bug before being removed.

I will have a closer look at the DolphinDOS drive-side ROM and also the MiSTer VIA6522 implementation.

But i still have no theory how the all-outputs C-64 side of the parallel port is overridden (by external forces majeures?) to something else.

sorgelig commented 2 years ago

this is build with drive_led used as gate, Transwarp works: C64.zip

May be it will help you to debug.

sorgelig commented 2 years ago

According to MOS6526 datasheet it uses both 0 and 1 drivers on output. So it's possible it competes with VIA on drive side. On real HW CIA will win if you read the bus on C64 side, and VIA will win if you will read the bus on drive side. Simply because cable and PCB traces have some resistance. Although it's a bad sign for real HW because your loader will put both sides into competition mode overloading their outputs. I think i can emulate such competition in core. Pin designated as output will simply ignore the bus state and will read itself state instead.

KrillPlush commented 2 years ago

Both sides driving the "bus" was the first thing i checked and then ruled out, before signing up on github to comment here. :)

Might have overlooked something, of course, but it's not in my immediate focus for now.

sorgelig commented 2 years ago

In this build i ignore input on pins configured as output (as mentioned earlier): C64_t.zip

Transwarp works now with disk_access as a gate. Problem can be considered as solved, although it just hides the original issue.

Besides this problem i still want to ask you to develop a shadow transwarp loader which will be always available, not only for initial prg loading. I can add shadow ROMs for both drive and C64. We can decide at which point shadow ROM should be triggered on/off, so it won't be visible for games. Then we won't need special kernels for C64 and drive and will use original ones. I think it would be really great feature - to have a fast loader all the time. Also it can be ported to 1581. It seems you are MiSTer user as well, right?

KrillPlush commented 2 years ago

I'm not yet a MiSTer user, i'm only now considering to become one, after FRauANtjeK has brought this issue to my attention.

Your tentative fix with outputs completely ignoring corresponding inputs indeed points at competing drivers on either end.

Will now again look in that direction. But some masked bug in emulation is also not ruled out yet.

As for Transwarp resident in ROM, it's not at that point yet. Keep in mind that it has its own native encoding (with 223 instead of 256 bytes per block), which was designed to be representable with d64 images, for ease of copying/transferring/spreading, and yet anything expecting to read plain Commodore GCR data will fail when going below the block level. You can disk-copy Transwarp files, but not file-copy them, nor read the files with anything oblivious to it.

A fallback fastloader to load standard-format files is planned, though. Still, it will load while any DMA is disabled (screen off), but it will do so with optimised GCR decoding and disk access routines. These may be what you want (but might not at all be applicable to 1581), but... not on my menu to shoehorn them into a complete replacement DOS... :) (The compatibility issues alone...)

KrillPlush commented 2 years ago

Found the issue, and it was indeed competing drivers on both sides.

The aforementioned DolphinDOS code

.C:f1be  A9 FF       LDA #$FF
.C:f1c0  8D 03 DD    STA $DD03
.C:f1c3  AD 01 DD    LDA $DD01
.C:f1c6  8D 95 02    STA $0295

is sometimes executed while the drive-side counterpart hasn't yet switched its parallel port back to input. As a result, a wrong value is read from $DD01 and later written back on restore.

This can be considered a DolphinDOS bug, but as mentioned, "On real HW CIA will win if you read the bus on C64 side, and VIA will win if you will read the bus on drive side. Simply because cable and PCB traces have some resistance.", the correct output value will be read and buffered on a real system.

The fix to make it work across all current MiSTer releases was to simply set $DD01 to the desired value $E0 again after some drive-code has been uploaded via DolphinDOS and Transwarp takes over.

Will you leave in the tentative new behaviour, or revert to the old one? The new one seems closer to the real thing, but i don't know what this means for your desired open-collector parallel bus architecture.

sorgelig commented 2 years ago

the correct output value will be read and buffered on a real system.

on real system there are no buffers. It's basically DIY mod for C64 and Drive, so CIA is connected to VIA directly without any buffers, so they simply competing each other.

Will you leave in the tentative new behaviour, or revert to the old one? The new one seems closer to the real thing, but i don't know what this means for your desired open-collector parallel bus architecture.

i've released a new version with new behavior. So CIA doesn't read inputs on pins configured as output. Original open collector implementation i've made automatically just under impression of IEC implementation (which is open collector on real HW). The fact that 2 naked chips simply connected to each other somehow got unnoticed originally.

sorgelig commented 2 years ago

i think issue can be considered as solved.

KrillPlush commented 2 years ago

Some parallel cables do provide proper buffering. I guess those would not change the read-output-value behaviour as observed with two naked chips connected, though.