TimingEvents: Fix events ending up out-of-order

data/resources/gamedb.yaml

@@ -22795,7 +22795,8 @@ SLES-01182:
     - DigitalController
     - NeGcon
   settings:
-    dmaMaxSliceTicks: 100
+    dmaMaxSliceTicks: 500 # Needs smaller sizes to avoid menu corruption.
+    dmaHaltTicks: 250
   metadata:
     publisher: "THQ"
     developer: "Interactive Entertainment"
@@ -22819,7 +22820,8 @@ SLUS-00882:
     rating: NoIssues
     comments: "Intro logos require the software renderer to display correctly."
   settings:
-    dmaMaxSliceTicks: 100
+    dmaMaxSliceTicks: 500 # Needs smaller sizes to avoid menu corruption.
+    dmaHaltTicks: 250
   controllers:
     - AnalogController
     - DigitalController
@@ -172114,6 +172116,9 @@ SLPS-00025:
     - DigitalController
   traits:
     - ForceRecompilerICache
+  settings:
+    dmaMaxSliceTicks: 500 # Stops a large GPU transfer breaking CD.
+    dmaHaltTicks: 300
   codes:
     - HASH-A8647D688C39B63F
     - HASH-21D86F0985C11667

src/core/dma.cpp

@@ -194,15 +194,12 @@ static TickCount TransferDeviceToMemory(u32 address, u32 increment, u32 word_cou
 template<Channel channel>
 static TickCount TransferMemoryToDevice(u32 address, u32 increment, u32 word_count);
 
-static TickCount GetMaxSliceTicks();
+static TickCount GetMaxSliceTicks(TickCount max_slice_size);
 
 // configuration
 namespace {
 struct DMAState
 {
-  TickCount max_slice_ticks = 1000;
-  TickCount halt_ticks = 100;
-
   std::vector<u32> transfer_buffer;
   TimingEvent unhalt_event{"DMA Transfer Unhalt", 1, 1, &DMA::UnhaltTransfer, nullptr};
   TickCount halt_ticks_remaining = 0;
@@ -241,9 +238,7 @@ struct fmt::formatter<DMA::Channel> : fmt::formatter<fmt::string_view>
 
 void DMA::Initialize()
 {
-  s_state.max_slice_ticks = g_settings.dma_max_slice_ticks;
-  s_state.halt_ticks = g_settings.dma_halt_ticks;
-  s_state.unhalt_event.SetInterval(s_state.max_slice_ticks);
+  s_state.unhalt_event.SetInterval(g_settings.dma_halt_ticks);
   Reset();
 }
 
@@ -473,16 +468,6 @@ void DMA::SetRequest(Channel channel, bool request)
     s_channel_transfer_functions[static_cast<u32>(channel)]();
 }
 
-void DMA::SetMaxSliceTicks(TickCount ticks)
-{
-  s_state.max_slice_ticks = ticks;
-}
-
-void DMA::SetHaltTicks(TickCount ticks)
-{
-  s_state.halt_ticks = ticks;
-}
-
 ALWAYS_INLINE_RELEASE bool DMA::CanTransferChannel(Channel channel, bool ignore_halt)
 {
   if (!s_state.DPCR.GetMasterEnable(channel))
@@ -547,15 +532,15 @@ ALWAYS_INLINE_RELEASE void DMA::CompleteTransfer(Channel channel, ChannelState&
   }
 }
 
-TickCount DMA::GetMaxSliceTicks()
+TickCount DMA::GetMaxSliceTicks(TickCount max_slice_size)
 {
-  const TickCount max = Pad::IsTransmitting() ? SLICE_SIZE_WHEN_TRANSMITTING_PAD : s_state.max_slice_ticks;
+  const TickCount max = Pad::IsTransmitting() ? SLICE_SIZE_WHEN_TRANSMITTING_PAD : max_slice_size;
   if (!TimingEvents::IsRunningEvents())
     return max;
 
-  const u32 current_ticks = TimingEvents::GetGlobalTickCounter();
-  const u32 max_ticks = TimingEvents::GetEventRunTickCounter() + static_cast<u32>(max);
-  return std::clamp(static_cast<TickCount>(max_ticks - current_ticks), 0, max);
+  const TickCount remaining_in_event_loop =
+    static_cast<TickCount>(TimingEvents::GetEventRunTickCounter() - TimingEvents::GetGlobalTickCounter());
+  return std::max<TickCount>(max - remaining_in_event_loop, 1);
 }
 
 template<DMA::Channel channel>
@@ -607,7 +592,7 @@ bool DMA::TransferChannel()
       const u8* const ram_ptr = Bus::g_ram;
       const u32 mask = Bus::g_ram_mask;
 
-      const TickCount slice_ticks = GetMaxSliceTicks();
+      const TickCount slice_ticks = GetMaxSliceTicks(g_settings.dma_max_slice_ticks);
       TickCount remaining_ticks = slice_ticks;
       while (cs.request && remaining_ticks > 0)
       {
@@ -658,7 +643,7 @@ bool DMA::TransferChannel()
       if (cs.request)
       {
         // stall the transfer for a bit if we ran for too long
-        HaltTransfer(s_state.halt_ticks);
+        HaltTransfer(g_settings.dma_halt_ticks);
         return false;
       }
       else
@@ -677,7 +662,7 @@ bool DMA::TransferChannel()
 
       const u32 block_size = cs.block_control.request.GetBlockSize();
       u32 blocks_remaining = cs.block_control.request.GetBlockCount();
-      TickCount ticks_remaining = GetMaxSliceTicks();
+      TickCount ticks_remaining = GetMaxSliceTicks(g_settings.dma_max_slice_ticks);
 
       if (copy_to_device)
       {
@@ -732,7 +717,7 @@ bool DMA::TransferChannel()
         {
           // we got halted
           if (!s_state.unhalt_event.IsActive())
-            HaltTransfer(s_state.halt_ticks);
+            HaltTransfer(g_settings.dma_halt_ticks);
 
           return false;
         }

src/core/dma.h

@@ -34,10 +34,6 @@ void WriteRegister(u32 offset, u32 value);
 
 void SetRequest(Channel channel, bool request);
 
-// changing interfaces
-void SetMaxSliceTicks(TickCount ticks);
-void SetHaltTicks(TickCount ticks);
-
 void DrawDebugStateWindow();
 
 } // namespace DMA

src/core/system.cpp

@@ -4135,9 +4135,6 @@ void System::CheckForSettingsChanges(const Settings& old_settings)
       TextureReplacements::Reload();
     }
 
-    DMA::SetMaxSliceTicks(g_settings.dma_max_slice_ticks);
-    DMA::SetHaltTicks(g_settings.dma_halt_ticks);
-
     if (g_settings.audio_backend != old_settings.audio_backend ||
         g_settings.increase_timer_resolution != old_settings.increase_timer_resolution ||
         g_settings.emulation_speed != old_settings.emulation_speed ||

src/core/timing_event.cpp

@@ -24,6 +24,7 @@ struct TimingEventsState
   TimingEvent* active_events_head = nullptr;
   TimingEvent* active_events_tail = nullptr;
   TimingEvent* current_event = nullptr;
+  TickCount current_event_new_downcount = 0;
   u32 active_event_count = 0;
   u32 global_tick_counter = 0;
   u32 event_run_tick_counter = 0;
@@ -326,14 +327,21 @@ void TimingEvents::RunEvents()
           // Factor late time into the time for the next invocation.
           const TickCount ticks_late = -event->m_downcount;
           const TickCount ticks_to_execute = event->m_time_since_last_run;
-          event->m_downcount += event->m_interval;
+
+          // Why don't we modify event->m_downcount directly? Because otherwise the event list won't be sorted.
+          // Adding the interval may cause this event to have a greater downcount than the next, and a new event
+          // may be inserted at the front, despite having a higher downcount than the next.
+          s_state.current_event_new_downcount = event->m_downcount + event->m_interval;
           event->m_time_since_last_run = 0;
 
           // The cycles_late is only an indicator, it doesn't modify the cycles to execute.
           event->m_callback(event->m_callback_param, ticks_to_execute, ticks_late);
           if (event->m_active)
+          {
+            event->m_downcount = s_state.current_event_new_downcount;
             SortEvent(event);
           }
+        }
       } while (pending_ticks > 0);
 
       s_state.current_event = nullptr;
@@ -458,25 +466,33 @@ void TimingEvent::Delay(TickCount ticks)
 
 void TimingEvent::Schedule(TickCount ticks)
 {
+  using namespace TimingEvents;
+
   const TickCount pending_ticks = CPU::GetPendingTicks();
-  m_downcount = pending_ticks + ticks;
+  const TickCount new_downcount = pending_ticks + ticks;
+
+  // See note in RunEvents().
+  s_state.current_event_new_downcount =
+    (s_state.current_event == this) ? new_downcount : s_state.current_event_new_downcount;
 
   if (!m_active)
   {
     // Event is going active, so we want it to only execute ticks from the current timestamp.
+    m_downcount = new_downcount;
     m_time_since_last_run = -pending_ticks;
     m_active = true;
-    TimingEvents::AddActiveEvent(this);
+    AddActiveEvent(this);
   }
   else
   {
     // Event is already active, so we leave the time since last run alone, and just modify the downcount.
     // If this is a call from an IO handler for example, re-sort the event queue.
-    if (TimingEvents::s_state.current_event != this)
+    if (s_state.current_event != this)
     {
-      TimingEvents::SortEvent(this);
-      if (TimingEvents::s_state.active_events_head == this)
-        TimingEvents::UpdateCPUDowncount();
+      m_downcount = new_downcount;
+      SortEvent(this);
+      if (s_state.active_events_head == this)
+        UpdateCPUDowncount();
     }
   }
 }
@@ -494,21 +510,6 @@ void TimingEvent::SetPeriodAndSchedule(TickCount ticks)
   Schedule(ticks);
 }
 
