git rekt — mgba.git: 4101fe54c670616fc4e5bc7416b3c98fa7aad102

GB Video: Make SGB packet transfers atomic (fixes #1054, closes #1030)

Vicki Pfau vi@endrift.com

Sat, 21 Apr 2018 16:56:51 -0700

commit

4101fe54c670616fc4e5bc7416b3c98fa7aad102

parent

5a7d5766d0131d6adee4f88e77950e3c37548d2f

6 files changed, 93 insertions(+), 103 deletions(-)

jump to

include/mgba/internal/gb/renderers/software.h

include/mgba/internal/gb/serialize.h

include/mgba/internal/gb/video.h

src/gb/renderers/software.c

src/gb/serialize.c

src/gb/video.c

M include/mgba/internal/gb/renderers/software.h → include/mgba/internal/gb/renderers/software.h

@@ -39,15 +39,9 @@ GBRegisterLCDC lcdc;
 	enum GBModel model;
 
 	int sgbTransfer;
-	uint8_t sgbPacket[16];
+	uint8_t sgbPacket[128];
 	uint8_t sgbCommandHeader;
-	int sgbPacketId;
-	int sgbDataSets;
-	uint8_t sgbPartialDataSet[15];
 	bool sgbBorders;
-	int sgbAttrX;
-	int sgbAttrY;
-	int sgbAttrDirection;
 };
 
 void GBVideoSoftwareRendererCreate(struct GBVideoSoftwareRenderer*);

M include/mgba/internal/gb/serialize.h → include/mgba/internal/gb/serialize.h

@@ -258,6 +258,7 @@
 DECL_BITFIELD(GBSerializedSGBFlags, uint32_t);
 DECL_BITS(GBSerializedSGBFlags, P1Bits, 0, 2);
 DECL_BITS(GBSerializedSGBFlags, RenderMode, 2, 2);
