Merge pull request #3987 from JosJuice/scheduleevent-cleanup

CoreTiming: ScheduleEvent cleanup

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<std::mutex> 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<std::mutex> 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)

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);

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

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

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);
 

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
   {

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()

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

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);
   }
 }
 

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)

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

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);
 }

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()

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()

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