From 72ff0c53375b487c5a95ba9702d391a19c629b88 Mon Sep 17 00:00:00 2001 From: SachinVin <26602104+SachinVin@users.noreply.github.com> Date: Wed, 4 Oct 2023 19:14:59 +0530 Subject: [PATCH] AudioCore: Refactor DSP interrupt handling (#7026) --- src/audio_core/dsp_interface.h | 7 +++--- src/audio_core/hle/hle.cpp | 37 +++++++++++++--------------- src/audio_core/hle/hle.h | 3 ++- src/audio_core/lle/lle.cpp | 32 ++++++------------------ src/audio_core/lle/lle.h | 6 ++--- src/core/hle/service/dsp/dsp_dsp.cpp | 15 ++++++++--- src/core/hle/service/dsp/dsp_dsp.h | 7 +++--- src/tests/audio_core/hle/hle.cpp | 28 +++++++-------------- src/tests/audio_core/lle/lle.cpp | 23 +++-------------- 9 files changed, 61 insertions(+), 97 deletions(-) diff --git a/src/audio_core/dsp_interface.h b/src/audio_core/dsp_interface.h index f1b315e38..f62980cdd 100644 --- a/src/audio_core/dsp_interface.h +++ b/src/audio_core/dsp_interface.h @@ -14,7 +14,7 @@ #include "core/memory.h" namespace Service::DSP { -class DSP_DSP; +enum class InterruptType : u32; } // namespace Service::DSP namespace AudioCore { @@ -85,8 +85,9 @@ public: /// Returns a reference to the array backing DSP memory virtual std::array& GetDspMemory() = 0; - /// Sets the dsp class that we trigger interrupts for - virtual void SetServiceToInterrupt(std::weak_ptr dsp) = 0; + /// Sets the handler for the interrupts we trigger + virtual void SetInterruptHandler( + std::function handler) = 0; /// Loads the DSP program virtual void LoadComponent(std::span buffer) = 0; diff --git a/src/audio_core/hle/hle.cpp b/src/audio_core/hle/hle.cpp index f24176ca8..6b331b277 100644 --- a/src/audio_core/hle/hle.cpp +++ b/src/audio_core/hle/hle.cpp @@ -34,8 +34,7 @@ SERIALIZE_EXPORT_IMPL(AudioCore::DspHle) -using InterruptType = Service::DSP::DSP_DSP::InterruptType; -using Service::DSP::DSP_DSP; +using InterruptType = Service::DSP::InterruptType; namespace AudioCore { @@ -71,7 +70,8 @@ public: std::array& GetDspMemory(); - void SetServiceToInterrupt(std::weak_ptr dsp); + void SetInterruptHandler( + std::function handler); private: void ResetPipes(); @@ -105,7 +105,7 @@ private: std::unique_ptr decoder{}; - std::weak_ptr dsp_dsp{}; + std::function interrupt_handler{}; template void serialize(Archive& ar, const unsigned int) { @@ -114,7 +114,8 @@ private: ar& dsp_memory.raw_memory; ar& sources; ar& mixers; - ar& dsp_dsp; + // interrupt_handler is function pointer and cant be serialised, fortunately though, it + // should be registerd before the game has started } friend class boost::serialization::access; }; @@ -320,10 +321,8 @@ void DspHle::Impl::PipeWrite(DspPipe pipe_number, std::span buffer) { pipe_data[static_cast(pipe_number)].resize(sizeof(value)); std::memcpy(pipe_data[static_cast(pipe_number)].data(), &value, sizeof(value)); } - auto dsp = dsp_dsp.lock(); - if (dsp) { - dsp->SignalInterrupt(InterruptType::Pipe, DspPipe::Binary); - } + + interrupt_handler(InterruptType::Pipe, DspPipe::Binary); break; } default: @@ -338,8 +337,9 @@ std::array& DspHle::Impl::GetDspMemory() { return dsp_memory.raw_memory; } -void DspHle::Impl::SetServiceToInterrupt(std::weak_ptr dsp) { - dsp_dsp = std::move(dsp); +void DspHle::Impl::SetInterruptHandler( + std::function handler) { + interrupt_handler = handler; } void DspHle::Impl::ResetPipes() { @@ -386,9 +386,7 @@ void DspHle::Impl::AudioPipeWriteStructAddresses() { WriteU16(DspPipe::Audio, addr); } // Signal that we have data on this pipe. - if (auto service = dsp_dsp.lock()) { - service->SignalInterrupt(InterruptType::Pipe, DspPipe::Audio); - } + interrupt_handler(InterruptType::Pipe, DspPipe::Audio); } size_t DspHle::Impl::CurrentRegionIndex() const { @@ -464,9 +462,7 @@ bool DspHle::Impl::Tick() { void DspHle::Impl::AudioTickCallback(s64 cycles_late) { if (Tick()) { // TODO(merry): Signal all the other interrupts as appropriate. - if (auto service = dsp_dsp.lock()) { - service->SignalInterrupt(InterruptType::Pipe, DspPipe::Audio); - } + interrupt_handler(InterruptType::Pipe, DspPipe::Audio); } // Reschedule recurrent event @@ -505,9 +501,10 @@ std::array& DspHle::GetDspMemory() { return impl->GetDspMemory(); } -void DspHle::SetServiceToInterrupt(std::weak_ptr dsp) { - impl->SetServiceToInterrupt(std::move(dsp)); -} +void DspHle::SetInterruptHandler( + std::function handler) { + impl->SetInterruptHandler(handler); +}; void DspHle::LoadComponent(std::span component_data) { // HLE doesn't need DSP program. Only log some info here diff --git a/src/audio_core/hle/hle.h b/src/audio_core/hle/hle.h index ea0577bad..aa2fd2c4f 100644 --- a/src/audio_core/hle/hle.h +++ b/src/audio_core/hle/hle.h @@ -34,7 +34,8 @@ public: std::array& GetDspMemory() override; - void SetServiceToInterrupt(std::weak_ptr dsp) override; + void SetInterruptHandler( + std::function handler) override; void LoadComponent(std::span buffer) override; void UnloadComponent() override; diff --git a/src/audio_core/lle/lle.cpp b/src/audio_core/lle/lle.cpp index fd30f0c5a..afcb9df3c 100644 --- a/src/audio_core/lle/lle.cpp +++ b/src/audio_core/lle/lle.cpp @@ -408,29 +408,24 @@ std::array& DspLle::GetDspMemory() { return impl->teakra.GetDspMemory(); } -void DspLle::SetServiceToInterrupt(std::weak_ptr dsp) { - impl->teakra.SetRecvDataHandler(0, [this, dsp]() { +void DspLle::SetInterruptHandler( + std::function handler) { + impl->teakra.SetRecvDataHandler(0, [this, handler]() { if (!impl->loaded) return; std::lock_guard lock(HLE::g_hle_lock); - if (auto locked = dsp.lock()) { - locked->SignalInterrupt(Service::DSP::DSP_DSP::InterruptType::Zero, - static_cast(0)); - } + handler(Service::DSP::InterruptType::Zero, static_cast(0)); }); - impl->teakra.SetRecvDataHandler(1, [this, dsp]() { + impl->teakra.SetRecvDataHandler(1, [this, handler]() { if (!impl->loaded) return; std::lock_guard lock(HLE::g_hle_lock); - if (auto locked = dsp.lock()) { - locked->SignalInterrupt(Service::DSP::DSP_DSP::InterruptType::One, - static_cast(0)); - } + handler(Service::DSP::InterruptType::One, static_cast(0)); }); - auto ProcessPipeEvent = [this, dsp](bool event_from_data) { + auto ProcessPipeEvent = [this, handler](bool event_from_data) { if (!impl->loaded) return; @@ -456,10 +451,7 @@ void DspLle::SetServiceToInterrupt(std::weak_ptr dsp) { impl->GetPipeReadableSize(static_cast(pipe))); } else { std::lock_guard lock(HLE::g_hle_lock); - if (auto locked = dsp.lock()) { - locked->SignalInterrupt(Service::DSP::DSP_DSP::InterruptType::Pipe, - static_cast(pipe)); - } + handler(Service::DSP::InterruptType::Pipe, static_cast(pipe)); } } }; @@ -468,14 +460,6 @@ void DspLle::SetServiceToInterrupt(std::weak_ptr dsp) { impl->teakra.SetSemaphoreHandler([ProcessPipeEvent]() { ProcessPipeEvent(false); }); } -void DspLle::SetSemaphoreHandler(std::function handler) { - impl->teakra.SetSemaphoreHandler(handler); -} - -void DspLle::SetRecvDataHandler(u8 index, std::function handler) { - impl->teakra.SetRecvDataHandler(index, handler); -} - void DspLle::LoadComponent(std::span buffer) { impl->LoadComponent(buffer); } diff --git a/src/audio_core/lle/lle.h b/src/audio_core/lle/lle.h index 7aec28da4..bcc5e3c7d 100644 --- a/src/audio_core/lle/lle.h +++ b/src/audio_core/lle/lle.h @@ -27,10 +27,8 @@ public: std::array& GetDspMemory() override; - void SetServiceToInterrupt(std::weak_ptr dsp) override; - - void SetSemaphoreHandler(std::function handler); - void SetRecvDataHandler(u8 index, std::function handler); + void SetInterruptHandler( + std::function handler) override; void LoadComponent(const std::span buffer) override; void UnloadComponent() override; diff --git a/src/core/hle/service/dsp/dsp_dsp.cpp b/src/core/hle/service/dsp/dsp_dsp.cpp index 14c799c70..fd28925a0 100644 --- a/src/core/hle/service/dsp/dsp_dsp.cpp +++ b/src/core/hle/service/dsp/dsp_dsp.cpp @@ -12,7 +12,7 @@ #include "core/hle/service/dsp/dsp_dsp.h" using DspPipe = AudioCore::DspPipe; -using InterruptType = Service::DSP::DSP_DSP::InterruptType; +using InterruptType = Service::DSP::InterruptType; SERIALIZE_EXPORT_IMPL(Service::DSP::DSP_DSP) SERVICE_CONSTRUCT_IMPL(Service::DSP::DSP_DSP) @@ -235,7 +235,8 @@ void DSP_DSP::RegisterInterruptEvents(Kernel::HLERequestContext& ctx) { const u32 channel = rp.Pop(); auto event = rp.PopObject(); - ASSERT_MSG(interrupt < NUM_INTERRUPT_TYPE && channel < AudioCore::num_dsp_pipe, + ASSERT_MSG(interrupt < static_cast(InterruptType::Count) && + channel < AudioCore::num_dsp_pipe, "Invalid type or pipe: interrupt = {}, channel = {}", interrupt, channel); const InterruptType type = static_cast(interrupt); @@ -326,6 +327,9 @@ std::shared_ptr& DSP_DSP::GetInterruptEvent(InterruptType type, D ASSERT(pipe_index < AudioCore::num_dsp_pipe); return pipes[pipe_index]; } + case InterruptType::Count: + default: + break; } UNREACHABLE_MSG("Invalid interrupt type = {}", type); } @@ -401,7 +405,12 @@ void InstallInterfaces(Core::System& system) { auto& service_manager = system.ServiceManager(); auto dsp = std::make_shared(system); dsp->InstallAsService(service_manager); - system.DSP().SetServiceToInterrupt(std::move(dsp)); + system.DSP().SetInterruptHandler( + [dsp_ref = std::weak_ptr(dsp)](InterruptType type, DspPipe pipe) { + if (auto locked = dsp_ref.lock()) { + locked->SignalInterrupt(type, pipe); + } + }); } } // namespace Service::DSP diff --git a/src/core/hle/service/dsp/dsp_dsp.h b/src/core/hle/service/dsp/dsp_dsp.h index d580b3d00..1eecb29cc 100644 --- a/src/core/hle/service/dsp/dsp_dsp.h +++ b/src/core/hle/service/dsp/dsp_dsp.h @@ -19,15 +19,14 @@ class System; namespace Service::DSP { +/// There are three types of interrupts +enum class InterruptType : u32 { Zero = 0, One = 1, Pipe = 2, Count }; + class DSP_DSP final : public ServiceFramework { public: explicit DSP_DSP(Core::System& system); ~DSP_DSP(); - /// There are three types of interrupts - static constexpr std::size_t NUM_INTERRUPT_TYPE = 3; - enum class InterruptType : u32 { Zero = 0, One = 1, Pipe = 2 }; - /// Actual service implementation only has 6 'slots' for interrupts. static constexpr std::size_t max_number_of_interrupt_events = 6; diff --git a/src/tests/audio_core/hle/hle.cpp b/src/tests/audio_core/hle/hle.cpp index 07d5e04af..7b77209f9 100644 --- a/src/tests/audio_core/hle/hle.cpp +++ b/src/tests/audio_core/hle/hle.cpp @@ -25,7 +25,7 @@ TEST_CASE("DSP LLE vs HLE", "[audio_core][hle]") { AudioCore::DspHle hle(hle_memory, hle_core_timing); AudioCore::DspLle lle(lle_memory, lle_core_timing, true); - // Initialiase LLE + // Initialise LLE { FileUtil::SetUserPath(); // see tests/audio_core/lle/lle.cpp for details on dspaudio.cdc @@ -41,25 +41,15 @@ TEST_CASE("DSP LLE vs HLE", "[audio_core][hle]") { std::vector firm_file_buf(firm_file.GetSize()); firm_file.ReadArray(firm_file_buf.data(), firm_file_buf.size()); lle.LoadComponent(firm_file_buf); - lle.SetSemaphoreHandler([&lle]() { - u16 slot = lle.RecvData(2); - u16 side = slot % 2; - u16 pipe = slot / 2; - fmt::print("SetSemaphoreHandler slot={}\n", slot); - if (pipe > 15) - return; - if (side != 0) - return; - if (pipe == 0) { - // pipe 0 is for debug. 3DS automatically drains this pipe and discards the - // data - lle.PipeRead(static_cast(pipe), - lle.GetPipeReadableSize(static_cast(pipe))); - } + lle.SetInterruptHandler([](Service::DSP::InterruptType type, AudioCore::DspPipe pipe) { + fmt::print("LLE SetInterruptHandler type={} pipe={}\n", type, pipe); + }); + } + // Initialise HLE + { + hle.SetInterruptHandler([](Service::DSP::InterruptType type, AudioCore::DspPipe pipe) { + fmt::print("HLE SetInterruptHandler type={} pipe={}\n", type, pipe); }); - lle.SetRecvDataHandler(0, []() { fmt::print("SetRecvDataHandler 0\n"); }); - lle.SetRecvDataHandler(1, []() { fmt::print("SetRecvDataHandler 1\n"); }); - lle.SetRecvDataHandler(2, []() { fmt::print("SetRecvDataHandler 2\n"); }); } SECTION("Initialise Audio Pipe") { diff --git a/src/tests/audio_core/lle/lle.cpp b/src/tests/audio_core/lle/lle.cpp index d44554210..e4a85e68c 100644 --- a/src/tests/audio_core/lle/lle.cpp +++ b/src/tests/audio_core/lle/lle.cpp @@ -37,26 +37,11 @@ TEST_CASE("DSP LLE Sanity", "[audio_core][lle]") { std::vector firm_file_buf(firm_file.GetSize()); firm_file.ReadArray(firm_file_buf.data(), firm_file_buf.size()); lle.LoadComponent(firm_file_buf); + + lle.SetInterruptHandler([](Service::DSP::InterruptType type, AudioCore::DspPipe pipe) { + fmt::print("SetInterruptHandler type={} pipe={}\n", type, pipe); + }); } - lle.SetSemaphoreHandler([&lle]() { - u16 slot = lle.RecvData(2); - u16 side = slot % 2; - u16 pipe = slot / 2; - fmt::print("SetSemaphoreHandler slot={}\n", slot); - if (pipe > 15) - return; - if (side != 0) - return; - if (pipe == 0) { - // pipe 0 is for debug. 3DS automatically drains this pipe and discards the - // data - lle.PipeRead(static_cast(pipe), - lle.GetPipeReadableSize(static_cast(pipe))); - } - }); - lle.SetRecvDataHandler(0, []() { fmt::print("SetRecvDataHandler 0\n"); }); - lle.SetRecvDataHandler(1, []() { fmt::print("SetRecvDataHandler 1\n"); }); - lle.SetRecvDataHandler(2, []() { fmt::print("SetRecvDataHandler 2\n"); }); SECTION("Initialise Audio Pipe") { std::vector buffer(4, 0); buffer[0] = 0;