HW/HSP: Change Read/Write interface to use span<u8, 32> instead of four calls with swapped u64 values.

Source/Core/Core/HW/DSP.cpp

@@ -505,12 +505,20 @@ void DSPManager::Do_ARAM_DMA()
     }
     else if (!m_aram.wii_mode)
     {
-      while (m_aram_dma.Cnt.count)
+      auto* mm_ptr = memory.GetPointerForRange(m_aram_dma.MMAddr, m_aram_dma.Cnt.count);
+      if (mm_ptr != nullptr)
       {
-        memory.Write_U64(m_system.GetHSP().Read(m_aram_dma.ARAddr), m_aram_dma.MMAddr);
-        m_aram_dma.MMAddr += 8;
-        m_aram_dma.ARAddr += 8;
-        m_aram_dma.Cnt.count -= 8;
+        auto& hsp = m_system.GetHSP();
+        while (m_aram_dma.Cnt.count)
+        {
+          hsp.Read(m_aram_dma.ARAddr,
+                   std::span<u8, HSP::TRANSFER_SIZE>(mm_ptr, HSP::TRANSFER_SIZE));
+
+          m_aram_dma.MMAddr += HSP::TRANSFER_SIZE;
+          m_aram_dma.ARAddr += HSP::TRANSFER_SIZE;
+          m_aram_dma.Cnt.count -= HSP::TRANSFER_SIZE;
+          mm_ptr += HSP::TRANSFER_SIZE;
+        }
       }
     }
   }
@@ -557,13 +565,20 @@ void DSPManager::Do_ARAM_DMA()
     }
     else if (!m_aram.wii_mode)
     {
-      while (m_aram_dma.Cnt.count)
+      auto* mm_ptr = memory.GetPointerForRange(m_aram_dma.MMAddr, m_aram_dma.Cnt.count);
+      if (mm_ptr != nullptr)
       {
-        m_system.GetHSP().Write(m_aram_dma.ARAddr, memory.Read_U64(m_aram_dma.MMAddr));
+        auto& hsp = m_system.GetHSP();
+        while (m_aram_dma.Cnt.count)
+        {
+          hsp.Write(m_aram_dma.ARAddr,
+                    std::span<const u8, HSP::TRANSFER_SIZE>(mm_ptr, HSP::TRANSFER_SIZE));
 
-        m_aram_dma.MMAddr += 8;
-        m_aram_dma.ARAddr += 8;
-        m_aram_dma.Cnt.count -= 8;
+          m_aram_dma.MMAddr += HSP::TRANSFER_SIZE;
+          m_aram_dma.ARAddr += HSP::TRANSFER_SIZE;
+          m_aram_dma.Cnt.count -= HSP::TRANSFER_SIZE;
+          mm_ptr += HSP::TRANSFER_SIZE;
+        }
       }
     }
   }

Source/Core/Core/HW/HSP/HSP.cpp

@@ -5,6 +5,8 @@
 
 #include <memory>
 
+#include <fmt/ranges.h>
+
 #include "Common/ChunkFile.h"
 #include "Core/Config/MainSettings.h"
 #include "Core/HW/HSP/HSP_Device.h"
@@ -24,18 +26,18 @@ void HSPManager::Shutdown()
   m_device.reset();
 }
 
-u64 HSPManager::Read(u32 address)
+void HSPManager::Read(u32 address, std::span<u8, TRANSFER_SIZE> data)
 {
   DEBUG_LOG_FMT(HSP, "HSP read from 0x{:08x}", address);
 
-  return m_device->Read(address);
+  m_device->Read(address, data);
 }
 
-void HSPManager::Write(u32 address, u64 value)
+void HSPManager::Write(u32 address, std::span<const u8, TRANSFER_SIZE> data)
 {
-  DEBUG_LOG_FMT(HSP, "HSP write to 0x{:08x}: 0x{:016x}", address, value);
+  DEBUG_LOG_FMT(HSP, "HSP write to 0x{:08x}: {:02x}", address, fmt::join(data, " "));
 
-  m_device->Write(address, value);
+  m_device->Write(address, data);
 }
 
 void HSPManager::DoState(PointerWrap& p)

Source/Core/Core/HW/HSP/HSP.h

@@ -4,16 +4,15 @@
 #pragma once
 
 #include <memory>
+#include <span>
 
 #include "Common/CommonTypes.h"
