From 613f5be97714ced459ad71474ec5a1bc05965902 Mon Sep 17 00:00:00 2001 From: negativeExponent Date: Fri, 5 Dec 2025 08:46:02 +0800 Subject: [PATCH 1/3] Refactor GB joypad code - Simplify GB joypad handling register logic - Add missing edge case when both bits 4 and 5 in P1/JOYP register is low. The result returns AND of direction and action face buttons (see Sameboy and other emus) --- src/core/gb/gb.cpp | 118 ++++++++++++++++++--------------------------- 1 file changed, 48 insertions(+), 70 deletions(-) diff --git a/src/core/gb/gb.cpp b/src/core/gb/gb.cpp index 52a60aeb..89e50262 100644 --- a/src/core/gb/gb.cpp +++ b/src/core/gb/gb.cpp @@ -2207,82 +2207,60 @@ uint8_t gbReadMemory(uint16_t address) int b = gbMemory[0xff00]; - if ((b & 0x30) == 0x20) { - b &= 0xf0; - - int joy = 0; - if (gbSgbMode && gbSgbMultiplayer) { - switch (gbSgbNextController) { - case 0x0f: - joy = 0; - break; - case 0x0e: - joy = 1; - break; - case 0x0d: - joy = 2; - break; - case 0x0c: - joy = 3; - break; - default: - joy = 0; - } - } - int joystate = gbJoymask[joy]; - if (!(joystate & 128)) - b |= 0x08; - if (!(joystate & 64)) - b |= 0x04; - if (!(joystate & 32)) - b |= 0x02; - if (!(joystate & 16)) - b |= 0x01; - - gbMemory[0xff00] = (uint8_t)b; - } else if ((b & 0x30) == 0x10) { - b &= 0xf0; - - int joy = 0; - if (gbSgbMode && gbSgbMultiplayer) { - switch (gbSgbNextController) { - case 0x0f: - joy = 0; - break; - case 0x0e: - joy = 1; - break; - case 0x0d: - joy = 2; - break; - case 0x0c: - joy = 3; - break; - default: - joy = 0; - } + // Choose joystick index for SGB multiplayer if enabled, otherwise 0 + int joy = 0; + + if (gbSgbMode && gbSgbMultiplayer) { + switch (gbSgbNextController) { + case 0x0f: joy = 0; break; + case 0x0e: joy = 1; break; + case 0x0d: joy = 2; break; + case 0x0c: joy = 3; break; + default: joy = 0; break; } - int joystate = gbJoymask[joy]; - if (!(joystate & 8)) - b |= 0x08; - if (!(joystate & 4)) - b |= 0x04; - if (!(joystate & 2)) - b |= 0x02; - if (!(joystate & 1)) - b |= 0x01; - - gbMemory[0xff00] = (uint8_t)b; - } else { + } + + int joystate = gbJoymask[joy]; + + if (gbSgbMode && gbSgbMultiplayer) { + } + + // Bit 7 - Not used + // Bit 6 - Not used + // Bit 5 - P15 Select Action buttons (0=Select) + // Bit 4 - P14 Select Direction buttons (0=Select) + // Bit 3 - P13 Input: Down or Start (0=Pressed) (Read Only) + // Bit 2 - P12 Input: Up or Select (0=Pressed) (Read Only) + // Bit 1 - P11 Input: Left or B (0=Pressed) (Read Only) + // Bit 0 - P10 Input: Right or A (0=Pressed) (Read Only) + uint8_t data_dir = + (!(joystate & 0x80) ? 0x08 : 0x00) | // Down -> bit3 + (!(joystate & 0x40) ? 0x04 : 0x00) | // Up -> bit2 + (!(joystate & 0x20) ? 0x02 : 0x00) | // Left -> bit1 + (!(joystate & 0x10) ? 0x01 : 0x00); // Right-> bit0 + + uint8_t data_action = + (!(joystate & 0x08) ? 0x08 : 0x00) | // Start -> bit3 + (!(joystate & 0x04) ? 0x04 : 0x00) | // Select -> bit2 + (!(joystate & 0x02) ? 0x02 : 0x00) | // B -> bit1 + (!(joystate & 0x01) ? 0x01 : 0x00); // A -> bit0 + + b &= 0xf0; // keep selector bits in upper nibble + switch (b & 0x30) { + case 0x00: b |= data_dir & data_action; break; + case 0x10: b |= data_action; break; + case 0x20: b |= data_dir; break; + case 0x30: if (gbSgbMode && gbSgbMultiplayer) { - gbMemory[0xff00] = 0xf0 | gbSgbNextController; + b = 0xf0 | gbSgbNextController; } else { - gbMemory[0xff00] = 0xff; + b = 0xff; } + break; + } + gbMemory[0xff00] = b; } - } return gbMemory[0xff00]; - break; case 0x01: return gbMemory[0xff01]; case 0x02: From d6d5d956793d446e5db4268b812bf5745769f0cc Mon Sep 17 00:00:00 2001 From: negativeExponent Date: Fri, 5 Dec 2025 08:58:37 +0800 Subject: [PATCH 2/3] libretro: Fix input handling for multiplayer games Input handler was just previously reading from port 1 --- src/libretro/libretro.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libretro/libretro.cpp b/src/libretro/libretro.cpp index 0d1b8796..64250493 100644 --- a/src/libretro/libretro.cpp +++ b/src/libretro/libretro.cpp @@ -1640,11 +1640,11 @@ static void updateInput_Joypad(void) if (retropad_device[port] == RETRO_DEVICE_JOYPAD) { if (libretro_supports_bitmasks) - inbuf = input_cb(0, RETRO_DEVICE_JOYPAD, 0, RETRO_DEVICE_ID_JOYPAD_MASK); + inbuf = input_cb(port, RETRO_DEVICE_JOYPAD, 0, RETRO_DEVICE_ID_JOYPAD_MASK); else { for (int i = 0; i < (RETRO_DEVICE_ID_JOYPAD_R3 + 1); i++) - inbuf |= input_cb(0, RETRO_DEVICE_JOYPAD, 0, i) ? (1 << i) : 0; + inbuf |= input_cb(port, RETRO_DEVICE_JOYPAD, 0, i) ? (1 << i) : 0; } for (unsigned button = 0; button < max_buttons; button++) From 8ba0c5a927f68371b18c92fc67a89eeabd7bd6bb Mon Sep 17 00:00:00 2001 From: negativeExponent Date: Wed, 10 Dec 2025 13:10:10 +0800 Subject: [PATCH 3/3] Revised gpio/eeprom/backup save checks / logic / range - fixes proper detection of save types or attached gpio device - really need a proper and deterministic way to detect and set all these instead of waiting for core to present. too many enabled AUTO for example --- src/core/gba/gbaInline.h | 125 +++++++++++++++++++++++++------------- src/libretro/libretro.cpp | 11 +++- 2 files changed, 92 insertions(+), 44 deletions(-) diff --git a/src/core/gba/gbaInline.h b/src/core/gba/gbaInline.h index fd64bb5d..99b43e24 100644 --- a/src/core/gba/gbaInline.h +++ b/src/core/gba/gbaInline.h @@ -123,6 +123,31 @@ static inline uint16_t ROMReadOOB(uint32_t address) { return (address >> 1) & 0xFFFF; } +static inline bool IsGPIO(uint32_t address) { + // TODO: Need to check which GPIO feature really is enabled + return (address == 0x80000c4 || address == 0x80000c6 || address == 0x80000c8); +} + +// used for ROM boundary check +static inline bool IsEEPROM(uint32_t address) { + return (cpuEEPROMEnabled && (address & eepromMask) == eepromMask); +} + +static inline bool isSaveGame() { + return (coreOptions.saveType != 5) && (!eepromInUse || cpuSramEnabled || cpuFlashEnabled); +} + +// Reads sram or flash +static inline uint8_t CPUReadBackup(uint32_t address) { + return flashRead(address); +} + +// writes sram or flash +static inline void CPUWriteBackup(uint32_t address, uint8_t value) { + if (cpuSaveGameFunc) + (*cpuSaveGameFunc)(address, value); +} + static inline uint32_t CPUReadMemory(uint32_t address) { #ifdef VBAM_ENABLE_DEBUGGER @@ -133,7 +158,7 @@ static inline uint32_t CPUReadMemory(uint32_t address) } } #endif - uint32_t value = 0xFFFFFFFF; + uint32_t value = 0; switch (address >> 24) { case REGION_BIOS: @@ -194,10 +219,10 @@ static inline uint32_t CPUReadMemory(uint32_t address) case REGION_ROM1: case REGION_ROM1EX: case REGION_ROM2: - if ((address & 0x01FFFFFC) <= (gbaGetRomSize() - 4)) + if (IsEEPROM(address)) + return 0; // ignore reads from eeprom region outside 0x0D page reads + else if ((address & 0x01FFFFFC) <= (gbaGetRomSize() - 4)) value = READ32LE(((uint32_t *)&g_rom[address & 0x01FFFFFC])); - else if (cpuEEPROMEnabled && ((address & eepromMask) == eepromMask)) - return 0; // ignore reads from eeprom region outside 0x0D page reads else { value = (uint16_t)ROMReadOOB(address & 0x01FFFFFC); value |= (uint16_t)ROMReadOOB((address & 0x01FFFFFC) + 2) << 16; @@ -205,16 +230,23 @@ static inline uint32_t CPUReadMemory(uint32_t address) break; case REGION_ROM2EX: if (cpuEEPROMEnabled) - // no need to swap this return eepromRead(address); goto unreadable; case REGION_SRAM: case REGION_SRAMEX: - if (cpuFlashEnabled | cpuSramEnabled) { // no need to swap this - value = flashRead(address) * 0x01010101; + if (isSaveGame()) { + value = CPUReadBackup(address) * 0x01010101; break; } - /* fallthrough */ +#ifdef GBA_LOGGING + // Just normal log, not openbus + if (systemVerbose & VERBOSE_ILLEGAL_READ) { + log("Illegal word read: %08x at %08x\n", + address, + armMode ? armNextPC - 4 : armNextPC - 2); + } +#endif + return 0xffffffff; default: unreadable: #ifdef GBA_LOGGING @@ -274,7 +306,7 @@ static inline uint32_t CPUReadHalfWord(uint32_t address) } #endif - uint32_t value = 0xFFFFFFFF; + uint32_t value = 0; switch (address >> 24) { case REGION_BIOS: @@ -349,28 +381,35 @@ static inline uint32_t CPUReadHalfWord(uint32_t address) case REGION_ROM1: case REGION_ROM1EX: case REGION_ROM2: - if ((address & 0x01FFFFFE) <= (gbaGetRomSize() - 2)) - value = READ16LE(((uint16_t *)&g_rom[address & 0x01FFFFFE])); - else if (address == 0x80000c4 || address == 0x80000c6 || address == 0x80000c8) + if (IsGPIO(address)) value = rtcRead(address); - else if (cpuEEPROMEnabled && ((address & eepromMask) == eepromMask)) + else if (IsEEPROM(address)) return 0; // ignore reads from eeprom region outside 0x0D page reads + else if ((address & 0x01FFFFFE) <= (gbaGetRomSize() - 2)) + value = READ16LE(((uint16_t *)&g_rom[address & 0x01FFFFFE])); else value = (uint16_t)ROMReadOOB(address & 0x01FFFFFE); break; case REGION_ROM2EX: if (cpuEEPROMEnabled) - // no need to swap this return eepromRead(address); goto unreadable; case REGION_SRAM: case REGION_SRAMEX: - if (cpuFlashEnabled | cpuSramEnabled) { - // no need to swap this - value = flashRead(address) * 0x0101; + if (isSaveGame()) { + value = CPUReadBackup(address) * 0x0101; break; } - /* fallthrough */ +#ifdef GBA_LOGGING + // Just normal log, not openbus + if (systemVerbose & VERBOSE_ILLEGAL_READ) { + log("Illegal halfword read: %08x at %08x (%08x)\n", + address, + reg[15].I, + value); + } +#endif + return 0xffff; default: unreadable: #ifdef GBA_LOGGING @@ -469,20 +508,21 @@ static inline uint8_t CPUReadByte(uint32_t address) case REGION_ROM1: case REGION_ROM1EX: case REGION_ROM2: - if ((address & 0x01FFFFFF) <= gbaGetRomSize()) - return g_rom[address & 0x01FFFFFF]; - else if (cpuEEPROMEnabled && ((address & eepromMask) == eepromMask)) + if (IsEEPROM(address)) return 0; // ignore reads from eeprom region outside 0x0D page reads - return (uint8_t)ROMReadOOB(address & 0x01FFFFFE); + else if ((address & 0x01FFFFFF) <= gbaGetRomSize()) + return g_rom[address & 0x01FFFFFF]; + else + return (uint8_t)ROMReadOOB(address & 0x01FFFFFE); case REGION_ROM2EX: if (cpuEEPROMEnabled) return DowncastU8(eepromRead(address)); goto unreadable; case REGION_SRAM: case REGION_SRAMEX: - if (cpuSramEnabled | cpuFlashEnabled) - return flashRead(address); - + if (isSaveGame()) { + return CPUReadBackup(address); + } switch (address & 0x00008f00) { case 0x8200: return DowncastU8(systemGetSensorX()); @@ -493,7 +533,15 @@ static inline uint8_t CPUReadByte(uint32_t address) case 0x8500: return DowncastU8(systemGetSensorY() >> 8); } - return 0xFF; +#ifdef GBA_LOGGING + // Just normal log, not openbus + if (systemVerbose & VERBOSE_ILLEGAL_READ) { + log("Illegal byte read: %08x at %08x\n", + address, + armMode ? armNextPC - 4 : armNextPC - 2); + } +#endif + return 0xff; default: unreadable: #ifdef GBA_LOGGING @@ -603,12 +651,11 @@ static inline void CPUWriteMemory(uint32_t address, uint32_t value) goto unwritable; case REGION_SRAM: case REGION_SRAMEX: - if ((!eepromInUse) | cpuSramEnabled | cpuFlashEnabled) { - (*cpuSaveGameFunc)(address, (uint8_t)(value >> (8 * (address & 3)))); + if (isSaveGame()) { + CPUWriteBackup(address, (uint8_t)(value >> (8 * (address & 3)))); break; } - goto unwritable; - // fallthrough + // fallthrough default: unwritable: #ifdef GBA_LOGGING @@ -704,7 +751,7 @@ static inline void CPUWriteHalfWord(uint32_t address, uint16_t value) GBAMatrixWrite16(&GBAMatrix, address & 0x3C, value); break; } - if (address == 0x80000c4 || address == 0x80000c6 || address == 0x80000c8) { + if (IsGPIO(address)) { if (!rtcWrite(address, value)) goto unwritable; } else if (!agbPrintWrite(address, value)) @@ -727,11 +774,11 @@ static inline void CPUWriteHalfWord(uint32_t address, uint16_t value) goto unwritable; case REGION_SRAM: case REGION_SRAMEX: - if ((!eepromInUse) | cpuSramEnabled | cpuFlashEnabled) { - (*cpuSaveGameFunc)(address, (uint8_t)(value >> (8 * (address & 1)))); + if (isSaveGame()) { + CPUWriteBackup(address, (uint8_t)(value >> (8 * (address & 1)))); break; } - /* fallthrough */ + // fallthrough default: unwritable: #ifdef GBA_LOGGING @@ -867,9 +914,6 @@ static inline void CPUWriteByte(uint32_t address, uint8_t b) } break; case REGION_OAM: - // no need to switch - // byte writes to OAM are ignored - // *((uint16_t *)&g_oam[address & 0x3FE]) = (b << 8) | b; break; case REGION_ROM2EX: if (cpuEEPROMEnabled) { @@ -879,12 +923,11 @@ static inline void CPUWriteByte(uint32_t address, uint8_t b) goto unwritable; case REGION_SRAM: case REGION_SRAMEX: - if ((coreOptions.saveType != 5) && ((!eepromInUse) | cpuSramEnabled | cpuFlashEnabled)) { - (*cpuSaveGameFunc)(address, b); + if (isSaveGame()) { + CPUWriteBackup(address, b); break; } - goto unwritable; - // default + // fallthrough default: unwritable: #ifdef GBA_LOGGING diff --git a/src/libretro/libretro.cpp b/src/libretro/libretro.cpp index 64250493..3382badd 100644 --- a/src/libretro/libretro.cpp +++ b/src/libretro/libretro.cpp @@ -840,7 +840,7 @@ static void load_image_preferences(void) }; bool found = false; - int saveType = 0; + int saveType = GBA_SAVE_AUTO; int saveSize = 0; bool hasRtc = false; bool hasRumble = false; @@ -909,7 +909,7 @@ static void load_image_preferences(void) } // Autodetect save type is needed - if (!saveType) { + if (saveType == GBA_SAVE_AUTO) { log("Autodetecting save type...\n"); // FLASH 1M Sanyo if (find_string(g_rom, romSize, "FLASH1M_")) @@ -942,11 +942,16 @@ static void load_image_preferences(void) { log("Found SRAM_\n"); saveType = GBA_SAVE_SRAM; + } else + + // no save type found + { + saveType = GBA_SAVE_NONE; } // RTC flag if (find_string(g_rom, romSize, "SIIRTC_V")) { - log("Found SRAM_\n"); + log("Found RTC\n"); hasRtc = true; } }