GBA Video: Fix threaded rendering race conditions
Jeffrey Pfau jeffrey@endrift.com
Tue, 09 Aug 2016 22:30:35 -0700
2 files changed,
56 insertions(+),
42 deletions(-)
M
src/gba/renderers/thread-proxy.c
→
src/gba/renderers/thread-proxy.c
@@ -70,7 +70,6 @@ ConditionInit(&proxyRenderer->fromThreadCond);
ConditionInit(&proxyRenderer->toThreadCond); MutexInit(&proxyRenderer->mutex); RingFIFOInit(&proxyRenderer->dirtyQueue, 0x40000, 0x1000); - proxyRenderer->threadState = PROXY_THREAD_STOPPED; proxyRenderer->vramProxy = anonymousMemoryMap(SIZE_VRAM); proxyRenderer->backend->palette = proxyRenderer->paletteProxy;@@ -230,10 +229,11 @@ 0,
0xDEADBEEF, }; RingFIFOWrite(&proxyRenderer->dirtyQueue, &dirty, sizeof(dirty)); - while (proxyRenderer->threadState == PROXY_THREAD_BUSY) { + do { + RingFIFOWrite(&proxyRenderer->dirtyQueue, &dirty, sizeof(dirty)); ConditionWake(&proxyRenderer->toThreadCond); ConditionWait(&proxyRenderer->fromThreadCond, &proxyRenderer->mutex); - } + } while (proxyRenderer->threadState == PROXY_THREAD_BUSY); proxyRenderer->backend->finishFrame(proxyRenderer->backend); proxyRenderer->vramDirtyBitmap = 0; MutexUnlock(&proxyRenderer->mutex);@@ -280,45 +280,48 @@ struct GBAVideoThreadProxyRenderer* proxyRenderer = renderer;
ThreadSetName("Proxy Renderer Thread"); MutexLock(&proxyRenderer->mutex); + struct GBAVideoDirtyInfo item = {0}; while (proxyRenderer->threadState != PROXY_THREAD_STOPPED) { ConditionWait(&proxyRenderer->toThreadCond, &proxyRenderer->mutex); if (proxyRenderer->threadState == PROXY_THREAD_STOPPED) { break; } - proxyRenderer->threadState = PROXY_THREAD_BUSY; - - MutexUnlock(&proxyRenderer->mutex); - struct GBAVideoDirtyInfo item; - while (RingFIFORead(&proxyRenderer->dirtyQueue, &item, sizeof(item))) { - switch (item.type) { - case DIRTY_REGISTER: - proxyRenderer->backend->writeVideoRegister(proxyRenderer->backend, item.address, item.value); - break; - case DIRTY_PALETTE: - proxyRenderer->paletteProxy[item.address >> 1] = item.value; - proxyRenderer->backend->writePalette(proxyRenderer->backend, item.address, item.value); - break; - case DIRTY_OAM: - proxyRenderer->oamProxy.raw[item.address] = item.value; - proxyRenderer->backend->writeOAM(proxyRenderer->backend, item.address); - break; - case DIRTY_VRAM: - while (!RingFIFORead(&proxyRenderer->dirtyQueue, &proxyRenderer->vramProxy[item.address >> 1], 0x1000)); - proxyRenderer->backend->writeVRAM(proxyRenderer->backend, item.address); - break; - case DIRTY_SCANLINE: - proxyRenderer->backend->drawScanline(proxyRenderer->backend, item.address); - break; - case DIRTY_FLUSH: - // This is only here to ensure the queue gets flushed - break; - default: - // FIFO was corrupted - proxyRenderer->threadState = PROXY_THREAD_STOPPED; - break; - } + if (RingFIFORead(&proxyRenderer->dirtyQueue, &item, sizeof(item))) { + proxyRenderer->threadState = PROXY_THREAD_BUSY; + MutexUnlock(&proxyRenderer->mutex); + do { + switch (item.type) { + case DIRTY_REGISTER: + proxyRenderer->backend->writeVideoRegister(proxyRenderer->backend, item.address, item.value); + break; + case DIRTY_PALETTE: + proxyRenderer->paletteProxy[item.address >> 1] = item.value; + proxyRenderer->backend->writePalette(proxyRenderer->backend, item.address, item.value); + break; + case DIRTY_OAM: + proxyRenderer->oamProxy.raw[item.address] = item.value; + proxyRenderer->backend->writeOAM(proxyRenderer->backend, item.address); + break; + case DIRTY_VRAM: + while (!RingFIFORead(&proxyRenderer->dirtyQueue, &proxyRenderer->vramProxy[item.address >> 1], 0x1000)); + proxyRenderer->backend->writeVRAM(proxyRenderer->backend, item.address); + break; + case DIRTY_SCANLINE: + proxyRenderer->backend->drawScanline(proxyRenderer->backend, item.address); + break; + case DIRTY_FLUSH: + MutexLock(&proxyRenderer->mutex); + goto out; + default: + // FIFO was corrupted + MutexLock(&proxyRenderer->mutex); + proxyRenderer->threadState = PROXY_THREAD_STOPPED; + goto out; + } + } while (proxyRenderer->threadState == PROXY_THREAD_BUSY && RingFIFORead(&proxyRenderer->dirtyQueue, &item, sizeof(item))); + MutexLock(&proxyRenderer->mutex); } - MutexLock(&proxyRenderer->mutex); + out: ConditionWake(&proxyRenderer->fromThreadCond); if (proxyRenderer->threadState != PROXY_THREAD_STOPPED) { proxyRenderer->threadState = PROXY_THREAD_IDLE;
M
src/util/ring-fifo.c
→
src/util/ring-fifo.c
@@ -7,6 +7,15 @@ #include "ring-fifo.h"
#include "util/memory.h" +#ifndef _MSC_VER +#define ATOMIC_STORE(DST, SRC) __atomic_store_n(&DST, SRC, __ATOMIC_RELEASE) +#define ATOMIC_LOAD(DST, SRC) DST = __atomic_load_n(&SRC, __ATOMIC_ACQUIRE) +#else +// TODO +#define ATOMIC_STORE(DST, SRC) DST = SRC +#define ATOMIC_LOAD(DST, SRC) DST = SRC +#endif + void RingFIFOInit(struct RingFIFO* buffer, size_t capacity, size_t maxalloc) { buffer->data = anonymousMemoryMap(capacity); buffer->capacity = capacity;@@ -24,13 +33,14 @@ return buffer->capacity;
} void RingFIFOClear(struct RingFIFO* buffer) { - buffer->readPtr = buffer->data; - buffer->writePtr = buffer->data; + ATOMIC_STORE(buffer->readPtr, buffer->data); + ATOMIC_STORE(buffer->writePtr, buffer->data); } size_t RingFIFOWrite(struct RingFIFO* buffer, const void* value, size_t length) { void* data = buffer->writePtr; - void* end = buffer->readPtr; + void* end; + ATOMIC_LOAD(end, buffer->readPtr); size_t remaining; if ((intptr_t) data - (intptr_t) buffer->data + buffer->maxalloc >= buffer->capacity) { data = buffer->data;@@ -46,13 +56,14 @@ }
if (value) { memcpy(data, value, length); } - buffer->writePtr = (void*) ((intptr_t) data + length); + ATOMIC_STORE(buffer->writePtr, (void*) ((intptr_t) data + length)); return length; } size_t RingFIFORead(struct RingFIFO* buffer, void* output, size_t length) { void* data = buffer->readPtr; - void* end = buffer->writePtr; + void* end; + ATOMIC_LOAD(end, buffer->writePtr); size_t remaining; if ((intptr_t) data - (intptr_t) buffer->data + buffer->maxalloc >= buffer->capacity) { data = buffer->data;@@ -68,6 +79,6 @@ }
if (output) { memcpy(output, data, length); } - buffer->readPtr = (void*) ((intptr_t) data + length); + ATOMIC_STORE(buffer->readPtr, (void*) ((intptr_t) data + length)); return length; }