AudioCore: Refactor DSP interrupt handling (#7026)

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<u8, Memory::DSP_RAM_SIZE>& GetDspMemory() = 0;
 
-    /// Sets the dsp class that we trigger interrupts for
+    /// Sets the handler for the interrupts we trigger
-    virtual void SetServiceToInterrupt(std::weak_ptr<Service::DSP::DSP_DSP> dsp) = 0;
+    virtual void SetInterruptHandler(
+        std::function<void(Service::DSP::InterruptType type, DspPipe pipe)> handler) = 0;
 
     /// Loads the DSP program
     virtual void LoadComponent(std::span<const u8> buffer) = 0;

src/audio_core/hle/hle.cpp

@@ -34,8 +34,7 @@
 
 SERIALIZE_EXPORT_IMPL(AudioCore::DspHle)
 
-using InterruptType = Service::DSP::DSP_DSP::InterruptType;
+using InterruptType = Service::DSP::InterruptType;
-using Service::DSP::DSP_DSP;
 
 namespace AudioCore {
 
@@ -71,7 +70,8 @@ public:
 
     std::array<u8, Memory::DSP_RAM_SIZE>& GetDspMemory();
 
-    void SetServiceToInterrupt(std::weak_ptr<DSP_DSP> dsp);
+    void SetInterruptHandler(
+        std::function<void(Service::DSP::InterruptType type, DspPipe pipe)> handler);
 
 private:
     void ResetPipes();
@@ -105,7 +105,7 @@ private:
 
     std::unique_ptr<HLE::DecoderBase> decoder{};
 
-    std::weak_ptr<DSP_DSP> dsp_dsp{};
+    std::function<void(Service::DSP::InterruptType type, DspPipe pipe)> interrupt_handler{};
 
     template <class Archive>
     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<const u8> buffer) {
             pipe_data[static_cast<u32>(pipe_number)].resize(sizeof(value));
             std::memcpy(pipe_data[static_cast<u32>(pipe_number)].data(), &value, sizeof(value));
         }
-        auto dsp = dsp_dsp.lock();
+
-        if (dsp) {
+        interrupt_handler(InterruptType::Pipe, DspPipe::Binary);
-            dsp->SignalInterrupt(InterruptType::Pipe, DspPipe::Binary);
-        }
         break;
     }
     default:
@@ -338,8 +337,9 @@ std::array<u8, Memory::DSP_RAM_SIZE>& DspHle::Impl::GetDspMemory() {
     return dsp_memory.raw_memory;
 }
 
