naTmeg / ScriptedAmigaEmulator

Amiga Emulator in javascript and HTML5
331 stars 63 forks source link

CIA timer acknowledgment bug #32

Open dirkwhoffmann opened 5 years ago

dirkwhoffmann commented 5 years ago

Hi Rupert,

while trying to wake up the Aros cat in vAmiga, I think I’ve found a bug in SAE. It is about test case timer5 in the vAmigaTS repo:

https://github.com/dirkwhoffmann/vAmigaTS/tree/master/CIA/Timer

SAE looks like this:

timer5_SAE_0 9 11

UAE looks like this (my A500 looks like this as well):

timer5_UAE

The test starts timer CIAA::B in the vsync handler. When the timer underflows, a level 2 interrupt is issued. In the interrupt handler, the following happens:

   lea CUSTOM,a5

    ; Change the background color to visualize where we are
    move.w  #$F00,COLOR00(a5) 
        
    ; Acknowledge the IRQ by clearing the IRQ bit in INTREQ
    ; Note that the CIA's IRQ line is still low.
    move.w  #$8,INTREQ(a5)

    ; Acknowledge the IRQ by reading the CIA ICR reg
    move.b  $BFED01,d0 

    ; Change the background color again
    move.w  #$FFF,COLOR00(a5) 

Each red bar indicates that the interrupt handler has been called.

The test picture reveals that the interrupt is triggered twice in SAE (it’s not acknowledged in the first run of the interrupt handler).

If the command order is reversed like in test timer3 (INTREQ is written after $BFED01), SAE is doing the right thing. It then acknowledges the IRQ in the first run of the interrupt handler. On a real machine, it doesn’t matter in which order INTREQ or ICR are written to. It’s only important that the IRQ bits are cleared in both registers (otherweise the IRQ retriggers).

I guess that SAE doesn’t reset the bit in INTREQ if the CIA IRQ line is still down (this would explain what we see), but I didn’t check the code.

In vAmiga, I’ve implemented it this way:

uint16_t
Paula::peekINTREQR()
{
    uint16_t result = intreq;

    if (amiga->ciaA.irqPin() == 0) SET_BIT(result, 3);
    if (amiga->ciaB.irqPin() == 0) SET_BIT(result, 13);

    debug(INT_DEBUG, "peekINTREQR(): %x\n", result);

    return result;
}

int
Paula::interruptLevel()
{
    if (intena & 0x4000) {

        uint16_t mask = intreq;

        if (amiga->ciaA.irqPin() == 0) SET_BIT(mask, 3);
        if (amiga->ciaB.irqPin() == 0) SET_BIT(mask, 13);

        mask &= intena;

        if (mask & 0b0110000000000000) return 6;
        if (mask & 0b0001100000000000) return 5;
        if (mask & 0b0000011110000000) return 4;
        if (mask & 0b0000000001110000) return 3;
        if (mask & 0b0000000000001000) return 2;
        if (mask & 0b0000000000000111) return 1;
    }

    return 0;
}

I don’t know if this is 100% correct, but it makes vAmiga pass all five timer tests (timer1 to timer5).

naTmeg commented 5 years ago

Hey Dirk,

many thanks for this great report. I've looked into it, but didn't found a solution yet..