INPTx registers not working in all cases

[sa666666]

The recent addition of the PaddleReader class allows paddle games to work correctly. However, other games that use those inputs are now broken. In particular, ROMs using keypad controller (Alpha Beam with Ernie). Also, Compumate ROMs make extensive use of these registers, and the display isn't even showing up there.

I'm not sure if there's a bug in the PaddleReader class itself, or a difference in how the relevant parts of Stella now interact with the new code (in particular, Keyboard.cxx and CompuMate.cxx). I suspect the latter.

[DirtyHairy]

Thanks, @sa666666 , I'll look into it.

[DirtyHairy]

Fixed in afae00a .

This suggests a possible keypad emulation bug that also affects the old core: writing value A to SWTCHA and later value B without reading from INPT[0123] between the two writes will have no effect on the emulation. On real hardware, the first read will potentially discharge the capacitor (depending on keyboard state) and thus affect subsequent reads even after B is written. The new core does slightly better than the old one as it updates the readout circuit state each frame, but this won't help if both writes happen within the same frame. Fixing this properly and efficiently would require crosstalk between RIOT and TIA (i.e. updating the readout circuit state before SWTCHA is written).

[sa666666]

We should consider your suggested improvement for a future release; I want to have the emulation as accurate as possible. Obviously there is much more to do first, of course.

[DirtyHairy]

I think this has come back from the grave to haunt us

[DirtyHairy]

It seems that at least BASIC is affected, and a bit of printf debugging suggests that the issue is indeed the order in which the paddle resistance is updated: multiple writes to SWCHA happen per frame.

[sa666666]

Many other controllers use paddle inputs too, and they seem to be fine. So this makes me think the problem is now in Keyboard.cxx.

[sa666666]

Synthcart plays a different sound on each key, and uses the keypad controller. For me, q/w/z/x don't work, which correspond to keypad 4/5/*/0, respectively. The corresponding keys for the right keypad aren't working either. What I don't understand is why some people are getting 6 keys not working, and I'm only getting 4.

synthcart.zip

[DirtyHairy]

I think the solution is to update the paddle readout circuit whenever an analog pin changes state. I am working on this, but I am not sure whether I'll finish it today 😏

The issue occurs only for complex controllers which configure SWCHA as output to drive a matrix circuit, and even for those, only games which change SWCHA multiple times per frame are affected (as paddle updates occur once every frame).

[sa666666]

Can we perhaps try to fix this at a lower level, so it doesn't come back to haunt us again 😄

Also, would it be worthwhile to now implement the crosstalk between TIA and RIOT that you mentioned above, so this area of the code is fixed for good (or is that what you're already doing)?

[DirtyHairy]

That's basically what I am doing: I am modifying Controller to force all analog pin updates through a method that calls a callback after updating the pin state. The callback will be wired to the TIA, which in turn updates the readout circuit. I think this is the right place --- RIOT was a bit too enthusastic I think 😄

[sa666666]

The problem of course is that Controller::write() and friends are empty virtual methods, only implemented by classes that need them. So maybe that will force the classes overriding them to do this instead??

[DirtyHairy]

I added a separate protected method updateAnalogPin. Only analog updates are routed through this, and it is separate from Controller::write and company ;)

[DirtyHairy]

I was a bit hasty; there is something else going wrong, and I am going to need considerably more debugging in order to find out what it is. Later this week...

[DirtyHairy]

Got it! Actually, this is not a bug at all, but a lesson in electronics 😏 Typing it out is too much for tonight though, so I'll add more detail (and commit a fix) later.

[DirtyHairy]

Time for a break (I skipped lunch today) and elaborate a bit 😏

The analog readout circuit
In this circuit, a capacitor is connected to pin 9 (5) (with a small series resistor). While bit 7 of VBLANK is asserted, the capacitor is pulled to ground and discharged. After bit 7 has been cleared, the capacitor starts charging through the connection to pin 9 (5) and, after the voltage reaches a threshold, INPTx will read 1.

Paddles
For the paddle controller, the paddle potentiometer is used as a variable resistor (not as a divider) and connects pin 9 (5) to pin 7 (+5 Volts). Changing the paddle position changes the resistance and such the rate at which the cap charges.

