From 41e8a2a7d14a125768141d43039e0c6c72f81c13 Mon Sep 17 00:00:00 2001 From: Connor McLaughlin Date: Thu, 5 May 2022 09:37:45 +1000 Subject: [PATCH] MTGS: Purge pxThread --- pcsx2/CDVD/CDVD.cpp | 7 +- pcsx2/DebugTools/DebugInterface.cpp | 4 + pcsx2/GS.h | 49 +++--- pcsx2/MTGS.cpp | 234 ++++++++++++++-------------- pcsx2/PerformanceMetrics.cpp | 4 +- pcsx2/System/SysCoreThread.cpp | 7 +- pcsx2/VMManager.cpp | 7 +- pcsx2/gui/AppInit.cpp | 5 - 8 files changed, 163 insertions(+), 154 deletions(-) diff --git a/pcsx2/CDVD/CDVD.cpp b/pcsx2/CDVD/CDVD.cpp index d8ce3eac8c..9ab1788ce9 100644 --- a/pcsx2/CDVD/CDVD.cpp +++ b/pcsx2/CDVD/CDVD.cpp @@ -25,6 +25,7 @@ #include "common/FileSystem.h" #include "common/StringUtil.h" +#include "common/Threading.h" #include "Ps1CD.h" #include "CDVD.h" @@ -372,7 +373,7 @@ s32 cdvdWriteConfig(const u8* config) return 0; } -static MutexRecursive Mutex_NewDiskCB; +static Threading::MutexRecursive Mutex_NewDiskCB; // Sets ElfCRC to the CRC of the game bound to the CDVD source. static __fi ElfObject* loadElf(std::string filename, bool isPSXElf) @@ -414,7 +415,7 @@ static __fi ElfObject* loadElf(std::string filename, bool isPSXElf) static __fi void _reloadElfInfo(std::string elfpath) { // Now's a good time to reload the ELF info... - ScopedLock locker(Mutex_NewDiskCB); + Threading::ScopedLock locker(Mutex_NewDiskCB); if (elfpath == LastELF) return; @@ -911,7 +912,7 @@ void SaveStateBase::cdvdFreeze() void cdvdNewDiskCB() { - ScopedTryLock lock(Mutex_NewDiskCB); + Threading::ScopedTryLock lock(Mutex_NewDiskCB); if (lock.Failed()) { DevCon.WriteLn(Color_Red, L"NewDiskCB Lock Failed"); diff --git a/pcsx2/DebugTools/DebugInterface.cpp b/pcsx2/DebugTools/DebugInterface.cpp index 19ea388c27..ff9aebe269 100644 --- a/pcsx2/DebugTools/DebugInterface.cpp +++ b/pcsx2/DebugTools/DebugInterface.cpp @@ -27,6 +27,10 @@ #include "IopMem.h" #include "SymbolMap.h" +#ifndef PCSX2_CORE +#include "System/SysThreads.h" +#endif + R5900DebugInterface r5900Debug; R3000DebugInterface r3000Debug; diff --git a/pcsx2/GS.h b/pcsx2/GS.h index 79d00c9399..caecb7143b 100644 --- a/pcsx2/GS.h +++ b/pcsx2/GS.h @@ -16,10 +16,11 @@ #pragma once #include "Common.h" -#include "System/SysThreads.h" #include "Gif.h" #include "GS/GS.h" +#include #include +#include extern double GetVerticalFrequency(); alignas(16) extern u8 g_RealGSMem[Ps2MemSize::GSregs]; @@ -316,10 +317,8 @@ struct MTGS_MemoryScreenshotData // -------------------------------------------------------------------------------------- // SysMtgsThread // -------------------------------------------------------------------------------------- -class SysMtgsThread : public SysThreadBase +class SysMtgsThread { - typedef SysThreadBase _parent; - public: using AsyncCallType = std::function; @@ -334,10 +333,11 @@ public: std::atomic m_QueuedFrameCount; std::atomic m_VsyncSignalListener; - Mutex m_mtx_RingBufferBusy2; // Gets released on semaXGkick waiting... - Mutex m_mtx_WaitGS; - Semaphore m_sem_OnRingReset; - Semaphore m_sem_Vsync; + Threading::Mutex m_mtx_RingBufferBusy2; // Gets released on semaXGkick waiting... + Threading::Mutex m_mtx_WaitGS; + Threading::WorkSema m_sem_event; + Threading::Semaphore m_sem_OnRingReset; + Threading::Semaphore m_sem_Vsync; // used to keep multiple threads from sending packets to the ringbuffer concurrently. // (currently not used or implemented -- is a planned feature for a future threaded VU1) @@ -347,9 +347,6 @@ public: // has more than one command in it when the thread is kicked. int m_CopyDataTally; - Semaphore m_sem_OpenDone; - std::atomic m_Opened; - // These vars maintain instance data for sending Data Packets. // Only one data packet can be constructed and uploaded at a time. @@ -361,10 +358,25 @@ public: Threading::Mutex m_lock_Stack; #endif + std::thread m_thread; + Threading::ThreadHandle m_thread_handle; + std::atomic_bool m_open_flag{false}; + std::atomic_bool m_shutdown_flag{false}; + Threading::KernelSemaphore m_open_or_close_done; + public: SysMtgsThread(); virtual ~SysMtgsThread(); + __fi const Threading::ThreadHandle& GetThreadHandle() const { return m_thread_handle; } + __fi bool IsOpen() const { return m_open_flag.load(std::memory_order_acquire); } + + /// Starts the thread, if it hasn't already been started. + void StartThread(); + + /// Fully stops the thread, closing in the process if needed. + void ShutdownThread(); + // Waits for the GS to empty out the entire ring buffer contents. void WaitGS(bool syncRegs=true, bool weakWait=false, bool isMTVU=false); void ResetGS(); @@ -374,6 +386,7 @@ public: void SendDataPacket(); void SendGameCRC( u32 crc ); bool WaitForOpen(); + void WaitForClose(); void Freeze( FreezeAction mode, MTGS_FreezeData& data ); void SendSimpleGSPacket( MTGS_RingCommand type, u32 offset, u32 size, GIF_PATH path ); @@ -385,8 +398,6 @@ public: void PostVsyncStart(bool registers_written); void InitAndReadFIFO(u8* mem, u32 qwc); - bool IsGSOpened() const { return m_Opened; } - void RunOnGSThread(AsyncCallType func); void ApplySettings(); void ResizeDisplayWindow(int width, int height, float scale); @@ -398,22 +409,16 @@ public: bool SaveMemorySnapshot(u32 width, u32 height, std::vector* pixels); protected: - void OpenGS(); + bool TryOpenGS(); void CloseGS(); - void OnStart() override; - void OnResumeReady() override; - - void OnSuspendInThread() override; - void OnPauseInThread(SystemsMask systemsToTearDown) override {} - void OnResumeInThread(SystemsMask systemsToReinstate) override; - void OnCleanupInThread() override; + void ThreadEntryPoint(); + void MainLoop(); void GenericStall( uint size ); // Used internally by SendSimplePacket type functions void _FinishSimplePacket(); - void ExecuteTaskInThread() override; }; // GetMTGS() is a required external implementation. This function is *NOT* provided diff --git a/pcsx2/MTGS.cpp b/pcsx2/MTGS.cpp index 4a85089c57..1675d9ac63 100644 --- a/pcsx2/MTGS.cpp +++ b/pcsx2/MTGS.cpp @@ -61,20 +61,10 @@ std::list ringposStack; #endif SysMtgsThread::SysMtgsThread() - : SysThreadBase() #ifdef RINGBUF_DEBUG_STACK - , m_lock_Stack() + : m_lock_Stack() #endif { - m_name = L"MTGS"; - - // All other state vars are initialized by OnStart(). -} - -void SysMtgsThread::OnStart() -{ - m_Opened = false; - m_ReadPos = 0; m_WritePos = 0; m_packet_size = 0; @@ -86,22 +76,85 @@ void SysMtgsThread::OnStart() m_SignalRingPosition = 0; m_CopyDataTally = 0; - - _parent::OnStart(); } SysMtgsThread::~SysMtgsThread() { - try - { - _parent::Cancel(); - } - DESTRUCTOR_CATCHALL + ShutdownThread(); } -void SysMtgsThread::OnResumeReady() +void SysMtgsThread::StartThread() { - m_sem_OpenDone.Reset(); + if (m_thread.joinable()) + return; + + pxAssertRel(!m_open_flag.load(), "GS thread should not be opened when starting"); + m_sem_event.Reset(); + m_shutdown_flag.store(false, std::memory_order_release); + m_thread = std::thread(&SysMtgsThread::ThreadEntryPoint, this); +} + +void SysMtgsThread::ShutdownThread() +{ + if (!m_thread.joinable()) + return; + + // just go straight to shutdown, don't wait-for-open again + m_shutdown_flag.store(true, std::memory_order_release); + if (IsOpen()) + WaitForClose(); + + // make sure the thread actually exits + m_sem_event.NotifyOfWork(); + m_thread.join(); +} + +void SysMtgsThread::ThreadEntryPoint() +{ + m_thread_handle = Threading::ThreadHandle::GetForCallingThread(); + Threading::SetNameOfCurrentThread("GS"); + + for (;;) + { + // wait until we're actually asked to initialize (and config has been loaded, etc) + while (!m_open_flag.load(std::memory_order_acquire)) + { + if (m_shutdown_flag.load(std::memory_order_acquire)) + { + m_sem_event.Kill(); + m_thread_handle = {}; + return; + } + + m_sem_event.WaitForWork(); + } + + // try initializing.. this could fail + const bool opened = TryOpenGS(); + m_open_flag.store(opened, std::memory_order_release); + + // notify emu thread that we finished opening (or failed) + m_open_or_close_done.Post(); + + // are we open? + if (!opened) + { + // wait until we're asked to try again... + continue; + } + + // we're ready to go + MainLoop(); + + // when we come back here, it's because we closed (or shutdown) + // that means the emu thread should be blocked, waiting for us to be done + pxAssertRel(!m_open_flag.load(std::memory_order_relaxed), "Open flag is clear on close"); + CloseGS(); + m_open_or_close_done.Post(); + + // we need to reset sem_event here, because MainLoop() kills it. + m_sem_event.Reset(); + } } void SysMtgsThread::ResetGS() @@ -203,26 +256,18 @@ union PacketTagType }; }; -void SysMtgsThread::OpenGS() +bool SysMtgsThread::TryOpenGS() { - if (m_Opened) - return; + std::memcpy(RingBuffer.Regs, PS2MEM_GS, sizeof(PS2MEM_GS)); - memcpy(RingBuffer.Regs, PS2MEM_GS, sizeof(PS2MEM_GS)); - - m_Opened = GSopen(EmuConfig.GS, EmuConfig.GS.Renderer, RingBuffer.Regs); - m_sem_OpenDone.Post(); - - if (!m_Opened) - { - Console.Error("GS failed to open"); - return; - } + if (!GSopen(EmuConfig.GS, EmuConfig.GS.Renderer, RingBuffer.Regs)) + return false; GSsetGameCRC(ElfCRC, 0); + return true; } -void SysMtgsThread::ExecuteTaskInThread() +void SysMtgsThread::MainLoop() { // Threading info: run in MTGS thread // m_ReadPos is only update by the MTGS thread so it is safe to load it with a relaxed atomic @@ -231,7 +276,6 @@ void SysMtgsThread::ExecuteTaskInThread() PacketTagType prevCmd; #endif - ScopedGuard kill_on_exception([this]{ m_sem_event.Kill(); }); ScopedLock mtvu_lock(m_mtx_RingBufferBusy2); while (true) @@ -244,7 +288,8 @@ void SysMtgsThread::ExecuteTaskInThread() m_mtx_RingBufferBusy2.Release(); m_sem_event.WaitForWork(); - StateCheckInThread(); + if (!m_open_flag.load(std::memory_order_acquire)) + break; m_mtx_RingBufferBusy2.Acquire(); @@ -516,52 +561,28 @@ void SysMtgsThread::ExecuteTaskInThread() //Console.Warning( "(MTGS Thread) Nothing to do! ringpos=0x%06x", m_ReadPos ); } + + // Unblock any threads in WaitGS in case MTGS gets cancelled while still processing work + m_ReadPos.store(m_WritePos.load(std::memory_order_acquire), std::memory_order_relaxed); + m_sem_event.Kill(); } void SysMtgsThread::CloseGS() { - if (!m_Opened) - return; #ifndef PCSX2_CORE if (GSDump::isRunning) return; #endif - m_Opened = false; GSclose(); } -void SysMtgsThread::OnSuspendInThread() -{ - CloseGS(); - _parent::OnSuspendInThread(); -} - -void SysMtgsThread::OnResumeInThread(SystemsMask systemsToReinstate) -{ - if (systemsToReinstate & System_GS) - OpenGS(); - - _parent::OnResumeInThread(systemsToReinstate); -} - -void SysMtgsThread::OnCleanupInThread() -{ - CloseGS(); - // Unblock any threads in WaitGS in case MTGS gets cancelled while still processing work - m_ReadPos.store(m_WritePos.load(std::memory_order_acquire), std::memory_order_relaxed); - _parent::OnCleanupInThread(); -} - // Waits for the GS to empty out the entire ring buffer contents. // If syncRegs, then writes pcsx2's gs regs to MTGS's internal copy // If weakWait, then this function is allowed to exit after MTGS finished a path1 packet // If isMTVU, then this implies this function is being called from the MTVU thread... void SysMtgsThread::WaitGS(bool syncRegs, bool weakWait, bool isMTVU) { - pxAssertDev(!IsSelf(), "This method is only allowed from threads *not* named MTGS."); - - if (m_ExecMode == ExecMode_NoThreadYet || !IsRunning()) - return; + pxAssertDev(std::this_thread::get_id() != m_thread.get_id(), "This method is only allowed from threads *not* named MTGS."); if (!pxAssertDev(IsOpen(), "MTGS Warning! WaitGS issued on a closed thread.")) return; @@ -585,7 +606,6 @@ void SysMtgsThread::WaitGS(bool syncRegs, bool weakWait, bool isMTVU) while (true) { m_mtx_RingBufferBusy2.Wait(); - RethrowException(); if (path.GetPendingGSPackets() != startP1Packs) break; } @@ -594,16 +614,7 @@ void SysMtgsThread::WaitGS(bool syncRegs, bool weakWait, bool isMTVU) else { if (!m_sem_event.WaitForEmpty()) - { - // There's a small race here as the semaphore is killed before the exception is set - // Try a few times to recover the actual exception before throwing something more generic - for (int i = 0; i < 5; i++) - { - std::this_thread::yield(); - RethrowException(); - } - throw Exception::RuntimeError(std::runtime_error("MTGS Thread Died")); - } + pxFailRel("MTGS Thread Died"); } if (syncRegs) @@ -836,54 +847,51 @@ void SysMtgsThread::SendGameCRC(u32 crc) bool SysMtgsThread::WaitForOpen() { - if (m_Opened) + if (IsOpen()) return true; - Resume(); - // Two-phase timeout on MTGS opening, so that possible errors are handled - // in a timely fashion. We check for errors after 2 seconds, and then give it - // another 12 seconds if no errors occurred (this might seem long, but sometimes our - // GS can be very stubborned, especially in debug mode builds). + StartThread(); + + // request open, and kick the thread. + m_open_flag.store(true, std::memory_order_release); + m_sem_event.NotifyOfWork(); + + // wait for it to finish its stuff + m_open_or_close_done.Wait(); + + // did we succeed? + const bool result = m_open_flag.load(std::memory_order_acquire); + if (!result) + Console.Error("GS failed to open."); #ifndef PCSX2_CORE - if (!m_sem_OpenDone.Wait(wxTimeSpan(0, 0, 2, 0))) - { - RethrowException(); - - if (!m_sem_OpenDone.Wait(wxTimeSpan(0, 0, 12, 0))) - { - RethrowException(); - - pxAssert(_("The MTGS thread has become unresponsive while waiting for GS to open.")); - } - } - - RethrowException(); - if (!m_Opened) // EE thread will continue running and explode everything if we don't throw an exception + if (!result) // EE thread will continue running and explode everything if we don't throw an exception throw Exception::RuntimeError(std::runtime_error("GS failed to open.")); - return m_Opened; -#else - if (!m_sem_OpenDone.Wait(wxTimeSpan(0, 0, 12, 0)) || !m_Opened) - { - Suspend(false); - return false; - } - - return true; #endif + + return result; +} + +void SysMtgsThread::WaitForClose() +{ + if (!IsOpen()) + return; + + // ask the thread to stop processing work, by clearing the open flag + m_open_flag.store(false, std::memory_order_release); + + // and kick the thread if it's sleeping + m_sem_event.NotifyOfWork(); + + // and wait for it to finish up.. + m_open_or_close_done.Wait(); } void SysMtgsThread::Freeze(FreezeAction mode, MTGS_FreezeData& data) { - pxAssertDev(!IsSelf(), "This method is only allowed from threads *not* named MTGS."); + pxAssertRel(IsOpen(), "GS thread is open"); + pxAssertDev(std::this_thread::get_id() != m_thread.get_id(), "This method is only allowed from threads *not* named MTGS."); SendPointerPacket(GS_RINGTYPE_FREEZE, (int)mode, &data); - // make sure MTGS is processing the packet we send it - Resume(); - // we are forced to wait for the semaphore to be released, otherwise - // we'll end up in a state where the main thread is stuck on WaitGS - // and MTGS stuck on sApp.OpenGSPanel, which post an event to the main - // thread. Obviously this ends up in a deadlock. -- govanify - WaitForOpen(); WaitGS(); } diff --git a/pcsx2/PerformanceMetrics.cpp b/pcsx2/PerformanceMetrics.cpp index 74983d712e..eae2da62dc 100644 --- a/pcsx2/PerformanceMetrics.cpp +++ b/pcsx2/PerformanceMetrics.cpp @@ -118,7 +118,7 @@ void PerformanceMetrics::Reset() s_last_frame_time.Reset(); s_last_cpu_time = s_cpu_thread_handle.GetCPUTime(); - s_last_gs_time = GetMTGS().GetCpuTime(); + s_last_gs_time = GetMTGS().GetThreadHandle().GetCPUTime(); s_last_vu_time = THREAD_VU1 ? vu1Thread.GetThreadHandle().GetCPUTime() : 0; s_last_ticks = GetCPUTicks(); @@ -183,7 +183,7 @@ void PerformanceMetrics::Update(bool gs_register_write, bool fb_blit) (1.0 / static_cast(s_frames_since_last_update)); const u64 cpu_time = s_cpu_thread_handle.GetCPUTime(); - const u64 gs_time = GetMTGS().GetCpuTime(); + const u64 gs_time = GetMTGS().GetThreadHandle().GetCPUTime(); const u64 vu_time = THREAD_VU1 ? vu1Thread.GetThreadHandle().GetCPUTime() : 0; const u64 cpu_delta = cpu_time - s_last_cpu_time; diff --git a/pcsx2/System/SysCoreThread.cpp b/pcsx2/System/SysCoreThread.cpp index 07afae96a8..c42d4d8ba3 100644 --- a/pcsx2/System/SysCoreThread.cpp +++ b/pcsx2/System/SysCoreThread.cpp @@ -212,7 +212,7 @@ void SysCoreThread::ApplySettings(const Pcsx2Config& src) // -------------------------------------------------------------------------------------- bool SysCoreThread::HasPendingStateChangeRequest() const { - return !m_hasActiveMachine || GetMTGS().HasPendingException() || _parent::HasPendingStateChangeRequest(); + return !m_hasActiveMachine || _parent::HasPendingStateChangeRequest(); } void SysCoreThread::_reset_stuff_as_needed() @@ -299,7 +299,6 @@ void SysCoreThread::GameStartingInThread() bool SysCoreThread::StateCheckInThread() { - GetMTGS().RethrowException(); return _parent::StateCheckInThread() && (_reset_stuff_as_needed(), true); } @@ -376,12 +375,12 @@ void SysCoreThread::OnCleanupInThread() DoCDVDclose(); FWclose(); FileMcd_EmuClose(); - GetMTGS().Suspend(); + GetMTGS().WaitForClose(); USBshutdown(); SPU2shutdown(); PADshutdown(); DEV9shutdown(); - GetMTGS().Cancel(); + GetMTGS().ShutdownThread(); _mm_setcsr(m_mxcsr_saved.bitmask); Threading::DisableHiresScheduler(); diff --git a/pcsx2/VMManager.cpp b/pcsx2/VMManager.cpp index 7adf73fe79..5f6fba279c 100644 --- a/pcsx2/VMManager.cpp +++ b/pcsx2/VMManager.cpp @@ -673,7 +673,7 @@ bool VMManager::Initialize(const VMBootParameters& boot_params) return false; } - ScopedGuard close_gs = []() { GetMTGS().Suspend(); }; + ScopedGuard close_gs = []() { GetMTGS().WaitForClose(); }; Console.WriteLn("Opening SPU2..."); if (SPU2init() != 0 || SPU2open() != 0) @@ -827,14 +827,11 @@ void VMManager::Shutdown(bool allow_save_resume_state /* = true */) DoCDVDclose(); FWclose(); FileMcd_EmuClose(); - GetMTGS().Suspend(); + GetMTGS().WaitForClose(); USBshutdown(); SPU2shutdown(); PADshutdown(); DEV9shutdown(); - - // GS mess here... - GetMTGS().Cancel(); GSshutdown(); s_vm_memory->DecommitAll(); diff --git a/pcsx2/gui/AppInit.cpp b/pcsx2/gui/AppInit.cpp index 731bf86b93..92c9f33b15 100644 --- a/pcsx2/gui/AppInit.cpp +++ b/pcsx2/gui/AppInit.cpp @@ -717,11 +717,6 @@ Pcsx2App::Pcsx2App() Pcsx2App::~Pcsx2App() { pxDoAssert = pxAssertImpl_LogIt; - try - { - vu1Thread.Cancel(); - } - DESTRUCTOR_CATCHALL } void Pcsx2App::CleanUp()