RTC state machine improvements (#3459)

* Try to correct the RTC state machine to only operate on edges Unsure if all of this is entirely right (haven't ran any tests to confirm behavior here). Fixes RTC within Pokemon games (and maybe other games) due to the added write latch code adding a _readPins call. * More correct behavior based on testing * Move this init to align to struct * Correctly handle rtc output on falling edges rather than raising edges also correctly handle rtc output in general, even in cases outside of an rtc read cmd _outputPins needed to be corrected here, it shouldn't be reading gpioBase here... * Simplify * More RTC state machine fixes Separate out command start and command write data processing Command start processing happens again if the command magic is invalid (note: doesn't apply to the unmapped command 5) Ensure command data processing loops Output 1s for commands with no actual output * Put SIO output in states * Try to correct light sensor too * inc state version * fix reserved names
2025-04-29 22:48:23 -07:00 · 2025-04-29 22:48:23 -07:00 · c5cddc0407
commit c5cddc0407
parent 4f9ad3a162
4 changed files with 129 additions and 110 deletions
--- a/include/mgba/internal/gba/cart/gpio.h
+++ b/include/mgba/internal/gba/cart/gpio.h
@ -49,10 +49,11 @@ DECL_BIT(RTCCommandData, Reading, 7);

 struct GBARTC {
 	int32_t bytesRemaining;
-	int32_t transferStep;
 	int32_t bitsRead;
 	int32_t bits;
 	int32_t commandActive;
+	bool sckEdge;
+	bool sioOutput;
 	RTCCommandData command;
 	RTCControl control;
 	uint8_t time[7];
--- a/include/mgba/internal/gba/serialize.h
+++ b/include/mgba/internal/gba/serialize.h
@ -168,7 +168,9 @@ mLOG_DECLARE_CATEGORY(GBA_STATE);
 * | 0x00290: Pin state
 * | 0x00291: Write latch
 * | 0x00292: Direction state
- * | 0x00293: Reserved
+ * | 0x00293: Flags
+ *   | bit 0: RTC SIO output
+ *   | bit 1 - 7: Reserved
 * | 0x00294 - 0x002B6: RTC state (see hardware.h for format)
 * | 0x002B7 - 0x002B7: GPIO devices
 *   | bit 0: Has RTC values
@ -185,7 +187,7 @@ mLOG_DECLARE_CATEGORY(GBA_STATE);
 *   | bit 0: Is read enabled
 *   | bit 1: Gyroscope sample is edge
 *   | bit 2: Light sample is edge
- *   | bit 3: Reserved
+ *   | bit 3: RTC SCK is edge
 *   | bits 4 - 15: Light counter
 * | 0x002C0 - 0x002C0: Light sample
 * | 0x002C1: Flags
@ -284,6 +286,7 @@ DECL_BITFIELD(GBASerializedHWFlags1, uint16_t);
 DECL_BIT(GBASerializedHWFlags1, ReadWrite, 0);
 DECL_BIT(GBASerializedHWFlags1, GyroEdge, 1);
 DECL_BIT(GBASerializedHWFlags1, LightEdge, 2);
+DECL_BIT(GBASerializedHWFlags1, RtcSckEdge, 3);
 DECL_BITS(GBASerializedHWFlags1, LightCounter, 4, 12);

 DECL_BITFIELD(GBASerializedHWFlags2, uint8_t);
@ -291,6 +294,9 @@ DECL_BITS(GBASerializedHWFlags2, TiltState, 0, 2);
 DECL_BITS(GBASerializedHWFlags2, GbpInputsPosted, 2, 2);
 DECL_BITS(GBASerializedHWFlags2, GbpTxPosition, 4, 4);

+DECL_BITFIELD(GBASerializedHWFlags3, uint8_t);
+DECL_BITS(GBASerializedHWFlags3, RtcSioOutput, 0, 1);
+
 DECL_BITFIELD(GBASerializedUnlCartFlags, uint16_t);
 DECL_BITS(GBASerializedUnlCartFlags, Type, 0, 5);
 DECL_BITS(GBASerializedUnlCartFlags, Subtype, 5, 3);
@ -381,9 +387,9 @@ struct GBASerializedState {
 		uint8_t pinState;
 		uint8_t writeLatch;
 		uint8_t pinDirection;
-		uint8_t reserved0;
+		GBASerializedHWFlags3 flags3;
 		int32_t rtcBytesRemaining;
-		int32_t rtcTransferStep;
+		int32_t reserved0;
 		int32_t rtcBitsRead;
 		int32_t rtcBits;
 		int32_t rtcCommandActive;
--- a/src/gba/cart/gpio.c
+++ b/src/gba/cart/gpio.c
@ -21,6 +21,7 @@ static void _outputPins(struct GBACartridgeHardware* hw, unsigned pins);

 static void _rtcReadPins(struct GBACartridgeHardware* hw);
 static unsigned _rtcOutput(struct GBACartridgeHardware* hw);
+static void _rtcBeginCommand(struct GBACartridgeHardware* hw);
 static void _rtcProcessByte(struct GBACartridgeHardware* hw);
 static void _rtcUpdateClock(struct GBACartridgeHardware* hw);
 static unsigned _rtcBCD(unsigned value);
@ -114,11 +115,11 @@ void GBAHardwareInitRTC(struct GBACartridgeHardware* hw) {
 	hw->devices |= HW_RTC;
 	hw->rtc.bytesRemaining = 0;

-	hw->rtc.transferStep = 0;
-
 	hw->rtc.bitsRead = 0;
 	hw->rtc.bits = 0;
-	hw->rtc.commandActive = 0;
+	hw->rtc.commandActive = false;
+	hw->rtc.sckEdge = true;
+	hw->rtc.sioOutput = true;
 	hw->rtc.command = 0;
 	hw->rtc.control = 0x40;
 	memset(hw->rtc.time, 0, sizeof(hw->rtc.time));
@ -146,11 +147,9 @@ void _readPins(struct GBACartridgeHardware* hw) {
 }

 void _outputPins(struct GBACartridgeHardware* hw, unsigned pins) {
+	hw->pinState &= hw->direction;
+	hw->pinState |= (pins & ~hw->direction & 0xF);
 	if (hw->readWrite) {
-		uint16_t old;
-		LOAD_16(old, 0, hw->gpioBase);
-		old &= hw->direction;
-		hw->pinState = old | (pins & ~hw->direction & 0xF);
 		STORE_16(hw->pinState, 0, hw->gpioBase);
 	}
 }
@ -158,121 +157,127 @@ void _outputPins(struct GBACartridgeHardware* hw, unsigned pins) {
 // == RTC

 void _rtcReadPins(struct GBACartridgeHardware* hw) {
-	// Transfer sequence:
-	// P: 0 | 1 |  2 | 3
-	// == Initiate
-	// > HI | - | LO | -
-	// > HI | - | HI | -
-	// == Transfer bit (x8)
-	// > LO | x | HI | -
-	// > HI | - | HI | -
-	// < ?? | x | ?? | -
-	// == Terminate
-	// >  - | - | LO | -
-	switch (hw->rtc.transferStep) {
-	case 0:
-		if ((hw->pinState & 5) == 1) {
-			hw->rtc.transferStep = 1;
-		}
-		break;
-	case 1:
-		if ((hw->pinState & 5) == 5) {
-			hw->rtc.transferStep = 2;
-		} else if ((hw->pinState & 5) != 1) {
-			hw->rtc.transferStep = 0;
-		}
-		break;
-	case 2:
+	// P: 0 - SCK | 1 - SIO | 2 - CS | 3 - Unused
+	// CS rising edge starts RTC transfer
+	// Conversely, CS falling edge aborts RTC transfer
+	// SCK rising edge shifts a bit from SIO into the transfer
+	// However, there appears to be a race condition if SIO changes at SCK rising edge
+	// For writing the command, the old SIO data is used in this race
+	// For writing command data, 0 is used in this race
+	// Note while CS is low, SCK is internally considered high by the RTC
+	// SCK falling edge shifts a bit from the transfer into SIO
+	// (Assuming a read command, outside of read commands SIO is held high)
+
+	// RTC keeps SCK/CS/Unused to low
+	_outputPins(hw, hw->pinState & 2);
+
+	if (!(hw->pinState & 4)) {
+		hw->rtc.bitsRead = 0;
+		hw->rtc.bytesRemaining = 0;
+		hw->rtc.commandActive = false;
+		hw->rtc.command = 0;
+		hw->rtc.sckEdge = true;
+		hw->rtc.sioOutput = true;
+		_outputPins(hw, 2);
+		return;
+	}
+
+	if (!hw->rtc.commandActive) {
+		_outputPins(hw, 2);
 		if (!(hw->pinState & 1)) {
 			hw->rtc.bits &= ~(1 << hw->rtc.bitsRead);
 			hw->rtc.bits |= ((hw->pinState & 2) >> 1) << hw->rtc.bitsRead;
-		} else {
-			if (hw->pinState & 4) {
-				if (!RTCCommandDataIsReading(hw->rtc.command)) {
-					++hw->rtc.bitsRead;
-					if (hw->rtc.bitsRead == 8) {
-						_rtcProcessByte(hw);
-					}
-				} else {
-					_outputPins(hw, 5 | (_rtcOutput(hw) << 1));
-					++hw->rtc.bitsRead;
-					if (hw->rtc.bitsRead == 8) {
-						--hw->rtc.bytesRemaining;
-						if (hw->rtc.bytesRemaining <= 0) {
-							hw->rtc.commandActive = 0;
-							hw->rtc.command = 0;
-						}
-						hw->rtc.bitsRead = 0;
-					}
-				}
-			} else {
-				hw->rtc.bitsRead = 0;
-				hw->rtc.bytesRemaining = 0;
-				hw->rtc.commandActive = 0;
-				hw->rtc.command = 0;
-				hw->rtc.transferStep = hw->pinState & 1;
-				_outputPins(hw, 1);
+		}
+		if (!hw->rtc.sckEdge && (hw->pinState & 1)) {
+			++hw->rtc.bitsRead;
+			if (hw->rtc.bitsRead == 8) {
+				_rtcBeginCommand(hw);
 			}
 		}
-		break;
-	}
-}
-
-void _rtcProcessByte(struct GBACartridgeHardware* hw) {
-	--hw->rtc.bytesRemaining;
-	if (!hw->rtc.commandActive) {
-		RTCCommandData command;
-		command = hw->rtc.bits;
-		if (RTCCommandDataGetMagic(command) == 0x06) {
-			hw->rtc.command = command;
-
-			hw->rtc.bytesRemaining = RTC_BYTES[RTCCommandDataGetCommand(command)];
-			hw->rtc.commandActive = hw->rtc.bytesRemaining > 0;
-			mLOG(GBA_HW, DEBUG, "Got RTC command %x", RTCCommandDataGetCommand(command));
-			switch (RTCCommandDataGetCommand(command)) {
-			case RTC_RESET:
-				hw->rtc.control = 0;
-				break;
-			case RTC_DATETIME:
-			case RTC_TIME:
-				_rtcUpdateClock(hw);
-				break;
-			case RTC_FORCE_IRQ:
-			case RTC_CONTROL:
-				break;
+	} else if (!RTCCommandDataIsReading(hw->rtc.command)) {
+		_outputPins(hw, 2);
+		if (!(hw->pinState & 1)) {
+			hw->rtc.bits &= ~(1 << hw->rtc.bitsRead);
+			hw->rtc.bits |= ((hw->pinState & 2) >> 1) << hw->rtc.bitsRead;
+		}
+		if (!hw->rtc.sckEdge && (hw->pinState & 1)) {
+			if ((((hw->rtc.bits >> hw->rtc.bitsRead) & 1) ^ ((hw->pinState & 2) >> 1))) {
+				hw->rtc.bits &= ~(1 << hw->rtc.bitsRead);
+			}
+			++hw->rtc.bitsRead;
+			if (hw->rtc.bitsRead == 8) {
+				_rtcProcessByte(hw);
 			}
-		} else {
-			mLOG(GBA_HW, WARN, "Invalid RTC command byte: %02X", hw->rtc.bits);
 		}
 	} else {
-		switch (RTCCommandDataGetCommand(hw->rtc.command)) {
-		case RTC_CONTROL:
-			hw->rtc.control = hw->rtc.bits;
-			break;
-		case RTC_FORCE_IRQ:
-			mLOG(GBA_HW, STUB, "Unimplemented RTC command %u", RTCCommandDataGetCommand(hw->rtc.command));
-			break;
+		if (hw->rtc.sckEdge && !(hw->pinState & 1)) {
+			hw->rtc.sioOutput = _rtcOutput(hw);
+			++hw->rtc.bitsRead;
+			if (hw->rtc.bitsRead == 8) {
+				--hw->rtc.bytesRemaining;
+				if (hw->rtc.bytesRemaining <= 0) {
+					hw->rtc.bytesRemaining = RTC_BYTES[RTCCommandDataGetCommand(hw->rtc.command)];
+				}
+				hw->rtc.bitsRead = 0;
+			}
+		}
+		_outputPins(hw, hw->rtc.sioOutput << 1);
+	}
+
+	hw->rtc.sckEdge = !!(hw->pinState & 1);
+}
+
+void _rtcBeginCommand(struct GBACartridgeHardware* hw) {
+	RTCCommandData command = hw->rtc.bits;
+	if (RTCCommandDataGetMagic(command) == 0x06) {
+		hw->rtc.command = command;
+		hw->rtc.bytesRemaining = RTC_BYTES[RTCCommandDataGetCommand(command)];
+		hw->rtc.commandActive = true;
+		mLOG(GBA_HW, DEBUG, "Got RTC command %x", RTCCommandDataGetCommand(command));
+		switch (RTCCommandDataGetCommand(command)) {
 		case RTC_RESET:
+			hw->rtc.control = 0;
+			break;
 		case RTC_DATETIME:
 		case RTC_TIME:
+			_rtcUpdateClock(hw);
+			break;
+		case RTC_FORCE_IRQ:
+		case RTC_CONTROL:
 			break;
 		}
+	} else {
+		mLOG(GBA_HW, WARN, "Invalid RTC command byte: %02X", hw->rtc.bits);
 	}

 	hw->rtc.bits = 0;
 	hw->rtc.bitsRead = 0;
-	if (!hw->rtc.bytesRemaining) {
-		hw->rtc.commandActive = 0;
-		hw->rtc.command = 0;
+}
+
+void _rtcProcessByte(struct GBACartridgeHardware* hw) {
+	switch (RTCCommandDataGetCommand(hw->rtc.command)) {
+	case RTC_CONTROL:
+		hw->rtc.control = hw->rtc.bits;
+		break;
+	case RTC_FORCE_IRQ:
+		mLOG(GBA_HW, STUB, "Unimplemented RTC command %u", RTCCommandDataGetCommand(hw->rtc.command));
+		break;
+	case RTC_RESET:
+	case RTC_DATETIME:
+	case RTC_TIME:
+		break;
+	}
+
+	hw->rtc.bits = 0;
+	hw->rtc.bitsRead = 0;
+	--hw->rtc.bytesRemaining;
+	if (hw->rtc.bytesRemaining <= 0) {
+		hw->rtc.bytesRemaining = RTC_BYTES[RTCCommandDataGetCommand(hw->rtc.command)];
 	}
 }

 unsigned _rtcOutput(struct GBACartridgeHardware* hw) {
-	uint8_t outputByte = 0;
-	if (!hw->rtc.commandActive) {
-		mLOG(GBA_HW, GAME_ERROR, "Attempting to use RTC without an active command");
-		return 0;
-	}
+	uint8_t outputByte = 0xFF;
 	switch (RTCCommandDataGetCommand(hw->rtc.command)) {
 	case RTC_CONTROL:
 		outputByte = hw->rtc.control;
@ -399,6 +404,7 @@ void _lightReadPins(struct GBACartridgeHardware* hw) {
 		struct GBALuminanceSource* lux = hw->p->luminanceSource;
 		mLOG(GBA_HW, DEBUG, "[SOLAR] Got reset");
 		hw->lightCounter = 0;
+		hw->lightEdge = true; // unverified (perhaps reset only happens on bit 1 rising edge?)
 		if (lux) {
 			if (lux->sample) {
 				lux->sample(lux);
@ -414,7 +420,7 @@ void _lightReadPins(struct GBACartridgeHardware* hw) {
 	hw->lightEdge = !(hw->pinState & 1);

 	bool sendBit = hw->lightCounter >= hw->lightSample;
-	_outputPins(hw, sendBit << 3);
+	_outputPins(hw, (sendBit << 3) | (hw->pinState & 0x7));
 	mLOG(GBA_HW, DEBUG, "[SOLAR] Output %u with pins %u", hw->lightCounter, hw->pinState);
 }

@ -488,11 +494,15 @@ void GBAHardwareSerialize(const struct GBACartridgeHardware* hw, struct GBASeria
 	state->hw.pinDirection = hw->direction;
 	state->hw.devices = hw->devices;

+	GBASerializedHWFlags3 flags3 = 0;
+	flags3 = GBASerializedHWFlags3SetRtcSioOutput(flags3, hw->rtc.sioOutput);
+	state->hw.flags3 = flags3;
+
 	STORE_32(hw->rtc.bytesRemaining, 0, &state->hw.rtcBytesRemaining);
-	STORE_32(hw->rtc.transferStep, 0, &state->hw.rtcTransferStep);
 	STORE_32(hw->rtc.bitsRead, 0, &state->hw.rtcBitsRead);
 	STORE_32(hw->rtc.bits, 0, &state->hw.rtcBits);
 	STORE_32(hw->rtc.commandActive, 0, &state->hw.rtcCommandActive);
+	flags1 = GBASerializedHWFlags1SetRtcSckEdge(flags1, hw->rtc.sckEdge);
 	STORE_32(hw->rtc.command, 0, &state->hw.rtcCommand);
 	STORE_32(hw->rtc.control, 0, &state->hw.rtcControl);
 	memcpy(state->hw.time, hw->rtc.time, sizeof(state->hw.time));
@ -539,11 +549,13 @@ void GBAHardwareDeserialize(struct GBACartridgeHardware* hw, const struct GBASer
 		}
 	}

+	hw->rtc.sioOutput = GBASerializedHWFlags3GetRtcSioOutput(state->hw.flags3);
+
 	LOAD_32(hw->rtc.bytesRemaining, 0, &state->hw.rtcBytesRemaining);
-	LOAD_32(hw->rtc.transferStep, 0, &state->hw.rtcTransferStep);
 	LOAD_32(hw->rtc.bitsRead, 0, &state->hw.rtcBitsRead);
 	LOAD_32(hw->rtc.bits, 0, &state->hw.rtcBits);
 	LOAD_32(hw->rtc.commandActive, 0, &state->hw.rtcCommandActive);
+	hw->rtc.sckEdge = GBASerializedHWFlags1GetRtcSckEdge(flags1);
 	LOAD_32(hw->rtc.command, 0, &state->hw.rtcCommand);
 	LOAD_32(hw->rtc.control, 0, &state->hw.rtcControl);
 	memcpy(hw->rtc.time, state->hw.time, sizeof(hw->rtc.time));
--- a/src/gba/serialize.c
+++ b/src/gba/serialize.c
@ -15,7 +15,7 @@
 #include <fcntl.h>

 MGBA_EXPORT const uint32_t GBASavestateMagic = 0x01000000;
-MGBA_EXPORT const uint32_t GBASavestateVersion = 0x00000008;
+MGBA_EXPORT const uint32_t GBASavestateVersion = 0x00000009;

 mLOG_DEFINE_CATEGORY(GBA_STATE, "GBA Savestate", "gba.serialize");