Merge pull request #10768 from Pokechu22/dsp-hle-gba-class

DSPHLE: Eliminate global state in GBA and AX uCode + accuracy improvements

Source/Core/Core/HW/DSPHLE/UCodes/AX.cpp

@@ -653,53 +653,72 @@ void AXUCode::SendAUXAndMix(u32 main_auxa_up, u32 auxb_s_up, u32 main_l_dl, u32
 
 void AXUCode::HandleMail(u32 mail)
 {
-  // Indicates if the next message is a command list address.
+  if (m_upload_setup_in_progress)
-  static bool next_is_cmdlist = false;
-  static u16 cmdlist_size = 0;
-
-  bool set_next_is_cmdlist = false;
-
-  if (next_is_cmdlist)
-  {
-    CopyCmdList(mail, cmdlist_size);
-    HandleCommandList();
-    m_cmdlist_size = 0;
-    SignalWorkEnd();
-  }
-  else if (m_upload_setup_in_progress)
   {
     PrepareBootUCode(mail);
+    return;
   }
-  else if (mail == MAIL_RESUME)
+
+  switch (m_mail_state)
   {
-    // Acknowledge the resume request
+  case MailState::WaitingForCmdListSize:
-    m_mail_handler.PushMail(DSP_RESUME, true);
+    if ((mail & MAIL_CMDLIST_MASK) == MAIL_CMDLIST)
-  }
-  else if (mail == MAIL_NEW_UCODE)
-  {
-    m_upload_setup_in_progress = true;
-  }
-  else if (mail == MAIL_RESET)
-  {
-    m_dsphle->SetUCode(UCODE_ROM);
-  }
-  else if (mail == MAIL_CONTINUE)
-  {
-    // We don't have to do anything here - the CPU does not wait for a ACK
-    // and sends a cmdlist mail just after.
-  }
-  else if ((mail & MAIL_CMDLIST_MASK) == MAIL_CMDLIST)
     {
       // A command list address is going to be sent next.
-    set_next_is_cmdlist = true;
+      m_cmdlist_size = static_cast<u16>(mail & ~MAIL_CMDLIST_MASK);
-    cmdlist_size = (u16)(mail & ~MAIL_CMDLIST_MASK);
+      m_mail_state = MailState::WaitingForCmdListAddress;
     }
     else
     {
-    ERROR_LOG_FMT(DSPHLE, "Unknown mail sent to AX::HandleMail: {:08x}", mail);
+      ERROR_LOG_FMT(DSPHLE, "Unknown mail sent to AX::HandleMail; expected command list: {:08x}",
+                    mail);
+    }
+    break;
+
+  case MailState::WaitingForCmdListAddress:
+    CopyCmdList(mail, m_cmdlist_size);
+    HandleCommandList();
+    m_cmdlist_size = 0;
+    SignalWorkEnd();
+    m_mail_state = MailState::WaitingForNextTask;
+    break;
+
+  case MailState::WaitingForNextTask:
+    if ((mail & TASK_MAIL_MASK) != TASK_MAIL_TO_DSP)
+    {
+      WARN_LOG_FMT(DSPHLE, "Rendering task without prefix CDD1: {:08x}", mail);
+      mail = TASK_MAIL_TO_DSP | (mail & ~TASK_MAIL_MASK);
+      // The actual uCode does not check for the CDD1 prefix.
     }
 
-  next_is_cmdlist = set_next_is_cmdlist;
+    switch (mail)
+    {
+    case MAIL_RESUME:
+      // Acknowledge the resume request
+      m_mail_handler.PushMail(DSP_RESUME, true);
+      m_mail_state = MailState::WaitingForCmdListSize;
+      break;
+
+    case MAIL_NEW_UCODE:
+      m_upload_setup_in_progress = true;
+      break;
+
+    case MAIL_RESET:
+      m_dsphle->SetUCode(UCODE_ROM);
+      break;
+
+    case MAIL_CONTINUE:
+      // We don't have to do anything here - the CPU does not wait for a ACK
+      // and sends a cmdlist mail just after.
+      m_mail_state = MailState::WaitingForCmdListSize;
+      break;
+
+    default:
+      WARN_LOG_FMT(DSPHLE, "Unknown task mail: {:08x}", mail);
+      break;
+    }
+    break;
+  }
 }
 
 void AXUCode::CopyCmdList(u32 addr, u16 size)
@@ -712,7 +731,6 @@ void AXUCode::CopyCmdList(u32 addr, u16 size)
 
   for (u32 i = 0; i < size; ++i, addr += 2)
     m_cmdlist[i] = HLEMemory_Read_U16(addr);
-  m_cmdlist_size = size;
 }
 
 void AXUCode::Update()
