DMA: Refactoring, support split block transfers

src/core/cdrom.cpp

@@ -2,6 +2,7 @@
 #include "YBaseLib/Log.h"
 #include "common/cd_image.h"
 #include "common/state_wrapper.h"
+#include "dma.h"
 #include "interrupt_controller.h"
 #include "system.h"
 Log_SetChannel(CDROM);
@@ -394,6 +395,8 @@ void CDROM::UpdateStatusRegister()
   m_status.RSLRRDY = !m_response_fifo.IsEmpty();
   m_status.DRQSTS = !m_data_fifo.IsEmpty();
   m_status.BUSYSTS = m_command_state == CommandState::WaitForExecute;
+
+  m_dma->SetRequest(DMA::Channel::CDROM, m_status.DRQSTS);
 }
 
 u32 CDROM::GetAckDelayForCommand() const

src/core/dma.cpp

@@ -30,7 +30,7 @@ bool DMA::Initialize(System* system, Bus* bus, InterruptController* interrupt_co
 void DMA::Reset()
 {
   m_transfer_ticks = 0;
-  m_transfer_pending = false;
+  m_transfer_in_progress = false;
   m_state = {};
   m_DPCR.bits = 0x07654321;
   m_DICR.bits = 0;
@@ -99,6 +99,7 @@ void DMA::WriteRegister(u32 offset, u32 value)
       {
         Log_TracePrintf("DMA channel %u block control <- 0x%08X", channel_index, value);
         state.block_control.bits = value;
+        Transfer();
         return;
       }
 
@@ -107,9 +108,7 @@ void DMA::WriteRegister(u32 offset, u32 value)
         state.channel_control.bits = (state.channel_control.bits & ~ChannelState::ChannelControl::WRITE_MASK) |
                                      (value & ChannelState::ChannelControl::WRITE_MASK);
         Log_TracePrintf("DMA channel %u channel control <- 0x%08X", channel_index, state.channel_control.bits);
-        if (CanRunChannel(static_cast<Channel>(channel_index)))
+        Transfer();
-          UpdateTransferPending();
-
         return;
       }
 
@@ -125,7 +124,7 @@ void DMA::WriteRegister(u32 offset, u32 value)
       {
         Log_TracePrintf("DPCR <- 0x%08X", value);
         m_DPCR.bits = value;
-        UpdateTransferPending();
+        Transfer();
         return;
       }
 
@@ -153,65 +152,70 @@ void DMA::SetRequest(Channel channel, bool request)
     return;
 
   cs.request = request;
-  UpdateTransferPending();
+  if (request)
+    Transfer();
 }
 
-void DMA::Execute(TickCount ticks)
+bool DMA::CanTransferChannel(Channel channel) const
-{
-  if (!m_transfer_pending)
-    return;
-
-  m_transfer_ticks -= ticks;
-  if (m_transfer_ticks <= 0)
-  {
-    m_transfer_pending = false;
-
-    for (u32 i = 0; i < NUM_CHANNELS; i++)
-    {
-      const Channel channel = static_cast<Channel>(i);
-      if (CanRunChannel(channel))
-      {
-        RunDMA(channel);
-        m_transfer_pending |= CanRunChannel(channel);
-      }
-    }
-
-    if (m_transfer_pending)
-    {
-      m_transfer_ticks += TRANSFER_TICKS;
-      m_system->SetDowncount(m_transfer_ticks);
-    }
-  }
-  else
-  {
-    m_system->SetDowncount(m_transfer_ticks);
-  }
-}
-
-bool DMA::CanRunChannel(Channel channel) const
 {
   if (!m_DPCR.GetMasterEnable(channel))
     return false;
 
   const ChannelState& cs = m_state[static_cast<u32>(channel)];
-  if (cs.channel_control.start_trigger)
+  if (!cs.channel_control.enable_busy)
-    return true;
+    return false;
 
-  return (cs.channel_control.enable_busy && cs.request);
+  if (!cs.request && channel != Channel::OTC)
+    return false;
+
+  if (cs.channel_control.sync_mode == SyncMode::Manual && !cs.channel_control.start_trigger)
+    return false;
+
+  return true;
 }
 
 bool DMA::CanRunAnyChannels() const
 {
   for (u32 i = 0; i < NUM_CHANNELS; i++)
   {
-    if (CanRunChannel(static_cast<Channel>(i)))
+    if (CanTransferChannel(static_cast<Channel>(i)))
       return true;
   }
 
   return false;
 }
 
