From 9c75c1e6fd6fae957aad28dae47a3f279f2dcfad Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Tue, 8 Apr 2014 22:20:33 -0700 Subject: [PATCH 1/6] Avoid triggering SYNC ge signals. Fixes #5806, bcause it avoids rescheduling at the signal, which it should not do. Possibly this is incorrect for all GE signals, but we don't need to trigger any behavior on a sync anyway. --- GPU/GPUCommon.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index 1bda70320d..9040b9ea6e 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -823,6 +823,10 @@ void GPUCommon::ExecuteOp(u32 op, u32 diff) { ERROR_LOG_REPORT(G3D, "Signal with Pause UNIMPLEMENTED! signal/end: %04x %04x", signal, enddata); break; case PSP_GE_SIGNAL_SYNC: + // Technically, this ought to trigger an interrupt, but it won't do anything. + // Triggering here can cause incorrect rescheduling, which breaks 3rd Birthday. + // However, this is likely a bug in how GE signal interrupts are handled. + trigger = false; currentList->signal = behaviour; DEBUG_LOG(G3D, "Signal with Sync. signal/end: %04x %04x", signal, enddata); break; From 702294fe60ae1bb87db27b7dfb3a98f36749574d Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Thu, 10 Apr 2014 00:23:36 -0700 Subject: [PATCH 2/6] Don't trigger pause GE signals either, comments. This better explains what each signal does. --- GPU/GPUCommon.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index 9040b9ea6e..c6b68030d1 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -808,21 +808,30 @@ void GPUCommon::ExecuteOp(u32 op, u32 diff) { switch (behaviour) { case PSP_GE_SIGNAL_HANDLER_SUSPEND: + // Suspend the list, and call the signal handler. When it's done, resume. + // Before sdkver 0x02000010, listsync should return paused. if (sceKernelGetCompiledSdkVersion() <= 0x02000010) currentList->state = PSP_GE_DL_STATE_PAUSED; currentList->signal = behaviour; DEBUG_LOG(G3D, "Signal with Wait UNIMPLEMENTED! signal/end: %04x %04x", signal, enddata); break; case PSP_GE_SIGNAL_HANDLER_CONTINUE: + // Resume the list right away, then call the handler. currentList->signal = behaviour; DEBUG_LOG(G3D, "Signal without wait. signal/end: %04x %04x", signal, enddata); break; case PSP_GE_SIGNAL_HANDLER_PAUSE: + // Pause the list instead of ending at the next FINISH. + // Call the handler with the PAUSE signal value at that FINISH. + // Technically, this ought to trigger an interrupt, but it won't do anything. + // But right now, signal is always reset by interrupts, so that causes pause to not work. + trigger = false; currentList->state = PSP_GE_DL_STATE_PAUSED; currentList->signal = behaviour; ERROR_LOG_REPORT(G3D, "Signal with Pause UNIMPLEMENTED! signal/end: %04x %04x", signal, enddata); break; case PSP_GE_SIGNAL_SYNC: + // Acts as a memory barrier, never calls any user code. // Technically, this ought to trigger an interrupt, but it won't do anything. // Triggering here can cause incorrect rescheduling, which breaks 3rd Birthday. // However, this is likely a bug in how GE signal interrupts are handled. From 4561440df795e80dcd217d5b72c0fe183cd8f234 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Fri, 11 Apr 2014 23:32:34 -0700 Subject: [PATCH 3/6] Handle GE pause signals per tests. They really should pause, but we were resetting them incorrectly. And most importantly, sceGeContinue() inside the signal handler absolutely must work. Games use this a lot. --- Core/HLE/sceGe.cpp | 4 +--- GPU/GPUCommon.cpp | 21 +++++++-------------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/Core/HLE/sceGe.cpp b/Core/HLE/sceGe.cpp index 6d7c7efd6c..cc9965a3d6 100644 --- a/Core/HLE/sceGe.cpp +++ b/Core/HLE/sceGe.cpp @@ -110,7 +110,7 @@ public: SubIntrHandler* handler = get(subintr); if (handler != NULL) { - DEBUG_LOG(CPU, "Entering interrupt handler %08x", handler->handlerAddress); + DEBUG_LOG(CPU, "Entering GE interrupt handler %08x", handler->handlerAddress); currentMIPS->pc = handler->handlerAddress; u32 data = dl->subIntrToken; currentMIPS->r[MIPS_REG_A0] = data & 0xFFFF; @@ -162,8 +162,6 @@ public: break; } - dl->signal = PSP_GE_SIGNAL_NONE; - gpu->InterruptEnd(intrdata.listid); } }; diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index c6b68030d1..43537ad282 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -330,9 +330,6 @@ u32 GPUCommon::UpdateStall(int listid, u32 newstall) { return SCE_KERNEL_ERROR_ALREADY; dl.stall = newstall & 0x0FFFFFFF; - - if (dl.signal == PSP_GE_SIGNAL_HANDLER_PAUSE) - dl.signal = PSP_GE_SIGNAL_HANDLER_SUSPEND; guard.unlock(); ProcessDLQueue(); @@ -349,8 +346,8 @@ u32 GPUCommon::Continue() { { if (!isbreak) { - if (currentList->signal == PSP_GE_SIGNAL_HANDLER_PAUSE) - return 0x80000021; + // TODO: Supposedly this returns SCE_KERNEL_ERROR_BUSY in some case, previously it had + // currentList->signal == PSP_GE_SIGNAL_HANDLER_PAUSE, but it doesn't reproduce. currentList->state = PSP_GE_DL_STATE_RUNNING; currentList->signal = PSP_GE_SIGNAL_NONE; @@ -457,9 +454,8 @@ bool GPUCommon::InterpretList(DisplayList &list) { easy_guard guard(listLock); - // TODO: This has to be right... but it freezes right now? - //if (list.state == PSP_GE_DL_STATE_PAUSED) - // return false; + if (list.state == PSP_GE_DL_STATE_PAUSED) + return false; currentList = &list; if (!list.started && list.context.IsValid()) { @@ -813,7 +809,7 @@ void GPUCommon::ExecuteOp(u32 op, u32 diff) { if (sceKernelGetCompiledSdkVersion() <= 0x02000010) currentList->state = PSP_GE_DL_STATE_PAUSED; currentList->signal = behaviour; - DEBUG_LOG(G3D, "Signal with Wait UNIMPLEMENTED! signal/end: %04x %04x", signal, enddata); + DEBUG_LOG(G3D, "Signal with wait. signal/end: %04x %04x", signal, enddata); break; case PSP_GE_SIGNAL_HANDLER_CONTINUE: // Resume the list right away, then call the handler. @@ -826,9 +822,8 @@ void GPUCommon::ExecuteOp(u32 op, u32 diff) { // Technically, this ought to trigger an interrupt, but it won't do anything. // But right now, signal is always reset by interrupts, so that causes pause to not work. trigger = false; - currentList->state = PSP_GE_DL_STATE_PAUSED; currentList->signal = behaviour; - ERROR_LOG_REPORT(G3D, "Signal with Pause UNIMPLEMENTED! signal/end: %04x %04x", signal, enddata); + ERROR_LOG_REPORT(G3D, "Signal with Pause. signal/end: %04x %04x", signal, enddata); break; case PSP_GE_SIGNAL_SYNC: // Acts as a memory barrier, never calls any user code. @@ -909,6 +904,7 @@ void GPUCommon::ExecuteOp(u32 op, u32 diff) { case GE_CMD_FINISH: switch (currentList->signal) { case PSP_GE_SIGNAL_HANDLER_PAUSE: + currentList->state = PSP_GE_DL_STATE_PAUSED; if (currentList->interruptsEnabled) { if (__GeTriggerInterrupt(currentList->id, currentList->pc, startingTicks + cyclesExecuted)) { currentList->pendingInterrupt = true; @@ -1039,9 +1035,6 @@ void GPUCommon::InterruptEnd(int listid) { __GeTriggerWait(GPU_SYNC_LIST, listid); } - if (dl.signal == PSP_GE_SIGNAL_HANDLER_PAUSE) - dl.signal = PSP_GE_SIGNAL_HANDLER_SUSPEND; - guard.unlock(); ProcessDLQueue(); } From 7fd7337911edbb89a0d70002615b31cc69916c12 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Fri, 11 Apr 2014 23:33:34 -0700 Subject: [PATCH 4/6] Update GE lists when switching. This uses a mutex, so it should be safe to do any time. Really helps debugging hung lists. --- Windows/GEDebugger/GEDebugger.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Windows/GEDebugger/GEDebugger.cpp b/Windows/GEDebugger/GEDebugger.cpp index 0c6d31b882..5877796f98 100644 --- a/Windows/GEDebugger/GEDebugger.cpp +++ b/Windows/GEDebugger/GEDebugger.cpp @@ -320,6 +320,9 @@ BOOL CGEDebugger::DlgProc(UINT message, WPARAM wParam, LPARAM lParam) { { case IDC_GEDBG_MAINTAB: tabs->HandleNotify(lParam); + if (gpuDebug != NULL) { + lists->Update(); + } break; case IDC_GEDBG_FBTABS: fbTabs->HandleNotify(lParam); From 26933384a7cecd88411ad1206716b92d3d8ca6e3 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Fri, 11 Apr 2014 23:49:33 -0700 Subject: [PATCH 5/6] Don't mark a list complete too early. This would matter probably mostly if we implemented CONTINUE properly. --- Core/HLE/sceGe.cpp | 6 ++++++ GPU/GPUCommon.cpp | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Core/HLE/sceGe.cpp b/Core/HLE/sceGe.cpp index cc9965a3d6..6bd25e6e45 100644 --- a/Core/HLE/sceGe.cpp +++ b/Core/HLE/sceGe.cpp @@ -107,6 +107,12 @@ public: } } + // Set the list as complete once the interrupt starts. + // In other words, not before another interrupt finishes. + if (dl->signal != PSP_GE_SIGNAL_HANDLER_PAUSE && cmd == GE_CMD_FINISH) { + dl->state = PSP_GE_DL_STATE_COMPLETED; + } + SubIntrHandler* handler = get(subintr); if (handler != NULL) { diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index 43537ad282..cdbf3d8fd5 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -920,11 +920,11 @@ void GPUCommon::ExecuteOp(u32 op, u32 diff) { default: currentList->subIntrToken = prev & 0xFFFF; - currentList->state = PSP_GE_DL_STATE_COMPLETED; UpdateState(GPUSTATE_DONE); if (currentList->interruptsEnabled && __GeTriggerInterrupt(currentList->id, currentList->pc, startingTicks + cyclesExecuted)) { currentList->pendingInterrupt = true; } else { + currentList->state = PSP_GE_DL_STATE_COMPLETED; currentList->waitTicks = startingTicks + cyclesExecuted; busyTicks = std::max(busyTicks, currentList->waitTicks); __GeTriggerSync(GPU_SYNC_LIST, currentList->id, currentList->waitTicks); From e93407f33ef9052257edfa1992bd53b5cd8f2783 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 12 Apr 2014 00:04:02 -0700 Subject: [PATCH 6/6] Oops, downgrade from ERROR to DEBUG, and -report. --- GPU/GPUCommon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index cdbf3d8fd5..13510e733c 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -823,7 +823,7 @@ void GPUCommon::ExecuteOp(u32 op, u32 diff) { // But right now, signal is always reset by interrupts, so that causes pause to not work. trigger = false; currentList->signal = behaviour; - ERROR_LOG_REPORT(G3D, "Signal with Pause. signal/end: %04x %04x", signal, enddata); + DEBUG_LOG(G3D, "Signal with Pause. signal/end: %04x %04x", signal, enddata); break; case PSP_GE_SIGNAL_SYNC: // Acts as a memory barrier, never calls any user code.