sfc: Really fix automatic joypad polling, for sure.

In commit e422ddc, Jonas Quinn updated higan's implementation of the auto-
joypad-poll mode. Those fixes were ported to bsnes in commit 39c37ec, but a
problem was found with the game "SpellCraft - Aspects of Valor", fixed in bsnes
in commit 22066b9. That fix was back-ported to higan in commit 9cd4b04, but
that change caused a regression in "Secret of Mana" (issue higan-emu#170).

This commit (helpfully provided by Jonas Quinn once again) re-ports the bsnes
changes back to higan, and the problems should be solved once and for all.

Fixes higan-emu#170.

higan/sfc/cpu/cpu.hpp

@@ -127,9 +127,7 @@ struct CPU : WDC65816, Thread, PPUcounter {
     bool hdmaPending = 0;
     bool hdmaMode = 0;  //0 = init, 1 = run
 
-    bool autoJoypadActive = 0;
-    bool autoJoypadLatch = 0;
-    uint autoJoypadCounter = 0;
+    uint autoJoypadCounter = 33;  //state machine; 4224 / 128 = 33 (inactive)
   } status;
 
   struct IO {

higan/sfc/cpu/io.cpp

@@ -126,7 +126,7 @@ auto CPU::writeCPU(uint24 address, uint8 data) -> void {
 
   case 0x4200:  //NMITIMEN
     io.autoJoypadPoll = data.bit(0);
-    if(!io.autoJoypadPoll) status.autoJoypadLatch = 0;
+    if(!io.autoJoypadPoll) status.autoJoypadCounter = 33; // Disable auto-joypad read
     nmitimenUpdate(data);
     return;
 

higan/sfc/cpu/serialization.cpp

@@ -41,8 +41,6 @@ auto CPU::serialize(serializer& s) -> void {
   s.integer(status.hdmaPending);
   s.integer(status.hdmaMode);
 
-  s.integer(status.autoJoypadActive);
-  s.integer(status.autoJoypadLatch);
   s.integer(status.autoJoypadCounter);
 
   s.integer(io.wramAddress);

higan/sfc/cpu/timing.cpp

@@ -5,7 +5,7 @@ inline auto CPU::dmaCounter() const -> uint {
 
 //joypad auto-poll clock divider
 inline auto CPU::joypadCounter() const -> uint {
-  return counter.cpu & 255;
+  return counter.cpu & 127;
 }
 
 auto CPU::step(uint clocks) -> void {
@@ -61,8 +61,7 @@ auto CPU::scanline() -> void {
     status.hdmaSetupPosition = (io.version == 1 ? 12 + 8 - dmaCounter() : 12 + dmaCounter());
     status.hdmaSetupTriggered = 0;
 
-    status.autoJoypadLatch = 0;
-    status.autoJoypadCounter = 0;
+    status.autoJoypadCounter = 33;  //33 = inactive
   }
 
   //DRAM refresh occurs once every scanline
@@ -137,7 +136,7 @@ alwaysinline auto CPU::dmaEdge() -> void {
   }
 }
 
-//called every 256 clocks; see CPU::step()
+//called every 128 clocks from inside the CPU::stepOnce() function
 alwaysinline auto CPU::joypadEdge() -> void {
   // Auto-Joypad-Read begins between dots 32.5 and 95.5 of the first V-Blank scanline,
   // and ends 4224 master cycles later. (Anomie's timing doc)
@@ -148,43 +147,48 @@ alwaysinline auto CPU::joypadEdge() -> void {
   // Todo: 4224 cycles would be 33*128 and not 16*256, hardware testing shows that higan ends auto-joypad read too early
   // Todo: What happens to auto-joypad read when you read/write 4816/4817 while it is in progress?
 
-  if(vcounter() == ppu.vdisp() && hcounter() >= 130 && hcounter() <= 384) {
-    // start auto joypad read on the first scanline of vblank
-    if(!status.autoJoypadCounter) {
-      status.autoJoypadLatch = io.autoJoypadPoll;
-
-      if(status.autoJoypadLatch) {
-        //shift registers are cleared at start of auto joypad polling
-        io.joy1 = 0;
-        io.joy2 = 0;
-        io.joy3 = 0;
-        io.joy4 = 0;
-      }
-    }
+  if(!io.autoJoypadPoll) return;
 
-    if(status.autoJoypadLatch && !status.autoJoypadCounter) {
-      controllerPort1.latch(1);
-      controllerPort2.latch(1);
-      controllerPort1.latch(0);
-      controllerPort2.latch(0);
-    }
+  //polling can only be started once per frame on the first line of vblank between hdot 130 and 384
+  //Todo: 256 is used here because the function is called every 128 clocks, instead of 256
+  //      this needs to be tested
+  //it cannot be restarted, once disabled, for the frame
+  if(vcounter() == ppu.vdisp() && hcounter() >= 130 && hcounter() <= 256) {
+    //begin new polling sequence
+    status.autoJoypadCounter = 0;
   }
 
-  if(!status.autoJoypadLatch) {
-    status.autoJoypadActive = 0;
-  } else {
-    status.autoJoypadActive = status.autoJoypadCounter <= 15;
+  //stop after polling has been completed for this frame
+  if(status.autoJoypadCounter >= 33) return;
 
-    if(status.autoJoypadActive) {
-      uint2 port0 = controllerPort1.data();
-      uint2 port1 = controllerPort2.data();
+  if(status.autoJoypadCounter == 0) {
+    //latch controller states on the first polling cycle
+    controllerPort1.latch(1);
+    controllerPort2.latch(1);
+  }
 
-      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);
-    }
+  if(status.autoJoypadCounter == 1) {
+    //release latch and begin reading on the second cycle
+    controllerPort1.latch(0);
+    controllerPort2.latch(0);
 
-    status.autoJoypadCounter++;
+    //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 >= 2 && !(status.autoJoypadCounter & 1)) {
+    //sixteen bits are shifted into joy{1-4}, one bit per 256 clocks
+    uint2 port0 = controllerPort1.data();
+    uint2 port1 = controllerPort2.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);
+  }
+
+  status.autoJoypadCounter++;
 }