From c1922783f8125dee4f12fdb20bdcbcf764e7ade4 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Thu, 12 May 2016 09:17:17 +0000 Subject: [PATCH 1/3] Core: Threadsafety Synchronization Fixes (Frame Advance / FifoPlayer) Fix Frame Advance and FifoPlayer pause/unpause/stop. CPU::EnableStepping is not atomic but is called from multiple threads which races and leaves the system in a random state; also instruction stepping was unstable, m_StepEvent had an almost random value because of the dual purpose it served which could cause races where CPU::Run would SingleStep when it was supposed to be sleeping. FifoPlayer never FinishStateMove()d which was causing it to deadlock. Rather than partially reimplementing CPU::Run, just use CPUCoreBase and then call CPU::Run(). More DRY and less likely to have weird bugs specific to the player (i.e the previous freezing on pause/stop). Refactor PowerPC::state into CPU since it manages the state of the CPU Thread which is controlled by CPU, not PowerPC. This simplifies the architecture somewhat and eliminates races that can be caused by calling PowerPC state functions directly instead of using CPU's (because they bypassed the EnableStepping lock). --- Source/Android/jni/MainAndroid.cpp | 22 +- Source/Core/AudioCommon/Mixer.cpp | 9 - Source/Core/Common/Event.h | 6 +- Source/Core/Core/Core.cpp | 83 +++-- Source/Core/Core/Core.h | 8 +- .../Core/Core/Debugger/PPCDebugInterface.cpp | 7 +- Source/Core/Core/FifoPlayer/FifoPlayer.cpp | 128 ++++--- Source/Core/Core/FifoPlayer/FifoPlayer.h | 14 +- Source/Core/Core/HLE/HLE_Misc.cpp | 3 +- Source/Core/Core/HW/CPU.cpp | 330 +++++++++++++----- Source/Core/Core/HW/CPU.h | 46 ++- Source/Core/Core/Movie.cpp | 7 +- .../Core/Core/PowerPC/CachedInterpreter.cpp | 6 +- .../Core/PowerPC/Interpreter/Interpreter.cpp | 5 +- Source/Core/Core/PowerPC/Jit64/Jit.cpp | 9 +- Source/Core/Core/PowerPC/Jit64/JitAsm.cpp | 12 +- .../Core/Core/PowerPC/Jit64/Jit_LoadStore.cpp | 3 +- Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp | 3 +- Source/Core/Core/PowerPC/Jit64IL/JitIL.cpp | 7 +- Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp | 7 +- .../Core/Core/PowerPC/JitCommon/JitBase.cpp | 3 +- .../JitILCommon/JitILBase_LoadStore.cpp | 3 +- Source/Core/Core/PowerPC/PowerPC.cpp | 146 ++++---- Source/Core/Core/PowerPC/PowerPC.h | 30 +- Source/Core/DolphinWX/Debugger/CodeWindow.cpp | 2 + Source/Core/DolphinWX/Debugger/WatchView.cpp | 7 +- Source/Core/DolphinWX/MainNoGUI.cpp | 3 - 27 files changed, 590 insertions(+), 319 deletions(-) diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp index d9b8a61fba..cd96fe46a2 100644 --- a/Source/Android/jni/MainAndroid.cpp +++ b/Source/Android/jni/MainAndroid.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -29,7 +30,6 @@ #include "Core/HW/Wiimote.h" #include "Core/HW/WiimoteReal/WiimoteReal.h" #include "Core/PowerPC/JitInterface.h" -#include "Core/PowerPC/PowerPC.h" #include "Core/PowerPC/Profiler.h" #include "DiscIO/Volume.h" @@ -66,8 +66,13 @@ void Host_NotifyMapLoaded() {} void Host_RefreshDSPDebuggerWindow() {} Common::Event updateMainFrameEvent; +static bool s_have_wm_user_stop = false; void Host_Message(int Id) { + if (Id == WM_USER_STOP) + { + s_have_wm_user_stop = true; + } } void* Host_GetRenderHandle() @@ -657,11 +662,22 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Run(JNIEnv * WiimoteReal::InitAdapterClass(); // No use running the loop when booting fails + s_have_wm_user_stop = false; if ( BootManager::BootCore( g_filename.c_str() ) ) { - PowerPC::Start(); - while (PowerPC::GetState() != PowerPC::CPU_POWERDOWN) + static constexpr int TIMEOUT = 10000; + static constexpr int WAIT_STEP = 25; + int time_waited = 0; + // A Core::CORE_ERROR state would be helpful here. + while (!Core::IsRunning() && time_waited < TIMEOUT && !s_have_wm_user_stop) + { + std::this_thread::sleep_for(std::chrono::milliseconds(WAIT_STEP)); + time_waited += WAIT_STEP; + } + while (Core::IsRunning()) + { updateMainFrameEvent.Wait(); + } } Core::Shutdown(); diff --git a/Source/Core/AudioCommon/Mixer.cpp b/Source/Core/AudioCommon/Mixer.cpp index f35a671094..5e3aef733a 100644 --- a/Source/Core/AudioCommon/Mixer.cpp +++ b/Source/Core/AudioCommon/Mixer.cpp @@ -12,9 +12,6 @@ #include "Common/Logging/Log.h" #include "Core/ConfigManager.h" -// UGLINESS -#include "Core/PowerPC/PowerPC.h" - #if _M_SSE >= 0x301 && !(defined __GNUC__ && !defined __SSSE3__) #include #endif @@ -121,12 +118,6 @@ unsigned int CMixer::Mix(short* samples, unsigned int num_samples, bool consider memset(samples, 0, num_samples * 2 * sizeof(short)); - if (PowerPC::GetState() != PowerPC::CPU_RUNNING) - { - // Silence - return num_samples; - } - m_dma_mixer.Mix(samples, num_samples, consider_framelimit); m_streaming_mixer.Mix(samples, num_samples, consider_framelimit); m_wiimote_speaker_mixer.Mix(samples, num_samples, consider_framelimit); diff --git a/Source/Core/Common/Event.h b/Source/Core/Common/Event.h index e05bafac1f..49cf543ead 100644 --- a/Source/Core/Common/Event.h +++ b/Source/Core/Common/Event.h @@ -42,8 +42,7 @@ public: return; std::unique_lock lk(m_mutex); - m_condvar.wait(lk, [&]{ return m_flag.IsSet(); }); - m_flag.Clear(); + m_condvar.wait(lk, [&]{ return m_flag.TestAndClear(); }); } template @@ -54,8 +53,7 @@ public: std::unique_lock lk(m_mutex); bool signaled = m_condvar.wait_for(lk, rel_time, - [&]{ return m_flag.IsSet(); }); - m_flag.Clear(); + [&]{ return m_flag.TestAndClear(); }); return signaled; } diff --git a/Source/Core/Core/Core.cpp b/Source/Core/Core/Core.cpp index 5c0258c37f..b5fbf14663 100644 --- a/Source/Core/Core/Core.cpp +++ b/Source/Core/Core/Core.cpp @@ -266,10 +266,7 @@ void Stop() // - Hammertime! // Stop the CPU INFO_LOG(CONSOLE, "%s", StopMessage(true, "Stop CPU").c_str()); - PowerPC::Stop(); - - // Kick it if it's waiting (code stepping wait loop) - CPU::StepOpcode(); + CPU::Stop(); if (_CoreParameter.bCPUThread) { @@ -376,6 +373,7 @@ static void CpuThread() static void FifoPlayerThread() { + DeclareAsCPUThread(); const SConfig& _CoreParameter = SConfig::GetInstance(); if (_CoreParameter.bCPUThread) @@ -388,18 +386,34 @@ static void FifoPlayerThread() Common::SetCurrentThreadName("FIFO-GPU thread"); } - s_is_started = true; - DeclareAsCPUThread(); - // Enter CPU run loop. When we leave it - we are done. if (FifoPlayer::GetInstance().Open(_CoreParameter.m_strFilename)) { - FifoPlayer::GetInstance().Play(); + if (auto cpu_core = FifoPlayer::GetInstance().GetCPUCore()) + { + PowerPC::InjectExternalCPUCore(cpu_core.get()); + s_is_started = true; + CPU::Run(); + s_is_started = false; + PowerPC::InjectExternalCPUCore(nullptr); + } FifoPlayer::GetInstance().Close(); } - UndeclareAsCPUThread(); - s_is_started = false; + // If we did not enter the CPU Run Loop above then run a fake one instead. + // We need to be IsRunningAndStarted() for DolphinWX to stop us. + if (CPU::GetState() != CPU::CPU_POWERDOWN) + { + s_is_started = true; + Host_Message(WM_USER_STOP); + while (CPU::GetState() != CPU::CPU_POWERDOWN) + { + if (!_CoreParameter.bCPUThread) + g_video_backend->PeekMessages(); + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + } + s_is_started = false; + } if (!_CoreParameter.bCPUThread) g_video_backend->Video_Cleanup(); @@ -487,6 +501,9 @@ void EmuThread() s_hardware_initialized = true; // Boot to pause or not + // NOTE: This violates the Host Thread requirement for SetState but we should + // not race the Host because the UI should have the buttons disabled until + // Host_UpdateMainFrame enables them. Core::SetState(core_parameter.bBootToPause ? Core::CORE_PAUSE : Core::CORE_RUN); // Load GCM/DOL/ELF whatever ... we boot with the interpreter core @@ -552,7 +569,7 @@ void EmuThread() // Spawn the CPU+GPU thread s_cpu_thread = std::thread(cpuThreadFunc); - while (PowerPC::GetState() != PowerPC::CPU_POWERDOWN) + while (CPU::GetState() != CPU::CPU_POWERDOWN) { g_video_backend->PeekMessages(); Common::SleepCurrentThread(20); @@ -621,11 +638,13 @@ void EmuThread() // Set or get the running state -void SetState(EState _State) +void SetState(EState state) { - switch (_State) + switch (state) { case CORE_PAUSE: + // NOTE: GetState() will return CORE_PAUSE immediately, even before anything has + // stopped (including the CPU). CPU::EnableStepping(true); // Break Wiimote::Pause(); #if defined(__LIBUSB__) || defined(_WIN32) @@ -719,30 +738,50 @@ void RequestRefreshInfo() s_request_refresh_info = true; } -bool PauseAndLock(bool doLock, bool unpauseOnUnlock) +bool PauseAndLock(bool do_lock, bool unpause_on_unlock) { + // WARNING: PauseAndLock is not fully threadsafe so is only valid on the Host Thread if (!IsRunning()) return true; // let's support recursive locking to simplify things on the caller's side, // and let's do it at this outer level in case the individual systems don't support it. - if (doLock ? s_pause_and_lock_depth++ : --s_pause_and_lock_depth) + if (do_lock ? s_pause_and_lock_depth++ : --s_pause_and_lock_depth) return true; - // first pause or unpause the CPU - bool wasUnpaused = CPU::PauseAndLock(doLock, unpauseOnUnlock); - ExpansionInterface::PauseAndLock(doLock, unpauseOnUnlock); + bool was_unpaused = true; + if (do_lock) + { + // first pause the CPU + // This acquires a wrapper mutex and converts the current thread into + // a temporary replacement CPU Thread. + was_unpaused = CPU::PauseAndLock(true); + } + + ExpansionInterface::PauseAndLock(do_lock, false); // audio has to come after CPU, because CPU thread can wait for audio thread (m_throttle). - DSP::GetDSPEmulator()->PauseAndLock(doLock, unpauseOnUnlock); + DSP::GetDSPEmulator()->PauseAndLock(do_lock, false); // video has to come after CPU, because CPU thread can wait for video thread (s_efbAccessRequested). - Fifo::PauseAndLock(doLock, unpauseOnUnlock); + Fifo::PauseAndLock(do_lock, false); #if defined(__LIBUSB__) || defined(_WIN32) GCAdapter::ResetRumble(); #endif - return wasUnpaused; + + // CPU is unlocked last because CPU::PauseAndLock contains the synchronization + // mechanism that prevents CPU::Break from racing. + if (!do_lock) + { + // The CPU is responsible for managing the Audio and FIFO state so we use its + // mechanism to unpause them. If we unpaused the systems above when releasing + // the locks then they could call CPU::Break which would require detecting it + // and re-pausing with CPU::EnableStepping. + was_unpaused = CPU::PauseAndLock(false, unpause_on_unlock, true); + } + + return was_unpaused; } // Display FPS info @@ -803,7 +842,7 @@ void UpdateTitle() float Speed = (float)(s_drawn_video.load() * (100 * 1000.0) / (VideoInterface::GetTargetRefreshRate() * ElapseTime)); // Settings are shown the same for both extended and summary info - std::string SSettings = StringFromFormat("%s %s | %s | %s", cpu_core_base->GetName(), _CoreParameter.bCPUThread ? "DC" : "SC", + std::string SSettings = StringFromFormat("%s %s | %s | %s", PowerPC::GetCPUName(), _CoreParameter.bCPUThread ? "DC" : "SC", g_video_backend->GetDisplayName().c_str(), _CoreParameter.bDSPHLE ? "HLE" : "LLE"); std::string SFPS; diff --git a/Source/Core/Core/Core.h b/Source/Core/Core/Core.h index e9f179579d..b5de60fd5e 100644 --- a/Source/Core/Core/Core.h +++ b/Source/Core/Core/Core.h @@ -52,7 +52,8 @@ bool IsRunningInCurrentThread(); // this tells us whether we are running in the bool IsCPUThread(); // this tells us whether we are the CPU thread. bool IsGPUThread(); -void SetState(EState _State); +// [NOT THREADSAFE] For use by Host only +void SetState(EState state); EState GetState(); void SaveScreenShot(); @@ -80,13 +81,14 @@ void UpdateTitle(); // or, if doLock is false, releases a lock on that state and optionally unpauses. // calls must be balanced (once with doLock true, then once with doLock false) but may be recursive. // the return value of the first call should be passed in as the second argument of the second call. -bool PauseAndLock(bool doLock, bool unpauseOnUnlock=true); +// [NOT THREADSAFE] Host only +bool PauseAndLock(bool doLock, bool unpauseOnUnlock = true); // for calling back into UI code without introducing a dependency on it in core typedef void(*StoppedCallbackFunc)(void); void SetOnStoppedCallback(StoppedCallbackFunc callback); -// Run on the GUI thread when the factors change. +// Run on the Host thread when the factors change. [NOT THREADSAFE] void UpdateWantDeterminism(bool initial = false); } // namespace diff --git a/Source/Core/Core/Debugger/PPCDebugInterface.cpp b/Source/Core/Core/Debugger/PPCDebugInterface.cpp index 0f61bff49f..9d6eacef8e 100644 --- a/Source/Core/Core/Debugger/PPCDebugInterface.cpp +++ b/Source/Core/Core/Debugger/PPCDebugInterface.cpp @@ -10,7 +10,6 @@ #include "Core/Host.h" #include "Core/Debugger/Debugger_SymbolMap.h" #include "Core/Debugger/PPCDebugInterface.h" -#include "Core/HW/CPU.h" #include "Core/HW/DSP.h" #include "Core/HW/Memmap.h" #include "Core/PowerPC/PowerPC.h" @@ -20,7 +19,7 @@ std::string PPCDebugInterface::Disassemble(unsigned int address) { // PowerPC::HostRead_U32 seemed to crash on shutdown - if (PowerPC::GetState() == PowerPC::CPU_POWERDOWN) + if (!IsAlive()) return ""; if (Core::GetState() == Core::CORE_PAUSE) @@ -51,7 +50,7 @@ std::string PPCDebugInterface::Disassemble(unsigned int address) void PPCDebugInterface::GetRawMemoryString(int memory, unsigned int address, char *dest, int max_size) { - if (Core::GetState() != Core::CORE_UNINITIALIZED) + if (IsAlive()) { if (memory || PowerPC::HostIsRAMAddress(address)) { @@ -96,7 +95,7 @@ unsigned int PPCDebugInterface::ReadInstruction(unsigned int address) bool PPCDebugInterface::IsAlive() { - return Core::GetState() != Core::CORE_UNINITIALIZED; + return Core::IsRunning(); } bool PPCDebugInterface::IsBreakpoint(unsigned int address) diff --git a/Source/Core/Core/FifoPlayer/FifoPlayer.cpp b/Source/Core/Core/FifoPlayer/FifoPlayer.cpp index a61d4a5887..a59861cde6 100644 --- a/Source/Core/Core/FifoPlayer/FifoPlayer.cpp +++ b/Source/Core/Core/FifoPlayer/FifoPlayer.cpp @@ -7,12 +7,14 @@ #include "Common/Assert.h" #include "Common/CommonTypes.h" +#include "Common/MsgHandler.h" #include "Core/ConfigManager.h" #include "Core/Core.h" #include "Core/CoreTiming.h" #include "Core/Host.h" #include "Core/FifoPlayer/FifoDataFile.h" #include "Core/FifoPlayer/FifoPlayer.h" +#include "Core/HW/CPU.h" #include "Core/HW/GPFifo.h" #include "Core/HW/Memmap.h" #include "Core/HW/ProcessorInterface.h" @@ -58,55 +60,103 @@ void FifoPlayer::Close() m_FrameRangeEnd = 0; } -bool FifoPlayer::Play() +class FifoPlayer::CPUCore final : public CPUCoreBase { - if (!m_File) - return false; - - if (m_File->GetFrameCount() == 0) - return false; - - IsPlayingBackFifologWithBrokenEFBCopies = m_File->HasBrokenEFBCopies(); - - m_CurrentFrame = m_FrameRangeStart; - - LoadMemory(); - - // This loop replaces the CPU loop that occurs when a game is run - while (PowerPC::GetState() != PowerPC::CPU_POWERDOWN) +public: + explicit CPUCore(FifoPlayer* parent) + : m_parent(parent) { - if (PowerPC::GetState() == PowerPC::CPU_RUNNING) + } + CPUCore(const CPUCore&) = delete; + ~CPUCore() + { + } + CPUCore& operator=(const CPUCore&) = delete; + + void Init() override + { + IsPlayingBackFifologWithBrokenEFBCopies = m_parent->m_File->HasBrokenEFBCopies(); + + m_parent->m_CurrentFrame = m_parent->m_FrameRangeStart; + m_parent->LoadMemory(); + } + + void Shutdown() override + { + IsPlayingBackFifologWithBrokenEFBCopies = false; + } + + void ClearCache() override + { + // Nothing to clear. + } + + void SingleStep() override + { + // NOTE: AdvanceFrame() will get stuck forever in Dual Core because the FIFO + // is disabled by CPU::EnableStepping(true) so the frame never gets displayed. + PanicAlertT("Cannot SingleStep the FIFO. Use Frame Advance instead."); + } + + const char* GetName() override + { + return "FifoPlayer"; + } + + void Run() override + { + while (CPU::GetState() == CPU::CPU_RUNNING) { - if (m_CurrentFrame >= m_FrameRangeEnd) + switch (m_parent->AdvanceFrame()) { - if (m_Loop) - { - m_CurrentFrame = m_FrameRangeStart; - } - else - { - PowerPC::Stop(); - Host_Message(WM_USER_STOP); - } - } - else - { - if (m_FrameWrittenCb) - m_FrameWrittenCb(); + case CPU::CPU_POWERDOWN: + CPU::Break(); + Host_Message(WM_USER_STOP); + break; - if (m_EarlyMemoryUpdates && m_CurrentFrame == m_FrameRangeStart) - WriteAllMemoryUpdates(); - - WriteFrame(m_File->GetFrame(m_CurrentFrame), m_FrameInfo[m_CurrentFrame]); - - ++m_CurrentFrame; + case CPU::CPU_STEPPING: + CPU::Break(); + Host_UpdateMainFrame(); + break; } } } - IsPlayingBackFifologWithBrokenEFBCopies = false; +private: + FifoPlayer* m_parent; +}; - return true; +int FifoPlayer::AdvanceFrame() +{ + if (m_CurrentFrame >= m_FrameRangeEnd) + { + if (!m_Loop) + return CPU::CPU_POWERDOWN; + // If there are zero frames in the range then sleep instead of busy spinning + if (m_FrameRangeStart >= m_FrameRangeEnd) + return CPU::CPU_STEPPING; + + m_CurrentFrame = m_FrameRangeStart; + } + + if (m_FrameWrittenCb) + m_FrameWrittenCb(); + + if (m_EarlyMemoryUpdates && m_CurrentFrame == m_FrameRangeStart) + WriteAllMemoryUpdates(); + + WriteFrame(m_File->GetFrame(m_CurrentFrame), m_FrameInfo[m_CurrentFrame]); + + ++m_CurrentFrame; + return CPU::CPU_RUNNING; +} + +std::unique_ptr FifoPlayer::GetCPUCore() +{ + if (!m_File || m_File->GetFrameCount() == 0) + return nullptr; + + return std::make_unique(this); } u32 FifoPlayer::GetFrameObjectCount() diff --git a/Source/Core/Core/FifoPlayer/FifoPlayer.h b/Source/Core/Core/FifoPlayer/FifoPlayer.h index a61ab7cc86..78f40ac777 100644 --- a/Source/Core/Core/FifoPlayer/FifoPlayer.h +++ b/Source/Core/Core/FifoPlayer/FifoPlayer.h @@ -4,10 +4,12 @@ #pragma once +#include #include #include #include "Core/FifoPlayer/FifoPlaybackAnalyzer.h" +#include "Core/PowerPC/CPUCoreBase.h" class FifoDataFile; struct MemoryUpdate; @@ -50,8 +52,12 @@ public: bool Open(const std::string& filename); void Close(); - // Play is controlled by the state of PowerPC - bool Play(); + // Returns a CPUCoreBase instance that can be injected into PowerPC as a + // pseudo-CPU. The instance is only valid while the FifoPlayer is Open(). + // Returns nullptr if the FifoPlayer is not initialized correctly. + // Play/Pause/Stop of the FifoLog can be controlled normally via the + // PowerPC state. + std::unique_ptr GetCPUCore(); FifoDataFile *GetFile() { return m_File; } @@ -85,8 +91,12 @@ public: static FifoPlayer &GetInstance(); private: + class CPUCore; + FifoPlayer(); + int AdvanceFrame(); + void WriteFrame(const FifoFrameInfo& frame, const AnalyzedFrameInfo &info); void WriteFramePart(u32 dataStart, u32 dataEnd, u32 &nextMemUpdate, const FifoFrameInfo& frame, const AnalyzedFrameInfo& info); diff --git a/Source/Core/Core/HLE/HLE_Misc.cpp b/Source/Core/Core/HLE/HLE_Misc.cpp index cb77e0f215..26ce8c884f 100644 --- a/Source/Core/Core/HLE/HLE_Misc.cpp +++ b/Source/Core/Core/HLE/HLE_Misc.cpp @@ -9,6 +9,7 @@ #include "Core/ConfigManager.h" #include "Core/Host.h" #include "Core/HLE/HLE_Misc.h" +#include "Core/HW/CPU.h" #include "Core/PowerPC/PowerPC.h" #include "Core/PowerPC/PPCCache.h" @@ -35,7 +36,7 @@ void HLEPanicAlert() void HBReload() { // There isn't much we can do. Just stop cleanly. - PowerPC::Pause(); + CPU::Break(); Host_Message(WM_USER_STOP); } diff --git a/Source/Core/Core/HW/CPU.cpp b/Source/Core/Core/HW/CPU.cpp index a853fdbd73..e436b90d1b 100644 --- a/Source/Core/Core/HW/CPU.cpp +++ b/Source/Core/Core/HW/CPU.cpp @@ -2,11 +2,13 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. +#include #include #include "AudioCommon/AudioCommon.h" #include "Common/CommonTypes.h" #include "Common/Event.h" +#include "Common/Logging/Log.h" #include "Core/Core.h" #include "Core/Host.h" #include "Core/HW/CPU.h" @@ -14,82 +16,177 @@ #include "Core/PowerPC/PowerPC.h" #include "VideoCommon/Fifo.h" -namespace -{ - static Common::Event m_StepEvent; - static Common::Event *m_SyncEvent = nullptr; - static std::mutex m_csCpuOccupied; -} - namespace CPU { +// CPU Thread execution state. +// Requires s_state_change_lock to modify the value. +// Read access is unsynchronized. +static State s_state = CPU_POWERDOWN; + +// Synchronizes EnableStepping and PauseAndLock so only one instance can be +// active at a time. Simplifies code by eliminating several edge cases where +// the EnableStepping(true)/PauseAndLock(true) case must release the state lock +// and wait for the CPU Thread which would otherwise require additional flags. +// NOTE: When using the stepping lock, it must always be acquired first. If +// the lock is acquired after the state lock then that is guaranteed to +// deadlock because of the order inversion. (A -> X,Y; B -> Y,X; A waits for +// B, B waits for A) +static std::mutex s_stepping_lock; + +// Primary lock. Protects changing s_state, requesting instruction stepping and +// pause-and-locking. +static std::mutex s_state_change_lock; +// When s_state_cpu_thread_active changes to false +static std::condition_variable s_state_cpu_idle_cvar; +// When s_state changes / s_state_paused_and_locked becomes false (for CPU Thread only) +static std::condition_variable s_state_cpu_cvar; +static bool s_state_cpu_thread_active = false; +static bool s_state_paused_and_locked = false; +static bool s_state_system_request_stepping = false; +static bool s_state_cpu_step_instruction = false; +static Common::Event* s_state_cpu_step_instruction_sync = nullptr; + void Init(int cpu_core) { PowerPC::Init(cpu_core); - m_SyncEvent = nullptr; + s_state = CPU_STEPPING; } void Shutdown() { + Stop(); PowerPC::Shutdown(); - m_SyncEvent = nullptr; +} + +// Requires holding s_state_change_lock +static void FlushStepSyncEventLocked() +{ + if (s_state_cpu_step_instruction_sync) + { + s_state_cpu_step_instruction_sync->Set(); + s_state_cpu_step_instruction_sync = nullptr; + } + s_state_cpu_step_instruction = false; } void Run() { - std::lock_guard lk(m_csCpuOccupied); - Host_UpdateDisasmDialog(); - - while (true) + std::unique_lock state_lock(s_state_change_lock); + while (s_state != CPU_POWERDOWN) { - switch (PowerPC::GetState()) + s_state_cpu_cvar.wait(state_lock, [] { return !s_state_paused_and_locked; }); + + switch (s_state) { - case PowerPC::CPU_RUNNING: - //1: enter a fast runloop + case CPU_RUNNING: + s_state_cpu_thread_active = true; + state_lock.unlock(); + + // Adjust PC for JIT when debugging + // SingleStep so that the "continue", "step over" and "step out" debugger functions + // work when the PC is at a breakpoint at the beginning of the block + // If watchpoints are enabled, any instruction could be a breakpoint. + if (PowerPC::GetMode() != PowerPC::MODE_INTERPRETER) + { +#ifndef ENABLE_MEM_CHECK + if (PowerPC::breakpoints.IsAddressBreakPoint(PC)) +#endif + { + PowerPC::CoreMode old_mode = PowerPC::GetMode(); + PowerPC::SetMode(PowerPC::MODE_INTERPRETER); + PowerPC::SingleStep(); + PowerPC::SetMode(old_mode); + } + } + + // Enter a fast runloop PowerPC::RunLoop(); + + state_lock.lock(); + s_state_cpu_thread_active = false; + s_state_cpu_idle_cvar.notify_all(); break; - case PowerPC::CPU_STEPPING: - m_csCpuOccupied.unlock(); - - //1: wait for step command.. - m_StepEvent.Wait(); - - m_csCpuOccupied.lock(); - if (PowerPC::GetState() == PowerPC::CPU_POWERDOWN) - return; - if (PowerPC::GetState() != PowerPC::CPU_STEPPING) + case CPU_STEPPING: + // Wait for step command. + s_state_cpu_cvar.wait(state_lock, [] + { + return s_state_cpu_step_instruction || + s_state != CPU_STEPPING; + }); + if (s_state != CPU_STEPPING) + { + // Signal event if the mode changes. + FlushStepSyncEventLocked(); + continue; + } + if (s_state_paused_and_locked) continue; - //3: do a step + // Do step + s_state_cpu_thread_active = true; + state_lock.unlock(); + PowerPC::SingleStep(); - //4: update disasm dialog - if (m_SyncEvent) - { - m_SyncEvent->Set(); - m_SyncEvent = nullptr; - } + state_lock.lock(); + s_state_cpu_thread_active = false; + s_state_cpu_idle_cvar.notify_all(); + + // Update disasm dialog + FlushStepSyncEventLocked(); Host_UpdateDisasmDialog(); break; - case PowerPC::CPU_POWERDOWN: - //1: Exit loop!! - return; + case CPU_POWERDOWN: + break; } } + state_lock.unlock(); + Host_UpdateDisasmDialog(); +} + +// Requires holding s_state_change_lock +static void RunAdjacentSystems(bool running) +{ + // NOTE: We're assuming these will not try to call Break or EnableStepping. + Fifo::EmulatorState(running); + AudioCommon::ClearAudioBuffer(!running); } void Stop() { - PowerPC::Stop(); - m_StepEvent.Set(); + // Change state and wait for it to be acknowledged. + // We don't need the stepping lock because CPU_POWERDOWN is a priority state which + // will stick permanently. + std::unique_lock state_lock(s_state_change_lock); + s_state = CPU_POWERDOWN; + s_state_cpu_cvar.notify_one(); + // FIXME: MsgHandler can cause this to deadlock the GUI Thread. Remove the timeout. + bool success = s_state_cpu_idle_cvar.wait_for(state_lock, std::chrono::seconds(5), [] + { + return !s_state_cpu_thread_active; + }); + if (!success) + ERROR_LOG(POWERPC, "CPU Thread failed to acknowledge CPU_POWERDOWN. It may be deadlocked."); + RunAdjacentSystems(false); + FlushStepSyncEventLocked(); } bool IsStepping() { - return PowerPC::GetState() == PowerPC::CPU_STEPPING; + return s_state == CPU_STEPPING; +} + +State GetState() +{ + return s_state; +} + +const volatile State* GetStatePtr() +{ + return &s_state; } void Reset() @@ -98,87 +195,142 @@ void Reset() void StepOpcode(Common::Event* event) { - m_StepEvent.Set(); - if (PowerPC::GetState() == PowerPC::CPU_STEPPING) + std::lock_guard state_lock(s_state_change_lock); + // If we're not stepping then this is pointless + if (!IsStepping()) { - m_SyncEvent = event; + if (event) + event->Set(); + return; } + + // Potential race where the previous step has not been serviced yet. + if (s_state_cpu_step_instruction_sync && s_state_cpu_step_instruction_sync != event) + s_state_cpu_step_instruction_sync->Set(); + + s_state_cpu_step_instruction = true; + s_state_cpu_step_instruction_sync = event; + s_state_cpu_cvar.notify_one(); } -void EnableStepping(const bool stepping) +// Requires s_state_change_lock +static bool SetStateLocked(State s) { + if (s_state == CPU_POWERDOWN) + return false; + s_state = s; + return true; +} + +void EnableStepping(bool stepping) +{ + std::lock_guard stepping_lock(s_stepping_lock); + std::unique_lock state_lock(s_state_change_lock); + if (stepping) { - PowerPC::Pause(); - m_StepEvent.Reset(); - Fifo::EmulatorState(false); - AudioCommon::ClearAudioBuffer(true); - } - else - { - // SingleStep so that the "continue", "step over" and "step out" debugger functions - // work when the PC is at a breakpoint at the beginning of the block - // If watchpoints are enabled, any instruction could be a breakpoint. - bool could_be_bp; -#ifdef ENABLE_MEM_CHECK - could_be_bp = true; -#else - could_be_bp = PowerPC::breakpoints.IsAddressBreakPoint(PC); -#endif - if (could_be_bp && PowerPC::GetMode() != PowerPC::MODE_INTERPRETER) + SetStateLocked(CPU_STEPPING); + + // Wait for the CPU Thread to leave the run loop + // FIXME: MsgHandler can cause this to deadlock the GUI Thread. Remove the timeout. + bool success = s_state_cpu_idle_cvar.wait_for(state_lock, std::chrono::seconds(5), [] { - PowerPC::CoreMode oldMode = PowerPC::GetMode(); - PowerPC::SetMode(PowerPC::MODE_INTERPRETER); - PowerPC::SingleStep(); - PowerPC::SetMode(oldMode); - } - PowerPC::Start(); - m_StepEvent.Set(); - Fifo::EmulatorState(true); - AudioCommon::ClearAudioBuffer(false); + return !s_state_cpu_thread_active; + }); + if (!success) + ERROR_LOG(POWERPC, "Abandoned waiting for CPU Thread! The Core may be deadlocked."); + + RunAdjacentSystems(false); + } + else if (SetStateLocked(CPU_RUNNING)) + { + s_state_cpu_cvar.notify_one(); + RunAdjacentSystems(true); } } void Break() { - EnableStepping(true); + std::lock_guard state_lock(s_state_change_lock); + + // If another thread is trying to PauseAndLock then we need to remember this + // for later to ignore the unpause_on_unlock. + if (s_state_paused_and_locked) + { + s_state_system_request_stepping = true; + return; + } + + // We'll deadlock if we synchronize, the CPU may block waiting for our caller to + // finish resulting in the CPU loop never terminating. + SetStateLocked(CPU_STEPPING); + RunAdjacentSystems(false); } -bool PauseAndLock(bool do_lock, bool unpause_on_unlock) +bool PauseAndLock(bool do_lock, bool unpause_on_unlock, bool control_adjacent) { - static bool s_have_fake_cpu_thread; - bool wasUnpaused = !IsStepping(); + // NOTE: This is protected by s_stepping_lock. + static bool s_have_fake_cpu_thread = false; + bool was_unpaused = false; + if (do_lock) { - // we can't use EnableStepping, that would causes deadlocks with both audio and video - PowerPC::Pause(); + s_stepping_lock.lock(); + + std::unique_lock state_lock(s_state_change_lock); + s_state_paused_and_locked = true; + + was_unpaused = s_state == CPU_RUNNING; + SetStateLocked(CPU_STEPPING); + + // FIXME: MsgHandler can cause this to deadlock the GUI Thread. Remove the timeout. + bool success = s_state_cpu_idle_cvar.wait_for(state_lock, std::chrono::seconds(10), [] + { + return !s_state_cpu_thread_active; + }); + if (!success) + NOTICE_LOG(POWERPC, "Abandoned CPU Thread synchronization in CPU::PauseAndLock! We'll probably crash now."); + + if (control_adjacent) + RunAdjacentSystems(false); + state_lock.unlock(); + + // NOTE: It would make more sense for Core::DeclareAsCPUThread() to keep a + // depth counter instead of being a boolean. if (!Core::IsCPUThread()) { - m_csCpuOccupied.lock(); s_have_fake_cpu_thread = true; Core::DeclareAsCPUThread(); } - else - { - s_have_fake_cpu_thread = false; - } } else { - if (unpause_on_unlock) - { - PowerPC::Start(); - m_StepEvent.Set(); - } - + // Only need the stepping lock for this if (s_have_fake_cpu_thread) { - Core::UndeclareAsCPUThread(); - m_csCpuOccupied.unlock(); s_have_fake_cpu_thread = false; + Core::UndeclareAsCPUThread(); } + + { + std::lock_guard state_lock(s_state_change_lock); + if (s_state_system_request_stepping) + { + s_state_system_request_stepping = false; + } + else if (unpause_on_unlock && SetStateLocked(CPU_RUNNING)) + { + was_unpaused = true; + } + s_state_paused_and_locked = false; + s_state_cpu_cvar.notify_one(); + + if (control_adjacent) + RunAdjacentSystems(s_state == CPU_RUNNING); + } + s_stepping_lock.unlock(); } - return wasUnpaused; + return was_unpaused; } } diff --git a/Source/Core/Core/HW/CPU.h b/Source/Core/Core/HW/CPU.h index 41b2630f2e..f9fc6aaca9 100644 --- a/Source/Core/Core/HW/CPU.h +++ b/Source/Core/Core/HW/CPU.h @@ -11,6 +11,13 @@ namespace Common { namespace CPU { +enum State +{ + CPU_RUNNING = 0, + CPU_STEPPING = 2, + CPU_POWERDOWN = 3 +}; + // Init void Init(int cpu_core); @@ -18,32 +25,49 @@ void Init(int cpu_core); void Shutdown(); // Starts the CPU +// To be called by the CPU Thread. void Run(); // Causes shutdown +// Once started, CPU_POWERDOWN cannot be stopped. +// Synchronizes with the CPU Thread (waits for CPU::Run to exit). void Stop(); -// Reset +// Reset [NOT IMPLEMENTED] void Reset(); // StepOpcode (Steps one Opcode) void StepOpcode(Common::Event* event = nullptr); -// Enable or Disable Stepping +// Enable or Disable Stepping. [Will deadlock if called from a system thread] void EnableStepping(bool stepping); -// Break, same as EnableStepping(true). +// Breakpoint activation for system threads. Similar to EnableStepping(true). +// NOTE: Unlike EnableStepping, this does NOT synchronize with the CPU Thread +// which enables it to avoid deadlocks but also makes it less safe so it +// should not be used by the Host. void Break(); -// Is stepping ? +// Shorthand for GetState() == CPU_STEPPING. +// WARNING: CPU_POWERDOWN will return false, not just CPU_RUNNING. bool IsStepping(); -// Waits until is stepping and is ready for a command (paused and fully idle), and acquires a lock on that state. -// or, if doLock is false, releases a lock on that state and optionally re-disables stepping. -// calls must be balanced and non-recursive (once with doLock true, then once with doLock false). -// intended (but not required) to be called from another thread, -// e.g. when the GUI thread wants to make sure everything is paused so that it can create a savestate. -// the return value is whether the CPU was unpaused before the call. -bool PauseAndLock(bool do_lock, bool unpause_on_unlock = true); +// Get current CPU Thread State +State GetState(); + +// Direct State Access (Raw pointer for embedding into JIT Blocks) +// Strictly read-only. A lock is required to change the value. +const volatile State* GetStatePtr(); + +// Locks the CPU Thread (waiting for it to become idle). +// While this lock is held, the CPU Thread will not perform any action so it is safe to access +// PowerPC::ppcState, CoreTiming, etc. without racing the CPU Thread. +// Cannot be used recursively. Must be paired as PauseAndLock(true)/PauseAndLock(false). +// Return value for do_lock == true is whether the state was CPU_RUNNING or not. +// Return value for do_lock == false is whether the state was changed *to* CPU_RUNNING or not. +// Cannot be used by System threads as it will deadlock. It is threadsafe otherwise. +// "control_adjacent" causes PauseAndLock to behave like EnableStepping by modifying the +// state of the Audio and FIFO subsystems as well. +bool PauseAndLock(bool do_lock, bool unpause_on_unlock = true, bool control_adjacent = false); } diff --git a/Source/Core/Core/Movie.cpp b/Source/Core/Core/Movie.cpp index 23eaf160fe..368ad04eb9 100644 --- a/Source/Core/Core/Movie.cpp +++ b/Source/Core/Core/Movie.cpp @@ -21,6 +21,7 @@ #include "Core/NetPlayProto.h" #include "Core/State.h" #include "Core/DSP/DSPCore.h" +#include "Core/HW/CPU.h" #include "Core/HW/DVDInterface.h" #include "Core/HW/EXI_Device.h" #include "Core/HW/ProcessorInterface.h" @@ -155,8 +156,8 @@ void FrameUpdate() } if (s_bFrameStep) { - Core::SetState(Core::CORE_PAUSE); s_bFrameStep = false; + CPU::Break(); } if (s_framesToSkip) @@ -246,9 +247,9 @@ void DoFrameStep() if (Core::GetState() == Core::CORE_PAUSE) { // if already paused, frame advance for 1 frame - Core::SetState(Core::CORE_RUN); - Core::RequestRefreshInfo(); s_bFrameStep = true; + Core::RequestRefreshInfo(); + Core::SetState(Core::CORE_RUN); } else if (!s_bFrameStep) { diff --git a/Source/Core/Core/PowerPC/CachedInterpreter.cpp b/Source/Core/Core/PowerPC/CachedInterpreter.cpp index 3b52caaa62..6adc822349 100644 --- a/Source/Core/Core/PowerPC/CachedInterpreter.cpp +++ b/Source/Core/Core/PowerPC/CachedInterpreter.cpp @@ -7,6 +7,7 @@ #include "Core/ConfigManager.h" #include "Core/CoreTiming.h" #include "Core/HLE/HLE.h" +#include "Core/HW/CPU.h" #include "Core/PowerPC/CachedInterpreter.h" #include "Core/PowerPC/Gekko.h" #include "Core/PowerPC/PowerPC.h" @@ -32,13 +33,10 @@ void CachedInterpreter::Shutdown() void CachedInterpreter::Run() { - while (!PowerPC::GetState()) + while (!CPU::GetState()) { SingleStep(); } - - // Let the waiting thread know we are done leaving - PowerPC::FinishStateMove(); } void CachedInterpreter::SingleStep() diff --git a/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp b/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp index 280bb8d80a..ecf3bbb151 100644 --- a/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp +++ b/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp @@ -200,7 +200,7 @@ int ShowSteps = 300; // FastRun - inspired by GCemu (to imitate the JIT so that they can be compared). void Interpreter::Run() { - while (!PowerPC::GetState()) + while (!CPU::GetState()) { //we have to check exceptions at branches apparently (or maybe just rfi?) if (SConfig::GetInstance().bEnableDebugging) @@ -279,9 +279,6 @@ void Interpreter::Run() CoreTiming::Advance(); } - - // Let the waiting thread know we are done leaving - PowerPC::FinishStateMove(); } void Interpreter::unknown_instruction(UGeckoInstruction _inst) diff --git a/Source/Core/Core/PowerPC/Jit64/Jit.cpp b/Source/Core/Core/PowerPC/Jit64/Jit.cpp index 111dcb46ab..e4f211a090 100644 --- a/Source/Core/Core/PowerPC/Jit64/Jit.cpp +++ b/Source/Core/Core/PowerPC/Jit64/Jit.cpp @@ -19,6 +19,7 @@ #include "Core/CoreTiming.h" #include "Core/PatchEngine.h" #include "Core/HLE/HLE.h" +#include "Core/HW/CPU.h" #include "Core/HW/GPFifo.h" #include "Core/HW/ProcessorInterface.h" #include "Core/PowerPC/JitInterface.h" @@ -561,7 +562,7 @@ void Jit64::Jit(u32 em_address) // Comment out the following to disable breakpoints (speed-up) if (!Profiler::g_ProfileBlocks) { - if (GetState() == CPU_STEPPING) + if (CPU::GetState() == CPU::CPU_STEPPING) { blockSize = 1; @@ -800,7 +801,9 @@ const u8* Jit64::DoJit(u32 em_address, PPCAnalyst::CodeBuffer *code_buf, JitBloc js.firstFPInstructionFound = true; } - if (SConfig::GetInstance().bEnableDebugging && breakpoints.IsAddressBreakPoint(ops[i].address) && GetState() != CPU_STEPPING) + if (SConfig::GetInstance().bEnableDebugging && + breakpoints.IsAddressBreakPoint(ops[i].address) && + CPU::GetState() != CPU::CPU_STEPPING) { // Turn off block linking if there are breakpoints so that the Step Over command does not link this block. jo.enableBlocklink = false; @@ -812,7 +815,7 @@ const u8* Jit64::DoJit(u32 em_address, PPCAnalyst::CodeBuffer *code_buf, JitBloc ABI_PushRegistersAndAdjustStack({}, 0); ABI_CallFunction(reinterpret_cast(&PowerPC::CheckBreakPoints)); ABI_PopRegistersAndAdjustStack({}, 0); - TEST(32, M(PowerPC::GetStatePtr()), Imm32(0xFFFFFFFF)); + TEST(32, M(CPU::GetStatePtr()), Imm32(0xFFFFFFFF)); FixupBranch noBreakpoint = J_CC(CC_Z); WriteExit(ops[i].address); diff --git a/Source/Core/Core/PowerPC/Jit64/JitAsm.cpp b/Source/Core/Core/PowerPC/Jit64/JitAsm.cpp index 0b1638bbf3..28deee1053 100644 --- a/Source/Core/Core/PowerPC/Jit64/JitAsm.cpp +++ b/Source/Core/Core/PowerPC/Jit64/JitAsm.cpp @@ -8,6 +8,7 @@ #include "Common/x64Emitter.h" #include "Core/ConfigManager.h" #include "Core/CoreTiming.h" +#include "Core/HW/CPU.h" #include "Core/HW/Memmap.h" #include "Core/PowerPC/PowerPC.h" #include "Core/PowerPC/Jit64/Jit.h" @@ -75,12 +76,12 @@ void Jit64AsmRoutineManager::Generate() if (SConfig::GetInstance().bEnableDebugging) { - TEST(32, M(PowerPC::GetStatePtr()), Imm32(PowerPC::CPU_STEPPING)); + TEST(32, M(CPU::GetStatePtr()), Imm32(CPU::CPU_STEPPING)); FixupBranch notStepping = J_CC(CC_Z); ABI_PushRegistersAndAdjustStack({}, 0); ABI_CallFunction(reinterpret_cast(&PowerPC::CheckBreakPoints)); ABI_PopRegistersAndAdjustStack({}, 0); - TEST(32, M(PowerPC::GetStatePtr()), Imm32(0xFFFFFFFF)); + TEST(32, M(CPU::GetStatePtr()), Imm32(0xFFFFFFFF)); dbg_exit = J_CC(CC_NZ, true); SetJumpTarget(notStepping); } @@ -208,7 +209,7 @@ void Jit64AsmRoutineManager::Generate() // Check the state pointer to see if we are exiting // Gets checked on at the end of every slice - TEST(32, M(PowerPC::GetStatePtr()), Imm32(0xFFFFFFFF)); + TEST(32, M(CPU::GetStatePtr()), Imm32(0xFFFFFFFF)); J_CC(CC_Z, outerLoop); //Landing pad for drec space @@ -221,11 +222,6 @@ void Jit64AsmRoutineManager::Generate() POP(RSP); } - // Let the waiting thread know we are done leaving - ABI_PushRegistersAndAdjustStack({}, 0); - ABI_CallFunction(reinterpret_cast(&PowerPC::FinishStateMove)); - ABI_PopRegistersAndAdjustStack({}, 0); - ABI_PopRegistersAndAdjustStack(ABI_ALL_CALLEE_SAVED, 8, 16); RET(); diff --git a/Source/Core/Core/PowerPC/Jit64/Jit_LoadStore.cpp b/Source/Core/Core/PowerPC/Jit64/Jit_LoadStore.cpp index 70eef7373d..37fb86e17b 100644 --- a/Source/Core/Core/PowerPC/Jit64/Jit_LoadStore.cpp +++ b/Source/Core/Core/PowerPC/Jit64/Jit_LoadStore.cpp @@ -12,6 +12,7 @@ #include "Common/x64Emitter.h" #include "Core/ConfigManager.h" #include "Core/CoreTiming.h" +#include "Core/HW/CPU.h" #include "Core/HW/DSP.h" #include "Core/HW/Memmap.h" #include "Core/PowerPC/JitInterface.h" @@ -120,7 +121,7 @@ void Jit64::lXXx(UGeckoInstruction inst) // TODO: We shouldn't use a debug read here. It should be possible to get // the following instructions out of the JIT state. if (SConfig::GetInstance().bSkipIdle && - PowerPC::GetState() != PowerPC::CPU_STEPPING && + CPU::GetState() != CPU::CPU_STEPPING && inst.OPCD == 32 && MergeAllowedNextInstructions(2) && (inst.hex & 0xFFFF0000) == 0x800D0000 && diff --git a/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp b/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp index c05b3052bd..06102dc8f7 100644 --- a/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp +++ b/Source/Core/Core/PowerPC/Jit64IL/IR_X86.cpp @@ -35,6 +35,7 @@ The register allocation is linear scan allocation. #include "Common/x64ABI.h" #include "Common/x64Emitter.h" #include "Core/CoreTiming.h" +#include "Core/HW/CPU.h" #include "Core/HW/ProcessorInterface.h" #include "Core/PowerPC/Gekko.h" #include "Core/PowerPC/PowerPC.h" @@ -2299,7 +2300,7 @@ static void DoWriteCode(IRBuilder* ibuild, JitIL* Jit, u32 exitAddress) Jit->MOV(32, PPCSTATE(pc), Imm32(InstLoc)); Jit->ABI_CallFunction(reinterpret_cast(&PowerPC::CheckBreakPoints)); - Jit->TEST(32, M(PowerPC::GetStatePtr()), Imm32(0xFFFFFFFF)); + Jit->TEST(32, M(CPU::GetStatePtr()), Imm32(0xFFFFFFFF)); FixupBranch noBreakpoint = Jit->J_CC(CC_Z); Jit->WriteExit(InstLoc); Jit->SetJumpTarget(noBreakpoint); diff --git a/Source/Core/Core/PowerPC/Jit64IL/JitIL.cpp b/Source/Core/Core/PowerPC/Jit64IL/JitIL.cpp index 68fb1aa686..c8628b76e5 100644 --- a/Source/Core/Core/PowerPC/Jit64IL/JitIL.cpp +++ b/Source/Core/Core/PowerPC/Jit64IL/JitIL.cpp @@ -17,6 +17,7 @@ #include "Common/Logging/Log.h" #include "Core/PatchEngine.h" #include "Core/HLE/HLE.h" +#include "Core/HW/CPU.h" #include "Core/PowerPC/PowerPC.h" #include "Core/PowerPC/Profiler.h" #include "Core/PowerPC/Jit64IL/JitIL.h" @@ -478,7 +479,7 @@ void JitIL::Jit(u32 em_address) // Comment out the following to disable breakpoints (speed-up) if (!Profiler::g_ProfileBlocks) { - if (GetState() == CPU_STEPPING) + if (CPU::GetState() == CPU::CPU_STEPPING) { blockSize = 1; @@ -624,7 +625,9 @@ const u8* JitIL::DoJit(u32 em_address, PPCAnalyst::CodeBuffer *code_buf, JitBloc ibuild.EmitExtExceptionCheck(ibuild.EmitIntConst(ops[i].address)); } - if (SConfig::GetInstance().bEnableDebugging && breakpoints.IsAddressBreakPoint(ops[i].address) && GetState() != CPU_STEPPING) + if (SConfig::GetInstance().bEnableDebugging && + breakpoints.IsAddressBreakPoint(ops[i].address) && + CPU::GetState() != CPU::CPU_STEPPING) { // Turn off block linking if there are breakpoints so that the Step Over command does not link this block. jo.enableBlocklink = false; diff --git a/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp b/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp index e2d2b2cbcc..f5279a61d9 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp @@ -6,6 +6,7 @@ #include "Common/CommonTypes.h" #include "Common/JitRegister.h" #include "Core/CoreTiming.h" +#include "Core/HW/CPU.h" #include "Core/HW/Memmap.h" #include "Core/PowerPC/PowerPC.h" #include "Core/PowerPC/JitArm64/Jit.h" @@ -85,7 +86,7 @@ void JitArm64AsmRoutineManager::Generate() // Check the state pointer to see if we are exiting // Gets checked on at the end of every slice - MOVI2R(X0, (u64)PowerPC::GetStatePtr()); + MOVI2R(X0, (u64)CPU::GetStatePtr()); LDR(INDEX_UNSIGNED, W0, X0, 0); CMP(W0, 0); @@ -96,10 +97,6 @@ void JitArm64AsmRoutineManager::Generate() SetJumpTarget(Exit); STR(INDEX_UNSIGNED, DISPATCHER_PC, PPC_REG, PPCSTATE_OFF(pc)); - // Let the waiting thread know we are done leaving - MOVI2R(X0, (u64)&PowerPC::FinishStateMove); - BLR(X0); - ABI_PopRegisters(regs_to_save); RET(X30); diff --git a/Source/Core/Core/PowerPC/JitCommon/JitBase.cpp b/Source/Core/Core/PowerPC/JitCommon/JitBase.cpp index 89da8cef31..9beb4c8faa 100644 --- a/Source/Core/Core/PowerPC/JitCommon/JitBase.cpp +++ b/Source/Core/Core/PowerPC/JitCommon/JitBase.cpp @@ -12,6 +12,7 @@ #include "Common/StringUtil.h" #include "Common/Logging/Log.h" #include "Core/ConfigManager.h" +#include "Core/HW/CPU.h" #include "Core/PowerPC/PowerPC.h" #include "Core/PowerPC/PPCAnalyst.h" #include "Core/PowerPC/JitCommon/JitBase.h" @@ -67,7 +68,7 @@ void LogGeneratedX86(int size, PPCAnalyst::CodeBuffer *code_buffer, const u8 *no bool JitBase::MergeAllowedNextInstructions(int count) { - if (PowerPC::GetState() == PowerPC::CPU_STEPPING || js.instructionsLeft < count) + if (CPU::GetState() == CPU::CPU_STEPPING || js.instructionsLeft < count) return false; // Be careful: a breakpoint kills flags in between instructions for (int i = 1; i <= count; i++) diff --git a/Source/Core/Core/PowerPC/JitILCommon/JitILBase_LoadStore.cpp b/Source/Core/Core/PowerPC/JitILCommon/JitILBase_LoadStore.cpp index f47756d861..42aa779724 100644 --- a/Source/Core/Core/PowerPC/JitILCommon/JitILBase_LoadStore.cpp +++ b/Source/Core/Core/PowerPC/JitILCommon/JitILBase_LoadStore.cpp @@ -5,6 +5,7 @@ #include "Common/Assert.h" #include "Common/CommonTypes.h" #include "Core/ConfigManager.h" +#include "Core/HW/CPU.h" #include "Core/PowerPC/PowerPC.h" #include "Core/PowerPC/JitILCommon/JitILBase.h" @@ -57,7 +58,7 @@ void JitILBase::lXz(UGeckoInstruction inst) // or higher in PPCAnalyst // TODO: We shouldn't use debug reads here. if (SConfig::GetInstance().bSkipIdle && - PowerPC::GetState() != PowerPC::CPU_STEPPING && + CPU::GetState() != CPU::CPU_STEPPING && inst.OPCD == 32 && // Lwx (inst.hex & 0xFFFF0000) == 0x800D0000 && (PowerPC::HostRead_U32(js.compilerPC + 4) == 0x28000000 || diff --git a/Source/Core/Core/PowerPC/PowerPC.cpp b/Source/Core/Core/PowerPC/PowerPC.cpp index d709708456..433b94a11b 100644 --- a/Source/Core/Core/PowerPC/PowerPC.cpp +++ b/Source/Core/Core/PowerPC/PowerPC.cpp @@ -5,13 +5,13 @@ #include "Common/Assert.h" #include "Common/ChunkFile.h" #include "Common/CommonTypes.h" -#include "Common/Event.h" #include "Common/FPURoundMode.h" #include "Common/MathUtil.h" #include "Common/Logging/Log.h" #include "Core/ConfigManager.h" #include "Core/Host.h" +#include "Core/HW/CPU.h" #include "Core/HW/Memmap.h" #include "Core/HW/SystemTimers.h" #include "Core/PowerPC/CPUCoreBase.h" @@ -21,18 +21,16 @@ #include "Core/PowerPC/Interpreter/Interpreter.h" -CPUCoreBase *cpu_core_base; - namespace PowerPC { // STATE_TO_SAVE PowerPCState ppcState; -static volatile CPUState state = CPU_POWERDOWN; -Interpreter * const interpreter = Interpreter::getInstance(); -static CoreMode mode; -static Common::Event s_state_change; +static CPUCoreBase* s_cpu_core_base = nullptr; +static bool s_cpu_core_base_is_injected = false; +Interpreter* const s_interpreter = Interpreter::getInstance(); +static CoreMode s_mode = MODE_INTERPRETER; Watches watches; BreakPoints breakpoints; @@ -114,6 +112,8 @@ static void ResetRegisters() void Init(int cpu_core) { + // NOTE: This function runs on EmuThread, not the CPU Thread. + // Changing the rounding mode has a limited effect. FPURoundMode::SetPrecisionMode(FPURoundMode::PREC_53); memset(ppcState.sr, 0, sizeof(ppcState.sr)); @@ -139,33 +139,32 @@ void Init(int cpu_core) // We initialize the interpreter because // it is used on boot and code window independently. - interpreter->Init(); + s_interpreter->Init(); switch (cpu_core) { case PowerPC::CORE_INTERPRETER: - cpu_core_base = interpreter; + s_cpu_core_base = s_interpreter; break; default: - cpu_core_base = JitInterface::InitJitCore(cpu_core); - if (!cpu_core_base) // Handle Situations where JIT core isn't available + s_cpu_core_base = JitInterface::InitJitCore(cpu_core); + if (!s_cpu_core_base) // Handle Situations where JIT core isn't available { WARN_LOG(POWERPC, "Jit core %d not available. Defaulting to interpreter.", cpu_core); - cpu_core_base = interpreter; + s_cpu_core_base = s_interpreter; } break; } - if (cpu_core_base != interpreter) + if (s_cpu_core_base != s_interpreter) { - mode = MODE_JIT; + s_mode = MODE_JIT; } else { - mode = MODE_INTERPRETER; + s_mode = MODE_INTERPRETER; } - state = CPU_STEPPING; ppcState.iCache.Init(); @@ -175,94 +174,87 @@ void Init(int cpu_core) void Shutdown() { + InjectExternalCPUCore(nullptr); JitInterface::Shutdown(); - interpreter->Shutdown(); - cpu_core_base = nullptr; - state = CPU_POWERDOWN; + s_interpreter->Shutdown(); + s_cpu_core_base = nullptr; } CoreMode GetMode() { - return mode; + return !s_cpu_core_base_is_injected ? s_mode : MODE_INTERPRETER; } -void SetMode(CoreMode new_mode) +static void ApplyMode() { - if (new_mode == mode) - return; // We don't need to do anything. - - mode = new_mode; - - switch (mode) + switch (s_mode) { case MODE_INTERPRETER: // Switching from JIT to interpreter - cpu_core_base = interpreter; + s_cpu_core_base = s_interpreter; break; case MODE_JIT: // Switching from interpreter to JIT. // Don't really need to do much. It'll work, the cache will refill itself. - cpu_core_base = JitInterface::GetCore(); - if (!cpu_core_base) // Has a chance to not get a working JIT core if one isn't active on host - cpu_core_base = interpreter; + s_cpu_core_base = JitInterface::GetCore(); + if (!s_cpu_core_base) // Has a chance to not get a working JIT core if one isn't active on host + s_cpu_core_base = s_interpreter; break; } } +void SetMode(CoreMode new_mode) +{ + if (new_mode == s_mode) + return; // We don't need to do anything. + + s_mode = new_mode; + + // If we're using an external CPU core implementation then don't do anything. + if (s_cpu_core_base_is_injected) + return; + + ApplyMode(); +} + +const char* GetCPUName() +{ + return s_cpu_core_base->GetName(); +} + +void InjectExternalCPUCore(CPUCoreBase* new_cpu) +{ + // Previously injected. + if (s_cpu_core_base_is_injected) + s_cpu_core_base->Shutdown(); + + // nullptr means just remove + if (!new_cpu) + { + if (s_cpu_core_base_is_injected) + { + s_cpu_core_base_is_injected = false; + ApplyMode(); + } + return; + } + + new_cpu->Init(); + s_cpu_core_base = new_cpu; + s_cpu_core_base_is_injected = true; +} + void SingleStep() { - cpu_core_base->SingleStep(); + s_cpu_core_base->SingleStep(); } void RunLoop() { - state = CPU_RUNNING; - cpu_core_base->Run(); Host_UpdateDisasmDialog(); -} - -CPUState GetState() -{ - return state; -} - -const volatile CPUState *GetStatePtr() -{ - return &state; -} - -void Start() -{ - state = CPU_RUNNING; + s_cpu_core_base->Run(); Host_UpdateDisasmDialog(); } -void Pause() -{ - volatile CPUState old_state = state; - state = CPU_STEPPING; - - // Wait for the CPU core to leave - if (old_state == CPU_RUNNING) - s_state_change.WaitFor(std::chrono::seconds(1)); - Host_UpdateDisasmDialog(); -} - -void Stop() -{ - volatile CPUState old_state = state; - state = CPU_POWERDOWN; - - // Wait for the CPU core to leave - if (old_state == CPU_RUNNING) - s_state_change.WaitFor(std::chrono::seconds(1)); - Host_UpdateDisasmDialog(); -} - -void FinishStateMove() -{ - s_state_change.Set(); -} - void UpdatePerformanceMonitor(u32 cycles, u32 num_load_stores, u32 num_fp_inst) { switch (MMCR0.PMC1SELECT) @@ -521,7 +513,7 @@ void CheckBreakPoints() { if (PowerPC::breakpoints.IsAddressBreakPoint(PC)) { - PowerPC::Pause(); + CPU::Break(); if (PowerPC::breakpoints.IsTempBreakPoint(PC)) PowerPC::breakpoints.Remove(PC); } diff --git a/Source/Core/Core/PowerPC/PowerPC.h b/Source/Core/Core/PowerPC/PowerPC.h index 71faf219b5..5a9fe5842d 100644 --- a/Source/Core/Core/PowerPC/PowerPC.h +++ b/Source/Core/Core/PowerPC/PowerPC.h @@ -11,14 +11,12 @@ #include "Common/CommonTypes.h" #include "Core/Debugger/PPCDebugInterface.h" -#include "Core/PowerPC/CPUCoreBase.h" #include "Core/PowerPC/Gekko.h" #include "Core/PowerPC/PPCCache.h" +class CPUCoreBase; class PointerWrap; -extern CPUCoreBase *cpu_core_base; - namespace PowerPC { @@ -129,13 +127,6 @@ struct PowerPCState static_assert(offsetof(PowerPC::PowerPCState, above_fits_in_first_0x100) <= 0x100, "top of PowerPCState too big"); #endif -enum CPUState -{ - CPU_RUNNING = 0, - CPU_STEPPING = 2, - CPU_POWERDOWN = 3, -}; - extern PowerPCState ppcState; extern Watches watches; @@ -148,19 +139,26 @@ void Shutdown(); void DoState(PointerWrap &p); CoreMode GetMode(); +// [NOT THREADSAFE] CPU Thread or CPU::PauseAndLock or CORE_UNINITIALIZED void SetMode(CoreMode _coreType); +const char* GetCPUName(); +// Set the current CPU Core to the given implementation until removed. +// Remove the current injected CPU Core by passing nullptr. +// While an external CPUCoreBase is injected, GetMode() will return MODE_INTERPRETER. +// Init() will be called when added and Shutdown() when removed. +// [Threadsafety: Same as SetMode(), except it cannot be called from inside the CPU +// run loop on the CPU Thread - it doesn't make sense for a CPU to remove itself +// while it is CPU_RUNNING] +void InjectExternalCPUCore(CPUCoreBase* core); + +// Stepping requires the CPU Execution lock (CPU::PauseAndLock or CPU Thread) +// It's not threadsafe otherwise. void SingleStep(); void CheckExceptions(); void CheckExternalExceptions(); void CheckBreakPoints(); void RunLoop(); -void Start(); -void Pause(); -void Stop(); -void FinishStateMove(); -CPUState GetState(); -const volatile CPUState *GetStatePtr(); // this oddity is here instead of an extern declaration to easily be able to find all direct accesses throughout the code. u32 CompactCR(); void ExpandCR(u32 cr); diff --git a/Source/Core/DolphinWX/Debugger/CodeWindow.cpp b/Source/Core/DolphinWX/Debugger/CodeWindow.cpp index c0108dc026..4e5581ca8e 100644 --- a/Source/Core/DolphinWX/Debugger/CodeWindow.cpp +++ b/Source/Core/DolphinWX/Debugger/CodeWindow.cpp @@ -317,6 +317,7 @@ void CCodeWindow::StepOut() { if (CPU::IsStepping()) { + CPU::PauseAndLock(true, false); PowerPC::breakpoints.ClearAllTemporary(); // Keep stepping until the next blr or timeout after one second @@ -347,6 +348,7 @@ void CCodeWindow::StepOut() PowerPC::SingleStep(); PowerPC::SetMode(oldMode); + CPU::PauseAndLock(false, false); JumpToAddress(PC); Update(); diff --git a/Source/Core/DolphinWX/Debugger/WatchView.cpp b/Source/Core/DolphinWX/Debugger/WatchView.cpp index ea79573f12..4f46c8aab2 100644 --- a/Source/Core/DolphinWX/Debugger/WatchView.cpp +++ b/Source/Core/DolphinWX/Debugger/WatchView.cpp @@ -7,6 +7,7 @@ #include #include "Common/GekkoDisassembler.h" +#include "Core/Core.h" #include "Core/HW/Memmap.h" #include "Core/PowerPC/PowerPC.h" #include "DolphinWX/Frame.h" @@ -87,7 +88,7 @@ static wxString GetValueByRowCol(int row, int col) } else if (row <= (int)PowerPC::watches.GetWatches().size()) { - if (PowerPC::GetState() != PowerPC::CPU_POWERDOWN) + if (Core::IsRunning()) { switch (col) { @@ -198,7 +199,7 @@ wxGridCellAttr* CWatchTable::GetAttr(int row, int col, wxGridCellAttr::wxAttrKin attr->SetTextColour(red ? *wxRED : *wxBLACK); - if (row > (int)(PowerPC::watches.GetWatches().size() + 1) || (PowerPC::GetState() == PowerPC::CPU_POWERDOWN)) + if (row > (int)(PowerPC::watches.GetWatches().size() + 1) || !Core::IsRunning()) { attr->SetReadOnly(true); attr->SetBackgroundColour(*wxLIGHT_GREY); @@ -224,7 +225,7 @@ CWatchView::CWatchView(wxWindow* parent, wxWindowID id) void CWatchView::Update() { - if (PowerPC::GetState() != PowerPC::CPU_POWERDOWN) + if (Core::IsRunning()) { m_watch_table->UpdateWatch(); ForceRefresh(); diff --git a/Source/Core/DolphinWX/MainNoGUI.cpp b/Source/Core/DolphinWX/MainNoGUI.cpp index 2ccd9f633d..bf2089a5c8 100644 --- a/Source/Core/DolphinWX/MainNoGUI.cpp +++ b/Source/Core/DolphinWX/MainNoGUI.cpp @@ -22,7 +22,6 @@ #include "Core/HW/Wiimote.h" #include "Core/IPC_HLE/WII_IPC_HLE_Device_usb.h" #include "Core/IPC_HLE/WII_IPC_HLE_WiiMote.h" -#include "Core/PowerPC/PowerPC.h" #include "UICommon/UICommon.h" @@ -359,8 +358,6 @@ int main(int argc, char* argv[]) platform->MainLoop(); Core::Stop(); - while (PowerPC::GetState() != PowerPC::CPU_POWERDOWN) - updateMainFrameEvent.Wait(); Core::Shutdown(); platform->Shutdown(); From e8dfc8e654f7a8a92b126bdcff9e4b86b7463ba9 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Sun, 1 May 2016 12:09:58 +0000 Subject: [PATCH 2/3] Movie: Threadsafety Audit Fix TASInputDlg which was trying to access the GUI without the GUI lock from the CPU Thread. --- Source/Core/Core/Movie.cpp | 169 ++++++++++++++++---------- Source/Core/DolphinWX/TASInputDlg.cpp | 84 +++++++++++-- Source/Core/DolphinWX/TASInputDlg.h | 4 + 3 files changed, 183 insertions(+), 74 deletions(-) diff --git a/Source/Core/Core/Movie.cpp b/Source/Core/Core/Movie.cpp index 368ad04eb9..35bbae2762 100644 --- a/Source/Core/Core/Movie.cpp +++ b/Source/Core/Core/Movie.cpp @@ -85,11 +85,14 @@ static u8 s_language = 10; //Set to unknown until language is known static bool s_bRecordingFromSaveState = false; static bool s_bPolled = false; +// s_InputDisplay is used by both CPU and GPU (is mutable). +static std::mutex s_input_display_lock; static std::string s_InputDisplay[8]; static GCManipFunction gcmfunc = nullptr; static WiiManipFunction wiimfunc = nullptr; +// NOTE: Host / CPU Thread static void EnsureTmpInputSize(size_t bound) { if (tmpInputAllocated >= bound) @@ -119,6 +122,7 @@ static bool IsMovieHeader(u8 magic[4]) magic[3] == 0x1A; } +// NOTE: GPU Thread std::string GetInputDisplay() { if (!IsMovieActive()) @@ -133,14 +137,19 @@ std::string GetInputDisplay() } } - std::string inputDisplay = ""; - for (int i = 0; i < 8; ++i) - if ((s_numPads & (1 << i)) != 0) - inputDisplay.append(s_InputDisplay[i]); - - return inputDisplay; + std::string input_display; + { + std::lock_guard guard(s_input_display_lock); + for (int i = 0; i < 8; ++i) + { + if ((s_numPads & (1 << i)) != 0) + input_display += s_InputDisplay[i]; + } + } + return input_display; } +// NOTE: GPU Thread void FrameUpdate() { // TODO[comex]: This runs on the GPU thread, yet it messes with the CPU @@ -168,6 +177,7 @@ void FrameUpdate() // called when game is booting up, even if no movie is active, // but potentially after BeginRecordingInput or PlayInput has been called. +// NOTE: EmuThread void Init() { s_bPolled = false; @@ -213,6 +223,7 @@ void Init() } } +// NOTE: CPU Thread void InputUpdate() { g_currentInputCount++; @@ -224,6 +235,7 @@ void InputUpdate() } } +// NOTE: Host Thread void SetFrameSkipping(unsigned int framesToSkip) { std::lock_guard lk(cs_frameSkip); @@ -237,11 +249,13 @@ void SetFrameSkipping(unsigned int framesToSkip) Fifo::SetRendering(true); } +// NOTE: CPU Thread void SetPolledDevice() { s_bPolled = true; } +// NOTE: Host Thread void DoFrameStep() { if (Core::GetState() == Core::CORE_PAUSE) @@ -258,6 +272,7 @@ void DoFrameStep() } } +// NOTE: Host Thread void SetReadOnly(bool bEnabled) { if (s_bReadOnly != bEnabled) @@ -266,6 +281,7 @@ void SetReadOnly(bool bEnabled) s_bReadOnly = bEnabled; } +// NOTE: GPU Thread void FrameSkipping() { // Frameskipping will desync movie playback @@ -399,6 +415,7 @@ bool IsNetPlayRecording() return s_bNetPlay; } +// NOTE: Host / CPU Thread void ChangePads(bool instantly) { if (!Core::IsRunning()) @@ -431,6 +448,7 @@ void ChangePads(bool instantly) } } +// NOTE: Host / Emu Threads void ChangeWiiPads(bool instantly) { int controllers = 0; @@ -450,6 +468,7 @@ void ChangeWiiPads(bool instantly) } } +// NOTE: Host Thread bool BeginRecordingInput(int controllers) { if (s_playMode != MODE_NONE || controllers == 0) @@ -575,46 +594,51 @@ static std::string Analog1DToString(u8 v, const std::string& prefix, u8 range = } } +// NOTE: CPU Thread static void SetInputDisplayString(ControllerState padState, int controllerID) { - s_InputDisplay[controllerID] = StringFromFormat("P%d:", controllerID + 1); + std::string display_str = StringFromFormat("P%d:", controllerID + 1); if (padState.A) - s_InputDisplay[controllerID].append(" A"); + display_str += " A"; if (padState.B) - s_InputDisplay[controllerID].append(" B"); + display_str += " B"; if (padState.X) - s_InputDisplay[controllerID].append(" X"); + display_str += " X"; if (padState.Y) - s_InputDisplay[controllerID].append(" Y"); + display_str += " Y"; if (padState.Z) - s_InputDisplay[controllerID].append(" Z"); + display_str += " Z"; if (padState.Start) - s_InputDisplay[controllerID].append(" START"); + display_str += " START"; if (padState.DPadUp) - s_InputDisplay[controllerID].append(" UP"); + display_str += " UP"; if (padState.DPadDown) - s_InputDisplay[controllerID].append(" DOWN"); + display_str += " DOWN"; if (padState.DPadLeft) - s_InputDisplay[controllerID].append(" LEFT"); + display_str += " LEFT"; if (padState.DPadRight) - s_InputDisplay[controllerID].append(" RIGHT"); + display_str += " RIGHT"; if (padState.reset) - s_InputDisplay[controllerID].append(" RESET"); + display_str += " RESET"; - s_InputDisplay[controllerID].append(Analog1DToString(padState.TriggerL, " L")); - s_InputDisplay[controllerID].append(Analog1DToString(padState.TriggerR, " R")); - s_InputDisplay[controllerID].append(Analog2DToString(padState.AnalogStickX, padState.AnalogStickY, " ANA")); - s_InputDisplay[controllerID].append(Analog2DToString(padState.CStickX, padState.CStickY, " C")); - s_InputDisplay[controllerID].append("\n"); + display_str += Analog1DToString(padState.TriggerL, " L"); + display_str += Analog1DToString(padState.TriggerR, " R"); + display_str += Analog2DToString(padState.AnalogStickX, padState.AnalogStickY, " ANA"); + display_str += Analog2DToString(padState.CStickX, padState.CStickY, " C"); + display_str += '\n'; + + std::lock_guard guard(s_input_display_lock); + s_InputDisplay[controllerID] = std::move(display_str); } +// NOTE: CPU Thread static void SetWiiInputDisplayString(int remoteID, u8* const data, const WiimoteEmu::ReportFeatures& rptf, int ext, const wiimote_key key) { int controllerID = remoteID + 4; - s_InputDisplay[controllerID] = StringFromFormat("R%d:", remoteID + 1); + std::string display_str = StringFromFormat("R%d:", remoteID + 1); u8* const coreData = rptf.core ? (data + rptf.core) : nullptr; u8* const accelData = rptf.accel ? (data + rptf.accel) : nullptr; @@ -625,43 +649,43 @@ static void SetWiiInputDisplayString(int remoteID, u8* const data, const Wiimote { wm_buttons buttons = *(wm_buttons*)coreData; if(buttons.left) - s_InputDisplay[controllerID].append(" LEFT"); + display_str += " LEFT"; if(buttons.right) - s_InputDisplay[controllerID].append(" RIGHT"); + display_str += " RIGHT"; if(buttons.down) - s_InputDisplay[controllerID].append(" DOWN"); + display_str += " DOWN"; if(buttons.up) - s_InputDisplay[controllerID].append(" UP"); + display_str += " UP"; if(buttons.a) - s_InputDisplay[controllerID].append(" A"); + display_str += " A"; if(buttons.b) - s_InputDisplay[controllerID].append(" B"); + display_str += " B"; if(buttons.plus) - s_InputDisplay[controllerID].append(" +"); + display_str += " +"; if(buttons.minus) - s_InputDisplay[controllerID].append(" -"); + display_str += " -"; if(buttons.one) - s_InputDisplay[controllerID].append(" 1"); + display_str += " 1"; if(buttons.two) - s_InputDisplay[controllerID].append(" 2"); + display_str += " 2"; if(buttons.home) - s_InputDisplay[controllerID].append(" HOME"); + display_str += " HOME"; } if (accelData) { wm_accel* dt = (wm_accel*)accelData; - std::string accel = StringFromFormat(" ACC:%d,%d,%d", - dt->x << 2 | ((wm_buttons*)coreData)->acc_x_lsb, dt->y << 2 | ((wm_buttons*)coreData)->acc_y_lsb << 1, dt->z << 2 | ((wm_buttons*)coreData)->acc_z_lsb << 1); - s_InputDisplay[controllerID].append(accel); + display_str += StringFromFormat(" ACC:%d,%d,%d", + dt->x << 2 | ((wm_buttons*)coreData)->acc_x_lsb, + dt->y << 2 | ((wm_buttons*)coreData)->acc_y_lsb << 1, + dt->z << 2 | ((wm_buttons*)coreData)->acc_z_lsb << 1); } if (irData) { u16 x = irData[0] | ((irData[2] >> 4 & 0x3) << 8); u16 y = irData[1] | ((irData[2] >> 6 & 0x3) << 8); - std::string ir = StringFromFormat(" IR:%d,%d", x,y); - s_InputDisplay[controllerID].append(ir); + display_str += StringFromFormat(" IR:%d,%d", x, y); } // Nunchuk @@ -676,11 +700,11 @@ static void SetWiiInputDisplayString(int remoteID, u8* const data, const Wiimote (nunchuk.ax << 2) | nunchuk.bt.acc_x_lsb, (nunchuk.ay << 2) | nunchuk.bt.acc_y_lsb, (nunchuk.az << 2) | nunchuk.bt.acc_z_lsb); if (nunchuk.bt.c) - s_InputDisplay[controllerID].append(" C"); + display_str += " C"; if (nunchuk.bt.z) - s_InputDisplay[controllerID].append(" Z"); - s_InputDisplay[controllerID].append(accel); - s_InputDisplay[controllerID].append(Analog2DToString(nunchuk.jx, nunchuk.jy, " ANA")); + display_str += " Z"; + display_str += accel; + display_str += Analog2DToString(nunchuk.jx, nunchuk.jy, " ANA"); } // Classic controller @@ -692,41 +716,45 @@ static void SetWiiInputDisplayString(int remoteID, u8* const data, const Wiimote cc.bt.hex = cc.bt.hex ^ 0xFFFF; if (cc.bt.regular_data.dpad_left) - s_InputDisplay[controllerID].append(" LEFT"); + display_str += " LEFT"; if (cc.bt.dpad_right) - s_InputDisplay[controllerID].append(" RIGHT"); + display_str += " RIGHT"; if (cc.bt.dpad_down) - s_InputDisplay[controllerID].append(" DOWN"); + display_str += " DOWN"; if (cc.bt.regular_data.dpad_up) - s_InputDisplay[controllerID].append(" UP"); + display_str += " UP"; if (cc.bt.a) - s_InputDisplay[controllerID].append(" A"); + display_str += " A"; if (cc.bt.b) - s_InputDisplay[controllerID].append(" B"); + display_str += " B"; if (cc.bt.x) - s_InputDisplay[controllerID].append(" X"); + display_str += " X"; if (cc.bt.y) - s_InputDisplay[controllerID].append(" Y"); + display_str += " Y"; if (cc.bt.zl) - s_InputDisplay[controllerID].append(" ZL"); + display_str += " ZL"; if (cc.bt.zr) - s_InputDisplay[controllerID].append(" ZR"); + display_str += " ZR"; if (cc.bt.plus) - s_InputDisplay[controllerID].append(" +"); + display_str += " +"; if (cc.bt.minus) - s_InputDisplay[controllerID].append(" -"); + display_str += " -"; if (cc.bt.home) - s_InputDisplay[controllerID].append(" HOME"); + display_str += " HOME"; - s_InputDisplay[controllerID].append(Analog1DToString(cc.lt1 | (cc.lt2 << 3), " L", 31)); - s_InputDisplay[controllerID].append(Analog1DToString(cc.rt, " R", 31)); - s_InputDisplay[controllerID].append(Analog2DToString(cc.regular_data.lx, cc.regular_data.ly, " ANA", 63)); - s_InputDisplay[controllerID].append(Analog2DToString(cc.rx1 | (cc.rx2 << 1) | (cc.rx3 << 3), cc.ry, " R-ANA", 31)); + display_str += Analog1DToString(cc.lt1 | (cc.lt2 << 3), " L", 31); + display_str += Analog1DToString(cc.rt, " R", 31); + display_str += Analog2DToString(cc.regular_data.lx, cc.regular_data.ly, " ANA", 63); + display_str += Analog2DToString(cc.rx1 | (cc.rx2 << 1) | (cc.rx3 << 3), cc.ry, " R-ANA", 31); } - s_InputDisplay[controllerID].append("\n"); + display_str += '\n'; + + std::lock_guard guard(s_input_display_lock); + s_InputDisplay[controllerID] = std::move(display_str); } +// NOTE: CPU Thread void CheckPadStatus(GCPadStatus* PadStatus, int controllerID) { s_padState.A = ((PadStatus->button & PAD_BUTTON_A) != 0); @@ -760,6 +788,7 @@ void CheckPadStatus(GCPadStatus* PadStatus, int controllerID) SetInputDisplayString(s_padState, controllerID); } +// NOTE: CPU Thread void RecordInput(GCPadStatus* PadStatus, int controllerID) { if (!IsRecordingInput() || !IsUsingPad(controllerID)) @@ -773,6 +802,7 @@ void RecordInput(GCPadStatus* PadStatus, int controllerID) s_totalBytes = s_currentByte; } +// NOTE: CPU Thread void CheckWiimoteStatus(int wiimote, u8 *data, const WiimoteEmu::ReportFeatures& rptf, int ext, const wiimote_key key) { SetWiiInputDisplayString(wiimote, data, rptf, ext, key); @@ -794,6 +824,7 @@ void RecordWiimote(int wiimote, u8 *data, u8 size) s_totalBytes = s_currentByte; } +// NOTE: CPU / EmuThread / Host Thread void ReadHeader() { s_numPads = tmpHeader.numControllers; @@ -832,6 +863,7 @@ void ReadHeader() s_DSPcoefHash = tmpHeader.DSPcoefHash; } +// NOTE: Host Thread bool PlayInput(const std::string& filename) { if (s_playMode != MODE_NONE) @@ -902,6 +934,7 @@ void DoState(PointerWrap &p) // other variables (such as s_totalBytes and g_totalFrames) are set in LoadInput } +// NOTE: Host / CPU Thread void LoadInput(const std::string& filename) { File::IOFile t_record; @@ -1039,6 +1072,7 @@ void LoadInput(const std::string& filename) } } +// NOTE: CPU Thread static void CheckInputEnd() { if (g_currentFrame > g_totalFrames || s_currentByte >= s_totalBytes || (CoreTiming::GetTicks() > s_totalTickCount && !IsRecordingInputFromSaveState())) @@ -1047,6 +1081,7 @@ static void CheckInputEnd() } } +// NOTE: CPU Thread void PlayController(GCPadStatus* PadStatus, int controllerID) { // Correct playback is entirely dependent on the emulator polling the controllers @@ -1147,6 +1182,7 @@ void PlayController(GCPadStatus* PadStatus, int controllerID) CheckInputEnd(); } +// NOTE: CPU Thread bool PlayWiimote(int wiimote, u8 *data, const WiimoteEmu::ReportFeatures& rptf, int ext, const wiimote_key key) { if (!IsPlayingInput() || !IsUsingWiimote(wiimote) || tmpInput == nullptr) @@ -1189,6 +1225,7 @@ bool PlayWiimote(int wiimote, u8 *data, const WiimoteEmu::ReportFeatures& rptf, return true; } +// NOTE: Host / EmuThread / CPU Thread void EndPlayInput(bool cont) { if (cont) @@ -1214,6 +1251,7 @@ void EndPlayInput(bool cont) } } +// NOTE: Save State + Host Thread void SaveRecording(const std::string& filename) { File::IOFile save_record(filename, "wb"); @@ -1292,17 +1330,20 @@ void SetWiiInputManip(WiiManipFunction func) wiimfunc = func; } +// NOTE: CPU Thread void CallGCInputManip(GCPadStatus* PadStatus, int controllerID) { if (gcmfunc) (*gcmfunc)(PadStatus, controllerID); } +// NOTE: CPU Thread void CallWiiInputManip(u8* data, WiimoteEmu::ReportFeatures rptf, int controllerID, int ext, const wiimote_key key) { if (wiimfunc) (*wiimfunc)(data, rptf, controllerID, ext, key); } +// NOTE: GPU Thread void SetGraphicsConfig() { g_Config.bEFBAccessEnable = tmpHeader.bEFBAccessEnable; @@ -1312,6 +1353,7 @@ void SetGraphicsConfig() g_Config.bUseRealXFB = tmpHeader.bUseRealXFB; } +// NOTE: CPU / EmuThread / Host Thread void GetSettings() { s_bSaveConfig = true; @@ -1372,6 +1414,7 @@ void GetSettings() static const mbedtls_md_info_t* s_md5_info = mbedtls_md_info_from_type(MBEDTLS_MD_MD5); +// NOTE: Entrypoint for own thread void CheckMD5() { for (int i = 0, n = 0; i < 16; ++i) @@ -1393,6 +1436,7 @@ void CheckMD5() Core::DisplayMessage("Checksum of current game does not match the recorded game!", 3000); } +// NOTE: Entrypoint for own thread void GetMD5() { Core::DisplayMessage("Calculating checksum of game file...", 2000); @@ -1401,6 +1445,7 @@ void GetMD5() Core::DisplayMessage("Finished calculating checksum.", 2000); } +// NOTE: EmuThread void Shutdown() { g_currentInputCount = g_totalInputCount = g_totalFrames = s_totalBytes = s_tickCountAtLastInput = 0; diff --git a/Source/Core/DolphinWX/TASInputDlg.cpp b/Source/Core/DolphinWX/TASInputDlg.cpp index 814be18d29..9c3d8c1f33 100644 --- a/Source/Core/DolphinWX/TASInputDlg.cpp +++ b/Source/Core/DolphinWX/TASInputDlg.cpp @@ -29,6 +29,7 @@ wxDEFINE_EVENT(INVALIDATE_BUTTON_EVENT, wxCommandEvent); wxDEFINE_EVENT(INVALIDATE_CONTROL_EVENT, wxCommandEvent); +wxDEFINE_EVENT(INVALIDATE_EXTENSION_EVENT, wxThreadEvent); struct TASWiimoteReport { @@ -63,11 +64,17 @@ void TASInputDlg::CreateBaseLayout() m_controls[1] = &m_main_stick.y_cont; m_a = CreateButton("A"); + m_a.checkbox->SetClientData(&m_a); m_b = CreateButton("B"); + m_b.checkbox->SetClientData(&m_b); m_dpad_up = CreateButton("Up"); + m_dpad_up.checkbox->SetClientData(&m_dpad_up); m_dpad_right = CreateButton("Right"); + m_dpad_right.checkbox->SetClientData(&m_dpad_right); m_dpad_down = CreateButton("Down"); + m_dpad_down.checkbox->SetClientData(&m_dpad_down); m_dpad_left = CreateButton("Left"); + m_dpad_left.checkbox->SetClientData(&m_dpad_left); m_buttons_dpad = new wxGridSizer(3); m_buttons_dpad->AddSpacer(20); @@ -134,10 +141,15 @@ void TASInputDlg::CreateWiiLayout(int num) wxGridSizer* const m_buttons_grid = new wxGridSizer(4); m_plus = CreateButton("+"); + m_plus.checkbox->SetClientData(&m_plus); m_minus = CreateButton("-"); + m_minus.checkbox->SetClientData(&m_minus); m_one = CreateButton("1"); + m_one.checkbox->SetClientData(&m_one); m_two = CreateButton("2"); + m_two.checkbox->SetClientData(&m_two); m_home = CreateButton("Home"); + m_home.checkbox->SetClientData(&m_home); m_main_szr = new wxBoxSizer(wxVERTICAL); m_wiimote_szr = new wxBoxSizer(wxHORIZONTAL); @@ -178,7 +190,9 @@ void TASInputDlg::CreateWiiLayout(int num) wxStaticBoxSizer* const nunchukaxisBox = CreateAccelLayout(&m_nx_cont, &m_ny_cont, &m_nz_cont, _("Nunchuk orientation")); m_c = CreateButton("C"); + m_c.checkbox->SetClientData(&m_c); m_z = CreateButton("Z"); + m_z.checkbox->SetClientData(&m_z); m_ext_szr->Add(m_c_stick_szr, 0, wxLEFT | wxBOTTOM | wxRIGHT, 5); m_ext_szr->Add(nunchukaxisBox); @@ -212,6 +226,7 @@ void TASInputDlg::FinishLayout() Bind(wxEVT_CLOSE_WINDOW, &TASInputDlg::OnCloseWindow, this); Bind(INVALIDATE_BUTTON_EVENT, &TASInputDlg::UpdateFromInvalidatedButton, this); Bind(INVALIDATE_CONTROL_EVENT, &TASInputDlg::UpdateFromInvalidatedControl, this); + Bind(INVALIDATE_EXTENSION_EVENT, &TASInputDlg::UpdateFromInvalidatedExtension, this); m_has_layout = true; } @@ -220,7 +235,10 @@ wxBoxSizer* TASInputDlg::CreateCCLayout() wxBoxSizer* const szr = new wxBoxSizer(wxHORIZONTAL); for (size_t i = 0; i < ArraySize(m_cc_buttons); ++i) + { m_cc_buttons[i] = CreateButton(m_cc_button_names[i]); + m_cc_buttons[i].checkbox->SetClientData(&m_cc_buttons[i]); + } m_cc_l_stick = CreateStick(ID_CC_L_STICK, 63, 63, WiimoteEmu::Classic::LEFT_STICK_CENTER_X, WiimoteEmu::Classic::LEFT_STICK_CENTER_Y, false, true); m_cc_r_stick = CreateStick(ID_CC_R_STICK, 31, 31, WiimoteEmu::Classic::RIGHT_STICK_CENTER_X, WiimoteEmu::Classic::RIGHT_STICK_CENTER_Y, false, true); @@ -348,11 +366,17 @@ void TASInputDlg::CreateGCLayout() wxGridSizer* const m_buttons_grid = new wxGridSizer(4); m_x = CreateButton("X"); + m_x.checkbox->SetClientData(&m_x); m_y = CreateButton("Y"); + m_y.checkbox->SetClientData(&m_y); m_l = CreateButton("L"); + m_l.checkbox->SetClientData(&m_l); m_r = CreateButton("R"); + m_r.checkbox->SetClientData(&m_r); m_z = CreateButton("Z"); + m_z.checkbox->SetClientData(&m_z); m_start = CreateButton("Start"); + m_start.checkbox->SetClientData(&m_start); for (unsigned int i = 4; i < ArraySize(m_buttons); ++i) if (m_buttons[i] != nullptr) @@ -449,11 +473,18 @@ TASInputDlg::Button TASInputDlg::CreateButton(const std::string& name) wxCheckBox* checkbox = new wxCheckBox(this, m_eleID++, name); checkbox->Bind(wxEVT_RIGHT_DOWN, &TASInputDlg::SetTurbo, this); checkbox->Bind(wxEVT_LEFT_DOWN, &TASInputDlg::SetTurbo, this); + checkbox->Bind(wxEVT_CHECKBOX, &TASInputDlg::OnCheckboxToggle, this); temp.checkbox = checkbox; temp.id = m_eleID - 1; return temp; } +void TASInputDlg::OnCheckboxToggle(wxCommandEvent& event) +{ + auto cbox = static_cast(event.GetEventObject()); + static_cast(cbox->GetClientData())->is_checked = event.IsChecked(); +} + void TASInputDlg::ResetValues() { for (Button* const button : m_buttons) @@ -491,6 +522,7 @@ void TASInputDlg::ResetValues() } } +// NOTE: Host / CPU Thread void TASInputDlg::SetStickValue(Control* control, int CurrentValue, int center) { if (CurrentValue != center) @@ -511,6 +543,7 @@ void TASInputDlg::SetStickValue(Control* control, int CurrentValue, int center) InvalidateControl(control); } +// NOTE: Host / CPU Thread void TASInputDlg::SetSliderValue(Control* control, int CurrentValue) { if (CurrentValue != (int)control->default_value) @@ -531,6 +564,7 @@ void TASInputDlg::SetSliderValue(Control* control, int CurrentValue) InvalidateControl(control); } +// NOTE: Host / CPU Thread void TASInputDlg::SetButtonValue(Button* button, bool CurrentState) { if (CurrentState) @@ -550,16 +584,18 @@ void TASInputDlg::SetButtonValue(Button* button, bool CurrentState) InvalidateButton(button); } +// NOTE: Host / CPU Thread void TASInputDlg::SetWiiButtons(u16* butt) { for (unsigned int i = 0; i < 11; ++i) { if (m_buttons[i] != nullptr) - *butt |= (m_buttons[i]->checkbox->IsChecked()) ? m_wii_buttons_bitmask[i] : 0; + *butt |= (m_buttons[i]->is_checked) ? m_wii_buttons_bitmask[i] : 0; } ButtonTurbo(); } +// NOTE: Host / CPU Thread void TASInputDlg::GetKeyBoardInput(GCPadStatus* PadStatus) { SetStickValue(&m_main_stick.x_cont, PadStatus->stickX); @@ -579,6 +615,7 @@ void TASInputDlg::GetKeyBoardInput(GCPadStatus* PadStatus) SetButtonValue(&m_r, ((PadStatus->triggerRight) == 255) || ((PadStatus->button & PAD_TRIGGER_R) != 0)); } +// NOTE: Host / CPU Thread void TASInputDlg::GetKeyBoardInput(u8* data, WiimoteEmu::ReportFeatures rptf, int ext, const wiimote_key key) { u8* const coreData = rptf.core ? (data + rptf.core) : nullptr; @@ -651,6 +688,9 @@ void TASInputDlg::GetKeyBoardInput(u8* data, WiimoteEmu::ReportFeatures rptf, in } } +// NOTE: Host / CPU Thread +// Do not touch the GUI. Requires wxMutexGuiEnter which will deadlock against +// the GUI when pausing/stopping. void TASInputDlg::GetValues(u8* data, WiimoteEmu::ReportFeatures rptf, int ext, const wiimote_key key) { if (!IsShown() || !m_has_layout) @@ -743,7 +783,7 @@ void TASInputDlg::GetValues(u8* data, WiimoteEmu::ReportFeatures rptf, int ext, if (ext != m_ext) { m_ext = ext; - HandleExtensionChange(); + InvalidateExtension(); } else if (extData && ext == 1) { @@ -759,8 +799,8 @@ void TASInputDlg::GetValues(u8* data, WiimoteEmu::ReportFeatures rptf, int ext, nunchuk.az = m_nz_cont.value >> 2; nunchuk.bt.acc_z_lsb = m_nz_cont.value & 0x3; - nunchuk.bt.hex |= (m_buttons[11]->checkbox->IsChecked()) ? WiimoteEmu::Nunchuk::BUTTON_C : 0; - nunchuk.bt.hex |= (m_buttons[12]->checkbox->IsChecked()) ? WiimoteEmu::Nunchuk::BUTTON_Z : 0; + nunchuk.bt.hex |= (m_buttons[11]->is_checked) ? WiimoteEmu::Nunchuk::BUTTON_C : 0; + nunchuk.bt.hex |= (m_buttons[12]->is_checked) ? WiimoteEmu::Nunchuk::BUTTON_Z : 0; nunchuk.bt.hex = nunchuk.bt.hex ^ 0x3; WiimoteEncrypt(&key, (u8*)&nunchuk, 0, sizeof(wm_nc)); } @@ -772,7 +812,7 @@ void TASInputDlg::GetValues(u8* data, WiimoteEmu::ReportFeatures rptf, int ext, for (unsigned int i = 0; i < ArraySize(m_cc_buttons); ++i) { - cc.bt.hex |= (m_cc_buttons[i].checkbox->IsChecked()) ? m_cc_buttons_bitmask[i] : 0; + cc.bt.hex |= (m_cc_buttons[i].is_checked) ? m_cc_buttons_bitmask[i] : 0; } cc.bt.hex ^= 0xFFFF; @@ -793,6 +833,7 @@ void TASInputDlg::GetValues(u8* data, WiimoteEmu::ReportFeatures rptf, int ext, } } +// NOTE: Host / CPU Thread void TASInputDlg::GetValues(GCPadStatus* PadStatus) { if (!IsShown() || !m_has_layout) @@ -805,26 +846,26 @@ void TASInputDlg::GetValues(GCPadStatus* PadStatus) PadStatus->stickY = m_main_stick.y_cont.value; PadStatus->substickX = m_c_stick.x_cont.value; PadStatus->substickY = m_c_stick.y_cont.value; - PadStatus->triggerLeft = m_l.checkbox->GetValue() ? 255 : m_l_cont.value; - PadStatus->triggerRight = m_r.checkbox->GetValue() ? 255 : m_r_cont.value; + PadStatus->triggerLeft = m_l.is_checked ? 255 : m_l_cont.value; + PadStatus->triggerRight = m_r.is_checked ? 255 : m_r_cont.value; for (unsigned int i = 0; i < ArraySize(m_buttons); ++i) { if (m_buttons[i] != nullptr) { - if (m_buttons[i]->checkbox->IsChecked()) + if (m_buttons[i]->is_checked) PadStatus->button |= m_gc_pad_buttons_bitmask[i]; else PadStatus->button &= ~m_gc_pad_buttons_bitmask[i]; } } - if (m_a.checkbox->IsChecked()) + if (m_a.is_checked) PadStatus->analogA = 0xFF; else PadStatus->analogA = 0x00; - if (m_b.checkbox->IsChecked()) + if (m_b.is_checked) PadStatus->analogB = 0xFF; else PadStatus->analogB = 0x00; @@ -1035,6 +1076,7 @@ void TASInputDlg::SetTurbo(wxMouseEvent& event) event.Skip(); } +// NOTE: Host / CPU Thread void TASInputDlg::ButtonTurbo() { static u64 frame = Movie::g_currentFrame; @@ -1046,7 +1088,7 @@ void TASInputDlg::ButtonTurbo() { if (button != nullptr && button->turbo_on) { - button->value = !button->checkbox->GetValue(); + button->value = !button->is_checked; InvalidateButton(button); } } @@ -1056,7 +1098,7 @@ void TASInputDlg::ButtonTurbo() { if (button.turbo_on) { - button.value = !button.checkbox->GetValue(); + button.value = !button.is_checked; InvalidateButton(&button); } } @@ -1075,6 +1117,7 @@ void TASInputDlg::InvalidateButton(Button* button) } button->checkbox->SetValue(button->value); + button->is_checked = button->value; } void TASInputDlg::InvalidateControl(Control* control) @@ -1090,11 +1133,23 @@ void TASInputDlg::InvalidateControl(Control* control) control->text->SetValue(std::to_string(control->value)); } +void TASInputDlg::InvalidateExtension() +{ + if (!wxIsMainThread()) + { + GetEventHandler()->QueueEvent(new wxThreadEvent(INVALIDATE_EXTENSION_EVENT)); + return; + } + + HandleExtensionChange(); +} + void TASInputDlg::UpdateFromInvalidatedButton(wxCommandEvent& event) { Button* button = static_cast(event.GetClientData()); _assert_msg_(PAD, button->id == button->checkbox->GetId(), "Button ids do not match: %i != %i", button->id, button->checkbox->GetId()); button->checkbox->SetValue(button->value); + button->is_checked = button->value; } void TASInputDlg::UpdateFromInvalidatedControl(wxCommandEvent& event) @@ -1104,6 +1159,11 @@ void TASInputDlg::UpdateFromInvalidatedControl(wxCommandEvent& event) control->text->SetValue(std::to_string(control->value)); } +void TASInputDlg::UpdateFromInvalidatedExtension(wxThreadEvent&) +{ + HandleExtensionChange(); +} + wxBitmap TASInputDlg::CreateStickBitmap(int x, int y) { x = x / 2; diff --git a/Source/Core/DolphinWX/TASInputDlg.h b/Source/Core/DolphinWX/TASInputDlg.h index ba003fddf7..f7e4996374 100644 --- a/Source/Core/DolphinWX/TASInputDlg.h +++ b/Source/Core/DolphinWX/TASInputDlg.h @@ -70,6 +70,7 @@ class TASInputDlg : public wxDialog struct Button { wxCheckBox* checkbox; + bool is_checked = false; bool value = false; bool set_by_keyboard = false; bool turbo_on = false; @@ -92,8 +93,11 @@ class TASInputDlg : public wxDialog void UpdateStickBitmap(Stick stick); void InvalidateButton(Button* button); void InvalidateControl(Control* button); + void InvalidateExtension(); void UpdateFromInvalidatedButton(wxCommandEvent& event); void UpdateFromInvalidatedControl(wxCommandEvent& event); + void UpdateFromInvalidatedExtension(wxThreadEvent& event); + void OnCheckboxToggle(wxCommandEvent& event); Stick* FindStickByID(int id); Stick CreateStick(int id_stick, int xRange, int yRange, u32 defaultX, u32 defaultY, bool reverseX, bool reverseY); wxStaticBoxSizer* CreateStickLayout(Stick* tempStick, const wxString& title); From c1944f623b5918ab45152795dea9e49f57878138 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Thu, 12 May 2016 01:18:30 +0000 Subject: [PATCH 3/3] Core/Movie: Add ability to run code in Host context EndPlayInput runs on the CPU thread so it can't directly call UpdateWantDeterminism. PlayController also tries to ChangeDisc from the CPU Thread which is also invalid. It now just pauses execution and posts a request to the Host to fix it instead. The Core itself also did dodgy things like PauseAndLock-ing from the CPU Thread and SetState from EmuThread which have been removed. --- Source/Android/jni/MainAndroid.cpp | 28 +++++++- Source/Core/Common/Common.h | 1 + Source/Core/Core/Core.cpp | 104 +++++++++++++++++++++++++-- Source/Core/Core/Core.h | 16 +++++ Source/Core/Core/HW/DVDInterface.cpp | 7 +- Source/Core/Core/HW/DVDInterface.h | 2 +- Source/Core/Core/Movie.cpp | 35 ++++++--- Source/Core/DolphinQt2/Host.cpp | 10 +++ Source/Core/DolphinQt2/Main.cpp | 7 ++ Source/Core/DolphinWX/Main.cpp | 13 ++++ Source/Core/DolphinWX/Main.h | 1 + Source/Core/DolphinWX/MainNoGUI.cpp | 33 +++++++-- 12 files changed, 227 insertions(+), 30 deletions(-) diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp index cd96fe46a2..6b30bc5409 100644 --- a/Source/Android/jni/MainAndroid.cpp +++ b/Source/Android/jni/MainAndroid.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -65,13 +66,23 @@ jint JNI_OnLoad(JavaVM* vm, void* reserved) void Host_NotifyMapLoaded() {} void Host_RefreshDSPDebuggerWindow() {} +// The Core only supports using a single Host thread. +// If multiple threads want to call host functions then they need to queue +// sequentially for access. +static std::mutex s_host_identity_lock; Common::Event updateMainFrameEvent; static bool s_have_wm_user_stop = false; void Host_Message(int Id) { - if (Id == WM_USER_STOP) + if (Id == WM_USER_JOB_DISPATCH) + { + updateMainFrameEvent.Set(); + } + else if (Id == WM_USER_STOP) { s_have_wm_user_stop = true; + if (Core::IsRunning()) + Core::QueueHostJob(&Core::Stop); } } @@ -393,15 +404,18 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SurfaceDestr JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_UnPauseEmulation(JNIEnv *env, jobject obj) { + std::lock_guard guard(s_host_identity_lock); Core::SetState(Core::CORE_RUN); } JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_PauseEmulation(JNIEnv *env, jobject obj) { + std::lock_guard guard(s_host_identity_lock); Core::SetState(Core::CORE_PAUSE); } JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_StopEmulation(JNIEnv *env, jobject obj) { + std::lock_guard guard(s_host_identity_lock); Core::SaveScreenShot("thumb"); Renderer::s_screenshotCompleted.WaitFor(std::chrono::seconds(2)); Core::Stop(); @@ -490,6 +504,7 @@ JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Supports JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveScreenShot(JNIEnv *env, jobject obj) { + std::lock_guard guard(s_host_identity_lock); Core::SaveScreenShot(); } @@ -534,11 +549,13 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetFilename( JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveState(JNIEnv *env, jobject obj, jint slot) { + std::lock_guard guard(s_host_identity_lock); State::Save(slot); } JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_LoadState(JNIEnv *env, jobject obj, jint slot) { + std::lock_guard guard(s_host_identity_lock); State::Load(slot); } @@ -565,6 +582,7 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_CreateUserFo JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetUserDirectory(JNIEnv *env, jobject obj, jstring jDirectory) { + std::lock_guard guard(s_host_identity_lock); std::string directory = GetJString(env, jDirectory); g_set_userpath = directory; UICommon::SetUserDirectory(directory); @@ -577,6 +595,7 @@ JNIEXPORT jstring JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_GetUserDi JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetProfiling(JNIEnv *env, jobject obj, jboolean enable) { + std::lock_guard guard(s_host_identity_lock); Core::SetState(Core::CORE_PAUSE); JitInterface::ClearCache(); Profiler::g_ProfileBlocks = enable; @@ -585,6 +604,7 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetProfiling JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_WriteProfileResults(JNIEnv *env, jobject obj) { + std::lock_guard guard(s_host_identity_lock); std::string filename = File::GetUserPath(D_DUMP_IDX) + "Debug/profiler.txt"; File::CreateFullPath(filename); JitInterface::WriteProfileResults(filename); @@ -643,6 +663,7 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SurfaceDestr } JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_RefreshWiimotes(JNIEnv *env, jobject obj) { + std::lock_guard guard(s_host_identity_lock); WiimoteReal::Refresh(); } @@ -656,6 +677,7 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Run(JNIEnv * RegisterMsgAlertHandler(&MsgAlert); + std::unique_lock guard(s_host_identity_lock); UICommon::SetUserDirectory(g_set_userpath); UICommon::Init(); @@ -676,12 +698,16 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Run(JNIEnv * } while (Core::IsRunning()) { + guard.unlock(); updateMainFrameEvent.Wait(); + guard.lock(); + Core::HostDispatchJobs(); } } Core::Shutdown(); UICommon::Shutdown(); + guard.unlock(); if (surf) { diff --git a/Source/Core/Common/Common.h b/Source/Core/Common/Common.h index 36a1bc13bf..7f8e6ce639 100644 --- a/Source/Core/Common/Common.h +++ b/Source/Core/Common/Common.h @@ -83,6 +83,7 @@ enum HOST_COMM WM_USER_STOP = 10, WM_USER_CREATE, WM_USER_SETCURSOR, + WM_USER_JOB_DISPATCH, }; // Used for notification on emulation state diff --git a/Source/Core/Core/Core.cpp b/Source/Core/Core/Core.cpp index b5fbf14663..ab888de322 100644 --- a/Source/Core/Core/Core.cpp +++ b/Source/Core/Core/Core.cpp @@ -5,6 +5,9 @@ #include #include #include +#include +#include +#include #ifdef _WIN32 #include @@ -102,6 +105,7 @@ void EmuThread(); static bool s_is_stopping = false; static bool s_hardware_initialized = false; static bool s_is_started = false; +static std::atomic s_is_booting{ false }; static void* s_window_handle = nullptr; static std::string s_state_filename; static std::thread s_emu_thread; @@ -112,6 +116,14 @@ static bool s_request_refresh_info = false; static int s_pause_and_lock_depth = 0; static bool s_is_throttler_temp_disabled = false; +struct HostJob +{ + std::function job; + bool run_after_stop; +}; +static std::mutex s_host_jobs_lock; +static std::queue s_host_jobs_queue; + #ifdef ThreadLocalStorage static ThreadLocalStorage bool tls_is_cpu_thread = false; #else @@ -225,6 +237,9 @@ bool Init() s_emu_thread.join(); } + // Drain any left over jobs + HostDispatchJobs(); + Core::UpdateWantDeterminism(/*initial*/ true); INFO_LOG(OSREPORT, "Starting core = %s mode", @@ -260,6 +275,9 @@ void Stop() // - Hammertime! s_is_stopping = true; + // Dump left over jobs + HostDispatchJobs(); + Fifo::EmulatorState(false); INFO_LOG(CONSOLE, "Stop [Main Thread]\t\t---- Shutting down ----"); @@ -310,6 +328,16 @@ void UndeclareAsCPUThread() #endif } +// For the CPU Thread only. +static void CPUSetInitialExecutionState() +{ + QueueHostJob([] + { + SetState(SConfig::GetInstance().bBootToPause ? CORE_PAUSE : CORE_RUN); + Host_UpdateMainFrame(); + }); +} + // Create the CPU thread, which is a CPU + Video thread in Single Core mode. static void CpuThread() { @@ -331,10 +359,20 @@ static void CpuThread() EMM::InstallExceptionHandler(); // Let's run under memory watch if (!s_state_filename.empty()) - State::LoadAs(s_state_filename); + { + // Needs to PauseAndLock the Core + // NOTE: EmuThread should have left us in CPU_STEPPING so nothing will happen + // until after the job is serviced. + QueueHostJob([] + { + // Recheck in case Movie cleared it since. + if (!s_state_filename.empty()) + State::LoadAs(s_state_filename); + }); + } s_is_started = true; - + CPUSetInitialExecutionState(); #ifdef USE_GDBSTUB #ifndef _WIN32 @@ -393,7 +431,10 @@ static void FifoPlayerThread() { PowerPC::InjectExternalCPUCore(cpu_core.get()); s_is_started = true; + + CPUSetInitialExecutionState(); CPU::Run(); + s_is_started = false; PowerPC::InjectExternalCPUCore(nullptr); } @@ -427,6 +468,7 @@ static void FifoPlayerThread() void EmuThread() { const SConfig& core_parameter = SConfig::GetInstance(); + s_is_booting.store(true); Common::SetCurrentThreadName("Emuthread - Starting"); @@ -445,6 +487,7 @@ void EmuThread() if (!g_video_backend->Initialize(s_window_handle)) { + s_is_booting.store(false); PanicAlert("Failed to initialize video backend!"); Host_Message(WM_USER_STOP); return; @@ -459,6 +502,7 @@ void EmuThread() if (!DSP::GetDSPEmulator()->Initialize(core_parameter.bWii, core_parameter.bDSPThread)) { + s_is_booting.store(false); HW::Shutdown(); g_video_backend->Shutdown(); PanicAlert("Failed to initialize DSP emulation!"); @@ -499,12 +543,10 @@ void EmuThread() // The hardware is initialized. s_hardware_initialized = true; + s_is_booting.store(false); - // Boot to pause or not - // NOTE: This violates the Host Thread requirement for SetState but we should - // not race the Host because the UI should have the buttons disabled until - // Host_UpdateMainFrame enables them. - Core::SetState(core_parameter.bBootToPause ? Core::CORE_PAUSE : Core::CORE_RUN); + // Set execution state to known values (CPU/FIFO/Audio Paused) + CPU::Break(); // Load GCM/DOL/ELF whatever ... we boot with the interpreter core PowerPC::SetMode(PowerPC::MODE_INTERPRETER); @@ -640,6 +682,10 @@ void EmuThread() void SetState(EState state) { + // State cannot be controlled until the CPU Thread is operational + if (!IsRunningAndStarted()) + return; + switch (state) { case CORE_PAUSE: @@ -904,6 +950,9 @@ void Shutdown() // on MSDN. if (s_emu_thread.joinable()) s_emu_thread.join(); + + // Make sure there's nothing left over in case we're about to exit. + HostDispatchJobs(); } void SetOnStoppedCallback(StoppedCallbackFunc callback) @@ -937,4 +986,45 @@ void UpdateWantDeterminism(bool initial) } } +void QueueHostJob(std::function job, bool run_during_stop) +{ + if (!job) + return; + + bool send_message = false; + { + std::lock_guard guard(s_host_jobs_lock); + send_message = s_host_jobs_queue.empty(); + s_host_jobs_queue.emplace(HostJob{ std::move(job), run_during_stop }); + } + // If the the queue was empty then kick the Host to come and get this job. + if (send_message) + Host_Message(WM_USER_JOB_DISPATCH); +} + +void HostDispatchJobs() +{ + // WARNING: This should only run on the Host Thread. + // NOTE: This function is potentially re-entrant. If a job calls + // Core::Stop for instance then we'll enter this a second time. + std::unique_lock guard(s_host_jobs_lock); + while (!s_host_jobs_queue.empty()) + { + HostJob job = std::move(s_host_jobs_queue.front()); + s_host_jobs_queue.pop(); + + // NOTE: Memory ordering is important. The booting flag needs to be + // checked first because the state transition is: + // CORE_UNINITIALIZED: s_is_booting -> s_hardware_initialized + // We need to check variables in the same order as the state + // transition, otherwise we race and get transient failures. + if (!job.run_after_stop && !s_is_booting.load() && !IsRunning()) + continue; + + guard.unlock(); + job.job(); + guard.lock(); + } +} + } // Core diff --git a/Source/Core/Core/Core.h b/Source/Core/Core/Core.h index b5de60fd5e..f09f89bce2 100644 --- a/Source/Core/Core/Core.h +++ b/Source/Core/Core/Core.h @@ -11,6 +11,7 @@ #pragma once +#include #include #include @@ -91,4 +92,19 @@ void SetOnStoppedCallback(StoppedCallbackFunc callback); // Run on the Host thread when the factors change. [NOT THREADSAFE] void UpdateWantDeterminism(bool initial = false); +// Queue an arbitrary function to asynchronously run once on the Host thread later. +// Threadsafe. Can be called by any thread, including the Host itself. +// Jobs will be executed in RELATIVE order. If you queue 2 jobs from the same thread +// then they will be executed in the order they were queued; however, there is no +// global order guarantee across threads - jobs from other threads may execute in +// between. +// NOTE: Make sure the jobs check the global state instead of assuming everything is +// still the same as when the job was queued. +// NOTE: Jobs that are not set to run during stop will be discarded instead. +void QueueHostJob(std::function job, bool run_during_stop = false); + +// Should be called periodically by the Host to run pending jobs. +// WM_USER_JOB_DISPATCH will be sent when something is added to the queue. +void HostDispatchJobs(); + } // namespace diff --git a/Source/Core/Core/HW/DVDInterface.cpp b/Source/Core/Core/HW/DVDInterface.cpp index 2276150d27..c3626cd428 100644 --- a/Source/Core/Core/HW/DVDInterface.cpp +++ b/Source/Core/Core/HW/DVDInterface.cpp @@ -478,8 +478,8 @@ static void InsertDiscCallback(u64 userdata, s64 cyclesLate) void ChangeDisc(const std::string& newFileName) { - bool is_cpu = Core::IsCPUThread(); - bool was_unpaused = is_cpu ? false : Core::PauseAndLock(true); + // WARNING: Can only run on Host Thread + bool was_unpaused = Core::PauseAndLock(true); std::string* _FileName = new std::string(newFileName); CoreTiming::ScheduleEvent(0, s_eject_disc); CoreTiming::ScheduleEvent(500000000, s_insert_disc, (u64)_FileName); @@ -495,8 +495,7 @@ void ChangeDisc(const std::string& newFileName) } Movie::g_discChange = fileName.substr(sizeofpath); } - if (!is_cpu) - Core::PauseAndLock(false, was_unpaused); + Core::PauseAndLock(false, was_unpaused); } void SetLidOpen(bool open) diff --git a/Source/Core/Core/HW/DVDInterface.h b/Source/Core/Core/HW/DVDInterface.h index 8c26b3b7c8..96b065132e 100644 --- a/Source/Core/Core/HW/DVDInterface.h +++ b/Source/Core/Core/HW/DVDInterface.h @@ -102,7 +102,7 @@ bool VolumeIsValid(); // Disc detection and swapping void SetDiscInside(bool _DiscInside); bool IsDiscInside(); -void ChangeDisc(const std::string& fileName); +void ChangeDisc(const std::string& fileName); // [NOT THREADSAFE] Host only // DVD Access Functions bool ChangePartition(u64 offset); diff --git a/Source/Core/Core/Movie.cpp b/Source/Core/Core/Movie.cpp index 35bbae2762..176ef5b445 100644 --- a/Source/Core/Core/Movie.cpp +++ b/Source/Core/Core/Movie.cpp @@ -415,7 +415,7 @@ bool IsNetPlayRecording() return s_bNetPlay; } -// NOTE: Host / CPU Thread +// NOTE: Host Thread void ChangePads(bool instantly) { if (!Core::IsRunning()) @@ -824,7 +824,7 @@ void RecordWiimote(int wiimote, u8 *data, u8 size) s_totalBytes = s_currentByte; } -// NOTE: CPU / EmuThread / Host Thread +// NOTE: EmuThread / Host Thread void ReadHeader() { s_numPads = tmpHeader.numControllers; @@ -934,7 +934,7 @@ void DoState(PointerWrap &p) // other variables (such as s_totalBytes and g_totalFrames) are set in LoadInput } -// NOTE: Host / CPU Thread +// NOTE: Host Thread void LoadInput(const std::string& filename) { File::IOFile t_record; @@ -1152,7 +1152,7 @@ void PlayController(GCPadStatus* PadStatus, int controllerID) { // This implementation assumes the disc change will only happen once. Trying to change more than that will cause // it to load the last disc every time. As far as i know though, there are no 3+ disc games, so this should be fine. - Core::SetState(Core::CORE_PAUSE); + CPU::Break(); bool found = false; std::string path; for (size_t i = 0; i < SConfig::GetInstance().m_ISOFolder.size(); ++i) @@ -1166,8 +1166,16 @@ void PlayController(GCPadStatus* PadStatus, int controllerID) } if (found) { - DVDInterface::ChangeDisc(path + '/' + g_discChange); - Core::SetState(Core::CORE_RUN); + path += '/' + g_discChange; + + Core::QueueHostJob([=] + { + if (!Movie::IsPlayingInput()) + return; + + DVDInterface::ChangeDisc(path); + CPU::EnableStepping(false); + }); } else { @@ -1235,10 +1243,13 @@ void EndPlayInput(bool cont) } else if (s_playMode != MODE_NONE) { + // We can be called by EmuThread during boot (CPU_POWERDOWN) + bool was_running = Core::IsRunningAndStarted() && !CPU::IsStepping(); + if (was_running) + CPU::Break(); s_rerecords = 0; s_currentByte = 0; s_playMode = MODE_NONE; - Core::UpdateWantDeterminism(); Core::DisplayMessage("Movie End.", 2000); s_bRecordingFromSaveState = false; // we don't clear these things because otherwise we can't resume playback if we load a movie state later @@ -1246,8 +1257,12 @@ void EndPlayInput(bool cont) //delete tmpInput; //tmpInput = nullptr; - if (SConfig::GetInstance().m_PauseMovie) - Core::SetState(Core::CORE_PAUSE); + Core::QueueHostJob([=] + { + Core::UpdateWantDeterminism(); + if (was_running && !SConfig::GetInstance().m_PauseMovie) + CPU::EnableStepping(false); + }); } } @@ -1353,7 +1368,7 @@ void SetGraphicsConfig() g_Config.bUseRealXFB = tmpHeader.bUseRealXFB; } -// NOTE: CPU / EmuThread / Host Thread +// NOTE: EmuThread / Host Thread void GetSettings() { s_bSaveConfig = true; diff --git a/Source/Core/DolphinQt2/Host.cpp b/Source/Core/DolphinQt2/Host.cpp index 389e0508b8..db74d88b39 100644 --- a/Source/Core/DolphinQt2/Host.cpp +++ b/Source/Core/DolphinQt2/Host.cpp @@ -2,6 +2,8 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. +#include +#include #include #include "Common/Common.h" @@ -57,7 +59,15 @@ void Host::SetRenderFullscreen(bool fullscreen) void Host_Message(int id) { if (id == WM_USER_STOP) + { emit Host::GetInstance()->RequestStop(); + } + else if (id == WM_USER_JOB_DISPATCH) + { + // Just poke the main thread to get it to wake up, job dispatch + // will happen automatically before it goes back to sleep again. + QAbstractEventDispatcher::instance(qApp->thread())->wakeUp(); + } } void Host_UpdateTitle(const std::string& title) diff --git a/Source/Core/DolphinQt2/Main.cpp b/Source/Core/DolphinQt2/Main.cpp index 639f68a4b4..12963caf59 100644 --- a/Source/Core/DolphinQt2/Main.cpp +++ b/Source/Core/DolphinQt2/Main.cpp @@ -2,6 +2,7 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. +#include #include #include "Core/BootManager.h" @@ -20,6 +21,12 @@ int main(int argc, char* argv[]) UICommon::Init(); Resources::Init(); + // Whenever the event loop is about to go to sleep, dispatch the jobs + // queued in the Core first. + QObject::connect(QAbstractEventDispatcher::instance(), + &QAbstractEventDispatcher::aboutToBlock, + &app, &Core::HostDispatchJobs); + MainWindow win; win.show(); int retval = app.exec(); diff --git a/Source/Core/DolphinWX/Main.cpp b/Source/Core/DolphinWX/Main.cpp index e085a94bb2..5398bf244c 100644 --- a/Source/Core/DolphinWX/Main.cpp +++ b/Source/Core/DolphinWX/Main.cpp @@ -101,6 +101,7 @@ bool DolphinApp::OnInit() Bind(wxEVT_QUERY_END_SESSION, &DolphinApp::OnEndSession, this); Bind(wxEVT_END_SESSION, &DolphinApp::OnEndSession, this); + Bind(wxEVT_IDLE, &DolphinApp::OnIdle, this); // Register message box and translation handlers RegisterMsgAlertHandler(&wxMsgAlert); @@ -359,6 +360,12 @@ void DolphinApp::OnFatalException() WiimoteReal::Shutdown(); } +void DolphinApp::OnIdle(wxIdleEvent& ev) +{ + ev.Skip(); + Core::HostDispatchJobs(); +} + // ------------ // Talk to GUI @@ -395,6 +402,12 @@ CFrame* DolphinApp::GetCFrame() void Host_Message(int Id) { + if (Id == WM_USER_JOB_DISPATCH) + { + // Trigger a wxEVT_IDLE + wxWakeUpIdle(); + return; + } wxCommandEvent event(wxEVT_HOST_COMMAND, Id); main_frame->GetEventHandler()->AddPendingEvent(event); } diff --git a/Source/Core/DolphinWX/Main.h b/Source/Core/DolphinWX/Main.h index 9b3bf2fdd0..7e9e3f56ff 100644 --- a/Source/Core/DolphinWX/Main.h +++ b/Source/Core/DolphinWX/Main.h @@ -33,6 +33,7 @@ private: void OnEndSession(wxCloseEvent& event); void InitLanguageSupport(); void AfterInit(); + void OnIdle(wxIdleEvent&); bool m_batch_mode = false; bool m_confirm_stop = false; diff --git a/Source/Core/DolphinWX/MainNoGUI.cpp b/Source/Core/DolphinWX/MainNoGUI.cpp index bf2089a5c8..aaf207ade3 100644 --- a/Source/Core/DolphinWX/MainNoGUI.cpp +++ b/Source/Core/DolphinWX/MainNoGUI.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include "Common/CommonTypes.h" @@ -36,7 +37,14 @@ class Platform public: virtual void Init() {} virtual void SetTitle(const std::string &title) {} - virtual void MainLoop() { while(running) {} } + virtual void MainLoop() + { + while (running) + { + Core::HostDispatchJobs(); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } + } virtual void Shutdown() {} virtual ~Platform() {} }; @@ -50,7 +58,10 @@ static Common::Event updateMainFrameEvent; void Host_Message(int Id) { if (Id == WM_USER_STOP) + { running = false; + updateMainFrameEvent.Set(); + } } static void* s_window_handle = nullptr; @@ -101,10 +112,13 @@ void Host_ConnectWiimote(int wm_idx, bool connect) { if (Core::IsRunning() && SConfig::GetInstance().bWii) { - bool was_unpaused = Core::PauseAndLock(true); - GetUsbPointer()->AccessWiiMote(wm_idx | 0x100)->Activate(connect); - Host_UpdateMainFrame(); - Core::PauseAndLock(false, was_unpaused); + Core::QueueHostJob([=] + { + bool was_unpaused = Core::PauseAndLock(true); + GetUsbPointer()->AccessWiiMote(wm_idx | 0x100)->Activate(connect); + Host_UpdateMainFrame(); + Core::PauseAndLock(false, was_unpaused); + }); } } @@ -270,6 +284,7 @@ class PlatformX11 : public Platform &borderDummy, &depthDummy); rendererIsFullscreen = false; } + Core::HostDispatchJobs(); usleep(100000); } } @@ -353,10 +368,14 @@ int main(int argc, char* argv[]) return 1; } - while (!Core::IsRunning()) + while (!Core::IsRunning() && running) + { + Core::HostDispatchJobs(); updateMainFrameEvent.Wait(); + } - platform->MainLoop(); + if (running) + platform->MainLoop(); Core::Stop(); Core::Shutdown();