Keyboard
The keyboard controller connects pin 9 (5) to pin 7 (+5 Volts) via a pullup resistor and to pin 1 / 2 / 3 / 4 (the RIOT GPIO pins configured as output) via the key matrix (the GPIO pin selects the row). In order to read one of the keys you set the corresponding GPIO pin to 0 (pulling it to ground) and (optionally) discharge the cap. If the key is pressed, the cap is connected to ground and stays discharged (or discharges) -> INPTx reads 0. If it is unpressed, the cap charges via the pullup -> INPTx reads 1.

The important part is that this is different from connecting pin 5 (9) to +5 Volts via a potentiometer. For a paddle controller, the cap charge will always increase after VBLANK is cleared, while reading a pressed button on the kids controller will actively discharge it even if it was charged before.

Stella (and all other emulators, I guess) model analog input with a single resistance value which fully describes the paddles but is inadequate for the keyboard. A better choice would be voltage and resistance, and the only precise way would be to model the whole circuit within the controller itself 😄

Instead of dumping the cap if the button is pressed, Stella maxes out the resistance value. The old code contains a check whether the resistance is maxed out and always returns 0 in this case. This is not a correct physical description as the resistance can become large for other reasons as well (in which any existing charge is held if the voltage is unchanged), but it does the job for the actual controller implementations.

The new code calculates the charge buildup on the cap between resistance changes. Maxing out the resistance immediatelly after VBLANK is cleared will prevent the cap from charging, but it will not discharge it. This is why some games work, and some don't. In the best of all worlds, the controller would supply a voltage and a resistance to pin 9 (5), and the cap would either charge or discharge accordingly. Homever, I think the real-world solution is to discharge the cap if the "resistance" is maxed out. Technically, this is not correct and could fail for actual high resistances, but those never occur for any real-world controller, so I think it is good enough 😏 I have tested this and it works, but I also have made the change I talked about before which I want to clean up and commit, too --- this will resolve any timing issues in updating the circuit state for good.

[sa666666]

Sounds like an excellent analysis of the problem. I particularly like the last sentence 'this will resolve any timing issues in updating the circuit state for good.' 😄

[thrust26]

Something for 5.0.1 I suppose? 😉

[ale-79]

Other controller using the paddle lines are Genesis pads and the Booster Grip.

It seems that genesis pad support is broken too in 5.0 (I just tried a few of the 2 button hacks from this thread: http://atariage.com/forums/topic/158430-rom-hacks-to-support-2-buttons-with-genesis-controllers)

On Genesis pad, pin 9 is connected to an ouptut of an IC (74157 in the original 3-buttons controller) that is at logic "0" if button "C" is pressed and "1" otherwise.

The Booster grip, just connects the paddle lines to pin 7 (+5v) when the trigger (pin 5) or top button (pin 9) are pressed, and leave them unconnected otherwise.

[sa666666]

Definitely. I haven't done a point-0 release yet that didn't have something we missed. But considering the huge number of features added, and the amount of code churn in 5.0, missing only just this one thing is very good.

I suggest we test all types of controllers for the next release, because most of the other ones use the INPTx registers in some way too.

Like @DirtyHairy, the ROM I used for testing was Alpha Beam with Ernie, and of course that ROM worked fine 👿

[DirtyHairy]

Implemented and fixed in 0d5d3de

[sa666666]

Good work on this; I didn't expect it would need to be so involved. But it's really nice that this is now emulated correctly at a lower level. It works with all the test ROMs I tried, so I think we're done. Now, would you consider looking at issue #176 before the next point release? 😄

[thrust26]

Sorry, but I think we need another look.

The attached binary worked until 5.0 and stopped working with 5.0.1 where this fix was added. And it is reported to work on real hardware (@sa666666 please double check). The problem is that you cannot move the ships.

https://atariage.com/forums/topic/318654-alpha-beam-proto-not-working-with-stella/

Alpha Beam (12-22-82).zip

[thrust26]

Changing MIN_RESISTANCE from 5600 back to 0 fixes the issue. Where did that number come from?

Update:
Pin 5 is not affected, only Pin 9. And the latter works up to a value of 1940. At 1941 it stops working. No idea what this means.

[thrust26]

@DirtyHairy Can we close this one again now?