-void DspHle::Impl::SetServiceToInterrupt(std::weak_ptr<DSP_DSP> dsp) {
+void DspHle::Impl::SetInterruptHandler(
-    dsp_dsp = std::move(dsp);
+    std::function<void(Service::DSP::InterruptType type, DspPipe pipe)> 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()) {
+    interrupt_handler(InterruptType::Pipe, DspPipe::Audio);
-        service->SignalInterrupt(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()) {
+        interrupt_handler(InterruptType::Pipe, DspPipe::Audio);
-            service->SignalInterrupt(InterruptType::Pipe, DspPipe::Audio);
-        }
     }
 
     // Reschedule recurrent event
@@ -505,9 +501,10 @@ std::array<u8, Memory::DSP_RAM_SIZE>& DspHle::GetDspMemory() {
     return impl->GetDspMemory();
 }
 
-void DspHle::SetServiceToInterrupt(std::weak_ptr<DSP_DSP> dsp) {
+void DspHle::SetInterruptHandler(
-    impl->SetServiceToInterrupt(std::move(dsp));
+    std::function<void(Service::DSP::InterruptType type, DspPipe pipe)> handler) {
-}
+    impl->SetInterruptHandler(handler);
+};
 
 void DspHle::LoadComponent(std::span<const u8> component_data) {
     // HLE doesn't need DSP program. Only log some info here

src/audio_core/hle/hle.h

@@ -34,7 +34,8 @@ public:
 
     std::array<u8, Memory::DSP_RAM_SIZE>& GetDspMemory() override;
 
-    void SetServiceToInterrupt(std::weak_ptr<Service::DSP::DSP_DSP> dsp) override;
+    void SetInterruptHandler(
+        std::function<void(Service::DSP::InterruptType type, DspPipe pipe)> handler) override;
 
     void LoadComponent(std::span<const u8> buffer) override;
     void UnloadComponent() override;

src/audio_core/lle/lle.cpp

@@ -408,29 +408,24 @@ std::array<u8, Memory::DSP_RAM_SIZE>& DspLle::GetDspMemory() {
     return impl->teakra.GetDspMemory();
 }
 
-void DspLle::SetServiceToInterrupt(std::weak_ptr<Service::DSP::DSP_DSP> dsp) {
+void DspLle::SetInterruptHandler(
-    impl->teakra.SetRecvDataHandler(0, [this, dsp]() {
+    std::function<void(Service::DSP::InterruptType type, DspPipe pipe)> handler) {
+    impl->teakra.SetRecvDataHandler(0, [this, handler]() {
         if (!impl->loaded)
             return;
 
         std::lock_guard lock(HLE::g_hle_lock);
-        if (auto locked = dsp.lock()) {
+        handler(Service::DSP::InterruptType::Zero, static_cast<DspPipe>(0));
-            locked->SignalInterrupt(Service::DSP::DSP_DSP::InterruptType::Zero,
-                                    static_cast<DspPipe>(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()) {
+        handler(Service::DSP::InterruptType::One, static_cast<DspPipe>(0));
-            locked->SignalInterrupt(Service::DSP::DSP_DSP::InterruptType::One,
-                                    static_cast<DspPipe>(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<Service::DSP::DSP_DSP> dsp) {
                                impl->GetPipeReadableSize(static_cast<u8>(pipe)));
             } else {
                 std::lock_guard lock(HLE::g_hle_lock);
-                if (auto locked = dsp.lock()) {
+                handler(Service::DSP::InterruptType::Pipe, static_cast<DspPipe>(pipe));
-                    locked->SignalInterrupt(Service::DSP::DSP_DSP::InterruptType::Pipe,
-                                            static_cast<DspPipe>(pipe));
-                }
             }
         }
     };
@@ -468,14 +460,6 @@ void DspLle::SetServiceToInterrupt(std::weak_ptr<Service::DSP::DSP_DSP> dsp) {
     impl->teakra.SetSemaphoreHandler([ProcessPipeEvent]() { ProcessPipeEvent(false); });
 }
 
-void DspLle::SetSemaphoreHandler(std::function<void()> handler) {
-    impl->teakra.SetSemaphoreHandler(handler);
-}
-
-void DspLle::SetRecvDataHandler(u8 index, std::function<void()> handler) {
-    impl->teakra.SetRecvDataHandler(index, handler);
-}
-
 void DspLle::LoadComponent(std::span<const u8> buffer) {
     impl->LoadComponent(buffer);
 }

src/audio_core/lle/lle.h

@@ -27,10 +27,8 @@ public:
 
     std::array<u8, Memory::DSP_RAM_SIZE>& GetDspMemory() override;
 
-    void SetServiceToInterrupt(std::weak_ptr<Service::DSP::DSP_DSP> dsp) override;
+    void SetInterruptHandler(
-
+        std::function<void(Service::DSP::InterruptType type, DspPipe pipe)> handler) override;
-    void SetSemaphoreHandler(std::function<void()> handler);
-    void SetRecvDataHandler(u8 index, std::function<void()> handler);
 
     void LoadComponent(const std::span<const u8> buffer) override;
     void UnloadComponent() override;

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<u32>();
     auto event = rp.PopObject<Kernel::Event>();
 
-    ASSERT_MSG(interrupt < NUM_INTERRUPT_TYPE && channel < AudioCore::num_dsp_pipe,
+    ASSERT_MSG(interrupt < static_cast<u32>(InterruptType::Count) &&
+                   channel < AudioCore::num_dsp_pipe,
                "Invalid type or pipe: interrupt = {}, channel = {}", interrupt, channel);
 
     const InterruptType type = static_cast<InterruptType>(interrupt);
@@ -326,6 +327,9 @@ std::shared_ptr<Kernel::Event>& 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<DSP_DSP>(system);
     dsp->InstallAsService(service_manager);
-    system.DSP().SetServiceToInterrupt(std::move(dsp));
+    system.DSP().SetInterruptHandler(
+        [dsp_ref = std::weak_ptr<DSP_DSP>(dsp)](InterruptType type, DspPipe pipe) {
+            if (auto locked = dsp_ref.lock()) {
+                locked->SignalInterrupt(type, pipe);
+            }
+        });
 }
 
 } // namespace Service::DSP

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<DSP_DSP> {
 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;
 

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<u8> 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]() {
+        lle.SetInterruptHandler([](Service::DSP::InterruptType type, AudioCore::DspPipe pipe) {
-            u16 slot = lle.RecvData(2);
+            fmt::print("LLE SetInterruptHandler type={} pipe={}\n", type, pipe);
-            u16 side = slot % 2;
+        });
-            u16 pipe = slot / 2;
+    }
-            fmt::print("SetSemaphoreHandler slot={}\n", slot);
+    // Initialise HLE
-            if (pipe > 15)
+    {
-                return;
+        hle.SetInterruptHandler([](Service::DSP::InterruptType type, AudioCore::DspPipe pipe) {
-            if (side != 0)
+            fmt::print("HLE SetInterruptHandler type={} pipe={}\n", type, pipe);
-                return;
-            if (pipe == 0) {
-                // pipe 0 is for debug. 3DS automatically drains this pipe and discards the
-                // data
-                lle.PipeRead(static_cast<AudioCore::DspPipe>(pipe),
-                             lle.GetPipeReadableSize(static_cast<AudioCore::DspPipe>(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") {

src/tests/audio_core/lle/lle.cpp

@@ -37,26 +37,11 @@ TEST_CASE("DSP LLE Sanity", "[audio_core][lle]") {
         std::vector<u8> 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<AudioCore::DspPipe>(pipe),
-                         lle.GetPipeReadableSize(static_cast<AudioCore::DspPipe>(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<u8> buffer(4, 0);
         buffer[0] = 0;