-void DMA::RunDMA(Channel channel)
+void DMA::Transfer()
+{
+  if (m_transfer_in_progress)
+    return;
+
+  // prevent recursive calls
+  m_transfer_in_progress = true;
+
+  // keep going until all transfers are done. one channel can start others (e.g. MDEC)
+  for (;;)
+  {
+    bool any_channels_active = false;
+
+    for (u32 i = 0; i < NUM_CHANNELS; i++)
+    {
+      const Channel channel = static_cast<Channel>(i);
+      if (CanTransferChannel(channel))
+      {
+        TransferChannel(channel);
+        any_channels_active = true;
+      }
+    }
+
+    if (!any_channels_active)
+      break;
+  }
+
+  m_transfer_in_progress = false;
+}
+
+void DMA::TransferChannel(Channel channel)
 {
   ChannelState& cs = m_state[static_cast<u32>(channel)];
   const bool copy_to_device = cs.channel_control.copy_to_device;
@@ -305,37 +309,56 @@ void DMA::RunDMA(Channel channel)
 
     case SyncMode::Request:
     {
-      const u32 block_size = cs.block_control.request.GetBlockSize();
+      Log_DebugPrintf("DMA%u: Copying %u blocks of size %u %s 0x%08X", static_cast<u32>(channel),
-      const u32 block_count = cs.block_control.request.GetBlockCount();
+                      cs.block_control.request.GetBlockCount(), cs.block_control.request.GetBlockSize(),
-      Log_DebugPrintf("DMA%u: Copying %u blocks of size %u %s 0x%08X", static_cast<u32>(channel), block_count,
+                      copy_to_device ? "from" : "to", current_address);
-                      block_size, copy_to_device ? "from" : "to", current_address);
+
+      u32 blocks_remaining = cs.block_control.request.block_count;
+
       if (copy_to_device)
       {
-        u32 words_remaining = block_size * block_count;
         do
         {
-          words_remaining--;
+          blocks_remaining--;
 
-          u32 value = 0;
+          u32 words_remaining = cs.block_control.request.block_size;
-          m_bus->DispatchAccess<MemoryAccessType::Read, MemoryAccessSize::Word>(current_address, value);
+          do
-          DMAWrite(channel, value, current_address, words_remaining);
+          {
+            words_remaining--;
 
-          current_address = (current_address + increment) & ADDRESS_MASK;
+            u32 value = 0;
-        } while (words_remaining > 0);
+            m_bus->DispatchAccess<MemoryAccessType::Read, MemoryAccessSize::Word>(current_address, value);
+            DMAWrite(channel, value, current_address, words_remaining);
+
+            current_address = (current_address + increment) & ADDRESS_MASK;
+          } while (words_remaining > 0);
+        } while (cs.request && blocks_remaining > 0);
       }
       else
       {
-        u32 words_remaining = block_size * block_count;
         do
         {
-          words_remaining--;
+          blocks_remaining--;
 
-          u32 value = DMARead(channel, current_address, words_remaining);
+          u32 words_remaining = cs.block_control.request.block_size;
-          m_bus->DispatchAccess<MemoryAccessType::Write, MemoryAccessSize::Word>(current_address, value);
+          do
+          {
+            words_remaining--;
 
-          current_address = (current_address + increment) & ADDRESS_MASK;
+            u32 value = DMARead(channel, current_address, words_remaining);
-        } while (words_remaining > 0);
+            m_bus->DispatchAccess<MemoryAccessType::Write, MemoryAccessSize::Word>(current_address, value);
+
+            current_address = (current_address + increment) & ADDRESS_MASK;
+          } while (words_remaining > 0);
+        } while (cs.request && blocks_remaining > 0);
       }
+
+      cs.base_address = current_address;
+      cs.block_control.request.block_count = blocks_remaining;
+
+      // finish transfer later if the request was cleared
+      if (blocks_remaining > 0)
+        return;
     }
     break;
 
