UltraWarp screen corruption (workaround attached)

[ryucats]

Hello,

Using ADTPro in a machine that has an UltraWarp accelerator installed has issues with screen corruption when receiving packets with an Uthernet.

The issue appears to be caused by the unilateral disable/re-enable of the ZipChip and TransWarp accelerators in the RECEIVE_LOOP_PAUSE routine in src/client/prodos/ethernet/ethproto.asm. Replacing those bits with an UltraWarp-specific disable/re-enable results in a working ADTPro client.

The workaround looks like this:

diff --git a/src/client/prodos/ethernet/ethproto.asm b/src/client/prodos/ethernet/ethproto.asm
index 80f9fb1..acffd00 100644
--- a/src/client/prodos/ethernet/ethproto.asm
+++ b/src/client/prodos/ethernet/ethproto.asm
@@ -956,30 +956,34 @@ RECEIVE_LOOP_WARM:
        rts

 RECEIVE_LOOP_PAUSE:
-       lda #$5a
-       sta $c05a
-       sta $c05a
-       sta $c05a
-       sta $c05a       ; Unlock ZipChip
+;      lda #$5a
+;      sta $c05a
+;      sta $c05a
+;      sta $c05a
+;      sta $c05a       ; Unlock ZipChip

-       lda #$00
-       sta $c05a       ; Disable ZipChip       
+;      lda #$00
+;      sta $c05a       ; Disable ZipChip       

-       lda #$01
-       sta $C074       ; Disable TransWarp
+;      lda #$01
+;      sta $C074       ; Disable TransWarp
+        lda #$01
+        sta $C05D       ; Slow down UltraWarp

 PauseValue:
        lda #$7f
        jsr DELAY       ; Wait!

-       lda #$00
-       sta $c05b       ; Enable ZipChip
+;      lda #$00
+;      sta $c05b       ; Enable ZipChip

-       lda #$a5
-       sta $c05a       ; Lock ZipChip
+;      lda #$a5
+;      sta $c05a       ; Lock ZipChip

-       lda #$00        ; Enable TransWarp
-       sta $C074
+;      lda #$00        ; Enable TransWarp
+;      sta $C074
+        lda #$01
+        sta $C05C       ; Speed up UltraWarp

        jmp RECEIVE_LOOP_WARM

I think that it's the ZipChip disable/enable that is frying the UltraWarp, as the address in question is adjacent to the UltraWarp address. I am going to engage Henry at ReactiveMicro for further investigation; there may be a way to detect the UltraWarp and skip the offending instructions thereby.

Edit: it's the ZipChip enable -- writing to $C05B disables the UltraWarp and leaves the system in an indeterminate state.

Cheers.

[david-schmidt]

Thanks for the legwork - I'm not sure yet how I'd work around this. The enablement/disablement switches were all mutually exclusive before. I don't otherwise have a way to detect one or the other. And Zip chips are much more likely to be found out in the wild than UltraWarps, though I actually do have one. :-)

[ryucats]

Yeah, I was dismayed to see that the UltraWarp incompatibly overlaps the same addresses as the Zip Chip.

My thoughts on fixing this in a compatible way in the ADTPro client, in descending order of desirability:

1. there's a way to detect the UltraWarp (signature byte or byte pattern somewhere in $C0xx space, for example); we test for that and branch accordingly,
2. we add a knob in the config screen ("ULTRAWARP SUPPORT" or similar) that sets a config flag, and we branch on that,
3. maintain a separate build of ADTPro and host it on the ReactiveMicro wiki,
4. I talk Henry into modifying the hardware softswitch logic to shift the UltraWarp kill location from $C05B to $C05E (same number of bits, so move the softswitch AND trigger from A0/A1/A3 to A1/A2/A3)

I suspect that the first option isn't feasible, but I'm waiting to hear back from Henry.

I've put a UW-specific build on the UltraWarp wiki page (https://wiki.reactivemicro.com/UltraWarp), so the third option is covered.

The fourth option is cleanest from a software perspective, but that's probably not going to happen for obvious reasons.