+#include "Core/HW/HSP/HSP_Device.h"
 
 class PointerWrap;
 
 namespace HSP
 {
-class IHSPDevice;
-enum class HSPDeviceType : int;
-
 class HSPManager
 {
 public:
@@ -27,8 +26,8 @@ public:
   void Init();
   void Shutdown();
 
-  u64 Read(u32 address);
-  void Write(u32 address, u64 value);
+  void Read(u32 address, std::span<u8, TRANSFER_SIZE> data);
+  void Write(u32 address, std::span<const u8, TRANSFER_SIZE> data);
 
   void DoState(PointerWrap& p);
 

Source/Core/Core/HW/HSP/HSP_Device.h

@@ -4,6 +4,7 @@
 #pragma once
 
 #include <memory>
+#include <span>
 
 #include "Common/CommonTypes.h"
 
@@ -11,6 +12,9 @@ class PointerWrap;
 
 namespace HSP
 {
+
+static constexpr std::size_t TRANSFER_SIZE = 32;
+
 enum class HSPDeviceType : int
 {
   None,
@@ -25,8 +29,10 @@ public:
 
   virtual HSPDeviceType GetDeviceType() const = 0;
 
-  virtual void Write(u32 address, u64 value) = 0;
-  virtual u64 Read(u32 address) = 0;
+  // Note: DSPManager::RegisterMMIO ensures addresses are 32 byte aligned.
+
+  virtual void Read(u32 address, std::span<u8, TRANSFER_SIZE> data) = 0;
+  virtual void Write(u32 address, std::span<const u8, TRANSFER_SIZE> data) = 0;
 
   // Savestate support
   virtual void DoState(PointerWrap& p);

Source/Core/Core/HW/HSP/HSP_DeviceARAMExpansion.cpp

@@ -8,36 +8,30 @@
 #include "Common/ChunkFile.h"
 #include "Common/MathUtil.h"
 #include "Common/MemoryUtil.h"
-#include "Common/Swap.h"
 
 #include "Core/Config/MainSettings.h"
 
 namespace HSP
 {
 CHSPDevice_ARAMExpansion::CHSPDevice_ARAMExpansion()
+    : m_size{MathUtil::NextPowerOf2(Config::Get(Config::MAIN_ARAM_EXPANSION_SIZE))},
+      m_mask{m_size - 1}, m_ptr{static_cast<u8*>(Common::AllocateMemoryPages(m_size))}
 {
-  m_size = MathUtil::NextPowerOf2(Config::Get(Config::MAIN_ARAM_EXPANSION_SIZE));
-  m_mask = m_size - 1;
-  m_ptr = static_cast<u8*>(Common::AllocateMemoryPages(m_size));
 }
 
 CHSPDevice_ARAMExpansion::~CHSPDevice_ARAMExpansion()
 {
   Common::FreeMemoryPages(m_ptr, m_size);
-  m_ptr = nullptr;
 }
 
-u64 CHSPDevice_ARAMExpansion::Read(u32 address)
+void CHSPDevice_ARAMExpansion::Read(u32 address, std::span<u8, TRANSFER_SIZE> data)
 {
-  u64 value;
-  std::memcpy(&value, &m_ptr[address & m_mask], sizeof(value));
-  return Common::swap64(value);
+  std::memcpy(data.data(), m_ptr + (address & m_mask), data.size());
 }
 
-void CHSPDevice_ARAMExpansion::Write(u32 address, u64 value)
+void CHSPDevice_ARAMExpansion::Write(u32 address, std::span<const u8, TRANSFER_SIZE> data)
 {
-  value = Common::swap64(value);
-  std::memcpy(&m_ptr[address & m_mask], &value, sizeof(value));
+  std::memcpy(m_ptr + (address & m_mask), data.data(), data.size());
 }
 
 void CHSPDevice_ARAMExpansion::DoState(PointerWrap& p)

Source/Core/Core/HW/HSP/HSP_DeviceARAMExpansion.h

@@ -15,14 +15,14 @@ public:
 
   HSPDeviceType GetDeviceType() const override { return HSPDeviceType::ARAMExpansion; }
 
-  void Write(u32 address, u64 value) override;
-  u64 Read(u32 address) override;
+  void Read(u32 address, std::span<u8, TRANSFER_SIZE> data) override;
+  void Write(u32 address, std::span<const u8, TRANSFER_SIZE> data) override;
 
   void DoState(PointerWrap&) override;
 
 private:
-  u32 m_size;
-  u32 m_mask;
-  u8* m_ptr = nullptr;
+  const u32 m_size;
+  const u32 m_mask;
+  u8* const m_ptr = nullptr;
 };
 }  // namespace HSP

Source/Core/Core/HW/HSP/HSP_DeviceGBPlayer.cpp

@@ -17,6 +17,7 @@
 
 #include "Common/ChunkFile.h"
 #include "Common/Logging/Log.h"
+#include "Common/Swap.h"
 
 #include "Core/Config/MainSettings.h"
 #include "Core/CoreTiming.h"
@@ -50,8 +51,7 @@ constexpr u8 CONTROL_MASK_IRQ = 0x10;
 // constexpr u8 CONTROL_LINK_CABLE = 0x40;
 // constexpr u8 CONTROL_LINK_ENABLE = 0x80;
 
-constexpr u32 COMMAND_ADDRESS_MASK = 0x1f;
-constexpr u32 AV_ADDRESS_MASK = 0xff8;
+constexpr u32 AV_ADDRESS_MASK = 0xfe0;
 
 class CGBPlayer_Dummy final : public HSP::IGBPlayer
 {
@@ -232,13 +232,13 @@ void CGBPlayer_mGBA::ReadScanlines(std::span<u32, AV_REGION_SIZE> scanlines)
   {
     const u32 color =
         M_RGB8_TO_RGB5(video_buffer[(m_current_scanline_index * GBA_VIDEO_HORIZONTAL_PIXELS) + i]);
-    u32 c = color & 0xFF;
-    c |= (color & 0xFF00u) << 8;
+    u32 c = color >> 8u;
+    c |= (color & 0xffu) << 16u;
     scanlines[i] = c | (c << 8);  // Parity
   }
 
   if (m_current_scanline_index == 0)
-    scanlines[0] |= 0x80800000;
+    scanlines[0] |= 0x00008080;
 
   m_current_scanline_index += 4;
 
@@ -398,14 +398,13 @@ CHSPDevice_GBPlayer::CHSPDevice_GBPlayer(Core::System& system) : m_system{system
 #endif
 }
 
-u64 CHSPDevice_GBPlayer::Read(u32 address)
+void CHSPDevice_GBPlayer::Read(u32 address, std::span<u8, TRANSFER_SIZE> data)
 {
-  u64 value = 0;
   switch (GBPRegister(address >> 20))
   {
   case GBPRegister::Test:
   {
-    std::memcpy(&value, &m_test[address & COMMAND_ADDRESS_MASK], sizeof(value));
+    std::ranges::copy(m_test, data.data());
     break;
   }
   case GBPRegister::Control:
@@ -423,14 +422,19 @@ u64 CHSPDevice_GBPlayer::Read(u32 address)
       m_control &= ~(CONTROL_CART_DETECTED | CONTROL_CART_INSERTED);
     }
 
-    value = m_control * 0x0101010101010101ULL;
+    std::ranges::fill(data, m_control);
     break;
   }
   case GBPRegister::IRQ:
   {
-    value = m_irq * 0x0000000100000001ULL;
-    value |= (m_irq & 0xFF00) * 0x0001010000010100ULL;
-    value &= 0xFFFF7FFFFFFF7FFFULL;
+    u32 value = m_irq;
+    value |= (m_irq & 0xff00) * 0x0100'0001u;
+    value &= 0x7fff'ffffu;
+
+    for (std::size_t i = 0; i != data.size(); i += sizeof(value))
+    {
+      std::memcpy(data.data() + i, &value, sizeof(value));
+    }
     break;
   }
   case GBPRegister::Video:
@@ -441,8 +445,7 @@ u64 CHSPDevice_GBPlayer::Read(u32 address)
 
     const u32 scanlines_pos = offset / 4;
 
-    value = u64(m_scanlines[scanlines_pos]) << 32;
-    value |= m_scanlines[scanlines_pos + 1];
+    std::memcpy(data.data(), m_scanlines.data() + scanlines_pos, data.size());
     break;
   }
   case GBPRegister::Audio:
@@ -451,10 +454,13 @@ u64 CHSPDevice_GBPlayer::Read(u32 address)
     if (offset == 0)
       m_gbp->ReadAudio(m_audio);
 
-    const u32 audio_pos = offset / 4;
+    u32 audio_pos = offset / 4;
 
-    value = 0x0101010100000000ULL * m_audio[audio_pos];
-    value |= 0x01010101ULL * m_audio[audio_pos + 1];
+    for (std::size_t i = 0; i != data.size(); i += sizeof(u32))
+    {
+      std::memset(data.data() + i, m_audio[audio_pos], sizeof(u32));
+      ++audio_pos;
+    }
     break;
   }
   default:
@@ -463,27 +469,22 @@ u64 CHSPDevice_GBPlayer::Read(u32 address)
     break;
   }
   }
