/mgba

Permalink
Browse files

GBA Video: Another attempt at patching the threaded renderer race con… 

…ditions
  • Loading branch information...
@endrift
endrift committed Aug 15, 2016
1 parent 812d063 commit ac115422264c242439c19aaf383b6b743a309c69
Unified Split
Showing with 66 additions and 34 deletions.
  1. +41 −21 src/gba/renderers/thread-proxy.c
  2. +1 −1 src/platform/psp2/psp2-context.c
  3. +23 −10 src/util/ring-fifo.c
  4. +1 −2 src/util/ring-fifo.h
View
62 src/gba/renderers/thread-proxy.c
@@ -69,7 +69,7 @@ void GBAVideoThreadProxyRendererInit(struct GBAVideoRenderer* renderer) {
ConditionInit(&proxyRenderer->fromThreadCond);
ConditionInit(&proxyRenderer->toThreadCond);
MutexInit(&proxyRenderer->mutex);
- RingFIFOInit(&proxyRenderer->dirtyQueue, 0x48000, 0x1000);
+ RingFIFOInit(&proxyRenderer->dirtyQueue, 0x40000);
proxyRenderer->vramProxy = anonymousMemoryMap(SIZE_VRAM);
proxyRenderer->backend->palette = proxyRenderer->paletteProxy;
@@ -123,6 +123,23 @@ void GBAVideoThreadProxyRendererDeinit(struct GBAVideoRenderer* renderer) {
mappedMemoryFree(proxyRenderer->vramProxy, SIZE_VRAM);
}
+static bool _writeData(struct GBAVideoThreadProxyRenderer* proxyRenderer, void* data, size_t length) {
+ while (!RingFIFOWrite(&proxyRenderer->dirtyQueue, data, length)) {
+ mLOG(GBA_VIDEO, WARN, "Can't write 0x%z bytes. Proxy thread asleep?", length);
+ mLOG(GBA_VIDEO, DEBUG, "Queue status: read: %p, write: %p", proxyRenderer->dirtyQueue.readPtr, proxyRenderer->dirtyQueue.writePtr);
+ MutexLock(&proxyRenderer->mutex);
+ if (proxyRenderer->threadState == PROXY_THREAD_STOPPED) {
+ mLOG(GBA_VIDEO, ERROR, "Proxy thread stopped prematurely!");
+ MutexUnlock(&proxyRenderer->mutex);
+ return false;
+ }
+ ConditionWake(&proxyRenderer->toThreadCond);
+ ConditionWait(&proxyRenderer->fromThreadCond, &proxyRenderer->mutex);
+ MutexUnlock(&proxyRenderer->mutex);
+ }
+ return true;
+}
+
uint16_t GBAVideoThreadProxyRendererWriteVideoRegister(struct GBAVideoRenderer* renderer, uint32_t address, uint16_t value) {
struct GBAVideoThreadProxyRenderer* proxyRenderer = (struct GBAVideoThreadProxyRenderer*) renderer;
switch (address) {
@@ -153,7 +170,7 @@ uint16_t GBAVideoThreadProxyRendererWriteVideoRegister(struct GBAVideoRenderer*
value,
0xDEADBEEF,
};
- RingFIFOWrite(&proxyRenderer->dirtyQueue, &dirty, sizeof(dirty));
+ _writeData(proxyRenderer, &dirty, sizeof(dirty));
return value;
}
@@ -174,7 +191,7 @@ void GBAVideoThreadProxyRendererWritePalette(struct GBAVideoRenderer* renderer,
value,
0xDEADBEEF,
};
- RingFIFOWrite(&proxyRenderer->dirtyQueue, &dirty, sizeof(dirty));
+ _writeData(proxyRenderer, &dirty, sizeof(dirty));
if (renderer->cache) {
GBAVideoTileCacheWritePalette(renderer->cache, address);
}
@@ -188,7 +205,7 @@ void GBAVideoThreadProxyRendererWriteOAM(struct GBAVideoRenderer* renderer, uint
proxyRenderer->d.oam->raw[oam],
0xDEADBEEF,
};
- RingFIFOWrite(&proxyRenderer->dirtyQueue, &dirty, sizeof(dirty));
+ _writeData(proxyRenderer, &dirty, sizeof(dirty));
}
void GBAVideoThreadProxyRendererDrawScanline(struct GBAVideoRenderer* renderer, int y) {
@@ -207,14 +224,8 @@ void GBAVideoThreadProxyRendererDrawScanline(struct GBAVideoRenderer* renderer,
0xABCD,
0xDEADBEEF,
};
- RingFIFOWrite(&proxyRenderer->dirtyQueue, &dirty, sizeof(dirty));
- while (!RingFIFOWrite(&proxyRenderer->dirtyQueue, &proxyRenderer->d.vram[j * 0x800], 0x1000)) {
- ConditionWake(&proxyRenderer->toThreadCond);
- if (proxyRenderer->threadState == PROXY_THREAD_STOPPED) {
- mLOG(GBA_VIDEO, FATAL, "Proxy thread stopped prematurely!");
- return;
- }
- }
+ _writeData(proxyRenderer, &dirty, sizeof(dirty));
+ _writeData(proxyRenderer, &proxyRenderer->d.vram[j * 0x800], 0x1000);
}
}
struct GBAVideoDirtyInfo dirty = {
@@ -223,7 +234,7 @@ void GBAVideoThreadProxyRendererDrawScanline(struct GBAVideoRenderer* renderer,
0,
0xDEADBEEF,
};
- RingFIFOWrite(&proxyRenderer->dirtyQueue, &dirty, sizeof(dirty));
+ _writeData(proxyRenderer, &dirty, sizeof(dirty));
if ((y & 15) == 15) {
ConditionWake(&proxyRenderer->toThreadCond);
}
@@ -232,7 +243,9 @@ void GBAVideoThreadProxyRendererDrawScanline(struct GBAVideoRenderer* renderer,
void GBAVideoThreadProxyRendererFinishFrame(struct GBAVideoRenderer* renderer) {
struct GBAVideoThreadProxyRenderer* proxyRenderer = (struct GBAVideoThreadProxyRenderer*) renderer;
if (proxyRenderer->threadState == PROXY_THREAD_STOPPED) {
- mLOG(GBA_VIDEO, FATAL, "Proxy thread stopped prematurely!");
+ mLOG(GBA_VIDEO, ERROR, "Proxy thread stopped prematurely!");
+ GBAVideoThreadProxyRendererDeinit(renderer);
+ GBAVideoThreadProxyRendererInit(renderer);
return;
}
MutexLock(&proxyRenderer->mutex);
@@ -243,9 +256,8 @@ void GBAVideoThreadProxyRendererFinishFrame(struct GBAVideoRenderer* renderer) {
0,
0xDEADBEEF,
};
- RingFIFOWrite(&proxyRenderer->dirtyQueue, &dirty, sizeof(dirty));
+ _writeData(proxyRenderer, &dirty, sizeof(dirty));
do {
- RingFIFOWrite(&proxyRenderer->dirtyQueue, &dirty, sizeof(dirty));
ConditionWake(&proxyRenderer->toThreadCond);
ConditionWait(&proxyRenderer->fromThreadCond, &proxyRenderer->mutex);
} while (proxyRenderer->threadState == PROXY_THREAD_BUSY);
@@ -255,15 +267,16 @@ void GBAVideoThreadProxyRendererFinishFrame(struct GBAVideoRenderer* renderer) {
}
static void GBAVideoThreadProxyRendererGetPixels(struct GBAVideoRenderer* renderer, unsigned* stride, const void** pixels) {
- struct GBAVideoThreadProxyRenderer* proxyRenderer = (struct GBAVideoThreadProxyRenderer*) renderer; MutexLock(&proxyRenderer->mutex);
+ struct GBAVideoThreadProxyRenderer* proxyRenderer = (struct GBAVideoThreadProxyRenderer*) renderer;
+ MutexLock(&proxyRenderer->mutex);
// Insert an extra item into the queue to make sure it gets flushed
struct GBAVideoDirtyInfo dirty = {
DIRTY_FLUSH,
0,
0,
0xDEADBEEF,
};
- RingFIFOWrite(&proxyRenderer->dirtyQueue, &dirty, sizeof(dirty));
+ _writeData(proxyRenderer, &dirty, sizeof(dirty));
while (proxyRenderer->threadState == PROXY_THREAD_BUSY) {
ConditionWake(&proxyRenderer->toThreadCond);
ConditionWait(&proxyRenderer->fromThreadCond, &proxyRenderer->mutex);
@@ -273,15 +286,16 @@ static void GBAVideoThreadProxyRendererGetPixels(struct GBAVideoRenderer* render
}
static void GBAVideoThreadProxyRendererPutPixels(struct GBAVideoRenderer* renderer, unsigned stride, void* pixels) {
- struct GBAVideoThreadProxyRenderer* proxyRenderer = (struct GBAVideoThreadProxyRenderer*) renderer; MutexLock(&proxyRenderer->mutex);
+ struct GBAVideoThreadProxyRenderer* proxyRenderer = (struct GBAVideoThreadProxyRenderer*) renderer;
+ MutexLock(&proxyRenderer->mutex);
// Insert an extra item into the queue to make sure it gets flushed
struct GBAVideoDirtyInfo dirty = {
DIRTY_FLUSH,
0,
0,
0xDEADBEEF,
};
- RingFIFOWrite(&proxyRenderer->dirtyQueue, &dirty, sizeof(dirty));
+ _writeData(proxyRenderer, &dirty, sizeof(dirty));
while (proxyRenderer->threadState == PROXY_THREAD_BUSY) {
ConditionWake(&proxyRenderer->toThreadCond);
ConditionWait(&proxyRenderer->fromThreadCond, &proxyRenderer->mutex);
@@ -319,7 +333,13 @@ static THREAD_ENTRY _proxyThread(void* renderer) {
break;
case DIRTY_VRAM:
while (!RingFIFORead(&proxyRenderer->dirtyQueue, &proxyRenderer->vramProxy[item.address >> 1], 0x1000)) {
+ mLOG(GBA_VIDEO, WARN, "Proxy thread can't read VRAM. CPU thread asleep?");
+ MutexLock(&proxyRenderer->mutex);
ConditionWake(&proxyRenderer->fromThreadCond);
+ proxyRenderer->threadState = PROXY_THREAD_IDLE;
+ ConditionWait(&proxyRenderer->toThreadCond, &proxyRenderer->mutex);
+ proxyRenderer->threadState = PROXY_THREAD_BUSY;
+ MutexUnlock(&proxyRenderer->mutex);
}
proxyRenderer->backend->writeVRAM(proxyRenderer->backend, item.address);
break;
@@ -333,7 +353,7 @@ static THREAD_ENTRY _proxyThread(void* renderer) {
// FIFO was corrupted
MutexLock(&proxyRenderer->mutex);
proxyRenderer->threadState = PROXY_THREAD_STOPPED;
- mLOG(GBA_VIDEO, FATAL, "Proxy thread queue got corrupted!");
+ mLOG(GBA_VIDEO, ERROR, "Proxy thread queue got corrupted!");
goto out;
}
} while (proxyRenderer->threadState == PROXY_THREAD_BUSY && RingFIFORead(&proxyRenderer->dirtyQueue, &item, sizeof(item)));
View
2 src/platform/psp2/psp2-context.c
@@ -233,7 +233,7 @@ void mPSP2LoadROM(struct mGUIRunner* runner) {
break;
}
- RingFIFOInit(&audioContext.buffer, PSP2_AUDIO_BUFFER_SIZE * sizeof(struct GBAStereoSample), PSP2_SAMPLES * 4);
+ RingFIFOInit(&audioContext.buffer, PSP2_AUDIO_BUFFER_SIZE * sizeof(struct GBAStereoSample));
MutexInit(&audioContext.mutex);
ConditionInit(&audioContext.cond);
audioContext.running = true;
View
33 src/util/ring-fifo.c
@@ -16,10 +16,9 @@
#define ATOMIC_LOAD(DST, SRC) DST = SRC
#endif
-void RingFIFOInit(struct RingFIFO* buffer, size_t capacity, size_t maxalloc) {
+void RingFIFOInit(struct RingFIFO* buffer, size_t capacity) {
buffer->data = anonymousMemoryMap(capacity);
buffer->capacity = capacity;
- buffer->maxalloc = maxalloc;
RingFIFOClear(buffer);
}
@@ -41,15 +40,24 @@ size_t RingFIFOWrite(struct RingFIFO* buffer, const void* value, size_t length)
void* data = buffer->writePtr;
void* end;
ATOMIC_LOAD(end, buffer->readPtr);
- size_t remaining;
- if ((intptr_t) data - (intptr_t) buffer->data + buffer->maxalloc >= buffer->capacity) {
+
+ // Wrap around if we can't fit enough in here
+ if ((intptr_t) data - (intptr_t) buffer->data + length >= buffer->capacity) {
+ if (end == buffer->data) {
+ // Oops! If we wrap now, it'll appear empty
+ return 0;
+ }
data = buffer->data;
}
+
+ size_t remaining;
if (data >= end) {
- remaining = (intptr_t) buffer->data + buffer->capacity - (intptr_t) data;
+ uintptr_t bufferEnd = (uintptr_t) buffer->data + buffer->capacity;
+ remaining = bufferEnd - (uintptr_t) data;
} else {
- remaining = (intptr_t) end - (intptr_t) data;
+ remaining = (uintptr_t) end - (uintptr_t) data;
}
+ // Note that we can't hit the end pointer
if (remaining <= length) {
return 0;
}
@@ -64,16 +72,21 @@ size_t RingFIFORead(struct RingFIFO* buffer, void* output, size_t length) {
void* data = buffer->readPtr;
void* end;
ATOMIC_LOAD(end, buffer->writePtr);
- size_t remaining;
- if ((intptr_t) data - (intptr_t) buffer->data + buffer->maxalloc >= buffer->capacity) {
+
+ // Wrap around if we can't fit enough in here
+ if ((intptr_t) data - (intptr_t) buffer->data + length >= buffer->capacity) {
data = buffer->data;
}
+
+ size_t remaining;
if (data > end) {
- remaining = (intptr_t) buffer->data + buffer->capacity - (intptr_t) data;
+ uintptr_t bufferEnd = (uintptr_t) buffer->data + buffer->capacity;
+ remaining = bufferEnd - (uintptr_t) data;
} else {
remaining = (intptr_t) end - (intptr_t) data;
}
- if (remaining <= length) {
+ // If the pointers touch, it's empty
+ if (remaining < length) {
return 0;
}
if (output) {
View
3 src/util/ring-fifo.h
@@ -11,12 +11,11 @@
struct RingFIFO {
void* data;
size_t capacity;
- size_t maxalloc;
void* readPtr;
void* writePtr;
};
-void RingFIFOInit(struct RingFIFO* buffer, size_t capacity, size_t maxalloc);
+void RingFIFOInit(struct RingFIFO* buffer, size_t capacity);
void RingFIFODeinit(struct RingFIFO* buffer);
size_t RingFIFOCapacity(const struct RingFIFO* buffer);
void RingFIFOClear(struct RingFIFO* buffer);

0 comments on commit ac11542

Please sign in to comment.