From 3443a1003089459029bf42ead4c5aabc9898be90 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Thu, 7 Jul 2016 15:52:08 +0200 Subject: [PATCH] CoreTiming: Merge ScheduleEvent variants into one function Now Core::IsCPUThread() only gets called once when using the AnyThread variant. Also, I think the enum approach makes calling code clearer. --- Source/Core/Core/CoreTiming.cpp | 86 +++++++++---------- Source/Core/Core/CoreTiming.h | 14 ++- Source/Core/Core/HW/DSP.cpp | 2 +- Source/Core/Core/HW/EXI.cpp | 22 ++--- Source/Core/Core/HW/EXI.h | 7 +- Source/Core/Core/HW/EXI_DeviceEthernet.cpp | 5 +- Source/Core/Core/HW/EXI_DeviceMic.cpp | 2 +- Source/Core/Core/HW/ProcessorInterface.cpp | 8 +- Source/Core/Core/HW/SI.cpp | 14 +-- Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp | 11 +-- Source/Core/Core/IPC_HLE/WII_IPC_HLE.h | 5 +- .../Core/IPC_HLE/WII_IPC_HLE_Device_hid.cpp | 6 +- Source/Core/Core/MemoryWatcher.cpp | 2 +- Source/Core/VideoCommon/CommandProcessor.cpp | 2 +- Source/Core/VideoCommon/PixelEngine.cpp | 7 +- 15 files changed, 99 insertions(+), 94 deletions(-) diff --git a/Source/Core/Core/CoreTiming.cpp b/Source/Core/Core/CoreTiming.cpp index b4de24cdfb..0accb2e66c 100644 --- a/Source/Core/Core/CoreTiming.cpp +++ b/Source/Core/Core/CoreTiming.cpp @@ -225,8 +225,8 @@ void DoState(PointerWrap& p) p.DoMarker("CoreTimingEvents"); } -// This should only be called from the CPU thread, if you are calling it any other thread, you are -// doing something evil +// This should only be called from the CPU thread. If you are calling +// it from any other thread, you are doing something evil u64 GetTicks() { u64 ticks = (u64)g_globalTimer; @@ -243,35 +243,6 @@ u64 GetIdleTicks() return (u64)idledCycles; } -// This is to be called when outside threads, such as the graphics thread, wants to -// schedule things to be executed on the main thread. -void ScheduleEvent_Threadsafe(s64 cyclesIntoFuture, int event_type, u64 userdata) -{ - _assert_msg_(POWERPC, !Core::IsCPUThread(), "ScheduleEvent_Threadsafe from wrong thread"); - if (Core::g_want_determinism) - { - ERROR_LOG(POWERPC, - "Someone scheduled an off-thread \"%s\" event while netplay or movie play/record " - "was active. This is likely to cause a desync.", - event_types[event_type].name.c_str()); - } - std::lock_guard lk(tsWriteLock); - Event ne; - ne.time = g_globalTimer + cyclesIntoFuture; - ne.type = event_type; - ne.userdata = userdata; - tsQueue.Push(ne); -} - -// To be used from any thread, including the CPU thread -void ScheduleEvent_AnyThread(s64 cyclesIntoFuture, int event_type, u64 userdata) -{ - if (Core::IsCPUThread()) - ScheduleEvent(cyclesIntoFuture, event_type, userdata); - else - ScheduleEvent_Threadsafe(cyclesIntoFuture, event_type, userdata); -} - void ClearPendingEvents() { while (first) @@ -300,24 +271,49 @@ static void AddEventToQueue(Event* ne) } } -// This must be run ONLY from within the CPU thread -// cyclesIntoFuture may be VERY inaccurate if called from anything else -// than Advance -void ScheduleEvent(s64 cyclesIntoFuture, int event_type, u64 userdata) +void ScheduleEvent(s64 cycles_into_future, int event_type, u64 userdata, FromThread from) { - _assert_msg_(POWERPC, Core::IsCPUThread() || Core::GetState() == Core::CORE_PAUSE, - "ScheduleEvent from wrong thread"); + bool from_cpu_thread; + if (from == FromThread::ANY) + { + from_cpu_thread = Core::IsCPUThread(); + } + else + { + from_cpu_thread = from == FromThread::CPU; + _assert_msg_(POWERPC, from_cpu_thread == Core::IsCPUThread(), + "ScheduleEvent from wrong thread (%s)", from_cpu_thread ? "CPU" : "non-CPU"); + } - Event* ne = GetNewEvent(); - ne->userdata = userdata; - ne->type = event_type; - ne->time = GetTicks() + cyclesIntoFuture; + if (from_cpu_thread) + { + Event* ne = GetNewEvent(); + ne->time = GetTicks() + cycles_into_future; + ne->userdata = userdata; + ne->type = event_type; - // If this event needs to be scheduled before the next advance(), force one early - if (!globalTimerIsSane) - ForceExceptionCheck(cyclesIntoFuture); + // If this event needs to be scheduled before the next advance(), force one early + if (!globalTimerIsSane) + ForceExceptionCheck(cycles_into_future); - AddEventToQueue(ne); + AddEventToQueue(ne); + } + else + { + if (Core::g_want_determinism) + { + ERROR_LOG(POWERPC, "Someone scheduled an off-thread \"%s\" event while netplay or " + "movie play/record was active. This is likely to cause a desync.", + event_types[event_type].name.c_str()); + } + + std::lock_guard lk(tsWriteLock); + Event ne; + ne.time = g_globalTimer + cycles_into_future; + ne.type = event_type; + ne.userdata = userdata; + tsQueue.Push(ne); + } } void RemoveEvent(int event_type) diff --git a/Source/Core/Core/CoreTiming.h b/Source/Core/Core/CoreTiming.h index 759806f295..210aa7c5e6 100644 --- a/Source/Core/Core/CoreTiming.h +++ b/Source/Core/Core/CoreTiming.h @@ -48,10 +48,18 @@ void DoState(PointerWrap& p); int RegisterEvent(const std::string& name, TimedCallback callback); void UnregisterAllEvents(); +enum class FromThread +{ + CPU, + NON_CPU, + // Don't use ANY unless you're sure you need to call from + // both the CPU thread and at least one other thread + ANY +}; + // userdata MAY NOT CONTAIN POINTERS. userdata might get written and reloaded from savestates. -void ScheduleEvent(s64 cyclesIntoFuture, int event_type, u64 userdata = 0); -void ScheduleEvent_Threadsafe(s64 cyclesIntoFuture, int event_type, u64 userdata = 0); -void ScheduleEvent_AnyThread(s64 cyclesIntoFuture, int event_type, u64 userdata = 0); +void ScheduleEvent(s64 cycles_into_future, int event_type, u64 userdata = 0, + FromThread from = FromThread::CPU); // We only permit one event of each type in the queue at a time. void RemoveEvent(int event_type); diff --git a/Source/Core/Core/HW/DSP.cpp b/Source/Core/Core/HW/DSP.cpp index 7a748a43f3..f709db769e 100644 --- a/Source/Core/Core/HW/DSP.cpp +++ b/Source/Core/Core/HW/DSP.cpp @@ -465,7 +465,7 @@ static void GenerateDSPInterrupt(u64 DSPIntType, s64 cyclesLate) void GenerateDSPInterruptFromDSPEmu(DSPInterruptType type) { // TODO: Maybe rethink this? The timing is unpredictable. - CoreTiming::ScheduleEvent_AnyThread(0, et_GenerateDSPInterrupt, type); + CoreTiming::ScheduleEvent(0, et_GenerateDSPInterrupt, type, CoreTiming::FromThread::ANY); } // called whenever SystemTimers thinks the DSP deserves a few more cycles diff --git a/Source/Core/Core/HW/EXI.cpp b/Source/Core/Core/HW/EXI.cpp index 6f437b596e..9db0a83382 100644 --- a/Source/Core/Core/HW/EXI.cpp +++ b/Source/Core/Core/HW/EXI.cpp @@ -104,13 +104,14 @@ static void ChangeDeviceCallback(u64 userdata, s64 cyclesLate) void ChangeDevice(const u8 channel, const TEXIDevices device_type, const u8 device_num) { - // Called from GUI, so we need to make it thread safe. + // Called from GUI, so we need to use FromThread::NON_CPU. // Let the hardware see no device for 1 second - CoreTiming::ScheduleEvent_Threadsafe( - 0, changeDevice, ((u64)channel << 32) | ((u64)EXIDEVICE_NONE << 16) | device_num); - CoreTiming::ScheduleEvent_Threadsafe(SystemTimers::GetTicksPerSecond(), changeDevice, - ((u64)channel << 32) | ((u64)device_type << 16) | - device_num); + CoreTiming::ScheduleEvent(0, changeDevice, + ((u64)channel << 32) | ((u64)EXIDEVICE_NONE << 16) | device_num, + CoreTiming::FromThread::NON_CPU); + CoreTiming::ScheduleEvent(SystemTimers::GetTicksPerSecond(), changeDevice, + ((u64)channel << 32) | ((u64)device_type << 16) | device_num, + CoreTiming::FromThread::NON_CPU); } CEXIChannel* GetChannel(u32 index) @@ -149,14 +150,9 @@ static void UpdateInterruptsCallback(u64 userdata, s64 cycles_late) UpdateInterrupts(); } -void ScheduleUpdateInterrupts_Threadsafe(int cycles_late) +void ScheduleUpdateInterrupts(CoreTiming::FromThread from, int cycles_late) { - CoreTiming::ScheduleEvent_Threadsafe(cycles_late, updateInterrupts, 0); -} - -void ScheduleUpdateInterrupts(int cycles_late) -{ - CoreTiming::ScheduleEvent(cycles_late, updateInterrupts, 0); + CoreTiming::ScheduleEvent(cycles_late, updateInterrupts, 0, from); } } // end of namespace ExpansionInterface diff --git a/Source/Core/Core/HW/EXI.h b/Source/Core/Core/HW/EXI.h index 28629a09b4..f6c6395e18 100644 --- a/Source/Core/Core/HW/EXI.h +++ b/Source/Core/Core/HW/EXI.h @@ -10,6 +10,10 @@ class CEXIChannel; class IEXIDevice; class PointerWrap; enum TEXIDevices : int; +namespace CoreTiming +{ +enum class FromThread; +} namespace MMIO { class Mapping; @@ -30,8 +34,7 @@ void PauseAndLock(bool doLock, bool unpauseOnUnlock); void RegisterMMIO(MMIO::Mapping* mmio, u32 base); void UpdateInterrupts(); -void ScheduleUpdateInterrupts_Threadsafe(int cycles_late); -void ScheduleUpdateInterrupts(int cycles_late); +void ScheduleUpdateInterrupts(CoreTiming::FromThread from, int cycles_late); void ChangeDevice(const u8 channel, const TEXIDevices device_type, const u8 device_num); diff --git a/Source/Core/Core/HW/EXI_DeviceEthernet.cpp b/Source/Core/Core/HW/EXI_DeviceEthernet.cpp index 42352c7d98..64230a28b4 100644 --- a/Source/Core/Core/HW/EXI_DeviceEthernet.cpp +++ b/Source/Core/Core/HW/EXI_DeviceEthernet.cpp @@ -9,6 +9,7 @@ #include "Common/Logging/Log.h" #include "Common/Network.h" #include "Core/ConfigManager.h" +#include "Core/CoreTiming.h" #include "Core/HW/EXI.h" #include "Core/HW/EXI_DeviceEthernet.h" #include "Core/HW/Memmap.h" @@ -406,7 +407,7 @@ void CEXIETHERNET::SendComplete() mBbaMem[BBA_IR] |= INT_T; exi_status.interrupt |= exi_status.TRANSFER; - ExpansionInterface::ScheduleUpdateInterrupts(0); + ExpansionInterface::ScheduleUpdateInterrupts(CoreTiming::FromThread::CPU, 0); } mBbaMem[BBA_LTPS] = 0; @@ -571,7 +572,7 @@ bool CEXIETHERNET::RecvHandlePacket() mBbaMem[BBA_IR] |= INT_R; exi_status.interrupt |= exi_status.TRANSFER; - ExpansionInterface::ScheduleUpdateInterrupts_Threadsafe(0); + ExpansionInterface::ScheduleUpdateInterrupts(CoreTiming::FromThread::NON_CPU, 0); } else { diff --git a/Source/Core/Core/HW/EXI_DeviceMic.cpp b/Source/Core/Core/HW/EXI_DeviceMic.cpp index 27d8ec1d17..85b1c82827 100644 --- a/Source/Core/Core/HW/EXI_DeviceMic.cpp +++ b/Source/Core/Core/HW/EXI_DeviceMic.cpp @@ -179,7 +179,7 @@ void CEXIMic::UpdateNextInterruptTicks() { int diff = (SystemTimers::GetTicksPerSecond() / sample_rate) * buff_size_samples; next_int_ticks = CoreTiming::GetTicks() + diff; - ExpansionInterface::ScheduleUpdateInterrupts(diff); + ExpansionInterface::ScheduleUpdateInterrupts(CoreTiming::FromThread::CPU, diff); } bool CEXIMic::IsInterruptSet() diff --git a/Source/Core/Core/HW/ProcessorInterface.cpp b/Source/Core/Core/HW/ProcessorInterface.cpp index cfaae30679..59ce6a4569 100644 --- a/Source/Core/Core/HW/ProcessorInterface.cpp +++ b/Source/Core/Core/HW/ProcessorInterface.cpp @@ -216,10 +216,10 @@ static void IOSNotifyResetButtonCallback(u64 userdata, s64 cyclesLate) void ResetButton_Tap() { - CoreTiming::ScheduleEvent_AnyThread(0, toggleResetButton, true); - CoreTiming::ScheduleEvent_AnyThread(0, iosNotifyResetButton, 0); - CoreTiming::ScheduleEvent_AnyThread(SystemTimers::GetTicksPerSecond() / 2, toggleResetButton, - false); + CoreTiming::ScheduleEvent(0, toggleResetButton, true, CoreTiming::FromThread::ANY); + CoreTiming::ScheduleEvent(0, iosNotifyResetButton, 0, CoreTiming::FromThread::ANY); + CoreTiming::ScheduleEvent(SystemTimers::GetTicksPerSecond() / 2, toggleResetButton, false, + CoreTiming::FromThread::ANY); } } // namespace ProcessorInterface diff --git a/Source/Core/Core/HW/SI.cpp b/Source/Core/Core/HW/SI.cpp index 59a0bb317b..55c9c06904 100644 --- a/Source/Core/Core/HW/SI.cpp +++ b/Source/Core/Core/HW/SI.cpp @@ -527,24 +527,26 @@ static void ChangeDeviceCallback(u64 userdata, s64 cyclesLate) void ChangeDevice(SIDevices device, int channel) { - // Called from GUI, so we need to make it thread safe. + // Called from GUI, so we need to use FromThread::NON_CPU. // Let the hardware see no device for 1 second // TODO: Calling GetDeviceType here isn't threadsafe. if (GetDeviceType(channel) != device) { - CoreTiming::ScheduleEvent_Threadsafe(0, changeDevice, ((u64)channel << 32) | SIDEVICE_NONE); - CoreTiming::ScheduleEvent_Threadsafe(SystemTimers::GetTicksPerSecond(), changeDevice, - ((u64)channel << 32) | device); + CoreTiming::ScheduleEvent(0, changeDevice, ((u64)channel << 32) | SIDEVICE_NONE, + CoreTiming::FromThread::NON_CPU); + CoreTiming::ScheduleEvent(SystemTimers::GetTicksPerSecond(), changeDevice, + ((u64)channel << 32) | device, CoreTiming::FromThread::NON_CPU); } } void ChangeDeviceDeterministic(SIDevices device, int channel) { - // Called from savestates, so no need to make it thread safe. + // Called from savestates, so we don't use FromThread::NON_CPU. if (GetDeviceType(channel) != device) { CoreTiming::ScheduleEvent(0, changeDevice, ((u64)channel << 32) | SIDEVICE_NONE); - CoreTiming::ScheduleEvent(500000000, changeDevice, ((u64)channel << 32) | device); + CoreTiming::ScheduleEvent(SystemTimers::GetTicksPerSecond(), changeDevice, + ((u64)channel << 32) | device); } } diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp index 90784282c1..1720790b75 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp @@ -226,7 +226,7 @@ void SDIO_EventNotify() // TODO: Potential race condition: If IsRunning() becomes false after // it's checked, an event may be scheduled after CoreTiming shuts down. if (SConfig::GetInstance().bWii && Core::IsRunning()) - CoreTiming::ScheduleEvent_Threadsafe(0, event_sdio_notify); + CoreTiming::ScheduleEvent(0, event_sdio_notify, 0, CoreTiming::FromThread::NON_CPU); } int getFreeDeviceId() @@ -555,14 +555,9 @@ void EnqueueRequest(u32 address) // NOTE: Only call this if you have correctly handled // CommandAddress+0 and CommandAddress+8. // Please search for examples of this being called elsewhere. -void EnqueueReply(u32 address, int cycles_in_future) +void EnqueueReply(u32 address, int cycles_in_future, CoreTiming::FromThread from) { - CoreTiming::ScheduleEvent(cycles_in_future, event_enqueue, address); -} - -void EnqueueReply_Threadsafe(u32 address, int cycles_in_future) -{ - CoreTiming::ScheduleEvent_Threadsafe(cycles_in_future, event_enqueue, address); + CoreTiming::ScheduleEvent(cycles_in_future, event_enqueue, address, from); } void EnqueueCommandAcknowledgement(u32 address, int cycles_in_future) diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.h b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.h index 3ca8c4bbf4..3b75781a64 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.h +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.h @@ -10,6 +10,7 @@ #include "Common/CommonTypes.h" +#include "Core/CoreTiming.h" #include "Core/HW/SystemTimers.h" class IWII_IPC_HLE_Device; @@ -74,8 +75,8 @@ void UpdateDevices(); void ExecuteCommand(u32 _Address); void EnqueueRequest(u32 address); -void EnqueueReply(u32 address, int cycles_in_future = 0); -void EnqueueReply_Threadsafe(u32 address, int cycles_in_future = 0); +void EnqueueReply(u32 address, int cycles_in_future = 0, + CoreTiming::FromThread from = CoreTiming::FromThread::CPU); void EnqueueCommandAcknowledgement(u32 _Address, int cycles_in_future = 0); } // end of namespace WII_IPC_HLE_Interface diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_hid.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_hid.cpp index 4a57a03097..1845e55a12 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_hid.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_hid.cpp @@ -7,6 +7,7 @@ #include "Common/Thread.h" #include "Core/Core.h" +#include "Core/CoreTiming.h" #include "Core/Debugger/Debugger_SymbolMap.h" #include "Core/HW/WII_IPC.h" #include "Core/IPC_HLE/WII_IPC_HLE.h" @@ -40,7 +41,8 @@ void CWII_IPC_HLE_Device_hid::checkUsbUpdates(CWII_IPC_HLE_Device_hid* hid) // Return value Memory::Write_U32(0, hid->deviceCommandAddress + 4); - WII_IPC_HLE_Interface::EnqueueReply_Threadsafe(hid->deviceCommandAddress); + WII_IPC_HLE_Interface::EnqueueReply(hid->deviceCommandAddress, 0, + CoreTiming::FromThread::NON_CPU); hid->deviceCommandAddress = 0; } } @@ -68,7 +70,7 @@ void CWII_IPC_HLE_Device_hid::handleUsbUpdates(struct libusb_transfer* transfer) // Return value Memory::Write_U32(ret, replyAddress + 4); - WII_IPC_HLE_Interface::EnqueueReply_Threadsafe(replyAddress); + WII_IPC_HLE_Interface::EnqueueReply(replyAddress, 0, CoreTiming::FromThread::NON_CPU); // DEBUG_LOG(WII_IPC_HID, "OMG OMG OMG I GOT A CALLBACK, IMMA BE FAMOUS %d %d %d", // transfer->actual_length, transfer->length, transfer->status); } diff --git a/Source/Core/Core/MemoryWatcher.cpp b/Source/Core/Core/MemoryWatcher.cpp index 76ff2d9538..a0fccb973b 100644 --- a/Source/Core/Core/MemoryWatcher.cpp +++ b/Source/Core/Core/MemoryWatcher.cpp @@ -21,7 +21,7 @@ static const int MW_RATE = 600; // Steps per second static void MWCallback(u64 userdata, s64 cyclesLate) { s_memory_watcher->Step(); - CoreTiming::ScheduleEvent((SystemTimers::GetTicksPerSecond() / MW_RATE) - cyclesLate, s_event); + CoreTiming::ScheduleEvent(SystemTimers::GetTicksPerSecond() / MW_RATE - cyclesLate, s_event); } void MemoryWatcher::Init() diff --git a/Source/Core/VideoCommon/CommandProcessor.cpp b/Source/Core/VideoCommon/CommandProcessor.cpp index 2b82173110..0cf8dd3b7b 100644 --- a/Source/Core/VideoCommon/CommandProcessor.cpp +++ b/Source/Core/VideoCommon/CommandProcessor.cpp @@ -343,7 +343,7 @@ void UpdateInterrupts(u64 userdata) void UpdateInterruptsFromVideoBackend(u64 userdata) { if (!Fifo::UseDeterministicGPUThread()) - CoreTiming::ScheduleEvent_Threadsafe(0, et_UpdateInterrupts, userdata); + CoreTiming::ScheduleEvent(0, et_UpdateInterrupts, userdata, CoreTiming::FromThread::NON_CPU); } bool IsInterruptWaiting() diff --git a/Source/Core/VideoCommon/PixelEngine.cpp b/Source/Core/VideoCommon/PixelEngine.cpp index f774b54064..f5c0dea0a4 100644 --- a/Source/Core/VideoCommon/PixelEngine.cpp +++ b/Source/Core/VideoCommon/PixelEngine.cpp @@ -277,10 +277,11 @@ static void RaiseEvent() return; s_event_raised = true; + + CoreTiming::FromThread from = CoreTiming::FromThread::NON_CPU; if (!SConfig::GetInstance().bCPUThread || Fifo::UseDeterministicGPUThread()) - CoreTiming::ScheduleEvent(0, et_SetTokenFinishOnMainThread, 0); - else - CoreTiming::ScheduleEvent_Threadsafe(0, et_SetTokenFinishOnMainThread, 0); + from = CoreTiming::FromThread::CPU; + CoreTiming::ScheduleEvent(0, et_SetTokenFinishOnMainThread, 0, from); } // SetToken