made savestates synchronous and immediate. this allows saving or loading while the emulator is paused, fixes issues where savestate hotkeys would get ignored if pressed too close together, might speed up savestates in some cases, and hopefully makes savestates more stable too.

the intent is to replace the haphazard scheduling and finger-crossing associated with saving/loading with the correct and minimal necessary wait for each thread to reach a known safe location before commencing the savestate operation, and for any already-paused components to not need to be resumed to do so.

Source/Core/VideoCommon/Src/Fifo.cpp

@@ -26,6 +26,7 @@
 #include "ChunkFile.h"
 #include "Fifo.h"
 #include "HW/Memmap.h"
+#include "Core.h"
 
 volatile bool g_bSkipCurrentFrame = false;
 extern u8* g_pVideoData;
@@ -34,22 +35,44 @@ namespace
 {
 static volatile bool GpuRunningState = false;
 static volatile bool EmuRunningState = false;
-static u8 *videoBuffer;
+static std::mutex m_csHWVidOccupied;
 // STATE_TO_SAVE
+static u8 *videoBuffer;
 static int size = 0;
 }  // namespace
 
 void Fifo_DoState(PointerWrap &p) 
 {
-
     p.DoArray(videoBuffer, FIFO_SIZE);
     p.Do(size);
 	int pos = (int)(g_pVideoData - videoBuffer); // get offset
-	p.Do(pos); // read or write offset (depends on the mode afaik)
-	g_pVideoData = &videoBuffer[pos]; // overwrite g_pVideoData -> expected no change when load ss and change when save ss
-
+	p.Do(pos); // read or write offset (depending on the mode)
+	if (p.GetMode() == PointerWrap::MODE_READ)
+	{
+		g_pVideoData = &videoBuffer[pos];
+		g_bSkipCurrentFrame = false;
+	}
 }
 
+void Fifo_PauseAndLock(bool doLock, bool unpauseOnUnlock)
+{
+	if (doLock)
+	{
+		EmulatorState(false);
+		if (!Core::IsGPUThread())
+			m_csHWVidOccupied.lock();
+		_dbg_assert_(COMMON, !CommandProcessor::fifo.isGpuReadingData);
+	}
+	else
+	{
+		if (unpauseOnUnlock)
+			EmulatorState(true);
+		if (!Core::IsGPUThread())
+			m_csHWVidOccupied.unlock();
+	}
+}
+
+
 void Fifo_Init()
 {	
     videoBuffer = (u8*)AllocateMemoryPages(FIFO_SIZE);
@@ -127,6 +150,7 @@ void ResetVideoBuffer()
 // Purpose: Keep the Core HW updated about the CPU-GPU distance
 void RunGpuLoop()
 {
+	std::lock_guard<std::mutex> lk(m_csHWVidOccupied);
 	GpuRunningState = true;
 	SCPFifoStruct &fifo = CommandProcessor::fifo;
 
@@ -178,12 +202,13 @@ void RunGpuLoop()
 			Common::YieldCPU();
 		else
 		{
-			// While the emu is paused, we still handle async request such as Savestates then sleep.
+			// While the emu is paused, we still handle async requests then sleep.
 			while (!EmuRunningState)
 			{
 				g_video_backend->PeekMessages();
-				VideoFifo_CheckStateRequest();
+				m_csHWVidOccupied.unlock();
 				Common::SleepCurrentThread(1);
+				m_csHWVidOccupied.lock();
 			}
 		}
 	}

Source/Core/VideoCommon/Src/Fifo.h

@@ -30,7 +30,9 @@ extern volatile bool g_bSkipCurrentFrame;
 
 void Fifo_Init();
 void Fifo_Shutdown();
+
 void Fifo_DoState(PointerWrap &f);
+void Fifo_PauseAndLock(bool doLock, bool unpauseOnUnlock);
 
 void ReadDataFromFifo(u8* _uData, u32 len);
 
@@ -45,6 +47,5 @@ void Fifo_SetRendering(bool bEnabled);
 
 // Implemented by the Video Backend
 void VideoFifo_CheckAsyncRequest();
-void VideoFifo_CheckStateRequest();
 
 #endif // _FIFO_H

Source/Core/VideoCommon/Src/MainBase.cpp

@@ -169,8 +169,7 @@ u32 VideoBackendHardware::Video_AccessEFB(EFBAccessType type, u32 x, u32 y, u32
 	return 0;
 }
 
-static volatile u32 s_doStateRequested = false;
- 
+
 void VideoBackendHardware::InitializeShared()
 {
 	VideoCommon_Init();
@@ -183,52 +182,29 @@ void VideoBackendHardware::InitializeShared()
 	s_AccessEFBResult = 0;
 }
 
-static volatile struct
-{
-	unsigned char **ptr;
-	int mode;
-} s_doStateArgs;
-
-// Depending on the threading mode (DC/SC) this can be called 
-// from either the GPU thread or the CPU thread
-void VideoFifo_CheckStateRequest()
-{
-	if (Common::AtomicLoadAcquire(s_doStateRequested))
-	{
-		// Clear all caches that touch RAM
-		TextureCache::Invalidate(false);
-		VertexLoaderManager::MarkAllDirty();
-
-		PointerWrap p(s_doStateArgs.ptr, s_doStateArgs.mode);
-		VideoCommon_DoState(p);
-
-		// Refresh state.
-		if (s_doStateArgs.mode == PointerWrap::MODE_READ)
-		{
-			BPReload();
-			RecomputeCachedArraybases();
-		}
-
-		Common::AtomicStoreRelease(s_doStateRequested, false);
-	}
-}
-
 // Run from the CPU thread
 void VideoBackendHardware::DoState(PointerWrap& p)
 {
-	s_doStateArgs.ptr = p.ptr;
-	s_doStateArgs.mode = p.mode;
-	Common::AtomicStoreRelease(s_doStateRequested, true);
-	if (SConfig::GetInstance().m_LocalCoreStartupParameter.bCPUThread)
+	// Clear all caches that touch RAM
+	TextureCache::Invalidate(false);
+	VertexLoaderManager::MarkAllDirty();
+
+	VideoCommon_DoState(p);
+
+	// Refresh state.
+	if (p.GetMode() == PointerWrap::MODE_READ)
 	{
-		while (Common::AtomicLoadAcquire(s_doStateRequested) && !s_FifoShuttingDown)
-			//Common::SleepCurrentThread(1);
-			Common::YieldCPU();
+		BPReload();
+		RecomputeCachedArraybases();
 	}
-	else
-		VideoFifo_CheckStateRequest();
 }
 
+void VideoBackendHardware::PauseAndLock(bool doLock, bool unpauseOnUnlock)
+{
+	Fifo_PauseAndLock(doLock, unpauseOnUnlock);
+}
+
+
 void VideoBackendHardware::RunLoop(bool enable)
 {
 	VideoCommon_RunLoop(enable);

Source/Core/VideoCommon/Src/PixelEngine.cpp

@@ -367,8 +367,6 @@ void UpdateFinishInterrupt(bool active)
 {
 		ProcessorInterface::SetInterrupt(INT_CAUSE_PE_FINISH, active);
 		interruptSetFinish = active;
-		if (active)
-			State::ProcessRequestedStates(0);
 }
 
 // TODO(mb2): Refactor SetTokenINT_OnMainThread(u64 userdata, int cyclesLate).