@@ -728,6 +746,7 @@ void AXUCode::DoAXState(PointerWrap& p)
 {
   p.Do(m_cmdlist);
   p.Do(m_cmdlist_size);
+  p.Do(m_mail_state);
 
   p.Do(m_samples_main_left);
   p.Do(m_samples_main_right);

Source/Core/Core/HW/DSPHLE/UCodes/AX.h

@@ -62,7 +62,7 @@ enum AXMixControl
   // clang-format on
 };
 
-class AXUCode : public UCodeInterface
+class AXUCode /* not final: subclassed by AXWiiUCode */ : public UCodeInterface
 {
 public:
   AXUCode(DSPHLE* dsphle, u32 crc);
@@ -200,5 +200,14 @@ private:
     CMD_COMPRESSOR = 0x12,
     CMD_SEND_AUX_AND_MIX = 0x13,
   };
+
+  enum class MailState
+  {
+    WaitingForCmdListSize,
+    WaitingForCmdListAddress,
+    WaitingForNextTask,
+  };
+
+  MailState m_mail_state = MailState::WaitingForCmdListSize;
 };
 }  // namespace DSP::HLE

Source/Core/Core/HW/DSPHLE/UCodes/AXWii.h

@@ -11,7 +11,7 @@ namespace DSP::HLE
 struct AXPBWii;
 class DSPHLE;
 
-class AXWiiUCode : public AXUCode
+class AXWiiUCode final : public AXUCode
 {
 public:
   AXWiiUCode(DSPHLE* dsphle, u32 crc);

Source/Core/Core/HW/DSPHLE/UCodes/CARD.cpp

@@ -3,6 +3,7 @@
 
 #include "Core/HW/DSPHLE/UCodes/CARD.h"
 
+#include "Common/ChunkFile.h"
 #include "Common/CommonTypes.h"
 #include "Common/Logging/Log.h"
 #include "Core/HW/DSP.h"
@@ -44,4 +45,10 @@ void CARDUCode::HandleMail(u32 mail)
   m_mail_handler.PushMail(DSP_DONE);
   m_dsphle->SetUCode(UCODE_ROM);
 }
+
+void CARDUCode::DoState(PointerWrap& p)
+{
+  DoStateShared(p);
+  p.Do(m_state);
+}
 }  // namespace DSP::HLE

Source/Core/Core/HW/DSPHLE/UCodes/CARD.h

@@ -10,7 +10,7 @@ namespace DSP::HLE
 {
 class DSPHLE;
 
-class CARDUCode : public UCodeInterface
+class CARDUCode final : public UCodeInterface
 {
 public:
   CARDUCode(DSPHLE* dsphle, u32 crc);
@@ -18,5 +18,17 @@ public:
   void Initialize() override;
   void HandleMail(u32 mail) override;
   void Update() override;
+  void DoState(PointerWrap& p) override;
+
+private:
+  enum class State
+  {
+    WaitingForRequest,
+    WaitingForAddress,
+    WaitingForNextTask,
+  };
+
+  // Currently unused, will be used in a later version
+  State m_state = State::WaitingForRequest;
 };
 }  // namespace DSP::HLE

Source/Core/Core/HW/DSPHLE/UCodes/GBA.cpp

@@ -4,6 +4,7 @@
 #include "Core/HW/DSPHLE/UCodes/GBA.h"
 
 #include "Common/Align.h"
+#include "Common/ChunkFile.h"
 #include "Common/CommonTypes.h"
 #include "Common/Logging/Log.h"
 #include "Core/HW/DSP.h"
@@ -89,28 +90,44 @@ void GBAUCode::Update()
 
 void GBAUCode::HandleMail(u32 mail)
 {
-  static bool nextmail_is_mramaddr = false;
-  static bool calc_done = false;
-
   if (m_upload_setup_in_progress)
   {
     PrepareBootUCode(mail);
+    // The GBA ucode ignores the first 3 mails (mram_dest_addr, mram_size, mram_dram_addr)
+    // but we currently don't handle that (they're read when they shoudln't be, but DSP HLE doesn't
+    // implement them so it's fine).
+    return;
   }
-  else if ((mail >> 16 == 0xabba) && !nextmail_is_mramaddr)
+
+  switch (m_mail_state)
   {
-    nextmail_is_mramaddr = true;
+  case MailState::WaitingForRequest:
+  {
+    if (mail == REQUEST_MAIL)
+    {
+      INFO_LOG_FMT(DSPHLE, "GBAUCode - Recieved request mail");
+      m_mail_state = MailState::WaitingForAddress;
     }
-  else if (nextmail_is_mramaddr)
+    else
     {
-    nextmail_is_mramaddr = false;
+      WARN_LOG_FMT(DSPHLE, "GBAUCode - Expected request mail but got {:08x}", mail);
+    }
+    break;
+  }
+  case MailState::WaitingForAddress:
+  {
+    const u32 address = mail & 0x0fff'ffff;
 
-    ProcessGBACrypto(mail);
+    ProcessGBACrypto(address);
 
-    calc_done = true;
     m_mail_handler.PushMail(DSP_DONE);
+    m_mail_state = MailState::WaitingForNextTask;
+    break;
   }
-  else if (((mail & TASK_MAIL_MASK) == TASK_MAIL_TO_DSP) && calc_done)
+  case MailState::WaitingForNextTask:
   {
+    // The GBA uCode checks that the high word is cdd1, so we compare the full mail with
+    // MAIL_NEW_UCODE/MAIL_RESET without doing masking
     switch (mail)
     {
     case MAIL_NEW_UCODE:
@@ -124,9 +141,12 @@ void GBAUCode::HandleMail(u32 mail)
       break;
     }
   }
-  else
-  {
-    WARN_LOG_FMT(DSPHLE, "GBAUCode - unknown command: {:08x}", mail);
   }
 }
+
+void GBAUCode::DoState(PointerWrap& p)
+{
+  DoStateShared(p);
+  p.Do(m_mail_state);
+}
 }  // namespace DSP::HLE