-  return value;
 }
 
-void CHSPDevice_GBPlayer::Write(u32 address, u64 value)
+void CHSPDevice_GBPlayer::Write(u32 address, std::span<const u8, TRANSFER_SIZE> data)
 {
-  // This seems to compensate for DSP.cpp turning 32byte writes into 4 x 8byte writes.
-  // TODO: Rectify that and adjust this.
-  constexpr u32 last_chunk = 0x18;
-
   switch (GBPRegister(address >> 20))
   {
   case GBPRegister::Test:
   {
-    value = ~value;
-    std::memcpy(&m_test[address & COMMAND_ADDRESS_MASK], &value, sizeof(value));
+    u8* dest = m_test.data();
+    for (u8 value : data)
+      *(dest++) = value ^ 0xffu;
     break;
   }
   case GBPRegister::Control:
   {
-    if ((address & COMMAND_ADDRESS_MASK) != last_chunk)
-      break;
+    const u8 value = data[0x1f];
 
     if (!(m_control & (CONTROL_3V | CONTROL_5V)) && (value & (CONTROL_3V | CONTROL_5V)))
     {
@@ -506,24 +507,20 @@ void CHSPDevice_GBPlayer::Write(u32 address, u64 value)
   }
   case GBPRegister::IRQ:
   {
-    if ((address & COMMAND_ADDRESS_MASK) != last_chunk)
-      break;
-
-    m_irq &= ~value;
+    const u16 value = Common::swap16(data.data() + 0x1e);
+    m_irq &= u16(~value);
     UpdateInterrupts();
     break;
   }
   case GBPRegister::Keypad:
   {
-    if ((address & COMMAND_ADDRESS_MASK) != last_chunk)
-      break;
-
+    const u16 value = Common::swap16(data.data() + 0x1e);
     m_gbp->SetKeys(value & 0x3FF);
     break;
   }
   default:
   {
-    WARN_LOG_FMT(HSP, "GBPlayer: Unknown write to 0x{:08x}: 0x{:016x}", address, value);
+    WARN_LOG_FMT(HSP, "GBPlayer: Unknown write to 0x{:08x}", address);
     break;
   }
   }

Source/Core/Core/HW/HSP/HSP_DeviceGBPlayer.h

@@ -59,8 +59,8 @@ public:
 
   HSPDeviceType GetDeviceType() const override { return HSPDeviceType::GBPlayer; }
 
-  void Write(u32 address, u64 value) override;
-  u64 Read(u32 address) override;
+  void Read(u32 address, std::span<u8, TRANSFER_SIZE> data) override;
+  void Write(u32 address, std::span<const u8, TRANSFER_SIZE> data) override;
 
   void DoState(PointerWrap& p) override;
 

Source/Core/Core/HW/HSP/HSP_DeviceNull.cpp

@@ -6,12 +6,11 @@
 namespace HSP
 {
 
-u64 CHSPDevice_Null::Read(u32 address)
+void CHSPDevice_Null::Read(u32 address, std::span<u8, TRANSFER_SIZE> data)
 {
-  return 0;
 }
 
-void CHSPDevice_Null::Write(u32 address, u64 value)
+void CHSPDevice_Null::Write(u32 address, std::span<const u8, TRANSFER_SIZE> data)
 {
 }
 

Source/Core/Core/HW/HSP/HSP_DeviceNull.h

@@ -13,7 +13,7 @@ class CHSPDevice_Null final : public IHSPDevice
 public:
   HSPDeviceType GetDeviceType() const override { return HSPDeviceType::None; }
 
-  void Write(u32 address, u64 value) override;
-  u64 Read(u32 address) override;
+  void Read(u32 address, std::span<u8, TRANSFER_SIZE> data) override;
+  void Write(u32 address, std::span<const u8, TRANSFER_SIZE> data) override;
 };
 }  // namespace HSP