Skip to content

Improve auto-joypad polling timing #351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 5, 2025
Merged

Improve auto-joypad polling timing #351

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
51bf15a
improve autojoypad timing
Morilli Jun 22, 2025
a60c9e4
fix spellcraft
Morilli Jun 22, 2025
6e07142
re-fix test 14
Morilli Jun 22, 2025
d8ae2aa
implement additional latch logic
Morilli Jun 22, 2025
45fa018
bump SerializerVersion
Morilli Jun 28, 2025
e2d7b51
rename variables for consistency with existing code
Morilli Jun 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions bsnes/sfc/cpu/cpu.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,12 @@ struct CPU : Processor::WDC65816, Thread, PPUcounter {
bool hdmaMode = 0; //0 = init, 1 = run

uint autoJoypadCounter = 33; //state machine; 4224 / 128 = 33 (inactive)

uint2 autoJoypadPort0 = 0;
uint2 autoJoypadPort1 = 0;

bool cpuLatch = false;
bool autoJoypadLatch = false;
} status;

struct IO {
Expand Down
15 changes: 12 additions & 3 deletions bsnes/sfc/cpu/io.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,22 @@ auto CPU::writeCPU(uint addr, uint8 data) -> void {
//bit 0 is shared between JOYSER0 and JOYSER1:
//strobing $4016.d0 affects both controller port latches.
//$4017 bit 0 writes are ignored.
controllerPort1.device->latch(data & 1);
controllerPort2.device->latch(data & 1);
status.cpuLatch = data & 1;
controllerPort1.device->latch(status.autoJoypadLatch | status.cpuLatch);
controllerPort2.device->latch(status.autoJoypadLatch | status.cpuLatch);
return;

case 0x4200: //NMITIMEN
io.autoJoypadPoll = data & 1;
if(!io.autoJoypadPoll) status.autoJoypadCounter = 33; // Disable auto-joypad read
if(status.autoJoypadCounter == 0) {
// allow controller latches during this time
status.autoJoypadLatch = io.autoJoypadPoll;
controllerPort1.device->latch(status.autoJoypadLatch | status.cpuLatch);
controllerPort2.device->latch(status.autoJoypadLatch | status.cpuLatch);
} else if (!io.autoJoypadPoll && status.autoJoypadCounter >= 2) {
status.autoJoypadCounter = 33;
}

nmitimenUpdate(data);
return;

Expand Down
6 changes: 6 additions & 0 deletions bsnes/sfc/cpu/serialization.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ auto CPU::serialize(serializer& s) -> void {

s.integer(status.autoJoypadCounter);

s.integer(status.autoJoypadPort0);
s.integer(status.autoJoypadPort1);

s.boolean(status.cpuLatch);
s.boolean(status.autoJoypadLatch);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change what's being serialised, we should also bump the serialisation version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, and that's one thing I was contemplating while making the serialization and struct changes. But I looked at the last time serialization was changed (34af95d) and the version wasn't bumped after that either. Should I just bump it to something like 115.1?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the previous idea was that the serialisation version only needed to be changed once per release, so if it had been changed since the previous release there was no need to change it again. Given releases are a lot further apart now, it seems prudent to change the number more regularly.

115.1 seems like a good choice, yeah.

s.integer(io.wramAddress);

s.boolean(io.hirqEnable);
Expand Down
61 changes: 35 additions & 26 deletions bsnes/sfc/cpu/timing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,45 +202,54 @@ auto CPU::dmaEdge() -> void {

//called every 128 clocks from inside the CPU::stepOnce() function
auto CPU::joypadEdge() -> void {
//it is not yet confirmed if polling can be stopped early and/or (re)started later
if(!io.autoJoypadPoll) return;

if(vcounter() == ppu.vdisp() && hcounter() >= 130 && hcounter() <= 256) {
if(vcounter() == ppu.vdisp() && (counter.cpu & 255) == 0 && hcounter() >= 130 && hcounter() <= 384) {
//begin new polling sequence
status.autoJoypadCounter = 0;
}
} else {
//stop after polling has been completed for this frame
if(status.autoJoypadCounter >= 33) return;

//stop after polling has been completed for this frame
if(status.autoJoypadCounter >= 33) return;
status.autoJoypadCounter++;
}

if(status.autoJoypadCounter == 0) {
//latch controller states on the first polling cycle
controllerPort1.device->latch(1);
controllerPort2.device->latch(1);
status.autoJoypadLatch = io.autoJoypadPoll;
controllerPort1.device->latch(status.autoJoypadLatch | status.cpuLatch);
controllerPort2.device->latch(status.autoJoypadLatch | status.cpuLatch);
if(io.autoJoypadPoll) {
//shift registers are cleared to zero at start of auto-joypad polling
io.joy1 = 0;
io.joy2 = 0;
io.joy3 = 0;
io.joy4 = 0;
}
}

if(status.autoJoypadCounter == 1) {
//release latch and begin reading on the second cycle
controllerPort1.device->latch(0);
controllerPort2.device->latch(0);
status.autoJoypadLatch = 0;
controllerPort1.device->latch(status.autoJoypadLatch | status.cpuLatch);
controllerPort2.device->latch(status.autoJoypadLatch | status.cpuLatch);
}

//shift registers are cleared to zero at start of auto-joypad polling
io.joy1 = 0;
io.joy2 = 0;
io.joy3 = 0;
io.joy4 = 0;
if(status.autoJoypadCounter != 1 && !io.autoJoypadPoll) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the status.autoJoypadCounter != 1 condition here? If the counter is 0 or 2-33, and auto-polling is disabled, we can skip the rest of the process, but if the counter is 1, we just wait until the counter becomes 2 and then disable auto-polling? If a game enables auto-polling after the counter ticks 1, and we've skipped the latching inputs and clearing the shift registers, does the SNES start shifting a new copy of the previously-latched inputs into the shift registers? Does this not happen if the game enables auto-polling on ticks 2 or later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right that this seems peculiar but it does appear to be accurate. Without this condition the tests 12-blip-autojoy-timing-test and 13-blip-autojoy-latches-joypad-test no longer match hardware results.

The implication of this condition is that disabling auto-joypad polling during this one cycle has no effect, so it could for example be toggled off and toggled on again without affecting the auto-joypad polling sequence at all during this specific timing window.

As far as I understand that's precisely what the blip-autojoy-timing-test intends to test: It waits until HVBJOY reports auto-joypad polling to be active, disables auto-joypad polling per NMITIMEN write, waits a certain amount of cpu cycles and then re-enables it. Of course this depends on HVBJOY and NMITIMEN behavior, but changing those as well causes other tests to break which leads me to believe that this combination is most accurate.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fiskbit and I noticed that temporary disabling (NMITIMEN = 0) then quickly re-enabling auto-joy (NMITIMEN = 1) would not stop automatic joypad reading.

12-blip-autojoy-timing-test was used to determine if is a 128-clock or 256-clock window from when HVBJOY reports auto-joypad active. My results indicate it is a 256-clock window. If the enable auto-joy NMITIMEN write is > 256-clocks after auto-joypad starts then automatic joypad read stops.

Quoting my notes from that day:

It's matches what I expected for the 3chip. I didn't know if it was a 128 clock or 256 clock before autojoy could be re-enabled.
If my math is right, there's N + 6 fastROM cycles (48 mcycles) between HVBJOY reports auto-joy active and the NMITIMEN=1 write.

  • 206 delay = 254 m-cycles
  • 208 delay = 256 m-cycles.

For the 13-blip-autojoy-latches-joypad-test, Fiskbit asked for a test that checks if the S-CPU latches the controller twice if the NMITIMEN auto-joypad enable flag is blipped (disabled then quickly enabled). Details about the test can be found in the test's source code.

// if auto-joypad polling is disabled at this point skip the rest of the polling
status.autoJoypadCounter = 33;
return;
}

if(status.autoJoypadCounter >= 2 && !(status.autoJoypadCounter & 1)) {
if(status.autoJoypadCounter >= 2) {
//sixteen bits are shifted into joy{1-4}, one bit per 256 clocks
uint2 port0 = controllerPort1.device->data();
uint2 port1 = controllerPort2.device->data();

io.joy1 = io.joy1 << 1 | port0.bit(0);
io.joy2 = io.joy2 << 1 | port1.bit(0);
io.joy3 = io.joy3 << 1 | port0.bit(1);
io.joy4 = io.joy4 << 1 | port1.bit(1);
//the bits are read on one 128-clock cycle and written on the next
if ((status.autoJoypadCounter & 1) == 0) {
status.autoJoypadPort0 = controllerPort1.device->data();
status.autoJoypadPort1 = controllerPort2.device->data();
} else {
io.joy1 = io.joy1 << 1 | status.autoJoypadPort0.bit(0);
io.joy2 = io.joy2 << 1 | status.autoJoypadPort1.bit(0);
io.joy3 = io.joy3 << 1 | status.autoJoypadPort0.bit(1);
io.joy4 = io.joy4 << 1 | status.autoJoypadPort1.bit(1);
}
}

status.autoJoypadCounter++;
}