Which leaves us with number two. I had a quick look at hacking that in, but it will take me some time to unravel what's happening in the configuration logic.

Cheers :)

[ryucats]

... a half-formed thought:

The UltraWarp has a 65c816, the ZipChip and TransWarp have a 65c02. No accelerator has a NMOS 6502.

Absent a way to detect an Ultrawarp, how about adding a CPU configuration flag that gets set like this:

; detect CPU - 0 for 6502, 1 for 65c02, 2 for 65c816
CPU_TYPE:       .byte 0     ; default to NMOS 6502
.setcpu "6502"
                ldx #$00
                txa
.setcpu "65c02"
                bra CMOS      ; undocumented two-byte NOP on NMOS
.setcpu "6502"
                beq DONE
CMOS:           inx
.setcpu "65c816"
                rep #$02    ; NOP on 65C02
.setcpu "6502"
                cmp #$00
                beq DONE
                inx
DONE:           stx CPU_TYPE

If the detected CPU is 6502, don't attempt to disable accelerators at all. If CPU is 65c02, disable ZipChip and TransWarp. If CPU is 65c816, disable UltraWarp.

Does that sound reasonable?

[david-schmidt]

That’s an interesting approach for sure. I don’t remember if we have concerns about a GS and it’s various accelerators are?

[ryucats]

Hmmm. Yes, a ZipGS-equipped machine would throw a spanner into that scheme.

I was operating under the assumption that we want to handle this automatically, but that might not be possible. Would a configuration option ("ACCELERATOR") that cycles between "NONE", "ZIP", "TRANS", and "ULTRA" be acceptable?

[david-schmidt]

I prefer that computers make decisions when they have the information they need; I'd rather not require a human to know which they have. I think the GS can set itself to 1MHz and be done with it, so we might be covered in that case anyway.

[ryucats]

Fair enough, that makes sense.

I moved the accelerator logic to src/client/main.asm, as doing it on-the-fly in RECEIVE_LOOP_PAUSE altered the timing to the point where transfers became unreliable. Disabling at startup and re-enabling at exit seems to be rock-solid.

The below diff appears to do the right thing on my enhanced //e, both with and without the UltraWarp installed. If this looks good, I'll create a pull request.

index 9479b86..5d1c6d1 100644
--- a/src/client/main.asm
+++ b/src/client/main.asm
@@ -41,6 +41,9 @@ entrypoint:
    tsx     ; Get a handle to the stackptr
    stx top_stack   ; Save it for full pops during aborts

+        jsr DETECT_CPU  ; Try to figure out if we're running an accelerator
+        jsr ACCEL_DISABLE ; disable accelerators (if present)
+
    jsr INIT_SCREEN ; Sets up the screen for behaviors we expect
    jsr MAKETBL ; Prepare our CRC tables
    JSR_GET_PREFIX  ; Get our current prefix (ProDOS only)
@@ -170,6 +173,7 @@ KQUIT:
 KQUITENTRY:
    lda #MENU_QUIT
    jsr HILIGHT_MENU
+        jsr ACCEL_ENABLE
    jmp QUIT    ; Head into OS oblivion

 KLEFT:
@@ -354,6 +358,75 @@ BumpA1:
 BumpA1Done:
    rts

