breakintoprogram / agon-mos

Official AGON QUARK Firmware: eZ80 MOS
MIT License
73 stars 16 forks source link

UART0 full-duplex CTS/RTS #125

Open S0urceror opened 6 months ago

S0urceror commented 6 months ago

The current version does not support full-duplex CTS/RTS. It does support RTS (vdp) and CTS (mos) but not vice-versa. So fast transfers from EZ80 to ESP32 are guarded but the other way around not. If you want to send a lot of data from ESP32 to EZ80 you get buffer overruns quickly.

Relevant code in VDP:

void setupVDPProtocol() {
    VDPSerial.end();
    VDPSerial.setRxBufferSize(UART_RX_SIZE);                    // Can't be called when running
    VDPSerial.begin(UART_BR, SERIAL_8N1, UART_RX, UART_TX);
    VDPSerial.setHwFlowCtrlMode(HW_FLOWCTRL_RTS, 64);           // Can be called whenever
    VDPSerial.setPins(UART_NA, UART_NA, UART_CTS, UART_RTS);    // Must be called after begin
}

Should be:

void setupVDPProtocol() {
    VDPSerial.end();
    VDPSerial.setRxBufferSize(UART_RX_SIZE);                    // Can't be called when running
    VDPSerial.begin(UART_BR, SERIAL_8N1, UART_RX, UART_TX);
    VDPSerial.setHwFlowCtrlMode(HW_FLOWCTRL_CTS_RTS, 64);           // Can be called whenever
    VDPSerial.setPins(UART_NA, UART_NA, UART_CTS, UART_RTS);    // Must be called after begin
}

Relevant code in MOS:

    UCHAR   pins = PORTPIN_ZERO | PORTPIN_ONE;                      // The transmit and receive pins                                            
    serialFlags &= 0xF0;
    SETREG(PD_DDR, pins);                                           // Set Port D bits 0, 1 (TX. RX) for alternate function.
    RESETREG(PD_ALT1, pins);
    SETREG(PD_ALT2, pins);
    if(pUART->flowControl == FCTL_HW) {
        SETREG(PD_DDR, PORTPIN_THREE);                              // Set Port D bit 3 (CTS) for input
        RESETREG(PD_ALT1, PORTPIN_THREE);
        RESETREG(PD_ALT2, PORTPIN_THREE);
        serialFlags |= 0x02;
    }
        ...
        UART0_MCTL = 0x00;                                              // Bring modem control register to reset value

And:

UART0_wait_CTS:     GET_GPIO    PD_DR, 8        ; Check Port D, bit 3 (CTS)
            JR      NZ, UART0_wait_CTS
            RET

Should be:

    UCHAR   pins = PORTPIN_ZERO | PORTPIN_ONE;                      // The transmit and receive pins                                            
    serialFlags &= 0xF0;
    SETREG(PD_DDR, pins);                                           // Set Port D bits 0, 1 (TX. RX) for alternate function.
    RESETREG(PD_ALT1, pins);
    SETREG(PD_ALT2, pins);
    if(pUART->flowControl == FCTL_HW) {
        SETREG(PD_DDR, PORTPIN_THREE|PORTPIN_FOUR);             // Set Port D bit 3, 4 (CTS, RTS) for alternate function
        RESETREG(PD_ALT1, PORTPIN_THREE|PORTPIN_FOUR);
        SETREG(PD_ALT2, PORTPIN_THREE|PORTPIN_FOUR);
        serialFlags |= 0x02;
    }
        ...
        UART0_MCTL = 0x02;                                              // Enable RTS

And:

UART0_wait_CTS:     in0 a, (UART0_MSR)
            bit 4,a ; check inverted CTS bit, 1 = CTS, 0 = NOT CTS (clear to send)
            JR      Z, UART0_wait_CTS
            RET
stevesims commented 6 months ago

this is cool stuff, and should be useful, especially when we add in facilities to send more data back from the VDP to the eZ80

one quick question - and forgive my ignorance of comms things... this is a pair of PRs, so does that mean that both are needed to be implemented simultaneously?

if so then that could be problematic for upgrading - feels like it means that both MOS and VDP need to be updated at the same time for this to work, which may not be practical

perhaps this may mean that we need to have a command to enable proper full-duplex comms?

S0urceror commented 6 months ago

Yes, you are right we have to be careful in upgrading. Because both sides have to work full-duplex to make it work.

We can however do a graceful fallback. The VDUStreamProcessor::wait_eZ80() is waiting for the EZ80. We could extend it to go back to half-duplex if there is not a good handshake with MOS within a certain time-interval. Similar to how it goes back to a lower baudrate if things don't work as expected.

I can have a look at that if you think this is the direction to go. This would allow to upgrade VDP without a MOS upgrade. The other way around to do a MOS upgrade without VDP would require changes on the MOS side.

S0urceror commented 5 months ago

Okay, checked things:

So you cannot have new VDP with default MOS and vice-versa.

Proposed solution direction:

This allows people to first upgrade VDP and experience no problems. Then continue to flash a new MOS and continue without problems.

Let me know what you think of this. I'll make the necessary changes to VDP and test things with both the default MOS and the hw-cts-rts MOS.

stevesims commented 5 months ago

so as it goes, the "general poll" message that MOS sends to the VDP has one byte which gets sent as part of the handshake, which the VDP then echos back as part of the PACKET_GP message

up until right now, all versions of MOS send a 1 byte in that VDU command (it's aVDU 23,0,&80,1)

so what we could do instead is use a different sequence, and a different response

MOS could send a 2 byte instead. VDP could detect that and if initialised == false instead of just echoing it back respond with a 3 to indicate it supports hw-rts-cts. Once the 3 response has been sent, VDP should swap to hw-rts-cts, and when the 3 response is received MOS should also swap.

when initialised == true VDP would revert to doing its current simple echo for the "general poll" command - not that anything is really likely to be using it, but you never know

(if VDP receives a 1 then you know you have an old MOS that won't do hw-rts-cts)

this way there's no need to add in a new VDP protocol packet - we just use what's already there

S0urceror commented 5 months ago

:-) that was initially what I was thinking as well before settling on a separate packet. I'll give it some thought and create a proof of concept and let you know.

S0urceror commented 5 months ago

Made a couple of changes and tested things.

If you have the full-duplex capable VDP it will first start half-duplex and wait for the PACKET_GP. If PACKET_GP=1 this signals a half-duplex MOS. If PACKET_GP>1 this signals full-duplex. The full-duplex capable vdp will then switch to full-duplex mode and answer with a normal echo packet.

Since most people use the flash tool this assume they first upgrade VDP before upgrading MOS. And this allows for a clean upgrade path. I tested this and this works beautifully.

TurboVega commented 3 months ago

Is PORTPIN_FOUR the correct pin? The schematic shows DTR0 on that pin, not RTS0, which is on PORTPIN_TWO.

TurboVega commented 3 months ago

The changes that I have for BDPP on MOS and VDP include full duplex RTS and CTS, but the EZ80 RTS and ESP32 CTS are not setup until BDPP is enabled. The startup is using ESP32 RTS to EZ80 CTS only, albeit with several fixes to MOS, because it was using the wrong register and testing the value incorrectly, as was also discovered by @S0urceror, looking at the code changes that he posted above. Since BDPP also uses the initial version exchange to know what to do, we can modify that code to handle setting up the EZ80 RTS to ESP32 CTS, based on the protocol versions, even though BDPP is initially disabled on startup. Thus, I suggest that we just fix full duplex by merging the BDPP changes, after adding the mentioned addition, rather than fixing FD separately. Doing so may make any existing merge request based on this particular issue ticket moot.