indigodarkwolf / box16

A fork of the official X16 emulator, converted to C++20 and with a bunch of features tweaked and added.
MIT License
41 stars 19 forks source link

When reading from PRA ($f001\$f00f) the data direction register should be considered for all bits. #63

Closed Yazwh0 closed 1 year ago

Yazwh0 commented 1 year ago

It only looks like it's considered for the I2C bits, not the joystick data.

In via.cpp:

case 1: // PA
case 15:
    ps2_autostep(0);
    if (!debug) {
        via_clear_pra_irqs(&via[0]);
    }
    if (via[0].registers[11] & 1) {
        // CA1 is currently not connected to anything (?)
        return 0;
    } else {
        return (~via[0].registers[3] & ps2_port[0].out) |
                     (via[0].registers[3] & ps2_port[0].in) | Joystick_data;
    }
indigodarkwolf commented 1 year ago

You confused me briefly with the old code reference, the current code looks like this:

        case 1: // PA
        case 15:
            i2c_step();
            if (!debug) {
                via_clear_pra_irqs(&via[0]);
            }
            if (via[0].registers[11] & 1) {
                // CA1 is currently not connected to anything (?)
                return 0;
            } else {
                return (~via[0].registers[3] & i2c_port.data_out) | // I2C Data: PA0=1 if DDR bit is 0 (input) and data_out is 1; usage of data_out and data_in is a bit confusing...
                       (via[0].registers[3] & i2c_port.data_in) |   // I2C Data: PA0=1 if DDR bit is 1 (output) and data_in is 1
                       (~via[0].registers[3] & I2C_CLK_MASK) |      // I2C Clock: PA1=1 if DDR bit is 0 (input), simulating an input pull-up
                       (via[0].registers[3] & i2c_port.clk_in) |    // I2C Clock: PA1=1 if DDR bit is 1 (output) and clk_in is 1, simulating a pin driven by the VIA
                       Joystick_data;
            }

But I agree it appears that it does not respect the direction for joystick data. I assume that means the fix should be to bitwise-AND the Joystick_data with ~via[0].registers[3].

indigodarkwolf commented 1 year ago

The same error appears to occur at https://github.com/commanderx16/x16-emulator/blob/master/src/via.c#L292, so it may be worth filing a bug over there as well.

Yazwh0 commented 1 year ago

Already done. https://github.com/commanderx16/x16-emulator/issues/450

indigodarkwolf commented 1 year ago

Should be fixed by 4871184defd4a01cb28d684ce8cc207b5f099661.