+;---------------------------------------------------------
+; ACCEL_* - disable/enable accelerators (if present)
+;---------------------------------------------------------
+ACCEL_DISABLE:
+        lda CPU_TYPE
+        beq AD_EXIT     ; if NMOS, no accel, exit
+        cmp #2          ; if 65816, do UltraWarp
+        beq AD_65816
+        lda #$5a        ; ... otherwise, we're 65C02, do Zip/TransWarp
+        sta $c05a
+        sta $c05a
+        sta $c05a
+        sta $c05a       ; Unlock ZipChip
+ 
+        lda #$00
+        sta $c05a       ; Disable ZipChip       
+        lda #$01
+        sta $C074       ; Disable TransWarp
+        bne AD_EXIT
+
+AD_65816: 
+        lda #$01
+        sta $C05D       ; Slow down UltraWarp
+AD_EXIT:
+        rts
+
+ACCEL_ENABLE:
+        lda CPU_TYPE
+        beq AE_EXIT     ; if NMOS, no accel, exit
+        cmp #2          ; if 65816, do UltraWarp
+        beq AE_65816
+        lda #$00
+        sta $c05b       ; Enable ZipChip
+ 
+        lda #$a5
+        sta $c05a       ; Lock ZipChip
+ 
+        lda #$00        ; Enable TransWarp
+        sta $C074
+        beq AE_EXIT
+
+AE_65816:
+        lda #$01
+        sta $C05C       ; Speed up UltraWarp
+AE_EXIT:
+        rts
+
+;---------------------------------------------------------
+;DETECT_CPU - identify CPU, to try to logic our way through
+;             accelerator detection
+;---------------------------------------------------------
+CPU_TYPE:       .byte 0 ; default to NMOS 6502
+DETECT_CPU:
+.setcpu "6502"
+         lda #0
+.setcpu "65C02"
+         bra :+         ; undocumented two-byte NOP on NMOS
+.setcpu "6502"
+         beq CT_OUT     ; we have an NMOS 6502
+:        inc CPU_TYPE   ; we have a CMOS 65C02/65C816 ...
+.setcpu "65816"
+         lda CPU_TYPE
+         xba            ; exchange B and A (65C816 only)
+         dec            ; A=0 (65C02), A=1 (65C816, via B)
+         xba            ; swap back (again, 65C816 only)
+         inc            ; A=1 (65C02), A=2 (65C816, via B)
+         sta CPU_TYPE
+.setcpu "6502"
+CT_OUT:        rts

 ;---------------------------------------------------------
 ; Table of menu highlighting coordinates
@@ -374,4 +447,4 @@ MENUHITBL:
 CUR_MENU:
    .byte MENU_CONFIG
 PREV_MENU:
-   .byte $ff
\ No newline at end of file
+   .byte $ff
diff --git a/src/client/prodos/ethernet/ethproto.asm b/src/client/prodos/ethernet/ethproto.asm
index 80f9fb1..d899145 100644
--- a/src/client/prodos/ethernet/ethproto.asm
+++ b/src/client/prodos/ethernet/ethproto.asm
@@ -956,31 +956,9 @@ RECEIVE_LOOP_WARM:
    rts

 RECEIVE_LOOP_PAUSE:
-   lda #$5a
-   sta $c05a
-   sta $c05a
-   sta $c05a
-   sta $c05a   ; Unlock ZipChip
-
-   lda #$00
-   sta $c05a   ; Disable ZipChip   
-
-   lda #$01
-   sta $C074   ; Disable TransWarp
-
 PauseValue:
    lda #$7f
    jsr DELAY   ; Wait!
-
-   lda #$00
-   sta $c05b   ; Enable ZipChip
-
-   lda #$a5
-   sta $c05a   ; Lock ZipChip
-
-   lda #$00    ; Enable TransWarp
-   sta $C074
-
    jmp RECEIVE_LOOP_WARM

[ryucats]

It looks like the value at $FE1F can be used to determine if we're running on a GS -- it's RTS on a non-GS and the entry to the machine identification routine on a GS.

So we can do this:

ACCEL_DISABLE:
        lda $FE1F       ; ][ and //e have $60, GS has detect routine
        cmp #$60
        bne AD_65C02    ; if GS, do Zip/TransWarp
        lda CPU_TYPE
        beq AD_EXIT     ; NMOS 6502 isn't accelerated, skip
        cmp #2          ; non-GS 65816 is UltraWarp
        beq AD_65816
AD_65C02: 
        ...

(ACCEL_ENABLE is nearly identical)

... in the accelerator enable/disable routine and cover all bases thereby.

?

[david-schmidt]

That looks like a good way to be generic, yes.

[ryucats]

Thanks for that. I've created pull request #181.

[david-schmidt]

Great, I think I can test ZipChip-ness with a //c+. Thanks for the legwork!