Merge pull request #11171 from unknownbrackets/debugger

Fix some issues with GE dumps

Core/Core.cpp

@@ -54,7 +54,8 @@ static std::condition_variable m_InactiveCond;
 static std::mutex m_hInactiveMutex;
 static bool singleStepPending = false;
 static int steppingCounter = 0;
-static std::set<CoreLifecycleFunc> shutdownFuncs;
+static std::set<CoreLifecycleFunc> lifecycleFuncs;
+static std::set<CoreStopRequestFunc> stopFuncs;
 static bool windowHidden = false;
 static double lastActivity = 0.0;
 static double lastKeepAwake = 0.0;
@@ -76,17 +77,24 @@ void Core_NotifyActivity() {
 }
 
 void Core_ListenLifecycle(CoreLifecycleFunc func) {
-	shutdownFuncs.insert(func);
+	lifecycleFuncs.insert(func);
 }
 
 void Core_NotifyLifecycle(CoreLifecycle stage) {
-	for (auto it = shutdownFuncs.begin(); it != shutdownFuncs.end(); ++it) {
-		(*it)(stage);
+	for (auto func : lifecycleFuncs) {
+		func(stage);
 	}
 }
 
+void Core_ListenStopRequest(CoreStopRequestFunc func) {
+	stopFuncs.insert(func);
+}
+
 void Core_Stop() {
 	Core_UpdateState(CORE_POWERDOWN);
+	for (auto func : stopFuncs) {
+		func();
+	}
 }
 
 bool Core_IsStepping() {

Core/Core.h

@@ -51,10 +51,15 @@ enum class CoreLifecycle {
 	MEMORY_REINITED,
 };
 
+// Callback is called on the Emu thread.
 typedef void (* CoreLifecycleFunc)(CoreLifecycle stage);
 void Core_ListenLifecycle(CoreLifecycleFunc func);
 void Core_NotifyLifecycle(CoreLifecycle stage);
 
+// Callback is executed on requesting thread.
+typedef void (* CoreStopRequestFunc)();
+void Core_ListenStopRequest(CoreStopRequestFunc callback);
+
 bool Core_IsStepping();
 
 bool Core_IsActive();

GPU/Debugger/Record.cpp

@@ -17,6 +17,7 @@
 
 #include <algorithm>
 #include <cstring>
+#include <functional>
 #include <vector>
 #include <snappy-c.h>
 #include "base/stringutil.h"
@@ -86,12 +87,37 @@ static std::vector<u32> lastRegisters;
 static std::vector<u32> lastTextures;
 
 // TODO: Maybe move execute to another file?
-static u32 execMemcpyDest;
-static u32 execListBuf;
-static u32 execListPos;
-static u32 execListID;
-static const int LIST_BUF_SIZE = 256 * 1024;
-static std::vector<u32> execListQueue;
+class DumpExecute {
+public:
+	~DumpExecute();
+
+	bool Run();
+
+private:
+	void SyncStall();
+	bool SubmitCmds(void *p, u32 sz);
+	void SubmitListEnd();
+
+	void Init(u32 ptr, u32 sz);
+	void Registers(u32 ptr, u32 sz);
+	void Vertices(u32 ptr, u32 sz);
+	void Indices(u32 ptr, u32 sz);
+	void Clut(u32 ptr, u32 sz);
+	void TransferSrc(u32 ptr, u32 sz);
+	void Memset(u32 ptr, u32 sz);
+	void MemcpyDest(u32 ptr, u32 sz);
+	void Memcpy(u32 ptr, u32 sz);
+	void Texture(int level, u32 ptr, u32 sz);
+	void Display(u32 ptr, u32 sz);
+
+	u32 execMemcpyDest = 0;
+	u32 execListBuf = 0;
+	u32 execListPos = 0;
+	u32 execListID = 0;
+	const int LIST_BUF_SIZE = 256 * 1024;
+	std::vector<u32> execListQueue;
+	u16 lastBufw_[8]{};
+};
 
 // This class maps pushbuffer (dump data) sections to PSP memory.
 // Dumps can be larger than available PSP memory, because they include generated data too.
@@ -102,7 +128,7 @@ static std::vector<u32> execListQueue;
 class BufMapping {
 public:
 	// Returns a pointer to contiguous memory for this access, or else 0 (failure).
-	u32 Map(u32 bufpos, u32 sz);
+	u32 Map(u32 bufpos, u32 sz, const std::function<void()> &flush);
 
 	// Clear and reset allocations made.
 	void Reset() {
@@ -117,8 +143,8 @@ public:
 	}
 
 protected:
-	u32 MapSlab(u32 bufpos);
-	u32 MapExtra(u32 bufpos, u32 sz);
+	u32 MapSlab(u32 bufpos, const std::function<void()> &flush);
+	u32 MapExtra(u32 bufpos, u32 sz, const std::function<void()> &flush);
 
 	enum {
 		// These numbers kept low because we only have 24 MB of user memory to map into.
@@ -189,20 +215,20 @@ protected:
 
 static BufMapping execMapping;
 
-u32 BufMapping::Map(u32 bufpos, u32 sz) {
+u32 BufMapping::Map(u32 bufpos, u32 sz, const std::function<void()> &flush) {
 	int slab1 = bufpos / SLAB_SIZE;
 	int slab2 = (bufpos + sz - 1) / SLAB_SIZE;
 
 	if (slab1 == slab2) {
 		// Doesn't straddle, so we can just map to a slab.
-		return MapSlab(bufpos);
+		return MapSlab(bufpos, flush);
 	} else {
 		// We need contiguous, so we'll just allocate separately.
-		return MapExtra(bufpos, sz);
+		return MapExtra(bufpos, sz, flush);
 	}
 }
 
-u32 BufMapping::MapSlab(u32 bufpos) {
+u32 BufMapping::MapSlab(u32 bufpos, const std::function<void()> &flush) {
 	u32 slab_pos = (bufpos / SLAB_SIZE) * SLAB_SIZE;
 
 	int best = 0;
@@ -216,6 +242,9 @@ u32 BufMapping::MapSlab(u32 bufpos) {
 		}
 	}
 
+	// Stall before mapping a new slab.
+	flush();
+
 	// Okay, we need to allocate.
 	if (!slabs_[best].Setup(slab_pos)) {
 		return 0;
@@ -223,7 +252,7 @@ u32 BufMapping::MapSlab(u32 bufpos) {
 	return slabs_[best].Ptr(bufpos);
 }
 
-u32 BufMapping::MapExtra(u32 bufpos, u32 sz) {
+u32 BufMapping::MapExtra(u32 bufpos, u32 sz, const std::function<void()> &flush) {
 	for (int i = 0; i < EXTRA_COUNT; ++i) {
 		// Might be likely to reuse larger buffers straddling slabs.
 		if (extra_[i].Matches(bufpos, sz)) {
@@ -231,6 +260,9 @@ u32 BufMapping::MapExtra(u32 bufpos, u32 sz) {
 		}
 	}
 
+	// Stall first, so we don't stomp existing RAM.
+	flush();
+
 	int i = extraOffset_;
 	extraOffset_ = (extraOffset_ + 1) % EXTRA_COUNT;
 
@@ -731,7 +763,18 @@ void NotifyFrame() {
 	}
 }
 
-static bool ExecuteSubmitCmds(void *p, u32 sz) {
+void DumpExecute::SyncStall() {
+	gpu->UpdateStall(execListID, execListPos);
+	s64 listTicks = gpu->GetListTicks(execListID);
+	if (listTicks != -1) {
+		currentMIPS->downcount -= listTicks - CoreTiming::GetTicks();
+	}
+
+	// Make sure downcount doesn't overflow.
+	CoreTiming::ForceCheck();
+}
+
+bool DumpExecute::SubmitCmds(void *p, u32 sz) {
 	if (execListBuf == 0) {
 		u32 allocSize = LIST_BUF_SIZE;
 		execListBuf = userMemory.Alloc(allocSize, "List buf");
@@ -761,27 +804,47 @@ static bool ExecuteSubmitCmds(void *p, u32 sz) {
 		Memory::Write_U32((GE_CMD_JUMP << 24) | (execListBuf & 0x00FFFFFF), execListPos + 4);
 
 		execListPos = execListBuf;
+
+		// Don't continue until we've stalled.
+		SyncStall();
 	}
 
 	Memory::MemcpyUnchecked(execListPos, execListQueue.data(), pendingSize);
 	execListPos += pendingSize;
+	u32 writePos = execListPos;
 	Memory::MemcpyUnchecked(execListPos, p, sz);
 	execListPos += sz;
 
-	execListQueue.clear();
-	gpu->UpdateStall(execListID, execListPos);
-	s64 listTicks = gpu->GetListTicks(execListID);
-	if (listTicks != -1) {
-		currentMIPS->downcount -= listTicks - CoreTiming::GetTicks();
+	// TODO: Unfortunate.  Maybe Texture commands should contain the bufw instead.
+	// The goal here is to realistically combine prims in dumps.  Stalling for the bufw flushes.
+	u32_le *ops = (u32_le *)Memory::GetPointer(writePos);
+	for (u32 i = 0; i < sz / 4; ++i) {
+		u32 cmd = ops[i] >> 24;
+		if (cmd >= GE_CMD_TEXBUFWIDTH0 && cmd <= GE_CMD_TEXBUFWIDTH7) {
+			int level = cmd - GE_CMD_TEXBUFWIDTH0;
+			u16 bufw = ops[i] & 0xFFFF;
+
+			// NOP the address part of the command to avoid a flush too.
+			if (bufw == lastBufw_[level])
+				ops[i] = GE_CMD_NOP << 24;
+			else
+				ops[i] = (gstate.texbufwidth[level] & 0xFFFF0000) | bufw;
+			lastBufw_[level] = bufw;
+		}
+
+		// Since we're here anyway, also NOP out texture addresses.
+		// This makes Step Tex not hit phantom textures.
+		if (cmd >= GE_CMD_TEXADDR0 && cmd <= GE_CMD_TEXADDR7) {
+			ops[i] = GE_CMD_NOP << 24;
+		}
 	}
 
-	// Make sure downcount doesn't overflow.
-	CoreTiming::ForceCheck();
+	execListQueue.clear();
 
 	return true;
 }
 
-static void ExecuteSubmitListEnd() {
+void DumpExecute::SubmitListEnd() {
 	if (execListPos == 0) {
 		return;
 	}
@@ -791,26 +854,21 @@ static void ExecuteSubmitListEnd() {
 	Memory::Write_U32(GE_CMD_END << 24, execListPos + 4);
 	execListPos += 8;
 
-	gpu->UpdateStall(execListID, execListPos);
-	currentMIPS->downcount -= gpu->GetListTicks(execListID) - CoreTiming::GetTicks();
-
+	SyncStall();
 	gpu->ListSync(execListID, 0);
-
-	// Make sure downcount doesn't overflow.
-	CoreTiming::ForceCheck();
 }
 
-static void ExecuteInit(u32 ptr, u32 sz) {
+void DumpExecute::Init(u32 ptr, u32 sz) {
 	gstate.Restore((u32_le *)(pushbuf.data() + ptr));
 	gpu->ReapplyGfxState();
 }
 
-static void ExecuteRegisters(u32 ptr, u32 sz) {
-	ExecuteSubmitCmds(pushbuf.data() + ptr, sz);
+void DumpExecute::Registers(u32 ptr, u32 sz) {
+	SubmitCmds(pushbuf.data() + ptr, sz);
 }
 
-static void ExecuteVertices(u32 ptr, u32 sz) {
-	u32 psp = execMapping.Map(ptr, sz);
+void DumpExecute::Vertices(u32 ptr, u32 sz) {
+	u32 psp = execMapping.Map(ptr, sz, std::bind(&DumpExecute::SyncStall, this));
 	if (psp == 0) {
 		ERROR_LOG(SYSTEM, "Unable to allocate for vertices");
 		return;
@@ -820,8 +878,8 @@ static void ExecuteVertices(u32 ptr, u32 sz) {
 	execListQueue.push_back((GE_CMD_VADDR << 24) | (psp & 0x00FFFFFF));
 }
 
-static void ExecuteIndices(u32 ptr, u32 sz) {
-	u32 psp = execMapping.Map(ptr, sz);
+void DumpExecute::Indices(u32 ptr, u32 sz) {
+	u32 psp = execMapping.Map(ptr, sz, std::bind(&DumpExecute::SyncStall, this));
 	if (psp == 0) {
 		ERROR_LOG(SYSTEM, "Unable to allocate for indices");
 		return;
@@ -831,8 +889,8 @@ static void ExecuteIndices(u32 ptr, u32 sz) {
 	execListQueue.push_back((GE_CMD_IADDR << 24) | (psp & 0x00FFFFFF));
 }
 
-static void ExecuteClut(u32 ptr, u32 sz) {
-	u32 psp = execMapping.Map(ptr, sz);
+void DumpExecute::Clut(u32 ptr, u32 sz) {
+	u32 psp = execMapping.Map(ptr, sz, std::bind(&DumpExecute::SyncStall, this));
 	if (psp == 0) {
 		ERROR_LOG(SYSTEM, "Unable to allocate for clut");
 		return;
@@ -842,18 +900,21 @@ static void ExecuteClut(u32 ptr, u32 sz) {
 	execListQueue.push_back((GE_CMD_CLUTADDR << 24) | (psp & 0x00FFFFFF));
 }
 
-static void ExecuteTransferSrc(u32 ptr, u32 sz) {
-	u32 psp = execMapping.Map(ptr, sz);
+void DumpExecute::TransferSrc(u32 ptr, u32 sz) {
+	u32 psp = execMapping.Map(ptr, sz, std::bind(&DumpExecute::SyncStall, this));
 	if (psp == 0) {
 		ERROR_LOG(SYSTEM, "Unable to allocate for transfer");
 		return;
 	}
 
+	// Need to sync in order to access gstate.transfersrcw.
+	SyncStall();
+
 	execListQueue.push_back((gstate.transfersrcw & 0xFF00FFFF) | ((psp >> 8) & 0x00FF0000));
 	execListQueue.push_back(((GE_CMD_TRANSFERSRC) << 24) | (psp & 0x00FFFFFF));
 }
 
-static void ExecuteMemset(u32 ptr, u32 sz) {
+void DumpExecute::Memset(u32 ptr, u32 sz) {
 	struct MemsetCommand {
 		u32 dest;
 		int value;
@@ -863,33 +924,37 @@ static void ExecuteMemset(u32 ptr, u32 sz) {
 	const MemsetCommand *data = (const MemsetCommand *)(pushbuf.data() + ptr);
 
 	if (Memory::IsVRAMAddress(data->dest)) {
+		SyncStall();
 		gpu->PerformMemorySet(data->dest, (u8)data->value, data->sz);
 	}
 }
 
-static void ExecuteMemcpyDest(u32 ptr, u32 sz) {
+void DumpExecute::MemcpyDest(u32 ptr, u32 sz) {
 	execMemcpyDest = *(const u32 *)(pushbuf.data() + ptr);
 }
 
-static void ExecuteMemcpy(u32 ptr, u32 sz) {
+void DumpExecute::Memcpy(u32 ptr, u32 sz) {
 	if (Memory::IsVRAMAddress(execMemcpyDest)) {
+		SyncStall();
 		Memory::MemcpyUnchecked(execMemcpyDest, pushbuf.data() + ptr, sz);
 		gpu->PerformMemoryUpload(execMemcpyDest, sz);
 	}
 }
 
-static void ExecuteTexture(int level, u32 ptr, u32 sz) {
-	u32 psp = execMapping.Map(ptr, sz);
+void DumpExecute::Texture(int level, u32 ptr, u32 sz) {
+	u32 psp = execMapping.Map(ptr, sz, std::bind(&DumpExecute::SyncStall, this));
 	if (psp == 0) {
 		ERROR_LOG(SYSTEM, "Unable to allocate for texture");
 		return;
 	}
 
-	execListQueue.push_back((gstate.texbufwidth[level] & 0xFF00FFFF) | ((psp >> 8) & 0x00FF0000));
-	execListQueue.push_back(((GE_CMD_TEXADDR0 + level) << 24) | (psp & 0x00FFFFFF));
+	u32 bufwCmd = GE_CMD_TEXBUFWIDTH0 + level;
+	u32 addrCmd = GE_CMD_TEXADDR0 + level;
+	execListQueue.push_back((bufwCmd << 24) | ((psp >> 8) & 0x00FF0000) | lastBufw_[level]);
+	execListQueue.push_back((addrCmd << 24) | (psp & 0x00FFFFFF));
 }
 
-static void ExecuteDisplay(u32 ptr, u32 sz) {
+void DumpExecute::Display(u32 ptr, u32 sz) {
 	struct DisplayBufData {
 		PSPPointer<u8> topaddr;
 		u32 linesize, pixelFormat;
@@ -897,11 +962,14 @@ static void ExecuteDisplay(u32 ptr, u32 sz) {
 
 	DisplayBufData *disp = (DisplayBufData *)(pushbuf.data() + ptr);
 
+	// Sync up drawing.
+	SyncStall();
+
 	__DisplaySetFramebuf(disp->topaddr.ptr, disp->linesize, disp->pixelFormat, 1);
 	__DisplaySetFramebuf(disp->topaddr.ptr, disp->linesize, disp->pixelFormat, 0);
 }
 
-static void ExecuteFree() {
+DumpExecute::~DumpExecute() {
 	execMemcpyDest = 0;
 	if (execListBuf) {
 		userMemory.Free(execListBuf);
@@ -914,43 +982,43 @@ static void ExecuteFree() {
 	pushbuf.clear();
 }
 
-static bool ExecuteCommands() {
+bool DumpExecute::Run() {
 	for (const Command &cmd : commands) {
 		switch (cmd.type) {
 		case CommandType::INIT:
-			ExecuteInit(cmd.ptr, cmd.sz);
+			Init(cmd.ptr, cmd.sz);
 			break;
 
 		case CommandType::REGISTERS:
-			ExecuteRegisters(cmd.ptr, cmd.sz);
+			Registers(cmd.ptr, cmd.sz);
 			break;
 
 		case CommandType::VERTICES:
-			ExecuteVertices(cmd.ptr, cmd.sz);
+			Vertices(cmd.ptr, cmd.sz);
 			break;
 
 		case CommandType::INDICES:
-			ExecuteIndices(cmd.ptr, cmd.sz);
+			Indices(cmd.ptr, cmd.sz);
 			break;
 
 		case CommandType::CLUT:
-			ExecuteClut(cmd.ptr, cmd.sz);
+			Clut(cmd.ptr, cmd.sz);
 			break;
 
 		case CommandType::TRANSFERSRC:
-			ExecuteTransferSrc(cmd.ptr, cmd.sz);
+			TransferSrc(cmd.ptr, cmd.sz);
 			break;
 
 		case CommandType::MEMSET:
-			ExecuteMemset(cmd.ptr, cmd.sz);
+			Memset(cmd.ptr, cmd.sz);
 			break;
 
 		case CommandType::MEMCPYDEST:
-			ExecuteMemcpyDest(cmd.ptr, cmd.sz);
+			MemcpyDest(cmd.ptr, cmd.sz);
 			break;
 
 		case CommandType::MEMCPYDATA:
-			ExecuteMemcpy(cmd.ptr, cmd.sz);
+			Memcpy(cmd.ptr, cmd.sz);
 			break;
 
 		case CommandType::TEXTURE0:
@@ -961,11 +1029,11 @@ static bool ExecuteCommands() {
 		case CommandType::TEXTURE5:
 		case CommandType::TEXTURE6:
 		case CommandType::TEXTURE7:
-			ExecuteTexture((int)cmd.type - (int)CommandType::TEXTURE0, cmd.ptr, cmd.sz);
+			Texture((int)cmd.type - (int)CommandType::TEXTURE0, cmd.ptr, cmd.sz);
 			break;
 
 		case CommandType::DISPLAY:
-			ExecuteDisplay(cmd.ptr, cmd.sz);
+			Display(cmd.ptr, cmd.sz);
 			break;
 
 		default:
@@ -974,7 +1042,7 @@ static bool ExecuteCommands() {
 		}
 	}
 
-	ExecuteSubmitListEnd();
+	SubmitListEnd();
 	return true;
 }
 
@@ -1028,13 +1096,11 @@ bool RunMountedReplay(const std::string &filename) {
 
 	if (truncated) {
 		ERROR_LOG(SYSTEM, "Truncated GE dump");
-		ExecuteFree();
 		return false;
 	}
 
-	bool success = ExecuteCommands();
-	ExecuteFree();
-	return success;
+	DumpExecute executor;
+	return executor.Run();
 }
 
 };

GPU/Debugger/Stepping.cpp

@@ -197,12 +197,10 @@ void ResumeFromStepping() {
 	SetPauseAction(PAUSE_CONTINUE, false);
 }
 
-void ForceUnpause(CoreLifecycle stage) {
-	if (stage == CoreLifecycle::STOPPING) {
-		SetPauseAction(PAUSE_CONTINUE, false);
-		actionComplete = true;
-		actionWait.notify_all();
-	}
+void ForceUnpause() {
+	SetPauseAction(PAUSE_CONTINUE, false);
+	actionComplete = true;
+	actionWait.notify_all();
 }
 
 }  // namespace

GPU/Debugger/Stepping.h

@@ -38,5 +38,5 @@ namespace GPUStepping {
 	bool GPU_SetCmdValue(u32 op);
 
 	void ResumeFromStepping();
-	void ForceUnpause(CoreLifecycle stage);
+	void ForceUnpause();
 };

GPU/GLES/TextureCacheGLES.cpp

@@ -841,6 +841,8 @@ bool TextureCacheGLES::GetCurrentTextureDebug(GPUDebugBuffer &buffer, int level)
 
 	// Apply texture may need to rebuild the texture if we're about to render, or bind a framebuffer.
 	TexCacheEntry *entry = nextTexture_;
+	// We might need a render pass to set the sampling params, unfortunately.  Otherwise BuildTexture may crash.
+	framebufferManagerGL_->RebindFramebuffer();
 	ApplyTexture();
 
 	// TODO: Centralize?

Windows/GEDebugger/GEDebugger.cpp

@@ -67,7 +67,7 @@ void CGEDebugger::Init() {
 CGEDebugger::CGEDebugger(HINSTANCE _hInstance, HWND _hParent)
 	: Dialog((LPCSTR)IDD_GEDEBUGGER, _hInstance, _hParent) {
 	GPUBreakpoints::Init();
-	Core_ListenLifecycle(ForceUnpause);
+	Core_ListenStopRequest(ForceUnpause);
 
 	// minimum size = a little more than the default
 	RECT windowRect;