@@ -412,22 +435,3 @@ void DMA::DMAWrite(Channel channel, u32 value, PhysicalMemoryAddress src_address
       break;
   }
 }
-
-void DMA::UpdateTransferPending()
-{
-  if (CanRunAnyChannels())
-  {
-    if (m_transfer_pending)
-      return;
-
-    m_system->Synchronize();
-    m_transfer_pending = true;
-    m_transfer_ticks = TRANSFER_TICKS;
-    m_system->SetDowncount(m_transfer_ticks);
-  }
-  else
-  {
-    m_transfer_pending = false;
-    m_transfer_ticks = 0;
-  }
-}

src/core/dma.h

@@ -35,8 +35,8 @@ public:
   DMA();
   ~DMA();
 
-  bool Initialize(System* system, Bus* bus, InterruptController* interrupt_controller, GPU* gpu, CDROM* cdrom,
+  bool Initialize(System* system, Bus* bus, InterruptController* interrupt_controller, GPU* gpu, CDROM* cdrom, SPU* spu,
-                  SPU* spu, MDEC* mdec);
+                  MDEC* mdec);
   void Reset();
   bool DoState(StateWrapper& sw);
 
@@ -45,8 +45,6 @@ public:
 
   void SetRequest(Channel channel, bool request);
 
-  void Execute(TickCount ticks);
-
 private:
   static constexpr PhysicalMemoryAddress ADDRESS_MASK = UINT32_C(0x00FFFFFF);
   static constexpr u32 TRANSFER_TICKS = 10;
@@ -60,10 +58,11 @@ private:
   };
 
   // is everything enabled for a channel to operate?
-  bool CanRunChannel(Channel channel) const;
+  bool CanTransferChannel(Channel channel) const;
   bool CanRunAnyChannels() const;
 
-  void RunDMA(Channel channel);
+  void Transfer();
+  void TransferChannel(Channel channel);
 
   // from device -> memory
   u32 DMARead(Channel channel, PhysicalMemoryAddress dst_address, u32 remaining_words);
@@ -71,8 +70,6 @@ private:
   // from memory -> device
   void DMAWrite(Channel channel, u32 value, PhysicalMemoryAddress src_address, u32 remaining_words);
 
-  void UpdateTransferPending();
-
   System* m_system = nullptr;
   Bus* m_bus = nullptr;
   InterruptController* m_interrupt_controller = nullptr;
@@ -82,7 +79,7 @@ private:
   MDEC* m_mdec = nullptr;
 
   TickCount m_transfer_ticks = 0;
-  bool m_transfer_pending = false;
+  bool m_transfer_in_progress = false;
 
   struct ChannelState
   {

src/core/mdec.cpp

@@ -48,7 +48,6 @@ u32 MDEC::ReadRegister(u32 offset)
   switch (offset)
   {
     case 0:
-      UpdateStatusRegister();
       return ReadDataRegister();
 
     case 4:
@@ -85,6 +84,7 @@ void MDEC::WriteRegister(u32 offset, u32 value)
 
       m_enable_dma_in = cr.enable_dma_in;
       m_enable_dma_out = cr.enable_dma_out;
+      UpdateStatusRegister();
       UpdateDMARequest();
       return;
     }
@@ -114,6 +114,7 @@ void MDEC::SoftReset()
   m_enable_dma_out = false;
   m_data_in_fifo.Clear();
   m_data_out_fifo.Clear();
+  UpdateStatusRegister();
   UpdateDMARequest();
 }
 
@@ -122,7 +123,7 @@ void MDEC::UpdateStatusRegister()
   m_status.data_out_fifo_empty = m_data_out_fifo.IsEmpty();
   m_status.data_in_fifo_full = m_data_in_fifo.IsFull();
 
-  m_status.command_busy = m_command != Command::None;
+  m_status.command_busy = false;
   m_status.parameter_words_remaining = Truncate16(m_remaining_words - 1);
   m_status.current_block = (m_current_block + 4) % NUM_BLOCKS;
 }
