code: plan9front

Info  •  Files  •  Log

games/gb: various HDMA fixes
H-blank DMA should only transfer 16 bytes per h-blank, rather than
waiting for the first h-blank and then transferring the whole size.

HDMAC should read 0xff when the transfer is finished, and 0 in the
high bit when the transfer is ongoing. Also, if 0 is written in the
high bit, the current transfer should be aborted.

Introduce two flags, DMAREADY and DMAHBLANK rather than special
constants 1 and -1. If dma is non-zero, there is an ongoing DMA. If
DMAREADY is set, the next chunk is ready to transfer.

Reference: https://gbdev.io/pandocs/#ff55-hdma5-cgb-mode-only-new-dma-length-mode-start

Tested with pokemon crystal.

What was happening is that when the game was loading N background tiles
into vram (each 16 bytes, so one per h-blank), it did something like
this:
- start an hdma transfer for N+1 tiles
- after the Nth tile is transferred, it would read HDMA5, clear the
high bit, then write it back to abort the transfer.

games/gb would instead transfer all N+1 tiles at once, overwriting one
extra tile with whatever was 1 past the end of the source array, and
then would interpret the cancel request as the start of a new transfer
of 16 bytes, which would copy an additional tile past the end. The end
result is that every transfer would end up copying N+2 tiles instead
of just N, overwriting certain tiles with whatever was after the end
of the source data.

--- a/sys/src/games/gb/dat.h

+++ b/sys/src/games/gb/dat.h

@@ -8,7 +8,7 @@

 extern uchar vram[16384];

 extern int nrom, nback, nbackbank;

 extern u32int pal[64];

-extern s8int dma;

+extern u8int dma;

 extern u32int divclock;

 extern Event *elist;

@@ -113,6 +113,9 @@

 	FEATRAM = 1,

 	FEATBAT = 2,

 	FEATTIM = 4,

+

+	DMAREADY = 1,

+	DMAHBLANK = 2,

 	INIT = -1,

 	SAVE = -2,

--- a/sys/src/games/gb/gb.c

+++ b/sys/src/games/gb/gb.c

@@ -308,7 +308,7 @@

 			qlock(&pauselock);

 			qunlock(&pauselock);

 		}

-		if(dma > 0)

+		if(dma & DMAREADY)

 			t = dmastep();

 		else

 			t = step();

--- a/sys/src/games/gb/mem.c

+++ b/sys/src/games/gb/mem.c

@@ -15,7 +15,7 @@

 u32int divclock;

 int prish;

 MBC3Timer timer, timerl;

-s8int dma;

+u8int dma;

 u32int white;

 u32int moncols[4];

@@ -175,7 +175,14 @@

 	case HDMAC:

 		if((mode & COL) == 0)

 			goto ff;

-		dma = (v & 0x80) != 0 ? -1 : 1;

+		if(v & 0x80){

+			v &= 0x7f;

+			dma = DMAHBLANK;

+		}else if(dma){

+			v |= 0x80;

+			dma = 0;

+		}else

+			dma = DMAREADY;

 		break;

 	case NR10: v |= 0x80; goto snd;

 	case NR14: case NR24: v |= 0x38; goto snd;

@@ -534,6 +541,7 @@

 	reg[VBK] = 0xfe;

 	reg[SVBK] = 0xf8;

 	reg[IF] = 0xe0;

+	reg[HDMAC] = 0xff;

 }

 void

@@ -572,7 +580,7 @@

 {

 	int i;

 	u16int sa, da;

-	

+

 	sa = (reg[HDMASL] | reg[HDMASH] << 8) & 0xfff0;

 	da = (reg[HDMADL] | reg[HDMADH] << 8) & 0x1ff0 | 0x8000;

 	for(i = 0; i < 16; i++)

@@ -583,12 +591,9 @@

 	reg[HDMADL] += 16;

 	if((reg[HDMADL] & 0xf0) == 0)

 		reg[HDMADH]++;

-	if((reg[HDMAC] & 0x7f) == 0)

+	if(--reg[HDMAC] == 0xff)

 		dma = 0;

-	else{

-		reg[HDMAC]--;

-		if((reg[HDMAC] & 0x80) != 0)

-			dma = 1;

-	}

+	else if(dma & DMAHBLANK)

+		dma &= ~DMAREADY;

 	return 64;

 }

--- a/sys/src/games/gb/ppu.c

+++ b/sys/src/games/gb/ppu.c

@@ -338,8 +338,8 @@

 			reg[IF] |= IRQLCDS;

 		t = hblclock + 456 * 2 - clock;

 		addevent(&evhblank, t < 0 ? 456 * 2 : t);

-		if(dma < 0)

-			dma = 1;

+		if(dma & DMAHBLANK)

+			dma |= DMAREADY;

 		break;

 	}

 }