Merge pull request #10389 from Pokechu22/low-unknown-opcode

OpcodeDecoding: Don't raise panic alerts for unknown opcodes 0x01-0x07
dolphin-emu · Jan 23, 2022 · 481c859 · 481c859
2 parents 237947e + 8d7eff2
commit 481c859
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 33 deletions.
diff --git a/Source/Core/VideoCommon/CommandProcessor.cpp b/Source/Core/VideoCommon/CommandProcessor.cpp
@@ -41,6 +41,8 @@ static u16 m_tokenReg;
 static Common::Flag s_interrupt_set;
 static Common::Flag s_interrupt_waiting;
 
+static bool s_is_fifo_error_seen = false;
+
 static bool IsOnThread()
 {
   return Core::System::GetInstance().IsDualCoreMode();
@@ -73,6 +75,8 @@ void SCPFifoStruct::Init()
   bFF_HiWatermarkInt.store(0, std::memory_order_relaxed);
   bFF_LoWatermark.store(0, std::memory_order_relaxed);
   bFF_LoWatermarkInt.store(0, std::memory_order_relaxed);
+
+  s_is_fifo_error_seen = false;
 }
 
 void SCPFifoStruct::DoState(PointerWrap& p)
@@ -611,18 +615,26 @@ void SetCpClearRegister()
 
 void HandleUnknownOpcode(u8 cmd_byte, const u8* buffer, bool preprocess)
 {
-  // TODO(Omega): Maybe dump FIFO to file on this error
-  PanicAlertFmtT("GFX FIFO: Unknown Opcode ({0:#04x} @ {1}, preprocess={2}).\n"
-                 "This means one of the following:\n"
-                 "* The emulated GPU got desynced, disabling dual core can help\n"
-                 "* Command stream corrupted by some spurious memory bug\n"
-                 "* This really is an unknown opcode (unlikely)\n"
-                 "* Some other sort of bug\n\n"
-                 "Further errors will be sent to the Video Backend log and\n"
-                 "Dolphin will now likely crash or hang. Enjoy.",
-                 cmd_byte, fmt::ptr(buffer), preprocess);
-
+  // Datel software uses 0x01 during startup, and Mario Party 5's Wiggler capsule
+  // accidentally uses 0x01-0x03 due to sending 4 more vertices than intended.
+  // Hardware testing indicates that 0x01-0x07 do nothing, so to avoid annoying the user with
+  // spurious popups, we don't create a panic alert in those cases.  Other unknown opcodes
+  // (such as 0x18) seem to result in hangs.
+  if (!s_is_fifo_error_seen && cmd_byte > 0x07)
   {
+    s_is_fifo_error_seen = true;
+
+    // TODO(Omega): Maybe dump FIFO to file on this error
+    PanicAlertFmtT("GFX FIFO: Unknown Opcode ({0:#04x} @ {1}, preprocess={2}).\n"
+                   "This means one of the following:\n"
+                   "* The emulated GPU got desynced, disabling dual core can help\n"
+                   "* Command stream corrupted by some spurious memory bug\n"
+                   "* This really is an unknown opcode (unlikely)\n"
+                   "* Some other sort of bug\n\n"
+                   "Further errors will be sent to the Video Backend log and\n"
+                   "Dolphin will now likely crash or hang. Enjoy.",
+                   cmd_byte, fmt::ptr(buffer), preprocess);
+
     PanicAlertFmt("Illegal command {:02x}\n"
                   "CPBase: {:#010x}\n"
                   "CPEnd: {:#010x}\n"
@@ -653,6 +665,10 @@ void HandleUnknownOpcode(u8 cmd_byte, const u8* buffer, bool preprocess)
                   fifo.bFF_HiWatermarkInt.load(std::memory_order_relaxed) ? "true" : "false",
                   fifo.bFF_LoWatermarkInt.load(std::memory_order_relaxed) ? "true" : "false");
   }
+
+  // We always generate this log message, though we only generate the panic alerts once.
+  ERROR_LOG_FMT(VIDEO, "FIFO: Unknown Opcode ({:#04x} @ {}, preprocessing = {})", cmd_byte,
+                fmt::ptr(buffer), preprocess ? "yes" : "no");
 }
 
 }  // namespace CommandProcessor
diff --git a/Source/Core/VideoCommon/OpcodeDecoding.cpp b/Source/Core/VideoCommon/OpcodeDecoding.cpp
@@ -32,14 +32,8 @@
 
 namespace OpcodeDecoder
 {
-static bool s_is_fifo_error_seen = false;
 bool g_record_fifo_data = false;
 
-void Init()
-{
-  s_is_fifo_error_seen = false;
-}
-
 template <bool is_preprocess>
 class RunCallback final : public Callback
 {
@@ -193,13 +187,7 @@ class RunCallback final : public Callback
   }
   OPCODE_CALLBACK(void OnUnknown(u8 opcode, const u8* data))
   {
-    if (static_cast<Opcode>(opcode) == Opcode::GX_UNKNOWN_RESET)
-    {
-      // Datel software uses this command
-      m_cycles += 6;
-      DEBUG_LOG_FMT(VIDEO, "GX Reset?");
-    }
-    else if (static_cast<Opcode>(opcode) == Opcode::GX_CMD_UNKNOWN_METRICS)
+    if (static_cast<Opcode>(opcode) == Opcode::GX_CMD_UNKNOWN_METRICS)
     {
       // 'Zelda Four Swords' calls it and checks the metrics registers after that
       m_cycles += 6;
@@ -213,11 +201,7 @@ class RunCallback final : public Callback
     }
     else
     {
-      if (!s_is_fifo_error_seen)
-        CommandProcessor::HandleUnknownOpcode(opcode, data, is_preprocess);
-      ERROR_LOG_FMT(VIDEO, "FIFO: Unknown Opcode({:#04x} @ {}, preprocessing = {})", opcode,
-                    fmt::ptr(data), is_preprocess ? "yes" : "no");
-      s_is_fifo_error_seen = true;
+      CommandProcessor::HandleUnknownOpcode(opcode, data, is_preprocess);
       m_cycles += 1;
     }
   }

diff --git a/Source/Core/VideoCommon/OpcodeDecoding.h b/Source/Core/VideoCommon/OpcodeDecoding.h
@@ -24,7 +24,6 @@ extern bool g_record_fifo_data;
 enum class Opcode
 {
   GX_NOP = 0x00,
-  GX_UNKNOWN_RESET = 0x01,
 
   GX_LOAD_BP_REG = 0x61,
   GX_LOAD_CP_REG = 0x08,
@@ -61,8 +60,6 @@ enum class Primitive : u8
   GX_DRAW_POINTS = 0x7           // 0xB8
 };
 
-void Init();
-
 // Interface for the Run and RunCommand functions below.
 // The functions themselves are templates so that the compiler generates separate versions for each
 // callback (with the callback functions inlined), so the callback doesn't actually need to be

diff --git a/Source/Core/VideoCommon/VideoBackendBase.cpp b/Source/Core/VideoCommon/VideoBackendBase.cpp
@@ -316,7 +316,6 @@ void VideoBackendBase::InitializeShared()
 
   CommandProcessor::Init();
   Fifo::Init();
-  OpcodeDecoder::Init();
   PixelEngine::Init();
   BPInit();
   VertexLoaderManager::Init();