+DECL_BITS(GBSerializedSGBFlags, BufferIndex, 4, 3)
 
 #pragma pack(push, 1)
 struct GBSerializedState {
@@ -388,15 +389,15 @@
 	uint8_t vram[GB_SIZE_VRAM];
 	uint8_t wram[GB_SIZE_WORKING_RAM];
 
-	uint32_t reserved2[0xE0];
+	uint32_t reserved2[0xC4];
 
 	struct {
 		uint8_t attributes[90];
 		uint8_t command;
 		uint8_t bits;
 		GBSerializedSGBFlags flags;
-		uint8_t packet[16];
-		uint32_t reserved[4];
+		uint8_t inProgressPacket[16];
+		uint8_t packet[128];
 		uint8_t charRam[SGB_SIZE_CHAR_RAM];
 		uint8_t mapRam[SGB_SIZE_MAP_RAM];
 		uint8_t palRam[SGB_SIZE_PAL_RAM];

M include/mgba/internal/gb/video.h → include/mgba/internal/gb/video.h

@@ -143,6 +143,8 @@ bool bcpIncrement;
 	int ocpIndex;
 	bool ocpIncrement;
 	uint8_t sgbCommandHeader;
+	int sgbBufferIndex;
+	uint8_t sgbPacketBuffer[128];
 
 	uint16_t dmgPalette[12];
 	uint16_t palette[64];

M src/gb/renderers/software.c → src/gb/renderers/software.c

@@ -102,12 +102,7 @@ }
 
 static void _parseAttrBlock(struct GBVideoSoftwareRenderer* renderer, int start) {
 	uint8_t block[6];
-	if (start < 0) {
-		memcpy(block, renderer->sgbPartialDataSet, -start);
-		memcpy(&block[-start], renderer->sgbPacket, 6 + start);
-	} else {
-		memcpy(block, &renderer->sgbPacket[start], 6);
-	}
+	memcpy(block, &renderer->sgbPacket[start], 6);
 	unsigned x0 = block[2];
 	unsigned x1 = block[4];
 	unsigned y0 = block[3];
@@ -262,14 +257,13 @@ static void GBVideoSoftwareRendererWriteSGBPacket(struct GBVideoRenderer* renderer, uint8_t* data) {
 	struct GBVideoSoftwareRenderer* softwareRenderer = (struct GBVideoSoftwareRenderer*) renderer;
 	memcpy(softwareRenderer->sgbPacket, data, sizeof(softwareRenderer->sgbPacket));
 	int i;
-	if (!(softwareRenderer->sgbCommandHeader & 7)) {
-		softwareRenderer->sgbCommandHeader = data[0];
-		softwareRenderer->sgbPacketId = 0;
-		softwareRenderer->sgbTransfer = 0;
-	}
-	--softwareRenderer->sgbCommandHeader;
-	++softwareRenderer->sgbPacketId;
+	softwareRenderer->sgbCommandHeader = data[0];
+	softwareRenderer->sgbTransfer = 0;
 	int set;
+	int sets;
+	int attrX;
+	int attrY;
+	int attrDirection;
 	switch (softwareRenderer->sgbCommandHeader >> 3) {
 	case SGB_PAL_SET:
 		softwareRenderer->sgbPacket[1] = data[9];
@@ -284,58 +278,47 @@ memcpy(renderer->sgbAttributes, &renderer->sgbAttributeFiles[set * 90], 90);
 		}
 		break;
 	case SGB_ATTR_BLK:
-		if (softwareRenderer->sgbPacketId == 1) {
-			softwareRenderer->sgbDataSets = softwareRenderer->sgbPacket[1];
-			i = 2;
-		} else {
-			i = (9 - softwareRenderer->sgbPacketId) % 3 * -2;
-		}
-		for (; i <= 10 && softwareRenderer->sgbDataSets; i += 6, --softwareRenderer->sgbDataSets) {
+		sets = softwareRenderer->sgbPacket[1];
+		i = 2;
+		for (; i < (softwareRenderer->sgbCommandHeader & 7) << 4 && sets; i += 6, --sets) {
 			_parseAttrBlock(softwareRenderer, i);
 		}
-		if (i < 16 && softwareRenderer->sgbDataSets) {
-			memcpy(softwareRenderer->sgbPartialDataSet, &softwareRenderer->sgbPacket[i], 16 - i);
-		}
 		break;
 	case SGB_ATTR_CHR:
-		if (softwareRenderer->sgbPacketId == 1) {
-			softwareRenderer->sgbAttrX = softwareRenderer->sgbPacket[1];
-			softwareRenderer->sgbAttrY = softwareRenderer->sgbPacket[2];
-			if (softwareRenderer->sgbAttrX >= GB_VIDEO_HORIZONTAL_PIXELS / 8) {
-				softwareRenderer->sgbAttrX = 0;
-			}
-			if (softwareRenderer->sgbAttrY >= GB_VIDEO_VERTICAL_PIXELS / 8) {
-				softwareRenderer->sgbAttrY = 0;
-			}
-			softwareRenderer->sgbDataSets = softwareRenderer->sgbPacket[3];
-			softwareRenderer->sgbDataSets |= softwareRenderer->sgbPacket[4] << 8;
-			softwareRenderer->sgbAttrDirection = softwareRenderer->sgbPacket[5];
-			i = 6;
-		} else {
-			i = 0;
+		attrX = softwareRenderer->sgbPacket[1];
+		attrY = softwareRenderer->sgbPacket[2];
+		if (attrX >= GB_VIDEO_HORIZONTAL_PIXELS / 8) {
+			attrX = 0;
 		}
-		for (; i < 16 && softwareRenderer->sgbDataSets; ++i) {
+		if (attrY >= GB_VIDEO_VERTICAL_PIXELS / 8) {
+			attrY = 0;
+		}
+		sets = softwareRenderer->sgbPacket[3];
+		sets |= softwareRenderer->sgbPacket[4] << 8;
+		attrDirection = softwareRenderer->sgbPacket[5];
+		i = 6;
+		for (; i < (softwareRenderer->sgbCommandHeader & 7) << 4 && sets; ++i) {
 			int j;
-			for (j = 0; j < 4 && softwareRenderer->sgbDataSets; ++j, --softwareRenderer->sgbDataSets) {
+			for (j = 0; j < 4 && sets; ++j, --sets) {
 				uint8_t p = softwareRenderer->sgbPacket[i] >> (6 - j * 2);
-				_setAttribute(renderer->sgbAttributes, softwareRenderer->sgbAttrX, softwareRenderer->sgbAttrY, p & 3);
-				if (softwareRenderer->sgbAttrDirection) {
-					++softwareRenderer->sgbAttrY;
-					if (softwareRenderer->sgbAttrY >= GB_VIDEO_VERTICAL_PIXELS / 8) {
-						softwareRenderer->sgbAttrY = 0;
-						++softwareRenderer->sgbAttrX;
+				_setAttribute(renderer->sgbAttributes, attrX, attrY, p & 3);
+				if (attrDirection) {
+					++attrY;
+					if (attrY >= GB_VIDEO_VERTICAL_PIXELS / 8) {
+						attrY = 0;
+						++attrX;
 					}
-					if (softwareRenderer->sgbAttrX >= GB_VIDEO_HORIZONTAL_PIXELS / 8) {
-						softwareRenderer->sgbAttrX = 0;
+					if (attrX >= GB_VIDEO_HORIZONTAL_PIXELS / 8) {
+						attrX = 0;
 					}
 				} else {
-					++softwareRenderer->sgbAttrX;
-					if (softwareRenderer->sgbAttrX >= GB_VIDEO_HORIZONTAL_PIXELS / 8) {
-						softwareRenderer->sgbAttrX = 0;
-						++softwareRenderer->sgbAttrY;
+					++attrX;
+					if (attrX >= GB_VIDEO_HORIZONTAL_PIXELS / 8) {
+						attrX = 0;
+						++attrY;
 					}
-					if (softwareRenderer->sgbAttrY >= GB_VIDEO_VERTICAL_PIXELS / 8) {
-						softwareRenderer->sgbAttrY = 0;
+					if (attrY >= GB_VIDEO_VERTICAL_PIXELS / 8) {
+						attrY = 0;
 					}
 				}
 			}

M src/gb/serialize.c → src/gb/serialize.c

@@ -212,9 +212,11 @@
 	GBSerializedSGBFlags flags = 0;
 	flags = GBSerializedSGBFlagsSetP1Bits(flags, gb->currentSgbBits);
 	flags = GBSerializedSGBFlagsSetRenderMode(flags, gb->video.renderer->sgbRenderMode);
+	flags = GBSerializedSGBFlagsSetBufferIndex(flags, gb->video.sgbBufferIndex);
 	STORE_32LE(flags, 0, &state->sgb.flags);
 
-	memcpy(state->sgb.packet, gb->sgbPacket, sizeof(state->sgb.packet));
+	memcpy(state->sgb.packet, gb->video.sgbPacketBuffer, sizeof(state->sgb.packet));
+	memcpy(state->sgb.inProgressPacket, gb->sgbPacket, sizeof(state->sgb.inProgressPacket));
 
 	if (gb->video.renderer->sgbCharRam) {
 		memcpy(state->sgb.charRam, gb->video.renderer->sgbCharRam, sizeof(state->sgb.charRam));
@@ -241,8 +243,10 @@ GBSerializedSGBFlags flags;
 	LOAD_32LE(flags, 0, &state->sgb.flags);
 	gb->currentSgbBits = GBSerializedSGBFlagsGetP1Bits(flags);
 	gb->video.renderer->sgbRenderMode = GBSerializedSGBFlagsGetRenderMode(flags);
+	gb->video.sgbBufferIndex = GBSerializedSGBFlagsGetBufferIndex(flags);
 
-	memcpy(gb->sgbPacket, state->sgb.packet, sizeof(state->sgb.packet));
+	memcpy(gb->video.sgbPacketBuffer, state->sgb.packet, sizeof(state->sgb.packet));
+	memcpy(gb->sgbPacket, state->sgb.inProgressPacket, sizeof(state->sgb.inProgressPacket));
 
 	if (!gb->video.renderer->sgbCharRam) {
 		gb->video.renderer->sgbCharRam = anonymousMemoryMap(SGB_SIZE_CHAR_RAM);
@@ -267,5 +271,4 @@ memcpy(gb->video.renderer->sgbAttributeFiles, state->sgb.atfRam, sizeof(state->sgb.atfRam));
 	memcpy(gb->video.renderer->sgbAttributes, state->sgb.attributes, sizeof(state->sgb.attributes));
 
 	GBVideoWriteSGBPacket(&gb->video, (uint8_t[16]) { (SGB_ATRC_EN << 3) | 1, 0 });
-	GBVideoWriteSGBPacket(&gb->video, gb->sgbPacket);
 }

M src/gb/video.c → src/gb/video.c

@@ -113,6 +113,7 @@ video->renderer->sgbAttributeFiles = anonymousMemoryMap(SGB_SIZE_ATF_RAM);
 		video->renderer->sgbAttributes = malloc(90 * 45);
 		memset(video->renderer->sgbAttributes, 0, 90 * 45);
 		video->sgbCommandHeader = 0;
+		video->sgbBufferIndex = 0;
 	}
 
 	video->palette[0] = video->dmgPalette[0];
@@ -580,6 +581,7 @@
 void GBVideoWriteSGBPacket(struct GBVideo* video, uint8_t* data) {
 	int i;
 	if (!(video->sgbCommandHeader & 7)) {
+		video->sgbBufferIndex = 0;
 		if ((data[0] >> 3) > SGB_OBJ_TRN) {
 			video->sgbCommandHeader = 0;
 			return;
@@ -587,20 +589,25 @@ }
 		video->sgbCommandHeader = data[0];
 	}
 	--video->sgbCommandHeader;
+	memcpy(&video->sgbPacketBuffer[video->sgbBufferIndex << 4], data, 16);
+	++video->sgbBufferIndex;
+	if (video->sgbCommandHeader & 7) {
+		return;
+	}
 	switch (video->sgbCommandHeader >> 3) {
 	case SGB_PAL01:
-		video->palette[0] = data[1] | (data[2] << 8);
-		video->palette[1] = data[3] | (data[4] << 8);
-		video->palette[2] = data[5] | (data[6] << 8);
-		video->palette[3] = data[7] | (data[8] << 8);
+		video->palette[0] = video->sgbPacketBuffer[1] | (video->sgbPacketBuffer[2] << 8);
+		video->palette[1] = video->sgbPacketBuffer[3] | (video->sgbPacketBuffer[4] << 8);
+		video->palette[2] = video->sgbPacketBuffer[5] | (video->sgbPacketBuffer[6] << 8);
+		video->palette[3] = video->sgbPacketBuffer[7] | (video->sgbPacketBuffer[8] << 8);
 
-		video->palette[4] = data[1] | (data[2] << 8);
-		video->palette[5] = data[9] | (data[10] << 8);
-		video->palette[6] = data[11] | (data[12] << 8);
-		video->palette[7] = data[13] | (data[14] << 8);
+		video->palette[4] = video->sgbPacketBuffer[1] | (video->sgbPacketBuffer[2] << 8);
+		video->palette[5] = video->sgbPacketBuffer[9] | (video->sgbPacketBuffer[10] << 8);
+		video->palette[6] = video->sgbPacketBuffer[11] | (video->sgbPacketBuffer[12] << 8);
+		video->palette[7] = video->sgbPacketBuffer[13] | (video->sgbPacketBuffer[14] << 8);
 
-		video->palette[8] = data[1] | (data[2] << 8);
-		video->palette[12] = data[1] | (data[2] << 8);
+		video->palette[8] = video->sgbPacketBuffer[1] | (video->sgbPacketBuffer[2] << 8);
+		video->palette[12] = video->sgbPacketBuffer[1] | (video->sgbPacketBuffer[2] << 8);
 
 		video->renderer->writePalette(video->renderer, 0, video->palette[0]);
 		video->renderer->writePalette(video->renderer, 1, video->palette[1]);
@@ -614,13 +621,13 @@ video->renderer->writePalette(video->renderer, 8, video->palette[8]);
 		video->renderer->writePalette(video->renderer, 12, video->palette[12]);
 		break;
 	case SGB_PAL23:
-		video->palette[9] = data[3] | (data[4] << 8);
-		video->palette[10] = data[5] | (data[6] << 8);
-		video->palette[11] = data[7] | (data[8] << 8);
+		video->palette[9] = video->sgbPacketBuffer[3] | (video->sgbPacketBuffer[4] << 8);
+		video->palette[10] = video->sgbPacketBuffer[5] | (video->sgbPacketBuffer[6] << 8);
+		video->palette[11] = video->sgbPacketBuffer[7] | (video->sgbPacketBuffer[8] << 8);
 
-		video->palette[13] = data[9] | (data[10] << 8);
-		video->palette[14] = data[11] | (data[12] << 8);
-		video->palette[15] = data[13] | (data[14] << 8);
+		video->palette[13] = video->sgbPacketBuffer[9] | (video->sgbPacketBuffer[10] << 8);
+		video->palette[14] = video->sgbPacketBuffer[11] | (video->sgbPacketBuffer[12] << 8);
+		video->palette[15] = video->sgbPacketBuffer[13] | (video->sgbPacketBuffer[14] << 8);
 		video->renderer->writePalette(video->renderer, 9, video->palette[9]);
 		video->renderer->writePalette(video->renderer, 10, video->palette[10]);
 		video->renderer->writePalette(video->renderer, 11, video->palette[11]);
@@ -629,18 +636,18 @@ video->renderer->writePalette(video->renderer, 14, video->palette[14]);
 		video->renderer->writePalette(video->renderer, 15, video->palette[15]);
 		break;
 	case SGB_PAL03:
-		video->palette[0] = data[1] | (data[2] << 8);
-		video->palette[1] = data[3] | (data[4] << 8);
-		video->palette[2] = data[5] | (data[6] << 8);
-		video->palette[3] = data[7] | (data[8] << 8);
+		video->palette[0] = video->sgbPacketBuffer[1] | (video->sgbPacketBuffer[2] << 8);
+		video->palette[1] = video->sgbPacketBuffer[3] | (video->sgbPacketBuffer[4] << 8);
+		video->palette[2] = video->sgbPacketBuffer[5] | (video->sgbPacketBuffer[6] << 8);
+		video->palette[3] = video->sgbPacketBuffer[7] | (video->sgbPacketBuffer[8] << 8);
 
-		video->palette[4] = data[1] | (data[2] << 8);
-		video->palette[8] = data[1] | (data[2] << 8);
+		video->palette[4] = video->sgbPacketBuffer[1] | (video->sgbPacketBuffer[2] << 8);
+		video->palette[8] = video->sgbPacketBuffer[1] | (video->sgbPacketBuffer[2] << 8);
 
-		video->palette[12] = data[1] | (data[2] << 8);
-		video->palette[13] = data[9] | (data[10] << 8);
-		video->palette[14] = data[11] | (data[12] << 8);
-		video->palette[15] = data[13] | (data[14] << 8);
+		video->palette[12] = video->sgbPacketBuffer[1] | (video->sgbPacketBuffer[2] << 8);
+		video->palette[13] = video->sgbPacketBuffer[9] | (video->sgbPacketBuffer[10] << 8);
+		video->palette[14] = video->sgbPacketBuffer[11] | (video->sgbPacketBuffer[12] << 8);
+		video->palette[15] = video->sgbPacketBuffer[13] | (video->sgbPacketBuffer[14] << 8);
 		video->renderer->writePalette(video->renderer, 0, video->palette[0]);
 		video->renderer->writePalette(video->renderer, 1, video->palette[1]);
 		video->renderer->writePalette(video->renderer, 2, video->palette[2]);
@@ -653,13 +660,13 @@ video->renderer->writePalette(video->renderer, 14, video->palette[14]);
 		video->renderer->writePalette(video->renderer, 15, video->palette[15]);
 		break;
 	case SGB_PAL12:
-		video->palette[5] = data[3] | (data[4] << 8);
-		video->palette[6] = data[5] | (data[6] << 8);
-		video->palette[7] = data[7] | (data[8] << 8);
+		video->palette[5] = video->sgbPacketBuffer[3] | (video->sgbPacketBuffer[4] << 8);
+		video->palette[6] = video->sgbPacketBuffer[5] | (video->sgbPacketBuffer[6] << 8);
+		video->palette[7] = video->sgbPacketBuffer[7] | (video->sgbPacketBuffer[8] << 8);
 
-		video->palette[9] = data[9] | (data[10] << 8);
-		video->palette[10] = data[11] | (data[12] << 8);
-		video->palette[11] = data[13] | (data[14] << 8);
+		video->palette[9] = video->sgbPacketBuffer[9] | (video->sgbPacketBuffer[10] << 8);
+		video->palette[10] = video->sgbPacketBuffer[11] | (video->sgbPacketBuffer[12] << 8);
+		video->palette[11] = video->sgbPacketBuffer[13] | (video->sgbPacketBuffer[14] << 8);
 		video->renderer->writePalette(video->renderer, 5, video->palette[5]);
 		video->renderer->writePalette(video->renderer, 6, video->palette[6]);
 		video->renderer->writePalette(video->renderer, 7, video->palette[7]);
@@ -669,7 +676,7 @@ video->renderer->writePalette(video->renderer, 11, video->palette[11]);
 		break;
 	case SGB_PAL_SET:
 		for (i = 0; i < 4; ++i) {
-			uint16_t entry = (data[2 + (i * 2)] << 8) | data[1 + (i * 2)];
+			uint16_t entry = (video->sgbPacketBuffer[2 + (i * 2)] << 8) | video->sgbPacketBuffer[1 + (i * 2)];
 			if (entry >= 0x200) {
 				mLOG(GB, STUB, "Unimplemented SGB palette overflow: %03X", entry);
 				continue;
@@ -696,13 +703,13 @@ break;
 	case SGB_MLT_REG:
 		return;
 	case SGB_MASK_EN:
-		video->renderer->sgbRenderMode = data[1] & 0x3;
+		video->renderer->sgbRenderMode = video->sgbPacketBuffer[1] & 0x3;
 		break;
 	default:
-		mLOG(GB, STUB, "Unimplemented SGB command: %02X", data[0] >> 3);
+		mLOG(GB, STUB, "Unimplemented SGB command: %02X", video->sgbPacketBuffer[0] >> 3);
 		return;
 	}
-	video->renderer->writeSGBPacket(video->renderer, data);
+	video->renderer->writeSGBPacket(video->renderer, video->sgbPacketBuffer);
 }
 
 static void GBVideoDummyRendererInit(struct GBVideoRenderer* renderer, enum GBModel model, bool borders) {

all repos — mgba @ 4101fe54c670616fc4e5bc7416b3c98fa7aad102

mGBA Game Boy Advance Emulator