From 9410094d9ff89a526ce353f5607245120321962c Mon Sep 17 00:00:00 2001 From: Jordan Woyak Date: Mon, 30 Mar 2026 19:20:11 -0500 Subject: [PATCH] HW/HSP: Change Read/Write interface to use `span` instead of four calls with swapped u64 values. --- Source/Core/Core/HW/DSP.cpp | 35 +++++++--- Source/Core/Core/HW/HSP/HSP.cpp | 12 ++-- Source/Core/Core/HW/HSP/HSP.h | 9 ++- Source/Core/Core/HW/HSP/HSP_Device.h | 10 ++- .../Core/HW/HSP/HSP_DeviceARAMExpansion.cpp | 18 ++--- .../Core/HW/HSP/HSP_DeviceARAMExpansion.h | 10 +-- .../Core/Core/HW/HSP/HSP_DeviceGBPlayer.cpp | 67 +++++++++---------- Source/Core/Core/HW/HSP/HSP_DeviceGBPlayer.h | 4 +- Source/Core/Core/HW/HSP/HSP_DeviceNull.cpp | 5 +- Source/Core/Core/HW/HSP/HSP_DeviceNull.h | 4 +- 10 files changed, 93 insertions(+), 81 deletions(-) diff --git a/Source/Core/Core/HW/DSP.cpp b/Source/Core/Core/HW/DSP.cpp index 3abea2bea8..ae46113e79 100644 --- a/Source/Core/Core/HW/DSP.cpp +++ b/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(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(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; + } } } } diff --git a/Source/Core/Core/HW/HSP/HSP.cpp b/Source/Core/Core/HW/HSP/HSP.cpp index d53d0d2d26..c4cc5af029 100644 --- a/Source/Core/Core/HW/HSP/HSP.cpp +++ b/Source/Core/Core/HW/HSP/HSP.cpp @@ -5,6 +5,8 @@ #include +#include + #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 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 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) diff --git a/Source/Core/Core/HW/HSP/HSP.h b/Source/Core/Core/HW/HSP/HSP.h index b588d26624..f0ff14d5b9 100644 --- a/Source/Core/Core/HW/HSP/HSP.h +++ b/Source/Core/Core/HW/HSP/HSP.h @@ -4,16 +4,15 @@ #pragma once #include +#include #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 data); + void Write(u32 address, std::span data); void DoState(PointerWrap& p); diff --git a/Source/Core/Core/HW/HSP/HSP_Device.h b/Source/Core/Core/HW/HSP/HSP_Device.h index faca30f2b9..33cb69989b 100644 --- a/Source/Core/Core/HW/HSP/HSP_Device.h +++ b/Source/Core/Core/HW/HSP/HSP_Device.h @@ -4,6 +4,7 @@ #pragma once #include +#include #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 data) = 0; + virtual void Write(u32 address, std::span data) = 0; // Savestate support virtual void DoState(PointerWrap& p); diff --git a/Source/Core/Core/HW/HSP/HSP_DeviceARAMExpansion.cpp b/Source/Core/Core/HW/HSP/HSP_DeviceARAMExpansion.cpp index a182d6b932..1ad66c9830 100644 --- a/Source/Core/Core/HW/HSP/HSP_DeviceARAMExpansion.cpp +++ b/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(Common::AllocateMemoryPages(m_size))} { - m_size = MathUtil::NextPowerOf2(Config::Get(Config::MAIN_ARAM_EXPANSION_SIZE)); - m_mask = m_size - 1; - m_ptr = static_cast(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 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 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) diff --git a/Source/Core/Core/HW/HSP/HSP_DeviceARAMExpansion.h b/Source/Core/Core/HW/HSP/HSP_DeviceARAMExpansion.h index b584e4118b..e37a151201 100644 --- a/Source/Core/Core/HW/HSP/HSP_DeviceARAMExpansion.h +++ b/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 data) override; + void Write(u32 address, std::span 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 diff --git a/Source/Core/Core/HW/HSP/HSP_DeviceGBPlayer.cpp b/Source/Core/Core/HW/HSP/HSP_DeviceGBPlayer.cpp index 20a9998e15..68af359028 100644 --- a/Source/Core/Core/HW/HSP/HSP_DeviceGBPlayer.cpp +++ b/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 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 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 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; } } diff --git a/Source/Core/Core/HW/HSP/HSP_DeviceGBPlayer.h b/Source/Core/Core/HW/HSP/HSP_DeviceGBPlayer.h index 18f096e640..9d5fe079fd 100644 --- a/Source/Core/Core/HW/HSP/HSP_DeviceGBPlayer.h +++ b/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 data) override; + void Write(u32 address, std::span data) override; void DoState(PointerWrap& p) override; diff --git a/Source/Core/Core/HW/HSP/HSP_DeviceNull.cpp b/Source/Core/Core/HW/HSP/HSP_DeviceNull.cpp index e11586f97f..6a52697ed3 100644 --- a/Source/Core/Core/HW/HSP/HSP_DeviceNull.cpp +++ b/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 data) { - return 0; } -void CHSPDevice_Null::Write(u32 address, u64 value) +void CHSPDevice_Null::Write(u32 address, std::span data) { } diff --git a/Source/Core/Core/HW/HSP/HSP_DeviceNull.h b/Source/Core/Core/HW/HSP/HSP_DeviceNull.h index 62b500215b..5fcc552779 100644 --- a/Source/Core/Core/HW/HSP/HSP_DeviceNull.h +++ b/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 data) override; + void Write(u32 address, std::span data) override; }; } // namespace HSP