-void TimingEvent::Reset()
-{
-  if (!m_active)
-    return;
-
-  m_downcount = m_interval;
-  m_time_since_last_run = 0;
-  if (TimingEvents::s_state.current_event != this)
-  {
-    TimingEvents::SortEvent(this);
-    if (TimingEvents::s_state.active_events_head == this)
-      TimingEvents::UpdateCPUDowncount();
-  }
-}
-
 void TimingEvent::InvokeEarly(bool force /* = false */)
 {
   if (!m_active)
@@ -519,12 +520,14 @@ void TimingEvent::InvokeEarly(bool force /* = false */)
   if ((!force && ticks_to_execute < m_period) || ticks_to_execute <= 0)
     return;
 
+  // Shouldn't be invoking early when we're the current event running.
+  DebugAssert(TimingEvents::s_state.current_event != this);
+
   m_downcount = pending_ticks + m_interval;
   m_time_since_last_run -= ticks_to_execute;
   m_callback(m_callback_param, ticks_to_execute, 0);
 
   // Since we've changed the downcount, we need to re-sort the events.
-  DebugAssert(TimingEvents::s_state.current_event != this);
   TimingEvents::SortEvent(this);
   if (TimingEvents::s_state.active_events_head == this)
     TimingEvents::UpdateCPUDowncount();
@@ -536,6 +539,7 @@ void TimingEvent::Activate()
     return;
 
   // leave the downcount intact
+  // if we're running events, this is going to be zero, so no effect
   const TickCount pending_ticks = CPU::GetPendingTicks();
   m_downcount += pending_ticks;
   m_time_since_last_run -= pending_ticks;

src/core/timing_event.h

@@ -38,8 +38,6 @@ public:
   void SetIntervalAndSchedule(TickCount ticks);
   void SetPeriodAndSchedule(TickCount ticks);
 
-  void Reset();
-
   // Services the event with the current accmulated time. If force is set, when not enough time is pending to
   // simulate a single cycle, the callback will still be invoked, otherwise it won't be.
   void InvokeEarly(bool force = false);