Source/Core/Core/HW/DSPHLE/UCodes/GBA.h

@@ -15,12 +15,26 @@ class DSPHLE;
 // written back to RAM at the dest address provided in the crypto parameters.
 void ProcessGBACrypto(u32 address);
 
-struct GBAUCode : public UCodeInterface
+class GBAUCode final : public UCodeInterface
 {
+public:
   GBAUCode(DSPHLE* dsphle, u32 crc);
 
   void Initialize() override;
   void HandleMail(u32 mail) override;
   void Update() override;
+  void DoState(PointerWrap& p) override;
+
+private:
+  static constexpr u32 REQUEST_MAIL = 0xabba0000;
+
+  enum class MailState
+  {
+    WaitingForRequest,
+    WaitingForAddress,
+    WaitingForNextTask,
+  };
+
+  MailState m_mail_state = MailState::WaitingForRequest;
 };
 }  // namespace DSP::HLE

Source/Core/Core/HW/DSPHLE/UCodes/INIT.cpp

@@ -28,4 +28,9 @@ void INITUCode::Update()
 void INITUCode::HandleMail(u32 mail)
 {
 }
+
+void INITUCode::DoState(PointerWrap& p)
+{
+  // We don't need to call DoStateShared() as the init uCode doesn't support launching new uCode
+}
 }  // namespace DSP::HLE

Source/Core/Core/HW/DSPHLE/UCodes/INIT.h

@@ -10,7 +10,7 @@ namespace DSP::HLE
 {
 class DSPHLE;
 
-class INITUCode : public UCodeInterface
+class INITUCode final : public UCodeInterface
 {
 public:
   INITUCode(DSPHLE* dsphle, u32 crc);
@@ -18,5 +18,6 @@ public:
   void Initialize() override;
   void HandleMail(u32 mail) override;
   void Update() override;
+  void DoState(PointerWrap& p) override;
 };
 }  // namespace DSP::HLE

Source/Core/Core/HW/DSPHLE/UCodes/ROM.h

@@ -10,7 +10,7 @@ namespace DSP::HLE
 {
 class DSPHLE;
 
-class ROMUCode : public UCodeInterface
+class ROMUCode final : public UCodeInterface
 {
 public:
   ROMUCode(DSPHLE* dsphle, u32 crc);

Source/Core/Core/HW/DSPHLE/UCodes/UCodes.h

@@ -46,7 +46,7 @@ public:
   virtual void HandleMail(u32 mail) = 0;
   virtual void Update() = 0;
 
-  virtual void DoState(PointerWrap& p) { DoStateShared(p); }
+  virtual void DoState(PointerWrap& p) = 0;
   static u32 GetCRC(UCodeInterface* ucode) { return ucode ? ucode->m_crc : UCODE_NULL; }
 
 protected:

Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h

@@ -187,7 +187,7 @@ private:
   u32 m_reverb_pb_base_addr = 0;
 };
 
-class ZeldaUCode : public UCodeInterface
+class ZeldaUCode final : public UCodeInterface
 {
 public:
   ZeldaUCode(DSPHLE* dsphle, u32 crc);

Source/Core/Core/State.cpp

@@ -74,7 +74,7 @@ static std::recursive_mutex g_save_thread_mutex;
 static std::thread g_save_thread;
 
 // Don't forget to increase this after doing changes on the savestate system
-constexpr u32 STATE_VERSION = 147;  // Last changed in PR 10935
+constexpr u32 STATE_VERSION = 148;  // Last changed in PR 10768
 
 // Maps savestate versions to Dolphin versions.
 // Versions after 42 don't need to be added to this list,