@@ -130,7 +131,7 @@ void MDEC::UpdateStatusRegister()
 void MDEC::UpdateDMARequest()
 {
   // we always want data in if it's enabled
-  const bool data_in_request = m_enable_dma_in && !m_data_in_fifo.IsFull();
+  const bool data_in_request = m_enable_dma_in && m_data_in_fifo.GetSpace() >= (32 * 2) && !m_data_out_fifo.IsFull();
   m_status.data_in_request = data_in_request;
   m_dma->SetRequest(DMA::Channel::MDECin, data_in_request);
 
@@ -144,13 +145,22 @@ u32 MDEC::ReadDataRegister()
 {
   if (m_data_out_fifo.IsEmpty())
   {
-    Log_WarningPrintf("MDEC data out FIFO empty on read");
+    Execute();
-    return UINT32_C(0xFFFFFFFF);
+
+    if (m_data_out_fifo.IsEmpty())
+    {
+      Log_WarningPrintf("MDEC data out FIFO empty on read");
+      return UINT32_C(0xFFFFFFFF);
+    }
   }
 
   const u32 value = m_data_out_fifo.Pop();
-  UpdateStatusRegister();
+  if (m_data_out_fifo.IsEmpty())
-  UpdateDMARequest();
+  {
+    UpdateStatusRegister();
+    UpdateDMARequest();
+  }
+
   return value;
 }
 
@@ -197,29 +207,53 @@ void MDEC::WriteCommandRegister(u32 value)
     m_data_in_fifo.Push(Truncate16(value));
     m_data_in_fifo.Push(Truncate16(value >> 16));
     m_remaining_words--;
-    UpdateDMARequest();
   }
 
+  Execute();
+}
+
+void MDEC::Execute()
+{
   switch (m_command)
   {
     case Command::DecodeMacroblock:
     {
       if (!HandleDecodeMacroblockCommand())
+      {
+        UpdateStatusRegister();
+        UpdateDMARequest();
         return;
+      }
     }
     break;
 
     case Command::SetIqTab:
     {
       if (!HandleSetQuantTableCommand())
+      {
+        UpdateStatusRegister();
+        UpdateDMARequest();
         return;
+      }
     }
     break;
 
     case Command::SetScale:
     {
       if (!HandleSetScaleCommand())
+      {
+        UpdateStatusRegister();
+        UpdateDMARequest();
         return;
+      }
+    }
+    break;
+
+    default:
+    {
+      UpdateStatusRegister();
+      UpdateDMARequest();
+      return;
     }
     break;
   }
@@ -229,6 +263,7 @@ void MDEC::WriteCommandRegister(u32 value)
   m_current_block = 0;
   m_current_coefficient = 64;
   m_current_q_scale = 0;
+  UpdateStatusRegister();
   UpdateDMARequest();
 }
 
@@ -242,7 +277,7 @@ bool MDEC::HandleDecodeMacroblockCommand()
         break;
     }
 
-    return m_remaining_words == 0;
+    return m_data_in_fifo.IsEmpty() && m_remaining_words == 0;
   }
   else
   {
@@ -252,12 +287,24 @@ bool MDEC::HandleDecodeMacroblockCommand()
         break;
     }
 
