From e3de37e47bf6efd5989d56742e4090f1fa7e069b Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 29 Dec 2020 14:14:41 -0500 Subject: [PATCH 1/3] DSPCore: Make the accelerator private This is only used internally. --- Source/Core/Core/DSP/DSPCore.cpp | 4 +- Source/Core/Core/DSP/DSPCore.h | 3 +- Source/Core/Core/DSP/DSPHWInterface.cpp | 68 +++++++++++++------------ 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/Source/Core/Core/DSP/DSPCore.cpp b/Source/Core/Core/DSP/DSPCore.cpp index 45e7cdfe8a..c58aeb0249 100644 --- a/Source/Core/Core/DSP/DSPCore.cpp +++ b/Source/Core/Core/DSP/DSPCore.cpp @@ -120,7 +120,7 @@ SDSP::~SDSP() = default; bool SDSP::Initialize(const DSPInitOptions& opts) { step_counter = 0; - accelerator = std::make_unique(*this); + m_accelerator = std::make_unique(*this); irom = static_cast(Common::AllocateMemoryPages(DSP_IROM_BYTE_SIZE)); iram = static_cast(Common::AllocateMemoryPages(DSP_IRAM_BYTE_SIZE)); @@ -386,7 +386,7 @@ void SDSP::DoState(PointerWrap& p) p.Do(step_counter); p.DoArray(ifx_regs); - accelerator->DoState(p); + m_accelerator->DoState(p); p.Do(m_mailbox[0]); p.Do(m_mailbox[1]); Common::UnWriteProtectMemory(iram, DSP_IRAM_BYTE_SIZE, false); diff --git a/Source/Core/Core/DSP/DSPCore.h b/Source/Core/Core/DSP/DSPCore.h index 60b317eecf..504890dcb5 100644 --- a/Source/Core/Core/DSP/DSPCore.h +++ b/Source/Core/Core/DSP/DSPCore.h @@ -432,8 +432,6 @@ struct SDSP // Accelerator / DMA / other hardware registers. Not GPRs. std::array ifx_regs{}; - std::unique_ptr accelerator; - // When state saving, all of the above can just be memcpy'd into the save state. // The below needs special handling. u16* iram = nullptr; @@ -455,6 +453,7 @@ private: u16 ReadIFXImpl(u16 address); + std::unique_ptr m_accelerator; std::array, 2> m_mailbox; DSPCore& m_dsp_core; Analyzer m_analyzer; diff --git a/Source/Core/Core/DSP/DSPHWInterface.cpp b/Source/Core/Core/DSP/DSPHWInterface.cpp index 1b9cb2fe99..c0c2e0c36e 100644 --- a/Source/Core/Core/DSP/DSPHWInterface.cpp +++ b/Source/Core/Core/DSP/DSPHWInterface.cpp @@ -142,40 +142,42 @@ void SDSP::WriteIFX(u32 address, u16 value) break; case DSP_ACSAH: - accelerator->SetStartAddress(value << 16 | static_cast(accelerator->GetStartAddress())); + m_accelerator->SetStartAddress(value << 16 | + static_cast(m_accelerator->GetStartAddress())); break; case DSP_ACSAL: - accelerator->SetStartAddress(static_cast(accelerator->GetStartAddress() >> 16) << 16 | - value); - break; - case DSP_ACEAH: - accelerator->SetEndAddress(value << 16 | static_cast(accelerator->GetEndAddress())); - break; - case DSP_ACEAL: - accelerator->SetEndAddress(static_cast(accelerator->GetEndAddress() >> 16) << 16 | value); - break; - case DSP_ACCAH: - accelerator->SetCurrentAddress(value << 16 | - static_cast(accelerator->GetCurrentAddress())); - break; - case DSP_ACCAL: - accelerator->SetCurrentAddress(static_cast(accelerator->GetCurrentAddress() >> 16) << 16 | + m_accelerator->SetStartAddress(static_cast(m_accelerator->GetStartAddress() >> 16) << 16 | value); break; + case DSP_ACEAH: + m_accelerator->SetEndAddress(value << 16 | static_cast(m_accelerator->GetEndAddress())); + break; + case DSP_ACEAL: + m_accelerator->SetEndAddress(static_cast(m_accelerator->GetEndAddress() >> 16) << 16 | + value); + break; + case DSP_ACCAH: + m_accelerator->SetCurrentAddress(value << 16 | + static_cast(m_accelerator->GetCurrentAddress())); + break; + case DSP_ACCAL: + m_accelerator->SetCurrentAddress( + static_cast(m_accelerator->GetCurrentAddress() >> 16) << 16 | value); + break; case DSP_FORMAT: - accelerator->SetSampleFormat(value); + m_accelerator->SetSampleFormat(value); break; case DSP_YN1: - accelerator->SetYn1(value); + m_accelerator->SetYn1(value); break; case DSP_YN2: - accelerator->SetYn2(value); + m_accelerator->SetYn2(value); break; case DSP_PRED_SCALE: - accelerator->SetPredScale(value); + m_accelerator->SetPredScale(value); break; case DSP_ACDATA1: // Accelerator write (Zelda type) - "UnkZelda" - accelerator->WriteD3(value); + m_accelerator->WriteD3(value); break; default: @@ -222,29 +224,29 @@ u16 SDSP::ReadIFXImpl(u16 address) return ifx_regs[address & 0xFF]; case DSP_ACSAH: - return static_cast(accelerator->GetStartAddress() >> 16); + return static_cast(m_accelerator->GetStartAddress() >> 16); case DSP_ACSAL: - return static_cast(accelerator->GetStartAddress()); + return static_cast(m_accelerator->GetStartAddress()); case DSP_ACEAH: - return static_cast(accelerator->GetEndAddress() >> 16); + return static_cast(m_accelerator->GetEndAddress() >> 16); case DSP_ACEAL: - return static_cast(accelerator->GetEndAddress()); + return static_cast(m_accelerator->GetEndAddress()); case DSP_ACCAH: - return static_cast(accelerator->GetCurrentAddress() >> 16); + return static_cast(m_accelerator->GetCurrentAddress() >> 16); case DSP_ACCAL: - return static_cast(accelerator->GetCurrentAddress()); + return static_cast(m_accelerator->GetCurrentAddress()); case DSP_FORMAT: - return accelerator->GetSampleFormat(); + return m_accelerator->GetSampleFormat(); case DSP_YN1: - return accelerator->GetYn1(); + return m_accelerator->GetYn1(); case DSP_YN2: - return accelerator->GetYn2(); + return m_accelerator->GetYn2(); case DSP_PRED_SCALE: - return accelerator->GetPredScale(); + return m_accelerator->GetPredScale(); case DSP_ACCELERATOR: // ADPCM Accelerator reads - return accelerator->Read(reinterpret_cast(&ifx_regs[DSP_COEF_A1_0])); + return m_accelerator->Read(reinterpret_cast(&ifx_regs[DSP_COEF_A1_0])); case DSP_ACDATA1: // Accelerator reads (Zelda type) - "UnkZelda" - return accelerator->ReadD3(); + return m_accelerator->ReadD3(); default: { From 5fb1f0bfd34aba635da74cb760c426a3a9f962dc Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 29 Dec 2020 14:22:37 -0500 Subject: [PATCH 2/3] DSPCore: Make ifx registers private These aren't used externally, so they can be made private. --- Source/Core/Core/DSP/DSPCore.cpp | 2 +- Source/Core/Core/DSP/DSPCore.h | 6 ++--- Source/Core/Core/DSP/DSPHWInterface.cpp | 30 ++++++++++++------------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Source/Core/Core/DSP/DSPCore.cpp b/Source/Core/Core/DSP/DSPCore.cpp index c58aeb0249..a12bee76c2 100644 --- a/Source/Core/Core/DSP/DSPCore.cpp +++ b/Source/Core/Core/DSP/DSPCore.cpp @@ -385,7 +385,7 @@ void SDSP::DoState(PointerWrap& p) } p.Do(step_counter); - p.DoArray(ifx_regs); + p.DoArray(m_ifx_regs); m_accelerator->DoState(p); p.Do(m_mailbox[0]); p.Do(m_mailbox[1]); diff --git a/Source/Core/Core/DSP/DSPCore.h b/Source/Core/Core/DSP/DSPCore.h index 504890dcb5..e2f43f391c 100644 --- a/Source/Core/Core/DSP/DSPCore.h +++ b/Source/Core/Core/DSP/DSPCore.h @@ -429,9 +429,6 @@ struct SDSP u32 iram_crc = 0; u64 step_counter = 0; - // Accelerator / DMA / other hardware registers. Not GPRs. - std::array ifx_regs{}; - // When state saving, all of the above can just be memcpy'd into the save state. // The below needs special handling. u16* iram = nullptr; @@ -453,6 +450,9 @@ private: u16 ReadIFXImpl(u16 address); + // Accelerator / DMA / other hardware registers. Not GPRs. + std::array m_ifx_regs{}; + std::unique_ptr m_accelerator; std::array, 2> m_mailbox; DSPCore& m_dsp_core; diff --git a/Source/Core/Core/DSP/DSPHWInterface.cpp b/Source/Core/Core/DSP/DSPHWInterface.cpp index c0c2e0c36e..f182d7460f 100644 --- a/Source/Core/Core/DSP/DSPHWInterface.cpp +++ b/Source/Core/Core/DSP/DSPHWInterface.cpp @@ -23,7 +23,7 @@ namespace DSP { void SDSP::InitializeIFX() { - ifx_regs.fill(0); + m_ifx_regs.fill(0); GetMailbox(Mailbox::CPU).store(0); GetMailbox(Mailbox::DSP).store(0); @@ -118,14 +118,14 @@ void SDSP::WriteIFX(u32 address, u16 value) break; case DSP_DSBL: - ifx_regs[DSP_DSBL] = value; - ifx_regs[DSP_DSCR] |= 4; // Doesn't really matter since we do DMA instantly - if (!ifx_regs[DSP_AMDM]) + m_ifx_regs[DSP_DSBL] = value; + m_ifx_regs[DSP_DSCR] |= 4; // Doesn't really matter since we do DMA instantly + if (!m_ifx_regs[DSP_AMDM]) DoDMA(); else NOTICE_LOG_FMT(DSPLLE, "Masked DMA skipped"); - ifx_regs[DSP_DSCR] &= ~4; - ifx_regs[DSP_DSBL] = 0; + m_ifx_regs[DSP_DSCR] &= ~4; + m_ifx_regs[DSP_DSBL] = 0; break; case DSP_GAIN: @@ -138,7 +138,7 @@ void SDSP::WriteIFX(u32 address, u16 value) case DSP_DSMAH: case DSP_DSMAL: case DSP_DSCR: - ifx_regs[address & 0xFF] = value; + m_ifx_regs[address & 0xFF] = value; break; case DSP_ACSAH: @@ -199,7 +199,7 @@ void SDSP::WriteIFX(u32 address, u16 value) { ERROR_LOG_FMT(DSPLLE, "{:04x} MW {:04x} ({:04x})", pc, address, value); } - ifx_regs[address & 0xFF] = value; + m_ifx_regs[address & 0xFF] = value; break; } } @@ -221,7 +221,7 @@ u16 SDSP::ReadIFXImpl(u16 address) return ReadMailboxLow(Mailbox::CPU); case DSP_DSCR: - return ifx_regs[address & 0xFF]; + return m_ifx_regs[address & 0xFF]; case DSP_ACSAH: return static_cast(m_accelerator->GetStartAddress() >> 16); @@ -244,13 +244,13 @@ u16 SDSP::ReadIFXImpl(u16 address) case DSP_PRED_SCALE: return m_accelerator->GetPredScale(); case DSP_ACCELERATOR: // ADPCM Accelerator reads - return m_accelerator->Read(reinterpret_cast(&ifx_regs[DSP_COEF_A1_0])); + return m_accelerator->Read(reinterpret_cast(&m_ifx_regs[DSP_COEF_A1_0])); case DSP_ACDATA1: // Accelerator reads (Zelda type) - "UnkZelda" return m_accelerator->ReadD3(); default: { - const u16 ifx_reg = ifx_regs[address & 0xFF]; + const u16 ifx_reg = m_ifx_regs[address & 0xFF]; if ((address & 0xff) >= 0xa0) { @@ -323,10 +323,10 @@ const u8* SDSP::DDMAOut(u16 dsp_addr, u32 addr, u32 size) void SDSP::DoDMA() { - const u32 addr = (ifx_regs[DSP_DSMAH] << 16) | ifx_regs[DSP_DSMAL]; - const u16 ctl = ifx_regs[DSP_DSCR]; - const u16 dsp_addr = ifx_regs[DSP_DSPA] * 2; - const u16 len = ifx_regs[DSP_DSBL]; + const u32 addr = (m_ifx_regs[DSP_DSMAH] << 16) | m_ifx_regs[DSP_DSMAL]; + const u16 ctl = m_ifx_regs[DSP_DSCR]; + const u16 dsp_addr = m_ifx_regs[DSP_DSPA] * 2; + const u16 len = m_ifx_regs[DSP_DSBL]; if (len > 0x4000) { From f4e1f48b4fcfde29a50d297b5f7ec62377222442 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 29 Dec 2020 14:32:08 -0500 Subject: [PATCH 3/3] DSPCore: Make IRAM CRC and step counter private We can construct an API around these two members to allow them to be private. --- Source/Core/Core/DSP/DSPCore.cpp | 4 ++-- Source/Core/Core/DSP/DSPCore.h | 14 ++++++++++---- Source/Core/Core/DSP/DSPHWInterface.cpp | 2 +- .../Core/Core/DSP/Interpreter/DSPInterpreter.cpp | 2 +- Source/Core/Core/HW/DSPLLE/DSPHost.cpp | 2 +- 5 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/DSP/DSPCore.cpp b/Source/Core/Core/DSP/DSPCore.cpp index a12bee76c2..ca5eff4269 100644 --- a/Source/Core/Core/DSP/DSPCore.cpp +++ b/Source/Core/Core/DSP/DSPCore.cpp @@ -119,7 +119,7 @@ SDSP::~SDSP() = default; bool SDSP::Initialize(const DSPInitOptions& opts) { - step_counter = 0; + m_step_counter = 0; m_accelerator = std::make_unique(*this); irom = static_cast(Common::AllocateMemoryPages(DSP_IROM_BYTE_SIZE)); @@ -384,7 +384,7 @@ void SDSP::DoState(PointerWrap& p) p.Do(stack); } - p.Do(step_counter); + p.Do(m_step_counter); p.DoArray(m_ifx_regs); m_accelerator->DoState(p); p.Do(m_mailbox[0]); diff --git a/Source/Core/Core/DSP/DSPCore.h b/Source/Core/Core/DSP/DSPCore.h index e2f43f391c..298019aceb 100644 --- a/Source/Core/Core/DSP/DSPCore.h +++ b/Source/Core/Core/DSP/DSPCore.h @@ -396,6 +396,12 @@ struct SDSP // Writes a value to a given register. void WriteRegister(size_t reg, u16 val); + // Advances the step counter used for debugging purposes. + void AdvanceStepCounter() { ++m_step_counter; } + + // Sets the calculated IRAM CRC for debugging purposes. + void SetIRAMCRC(u32 crc) { m_iram_crc = crc; } + // Saves and loads any necessary state. void DoState(PointerWrap& p); @@ -425,10 +431,6 @@ struct SDSP // the stack overflows, you're screwed. u16 reg_stacks[4][DSP_STACK_DEPTH]{}; - // For debugging. - u32 iram_crc = 0; - u64 step_counter = 0; - // When state saving, all of the above can just be memcpy'd into the save state. // The below needs special handling. u16* iram = nullptr; @@ -450,6 +452,10 @@ private: u16 ReadIFXImpl(u16 address); + // For debugging. + u32 m_iram_crc = 0; + u64 m_step_counter = 0; + // Accelerator / DMA / other hardware registers. Not GPRs. std::array m_ifx_regs{}; diff --git a/Source/Core/Core/DSP/DSPHWInterface.cpp b/Source/Core/Core/DSP/DSPHWInterface.cpp index f182d7460f..ec5d449057 100644 --- a/Source/Core/Core/DSP/DSPHWInterface.cpp +++ b/Source/Core/Core/DSP/DSPHWInterface.cpp @@ -290,7 +290,7 @@ const u8* SDSP::IDMAIn(u16 dsp_addr, u32 addr, u32 size) Host::CodeLoaded(m_dsp_core, addr, size); NOTICE_LOG_FMT(DSPLLE, "*** Copy new UCode from {:#010x} to {:#06x} (crc: {:#08x})", addr, - dsp_addr, iram_crc); + dsp_addr, m_iram_crc); return reinterpret_cast(iram) + dsp_addr; } diff --git a/Source/Core/Core/DSP/Interpreter/DSPInterpreter.cpp b/Source/Core/Core/DSP/Interpreter/DSPInterpreter.cpp index b900d0585c..c142123fdc 100644 --- a/Source/Core/Core/DSP/Interpreter/DSPInterpreter.cpp +++ b/Source/Core/Core/DSP/Interpreter/DSPInterpreter.cpp @@ -49,7 +49,7 @@ void Interpreter::Step() auto& state = m_dsp_core.DSPState(); m_dsp_core.CheckExceptions(); - state.step_counter++; + state.AdvanceStepCounter(); const u16 opc = state.FetchInstruction(); ExecuteInstruction(UDSPInstruction{opc}); diff --git a/Source/Core/Core/HW/DSPLLE/DSPHost.cpp b/Source/Core/Core/HW/DSPLLE/DSPHost.cpp index 65e26ce238..f02472a081 100644 --- a/Source/Core/Core/HW/DSPLLE/DSPHost.cpp +++ b/Source/Core/Core/HW/DSPLLE/DSPHost.cpp @@ -77,7 +77,7 @@ void CodeLoaded(DSPCore& dsp, const u8* ptr, size_t size) { auto& state = dsp.DSPState(); const u32 iram_crc = Common::HashEctor(ptr, size); - state.iram_crc = iram_crc; + state.SetIRAMCRC(iram_crc); if (SConfig::GetInstance().m_DumpUCode) {