GBA Memory: Stall on VRAM access in mode 2 (fixes #190)

CHANGES

@@ -19,6 +19,7 @@ Emulation fixes:
  - GBA BIOS: Fix reloading video registers after reset (fixes mgba.io/i/1808)
  - GBA DMA: Linger last DMA on bus (fixes mgba.io/i/301 and mgba.io/i/1320)
  - GBA Memory: Improve gamepak prefetch timing
+ - GBA Memory: Stall on VRAM access in mode 2 (fixes mgba.io/i/190)
  - GBA SIO: Fix copying Normal mode transfer values
  - GBA Video: Latch scanline at end of Hblank (fixes mgba.io/i/1319)
  - GBA Video: Fix Hblank timing

include/mgba/internal/gba/video.h

@@ -206,8 +206,8 @@ struct GBAVideo {
 	struct GBAVideoRenderer* renderer;
 	struct mTimingEvent event;
 
-	// VCOUNT
 	int vcount;
+	int shouldStall;
 
 	uint16_t palette[512];
 	uint16_t* vram;

src/gba/memory.c

@@ -29,6 +29,7 @@ static uint8_t _agbPrintFunc[4] = { 0xFA, 0xDF /* swi 0xFF */, 0x70, 0x47 /* bx
 
 static void GBASetActiveRegion(struct ARMCore* cpu, uint32_t region);
 static int32_t GBAMemoryStall(struct ARMCore* cpu, int32_t wait);
+static int32_t GBAMemoryStallVRAM(struct GBA* gba, int32_t wait, int extra);
 
 static const char GBA_BASE_WAITSTATES[16] = { 0, 0, 2, 0, 0, 0, 0, 0, 4, 4, 4, 4, 4, 4, 4 };
 static const char GBA_BASE_WAITSTATES_32[16] = { 0, 0, 5, 0, 0, 1, 1, 0, 7, 7, 9, 9, 13, 13, 9 };
@@ -374,7 +375,10 @@ static void GBASetActiveRegion(struct ARMCore* cpu, uint32_t address) {
 	} else { \
 		LOAD_32(value, address & 0x0001FFFC, gba->video.vram); \
 	} \
-	wait += waitstatesRegion[REGION_VRAM];
+	++wait; \
+	if (gba->video.shouldStall) { \
+		wait += GBAMemoryStallVRAM(gba, wait, 1); \
+	}
 
 #define LOAD_OAM LOAD_32(value, address & (SIZE_OAM - 4), gba->video.oam.raw);
 
@@ -534,6 +538,9 @@ uint32_t GBALoad16(struct ARMCore* cpu, uint32_t address, int* cycleCounter) {
 		} else {
 			LOAD_16(value, address & 0x0001FFFE, gba->video.vram);
 		}
+		if (gba->video.shouldStall) {
+			wait += GBAMemoryStallVRAM(gba, wait, 0);
+		}
 		break;
 	case REGION_OAM:
 		LOAD_16(value, address & (SIZE_OAM - 2), gba->video.oam.raw);
@@ -650,6 +657,9 @@ uint32_t GBALoad8(struct ARMCore* cpu, uint32_t address, int* cycleCounter) {
 		} else {
 			value = ((uint8_t*) gba->video.vram)[address & 0x0001FFFF];
 		}
+		if (gba->video.shouldStall) {
+			wait += GBAMemoryStallVRAM(gba, wait, 0);
+		}
 		break;
 	case REGION_OAM:
 		value = ((uint8_t*) gba->video.oam.raw)[address & (SIZE_OAM - 1)];
@@ -751,7 +761,10 @@ uint32_t GBALoad8(struct ARMCore* cpu, uint32_t address, int* cycleCounter) {
 			gba->video.renderer->writeVRAM(gba->video.renderer, (address & 0x0001FFFC)); \
 		} \
 	} \
-	wait += waitstatesRegion[REGION_VRAM];
+	++wait; \
+	if (gba->video.shouldStall) { \
+		wait += GBAMemoryStallVRAM(gba, wait, 1); \
+	}
 
 #define STORE_OAM \
 	LOAD_32(oldValue, address & (SIZE_OAM - 4), gba->video.oam.raw); \
@@ -876,6 +889,9 @@ void GBAStore16(struct ARMCore* cpu, uint32_t address, int16_t value, int* cycle
 				gba->video.renderer->writeVRAM(gba->video.renderer, address & 0x0001FFFE);
 			}
 		}
+		if (gba->video.shouldStall) {
+			wait += GBAMemoryStallVRAM(gba, wait, 0);
+		}
 		break;
 	case REGION_OAM:
 		LOAD_16(oldValue, address & (SIZE_OAM - 2), gba->video.oam.raw);
@@ -977,6 +993,9 @@ void GBAStore8(struct ARMCore* cpu, uint32_t address, int8_t value, int* cycleCo
 			gba->video.renderer->vram[(address & 0x1FFFE) >> 1] = ((uint8_t) value) | (value << 8);
 			gba->video.renderer->writeVRAM(gba->video.renderer, address & 0x0001FFFE);
 		}
+		if (gba->video.shouldStall) {
+			wait += GBAMemoryStallVRAM(gba, wait, 0);
+		}
 		break;
 	case REGION_OAM:
 		mLOG(GBA_MEM, GAME_ERROR, "Cannot Store8 to OAM: 0x%08X", address);
@@ -1658,6 +1677,28 @@ int32_t GBAMemoryStall(struct ARMCore* cpu, int32_t wait) {
 	return wait;
 }
 
+int32_t GBAMemoryStallVRAM(struct GBA* gba, int32_t wait, int extra) {
+	UNUSED(extra);
+	// TODO
+	uint16_t dispcnt = gba->memory.io[REG_DISPCNT >> 1];
+	int32_t stall = 0;
+	switch (GBARegisterDISPCNTGetMode(dispcnt)) {
+	case 2:
+		if (GBARegisterDISPCNTIsBg2Enable(dispcnt) && GBARegisterDISPCNTIsBg3Enable(dispcnt)) {
+			// If both backgrounds are enabled, VRAM access is entirely blocked during hdraw
+			stall = mTimingUntil(&gba->timing, &gba->video.event);
+		}
+		break;
+	default:
+		return 0;
+	}
+	stall -= wait;
+	if (stall < 0) {
+		return 0;
+	}
+	return stall;
+}
+
 void GBAMemorySerialize(const struct GBAMemory* memory, struct GBASerializedState* state) {
 	memcpy(state->wram, memory->wram, SIZE_WORKING_RAM);
 	memcpy(state->iwram, memory->iwram, SIZE_WORKING_IRAM);

src/gba/video.c

@@ -95,6 +95,7 @@ void GBAVideoReset(struct GBAVideo* video) {
 	video->frameCounter = 0;
 	video->frameskipCounter = 0;
 	video->renderer->vram = video->vram;
+	video->shouldStall = 0;
 
 	memset(video->palette, 0, sizeof(video->palette));
 	memset(video->oam.raw, 0, sizeof(video->oam.raw));
@@ -139,6 +140,7 @@ void _startHdraw(struct mTiming* timing, void* context, uint32_t cyclesLate) {
 
 	if (video->vcount < GBA_VIDEO_VERTICAL_PIXELS && video->frameskipCounter <= 0) {
 		video->renderer->drawScanline(video->renderer, video->vcount);
+		video->shouldStall = 1;
 	}
 
 	GBARegisterDISPSTAT dispstat = video->p->memory.io[REG_DISPSTAT >> 1];
@@ -198,6 +200,7 @@ void _startHblank(struct mTiming* timing, void* context, uint32_t cyclesLate) {
 	if (GBARegisterDISPSTATIsHblankIRQ(dispstat)) {
 		GBARaiseIRQ(video->p, IRQ_HBLANK, cyclesLate);
 	}
+	video->shouldStall = 0;
 	video->p->memory.io[REG_DISPSTAT >> 1] = dispstat;
 }
 
@@ -338,6 +341,7 @@ void GBAVideoDeserialize(struct GBAVideo* video, const struct GBASerializedState
 	}
 	LOAD_32(video->frameCounter, 0, &state->video.frameCounter);
 
+	video->shouldStall = 0;
 	int32_t flags;
 	LOAD_32(flags, 0, &state->video.flags);
 	GBARegisterDISPSTAT dispstat = state->io[REG_DISPSTAT >> 1];
@@ -354,6 +358,7 @@ void GBAVideoDeserialize(struct GBAVideo* video, const struct GBASerializedState
 		break;
 	case 2:
 		video->event.callback = _startHblank;
+		video->shouldStall = 1;
 		break;
 	case 3:
 		video->event.callback = _midHblank;