-    return m_remaining_words == 0;
+    return m_data_in_fifo.IsEmpty() && m_remaining_words == 0;
   }
 }
 
 bool MDEC::DecodeMonoMacroblock()
 {
+  // sufficient space in output?
+  if (m_status.data_output_depth == DataOutputDepth_4Bit)
+  {
+    if (m_data_out_fifo.GetSpace() < (64 / 8))
+      return false;
+  }
+  else
+  {
+    if (m_data_out_fifo.GetSpace() < (64 / 4))
+      return false;
+  }
+
   if (!rl_decode_block(m_blocks[0].data(), m_iq_y.data()))
     return false;
 
@@ -310,7 +357,17 @@ bool MDEC::DecodeMonoMacroblock()
 
 bool MDEC::DecodeColoredMacroblock()
 {
-  std::array<u32, 256> out_rgb;
+  // sufficient space in output?
+  if (m_status.data_output_depth == DataOutputDepth_24Bit)
+  {
+    if (m_data_out_fifo.GetSpace() < (256 - (256 / 4)))
+      return false;
+  }
+  else
+  {
+    if (m_data_out_fifo.GetSpace() < (256 / 2))
+      return false;
+  }
 
   for (; m_current_block < NUM_BLOCKS; m_current_block++)
   {
@@ -322,7 +379,9 @@ bool MDEC::DecodeColoredMacroblock()
 
   // done decoding
   m_current_block = 0;
+  Log_DebugPrintf("Decoded colored macroblock");
 
+  std::array<u32, 256> out_rgb;
   yuv_to_rgb(0, 0, m_blocks[0], m_blocks[1], m_blocks[2], out_rgb);
   yuv_to_rgb(8, 0, m_blocks[0], m_blocks[1], m_blocks[3], out_rgb);
   yuv_to_rgb(0, 8, m_blocks[0], m_blocks[1], m_blocks[4], out_rgb);
@@ -612,8 +671,8 @@ void MDEC::DrawDebugWindow()
   if (ImGui::CollapsingHeader("Status", ImGuiTreeNodeFlags_DefaultOpen))
   {
     ImGui::Text("Data-Out FIFO Empty: %s", m_status.data_out_fifo_empty ? "Yes" : "No");
-    ImGui::Text("Data-In FIFO Empty: %s", m_status.data_in_fifo_full ? "Yes" : "No");
+    ImGui::Text("Data-In FIFO Full: %s", m_status.data_in_fifo_full ? "Yes" : "No");
-    ImGui::Text("Command Busy FIFO Empty: %s", m_status.command_busy ? "Yes" : "No");
+    ImGui::Text("Command Busy: %s", m_status.command_busy ? "Yes" : "No");
     ImGui::Text("Data-In Request: %s", m_status.data_in_request ? "Yes" : "No");
     ImGui::Text("Output Depth: %s", output_depths[static_cast<u8>(m_status.data_output_depth.GetValue())]);
     ImGui::Text("Output Signed: %s", m_status.data_output_signed ? "Yes" : "No");

src/core/mdec.h

@@ -91,6 +91,7 @@ private:
 
   u32 ReadDataRegister();
   void WriteCommandRegister(u32 value);
+  void Execute();
 
   bool HandleDecodeMacroblockCommand();
   bool HandleSetQuantTableCommand();
@@ -114,8 +115,8 @@ private:
   bool m_enable_dma_out = false;
 
   // Even though the DMA is in words, we access the FIFO as halfwords.
-  InlineFIFOQueue<u16, DATA_IN_FIFO_SIZE> m_data_in_fifo;
+  InlineFIFOQueue<u16, DATA_IN_FIFO_SIZE / sizeof(u16)> m_data_in_fifo;
-  InlineFIFOQueue<u32, DATA_OUT_FIFO_SIZE> m_data_out_fifo;
+  InlineFIFOQueue<u32, DATA_OUT_FIFO_SIZE / sizeof(u32)> m_data_out_fifo;
   Command m_command = Command::None;
   u32 m_remaining_words = 0;
 

src/core/system.cpp

@@ -288,7 +288,6 @@ void System::Synchronize()
   m_timers->Execute(pending_ticks);
   m_cdrom->Execute(pending_ticks);
   m_pad->Execute(pending_ticks);
-  m_dma->Execute(pending_ticks);
   m_spu->Execute(pending_ticks);
 }
 

src/duckstation/main.cpp

@@ -96,7 +96,7 @@ int main(int argc, char* argv[])
 #else
   g_pLog->SetConsoleOutputParams(true, nullptr, LOGLEVEL_DEBUG);
   // g_pLog->SetConsoleOutputParams(true, "GPU GPU_HW_OpenGL SPU Pad DigitalController", LOGLEVEL_DEBUG);
-  g_pLog->SetConsoleOutputParams(true, "GPU GPU_HW_OpenGL Pad DigitalController InterruptController", LOGLEVEL_DEBUG);
+  g_pLog->SetConsoleOutputParams(true, "Pad DigitalController InterruptController", LOGLEVEL_DEBUG);
   // g_pLog->SetFilterLevel(LOGLEVEL_TRACE);
   g_pLog->SetFilterLevel(LOGLEVEL_DEBUG);
   // g_pLog->SetFilterLevel(LOGLEVEL_DEV);