GBA: All IRQs have 7 cycle delay (fixes #539, #1208)

CHANGES

@@ -1,3 +1,7 @@
+0.8.0: (Future)
+Bugfixes:
+ - GBA: All IRQs have 7 cycle delay (fixes mgba.io/i/539, mgba.io/i/1208)
+
 0.7.0: (Future)
 Features:
  - ELF support

include/mgba/internal/gba/gba.h

@@ -82,6 +82,8 @@ struct GBA {
 	struct GBATimer timers[4];
 
 	int springIRQ;
+	struct mTimingEvent irqEvent;
+
 	uint32_t biosChecksum;
 	int* keySource;
 	struct mRotationSource* rotationSource;
@@ -141,8 +143,6 @@ void GBADestroy(struct GBA* gba);
 void GBAReset(struct ARMCore* cpu);
 void GBASkipBIOS(struct GBA* gba);
 
-void GBAWriteIE(struct GBA* gba, uint16_t value);
-void GBAWriteIME(struct GBA* gba, uint16_t value);
 void GBARaiseIRQ(struct GBA* gba, enum GBAIRQ irq);
 void GBATestIRQ(struct ARMCore* cpu);
 void GBAHalt(struct GBA* gba);

include/mgba/internal/gba/serialize.h

@@ -98,28 +98,28 @@ mLOG_DECLARE_CATEGORY(GBA_STATE);
  * | 0x00202 - 0x00203: Old reload value
  * | 0x00204 - 0x00207: Last event
  * | 0x00208 - 0x0020B: Next event
- * | 0x0020C - 0x0020F: Next IRQ
+ * | 0x0020C - 0x0020F: Reserved
  * | 0x00210 - 0x00213: Miscellaneous flags
  * 0x00214 - 0x00227: Timer 1
  * | 0x00214 - 0x00215: Reload value
  * | 0x00216 - 0x00217: Old reload value
  * | 0x00218 - 0x0021B: Last event
  * | 0x0021C - 0x0021F: Next event
- * | 0x00220 - 0x00223: Next IRQ
+ * | 0x00220 - 0x00223: Reserved
  * | 0x00224 - 0x00227: Miscellaneous flags
  * 0x00228 - 0x0023B: Timer 2
  * | 0x00228 - 0x00229: Reload value
  * | 0x0022A - 0x0022B: Old reload value
  * | 0x0022C - 0x0022F: Last event
  * | 0x00230 - 0x00233: Next event
- * | 0x00234 - 0x00237: Next IRQ
+ * | 0x00234 - 0x00237: Reserved
  * | 0x00238 - 0x0023B: Miscellaneous flags
  * 0x0023C - 0x00250: Timer 3
  * | 0x0023C - 0x0023D: Reload value
  * | 0x0023E - 0x0023F: Old reload value
  * | 0x00240 - 0x00243: Last event
  * | 0x00244 - 0x00247: Next event
- * | 0x00248 - 0x0024B: Next IRQ
+ * | 0x00248 - 0x0024B: Reserved
  * | 0x0024C - 0x0024F: Miscellaneous flags
  * 0x00250 - 0x0025F: DMA 0
  * | 0x00250 - 0x00253: DMA next source
@@ -196,7 +196,9 @@ mLOG_DECLARE_CATEGORY(GBA_STATE);
  * 0x0031C - 0x0031F: Miscellaneous flags
  *  | bit 0: Is CPU halted?
  *  | bit 1: POSTFLG
- * 0x00320 - 0x003FF: Reserved (leave zero)
+ *  | bit 2: Is IRQ pending?
+ * 0x00320 - 0x00323: Next IRQ event
+ * 0x00324 - 0x003FF: Reserved (leave zero)
  * 0x00400 - 0x007FF: I/O memory
  * 0x00800 - 0x00BFF: Palette
  * 0x00C00 - 0x00FFF: OAM
@@ -227,6 +229,7 @@ DECL_BIT(GBASerializedSavedataFlags, DustSettling, 5);
 DECL_BITFIELD(GBASerializedMiscFlags, uint32_t);
 DECL_BIT(GBASerializedMiscFlags, Halted, 0);
 DECL_BIT(GBASerializedMiscFlags, POSTFLG, 1);
+DECL_BIT(GBASerializedMiscFlags, IrqPending, 2);
 
 struct GBASerializedState {
 	uint32_t versionMagic;
@@ -267,10 +270,10 @@ struct GBASerializedState {
 
 	struct {
 		uint16_t reload;
-		uint16_t reserved;
+		uint16_t reserved0;
 		uint32_t lastEvent;
 		uint32_t nextEvent;
-		uint32_t nextIrq;
+		uint32_t reserved1;
 		GBATimerFlags flags;
 	} timers[4];
 
@@ -320,8 +323,9 @@ struct GBASerializedState {
 
 	uint32_t lastPrefetchedPc;
 	GBASerializedMiscFlags miscFlags;
+	uint32_t nextIrq;
 
-	uint32_t reserved[56];
+	uint32_t reserved[55];
 
 	uint16_t io[SIZE_IO >> 1];
 	uint16_t pram[SIZE_PALETTE_RAM >> 1];

include/mgba/internal/gba/timer.h

@@ -17,14 +17,12 @@ DECL_BITS(GBATimerFlags, PrescaleBits, 0, 4);
 DECL_BIT(GBATimerFlags, CountUp, 4);
 DECL_BIT(GBATimerFlags, DoIrq, 5);
 DECL_BIT(GBATimerFlags, Enable, 6);
-DECL_BIT(GBATimerFlags, IrqPending, 7);
 
 struct GBA;
 struct GBATimer {
 	uint16_t reload;
 	int32_t lastEvent;
 	struct mTimingEvent event;
-	struct mTimingEvent irq;
 	GBATimerFlags flags;
 };
 

src/arm/arm.c

@@ -167,6 +167,7 @@ void ARMRaiseIRQ(struct ARMCore* cpu) {
 	cpu->spsr = cpsr;
 	cpu->cpsr.i = 1;
 	cpu->cycles += currentCycles;
+	cpu->halted = 0;
 }
 
 void ARMRaiseSWI(struct ARMCore* cpu) {

src/gba/gba.c

@@ -25,6 +25,8 @@
 #include <mgba-util/elf-read.h>
 #endif
 
+#define GBA_IRQ_DELAY 7
+
 mLOG_DEFINE_CATEGORY(GBA, "GBA", "gba");
 mLOG_DEFINE_CATEGORY(GBA_DEBUG, "GBA Debug", "gba.debug");
 
@@ -45,6 +47,8 @@ static void GBAHitStub(struct ARMCore* cpu, uint32_t opcode);
 static void GBAIllegal(struct ARMCore* cpu, uint32_t opcode);
 static void GBABreakpoint(struct ARMCore* cpu, int immediate);
 
+static void _triggerIRQ(struct mTiming*, void* user, uint32_t cyclesLate);
+
 #ifdef USE_DEBUGGERS
 static bool _setSoftwareBreakpoint(struct ARMDebugger*, uint32_t address, enum ExecutionMode mode, uint32_t* opcode);
 static bool _clearSoftwareBreakpoint(struct ARMDebugger*, uint32_t address, enum ExecutionMode mode, uint32_t opcode);
@@ -86,7 +90,6 @@ static void GBAInit(void* cpu, struct mCPUComponent* component) {
 
 	GBAHardwareInit(&gba->memory.hw, NULL);
 
-	gba->springIRQ = 0;
 	gba->keySource = 0;
 	gba->rotationSource = 0;
 	gba->luminanceSource = 0;
@@ -116,6 +119,11 @@ static void GBAInit(void* cpu, struct mCPUComponent* component) {
 	gba->yankedRomSize = 0;
 
 	mTimingInit(&gba->timing, &gba->cpu->cycles, &gba->cpu->nextEvent);
+
+	gba->irqEvent.name = "GBA IRQ Event";
+	gba->irqEvent.callback = _triggerIRQ;
+	gba->irqEvent.context = gba;
+	gba->irqEvent.priority = 0;
 }
 
 void GBAUnloadROM(struct GBA* gba) {
@@ -247,11 +255,6 @@ static void GBAProcessEvents(struct ARMCore* cpu) {
 		gba->bus |= cpu->prefetch[1] << 16;
 	}
 
-	if (gba->springIRQ && !cpu->cpsr.i) {
-		ARMRaiseIRQ(cpu);
-		gba->springIRQ = 0;
-	}
-
 	int32_t nextEvent = cpu->nextEvent;
 	while (cpu->cycles >= nextEvent) {
 		cpu->nextEvent = INT_MAX;
@@ -474,34 +477,17 @@ void GBAApplyPatch(struct GBA* gba, struct Patch* patch) {
 	gba->romCrc32 = doCrc32(gba->memory.rom, gba->memory.romSize);
 }
 
-void GBAWriteIE(struct GBA* gba, uint16_t value) {
-	if (gba->memory.io[REG_IME >> 1] && value & gba->memory.io[REG_IF >> 1]) {
-		ARMRaiseIRQ(gba->cpu);
-	}
-}
-
-void GBAWriteIME(struct GBA* gba, uint16_t value) {
-	if (value && gba->memory.io[REG_IE >> 1] & gba->memory.io[REG_IF >> 1]) {
-		ARMRaiseIRQ(gba->cpu);
-	}
-}
-
 void GBARaiseIRQ(struct GBA* gba, enum GBAIRQ irq) {
 	gba->memory.io[REG_IF >> 1] |= 1 << irq;
-
+	GBATestIRQ(gba->cpu);
-	if (gba->memory.io[REG_IE >> 1] & 1 << irq) {
-		gba->cpu->halted = 0;
-		if (gba->memory.io[REG_IME >> 1]) {
-			ARMRaiseIRQ(gba->cpu);
-		}
-	}
 }
 
 void GBATestIRQ(struct ARMCore* cpu) {
 	struct GBA* gba = (struct GBA*) cpu->master;
-	if (gba->memory.io[REG_IME >> 1] && gba->memory.io[REG_IE >> 1] & gba->memory.io[REG_IF >> 1]) {
+	if (gba->memory.io[REG_IE >> 1] & gba->memory.io[REG_IF >> 1]) {
-		gba->springIRQ = gba->memory.io[REG_IE >> 1] & gba->memory.io[REG_IF >> 1];
+		if (!mTimingIsScheduled(&gba->timing, &gba->irqEvent)) {
-		gba->cpu->nextEvent = gba->cpu->cycles;
+			mTimingSchedule(&gba->timing, &gba->irqEvent, GBA_IRQ_DELAY);
+		}
 	}
 }
 
@@ -857,6 +843,20 @@ void GBATestKeypadIRQ(struct GBA* gba) {
 	}
 }
 
+static void _triggerIRQ(struct mTiming* timing, void* user, uint32_t cyclesLate) {
+	UNUSED(timing);
+	UNUSED(cyclesLate);
+	struct GBA* gba = user;
+	gba->cpu->halted = 0;
+	if (!(gba->memory.io[REG_IE >> 1] & gba->memory.io[REG_IF >> 1])) {
+		return;
+	}
+
+	if (gba->memory.io[REG_IME >> 1] && !gba->cpu->cpsr.i) {
+		ARMRaiseIRQ(gba->cpu);
+	}
+}
+
 void GBASetBreakpoint(struct GBA* gba, struct mCPUComponent* component, uint32_t address, enum ExecutionMode mode, uint32_t* opcode) {
 	size_t immediate;
 	for (immediate = 0; immediate < gba->cpu->numComponents; ++immediate) {

src/gba/io.c

@@ -547,15 +547,18 @@ void GBAIOWrite(struct GBA* gba, uint32_t address, uint16_t value) {
 			GBAAdjustWaitstates(gba, value);
 			break;
 		case REG_IE:
-			GBAWriteIE(gba, value);
+			gba->memory.io[REG_IE >> 1] = value;
-			break;
+			GBATestIRQ(gba->cpu);
+			return;
 		case REG_IF:
-			gba->springIRQ &= ~value;
 			value = gba->memory.io[REG_IF >> 1] & ~value;
-			break;
+			gba->memory.io[REG_IF >> 1] = value;
+			GBATestIRQ(gba->cpu);
+			return;
 		case REG_IME:
-			GBAWriteIME(gba, value);
+			gba->memory.io[REG_IME >> 1] = value;
-			break;
+			GBATestIRQ(gba->cpu);
+			return;
 		case REG_MAX:
 			// Some bad interrupt libraries will write to this
 			break;
@@ -931,7 +934,6 @@ void GBAIOSerialize(struct GBA* gba, struct GBASerializedState* state) {
 		STORE_16(gba->timers[i].reload, 0, &state->timers[i].reload);
 		STORE_32(gba->timers[i].lastEvent - mTimingCurrentTime(&gba->timing), 0, &state->timers[i].lastEvent);
 		STORE_32(gba->timers[i].event.when - mTimingCurrentTime(&gba->timing), 0, &state->timers[i].nextEvent);
-		STORE_32(gba->timers[i].irq.when - mTimingCurrentTime(&gba->timing), 0, &state->timers[i].nextIrq);
 		STORE_32(gba->timers[i].flags, 0, &state->timers[i].flags);
 		STORE_32(gba->memory.dma[i].nextSource, 0, &state->dma[i].nextSource);
 		STORE_32(gba->memory.dma[i].nextDest, 0, &state->dma[i].nextDest);
@@ -971,10 +973,6 @@ void GBAIODeserialize(struct GBA* gba, const struct GBASerializedState* state) {
 		if (GBATimerFlagsIsEnable(gba->timers[i].flags)) {
 			mTimingSchedule(&gba->timing, &gba->timers[i].event, when);
 		}
-		LOAD_32(when, 0, &state->timers[i].nextIrq);
-		if (GBATimerFlagsIsIrqPending(gba->timers[i].flags)) {
-			mTimingSchedule(&gba->timing, &gba->timers[i].irq, when);
-		}
 
 		LOAD_16(gba->memory.dma[i].reg, (REG_DMA0CNT_HI + i * 12), state->io);
 		LOAD_32(gba->memory.dma[i].nextSource, 0, &state->dma[i].nextSource);

src/gba/serialize.c

@@ -15,7 +15,7 @@
 #include <fcntl.h>
 
 const uint32_t GBA_SAVESTATE_MAGIC = 0x01000000;
-const uint32_t GBA_SAVESTATE_VERSION = 0x00000002;
+const uint32_t GBA_SAVESTATE_VERSION = 0x00000003;
 
 mLOG_DEFINE_CATEGORY(GBA_STATE, "GBA Savestate", "gba.serialize");
 
@@ -62,6 +62,10 @@ void GBASerialize(struct GBA* gba, struct GBASerializedState* state) {
 	GBASerializedMiscFlags miscFlags = 0;
 	miscFlags = GBASerializedMiscFlagsSetHalted(miscFlags, gba->cpu->halted);
 	miscFlags = GBASerializedMiscFlagsSetPOSTFLG(miscFlags, gba->memory.io[REG_POSTFLG >> 1] & 1);
+	if (mTimingIsScheduled(&gba->timing, &gba->irqEvent)) {
+		miscFlags = GBASerializedMiscFlagsFillIrqPending(miscFlags);
+		STORE_32(gba->irqEvent.when - mTimingCurrentTime(&gba->timing), 0, &state->nextIrq);
+	}
 	STORE_32(miscFlags, 0, &state->miscFlags);
 
 	GBAMemorySerialize(&gba->memory, state);
@@ -179,6 +183,11 @@ bool GBADeserialize(struct GBA* gba, const struct GBASerializedState* state) {
 	LOAD_32(miscFlags, 0, &state->miscFlags);
 	gba->cpu->halted = GBASerializedMiscFlagsGetHalted(miscFlags);
 	gba->memory.io[REG_POSTFLG >> 1] = GBASerializedMiscFlagsGetPOSTFLG(miscFlags);
+	if (GBASerializedMiscFlagsIsIrqPending(miscFlags)) {
+		int32_t when;
+		LOAD_32(when, 0, &state->nextIrq);
+		mTimingSchedule(&gba->timing, &gba->irqEvent, when);		
+	}
 
 	GBAVideoDeserialize(&gba->video, state);
 	GBAMemoryDeserialize(&gba->memory, state);

src/gba/timer.c

@@ -8,44 +8,11 @@
 #include <mgba/internal/gba/gba.h>
 #include <mgba/internal/gba/io.h>
 
-#define TIMER_IRQ_DELAY 7
 #define TIMER_RELOAD_DELAY 0
 #define TIMER_STARTUP_DELAY 2
 
 #define REG_TMCNT_LO(X) (REG_TM0CNT_LO + ((X) << 2))
 
-static void GBATimerIrq(struct GBA* gba, int timerId) {
-	struct GBATimer* timer = &gba->timers[timerId];
-	if (GBATimerFlagsIsIrqPending(timer->flags)) {
-		timer->flags = GBATimerFlagsClearIrqPending(timer->flags);
-		GBARaiseIRQ(gba, IRQ_TIMER0 + timerId);
-	}
-}
-
-static void GBATimerIrq0(struct mTiming* timing, void* context, uint32_t cyclesLate) {
-	UNUSED(timing);
-	UNUSED(cyclesLate);
-	GBATimerIrq(context, 0);
-}
-
-static void GBATimerIrq1(struct mTiming* timing, void* context, uint32_t cyclesLate) {
-	UNUSED(timing);
-	UNUSED(cyclesLate);
-	GBATimerIrq(context, 1);
-}
-
-static void GBATimerIrq2(struct mTiming* timing, void* context, uint32_t cyclesLate) {
-	UNUSED(timing);
-	UNUSED(cyclesLate);
-	GBATimerIrq(context, 2);
-}
-
-static void GBATimerIrq3(struct mTiming* timing, void* context, uint32_t cyclesLate) {
-	UNUSED(timing);
-	UNUSED(cyclesLate);
-	GBATimerIrq(context, 3);
-}
-
 static void GBATimerUpdate(struct GBA* gba, int timerId, uint32_t cyclesLate) {
 	struct GBATimer* timer = &gba->timers[timerId];
 	if (GBATimerFlagsIsCountUp(timer->flags)) {
@@ -55,10 +22,7 @@ static void GBATimerUpdate(struct GBA* gba, int timerId, uint32_t cyclesLate) {
 	}
 
 	if (GBATimerFlagsIsDoIrq(timer->flags)) {
-		timer->flags = GBATimerFlagsFillIrqPending(timer->flags);
+		GBARaiseIRQ(gba, IRQ_TIMER0 + timerId);
-		if (!mTimingIsScheduled(&gba->timing, &timer->irq)) {
-			mTimingSchedule(&gba->timing, &timer->irq, TIMER_IRQ_DELAY - cyclesLate);
-		}
 	}
 
 	if (gba->audio.enable && timerId < 2) {
@@ -120,22 +84,6 @@ void GBATimerInit(struct GBA* gba) {
 	gba->timers[3].event.callback = GBATimerUpdate3;
 	gba->timers[3].event.context = gba;
 	gba->timers[3].event.priority = 0x23;
-	gba->timers[0].irq.name = "GBA Timer 0 IRQ";
-	gba->timers[0].irq.callback = GBATimerIrq0;
-	gba->timers[0].irq.context = gba;
-	gba->timers[0].irq.priority = 0x28;
-	gba->timers[1].irq.name = "GBA Timer 1 IRQ";
-	gba->timers[1].irq.callback = GBATimerIrq1;
-	gba->timers[1].irq.context = gba;
-	gba->timers[1].irq.priority = 0x29;
-	gba->timers[2].irq.name = "GBA Timer 2 IRQ";
-	gba->timers[2].irq.callback = GBATimerIrq2;
-	gba->timers[2].irq.context = gba;
-	gba->timers[2].irq.priority = 0x2A;
-	gba->timers[3].irq.name = "GBA Timer 3 IRQ";
-	gba->timers[3].irq.callback = GBATimerIrq3;
-	gba->timers[3].irq.context = gba;
-	gba->timers[3].irq.priority = 0x2B;
 }
 
 void GBATimerUpdateRegister(struct GBA* gba, int timer, int32_t cyclesLate) {