Merge pull request #9576 from Pokechu22/invalid-gfx-reg-quirk

Add game quirks for unknown BP/CP/XF commands

9 files changed, 137 insertions, 25 deletions

diff --git a/Source/Core/Core/DolphinAnalytics.cpp b/Source/Core/Core/DolphinAnalytics.cpp
index 81dbc16643..07e541af2b 100644
--- a/Source/Core/Core/DolphinAnalytics.cpp
+++ b/Source/Core/Core/DolphinAnalytics.cpp
@@ -133,20 +133,27 @@ void DolphinAnalytics::ReportGameStart()
 }
 
 // Keep in sync with enum class GameQuirk definition.
-constexpr std::array<const char*, 14> GAME_QUIRKS_NAMES{"icache-matters",
-                                                        "directly-reads-wiimote-input",
-                                                        "uses-DVDLowStopLaser",
-                                                        "uses-DVDLowOffset",
-                                                        "uses-DVDLowReadDiskBca",
-                                                        "uses-DVDLowRequestDiscStatus",
-                                                        "uses-DVDLowRequestRetryNumber",
-                                                        "uses-DVDLowSerMeasControl",
-                                                        "uses-different-partition-command",
-                                                        "uses-di-interrupt-command",
-                                                        "mismatched-gpu-texgens-between-xf-and-bp",
-                                                        "mismatched-gpu-colors-between-xf-and-bp",
-                                                        "uses-uncommon-wd-mode",
-                                                        "uses-wd-unimplemented-ioctl"};
+constexpr std::array<const char*, 19> GAME_QUIRKS_NAMES{
+    "icache-matters",
+    "directly-reads-wiimote-input",
+    "uses-DVDLowStopLaser",
+    "uses-DVDLowOffset",
+    "uses-DVDLowReadDiskBca",
+    "uses-DVDLowRequestDiscStatus",
+    "uses-DVDLowRequestRetryNumber",
+    "uses-DVDLowSerMeasControl",
+    "uses-different-partition-command",
+    "uses-di-interrupt-command",
+    "mismatched-gpu-texgens-between-xf-and-bp",
+    "mismatched-gpu-colors-between-xf-and-bp",
+    "uses-uncommon-wd-mode",
+    "uses-wd-unimplemented-ioctl",
+    "uses-unknown-bp-command",
+    "uses-unknown-cp-command",
+    "uses-unknown-xf-command",
+    "uses-maybe-invalid-cp-command",
+    "uses-cp-perf-command",
+};
 static_assert(GAME_QUIRKS_NAMES.size() == static_cast<u32>(GameQuirk::COUNT),
               "Game quirks names and enum definition are out of sync.");
 
diff --git a/Source/Core/Core/DolphinAnalytics.h b/Source/Core/Core/DolphinAnalytics.h
index 6416257b93..711117ea09 100644
--- a/Source/Core/Core/DolphinAnalytics.h
+++ b/Source/Core/Core/DolphinAnalytics.h
@@ -61,6 +61,18 @@ enum class GameQuirk
 
   USES_WD_UNIMPLEMENTED_IOCTL,
 
+  // Some games use invalid/unknown graphics commands (see e.g. bug 10931).
+  // These are different from unknown opcodes: it is known that a BP/CP/XF command is being used,
+  // but the command itself is not understood.
+  USES_UNKNOWN_BP_COMMAND,
+  USES_UNKNOWN_CP_COMMAND,
+  USES_UNKNOWN_XF_COMMAND,
+  // YAGCD and Dolphin's implementation disagree about what is valid in some cases
+  USES_MAYBE_INVALID_CP_COMMAND,
+  // These commands are used by a few games (e.g. bug 12461), and seem to relate to perf queries.
+  // Track them separately.
+  USES_CP_PERF_COMMAND,
+
   COUNT,
 };
 
diff --git a/Source/Core/Core/FifoPlayer/FifoPlayer.cpp b/Source/Core/Core/FifoPlayer/FifoPlayer.cpp
index 0ed9c94630..49a89749dd 100644
--- a/Source/Core/Core/FifoPlayer/FifoPlayer.cpp
+++ b/Source/Core/Core/FifoPlayer/FifoPlayer.cpp
@@ -491,7 +491,10 @@ void FifoPlayer::LoadRegisters()
 
   regs = m_File->GetXFRegs();
   for (int i = 0; i < FifoDataFile::XF_REGS_SIZE; ++i)
-    LoadXFReg(i, regs[i]);
+  {
+    if (ShouldLoadXF(i))
+      LoadXFReg(i, regs[i]);
+  }
 }
 
 void FifoPlayer::LoadTextureMemory()
@@ -571,6 +574,16 @@ bool FifoPlayer::ShouldLoadBP(u8 address)
   }
 }
 
+bool FifoPlayer::ShouldLoadXF(u8 reg)
+{
+  // Ignore unknown addresses
+  u16 address = reg + 0x1000;
+  return !(address == XFMEM_UNKNOWN_1007 ||
+           (address >= XFMEM_UNKNOWN_GROUP_1_START && address <= XFMEM_UNKNOWN_GROUP_1_END) ||
+           (address >= XFMEM_UNKNOWN_GROUP_2_START && address <= XFMEM_UNKNOWN_GROUP_2_END) ||
+           (address >= XFMEM_UNKNOWN_GROUP_3_START && address <= XFMEM_UNKNOWN_GROUP_3_END));
+}
+
 bool FifoPlayer::IsIdleSet()
 {
   CommandProcessor::UCPStatusReg status =
diff --git a/Source/Core/Core/FifoPlayer/FifoPlayer.h b/Source/Core/Core/FifoPlayer/FifoPlayer.h
index bb36795d47..5302e8cfcb 100644
--- a/Source/Core/Core/FifoPlayer/FifoPlayer.h
+++ b/Source/Core/Core/FifoPlayer/FifoPlayer.h
@@ -134,6 +134,7 @@ private:
   void LoadXFMem16(u16 address, const u32* data);
 
   bool ShouldLoadBP(u8 address);
+  bool ShouldLoadXF(u8 address);
 
   static bool IsIdleSet();
   static bool IsHighWatermarkSet();
diff --git a/Source/Core/VideoCommon/BPStructs.cpp b/Source/Core/VideoCommon/BPStructs.cpp
index 2ad90a350a..699b92ce17 100644
--- a/Source/Core/VideoCommon/BPStructs.cpp
+++ b/Source/Core/VideoCommon/BPStructs.cpp
@@ -12,8 +12,10 @@
 
 #include "Common/CommonTypes.h"
 #include "Common/Logging/Log.h"
+
 #include "Core/ConfigManager.h"
 #include "Core/CoreTiming.h"
+#include "Core/DolphinAnalytics.h"
 #include "Core/FifoPlayer/FifoPlayer.h"
 #include "Core/FifoPlayer/FifoRecorder.h"
 #include "Core/HW/Memmap.h"
@@ -704,6 +706,7 @@ static void BPWritten(const BPCmd& bp)
     break;
   }
 
+  DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_UNKNOWN_BP_COMMAND);
   WARN_LOG_FMT(VIDEO, "Unknown BP opcode: address = {:#010x} value = {:#010x}", bp.address,
                bp.newvalue);
 }
diff --git a/Source/Core/VideoCommon/CPMemory.h b/Source/Core/VideoCommon/CPMemory.h
index 2ede79578b..f54445940f 100644
--- a/Source/Core/VideoCommon/CPMemory.h
+++ b/Source/Core/VideoCommon/CPMemory.h
@@ -20,6 +20,13 @@ enum
   // TODO: However, Dolphin's implementation (in LoadCPReg) and YAGCD disagree about what values are
   // valid for the lower nybble.
 
+  // YAGCD mentions 0x20 as "?", and does not mention the others
+  // Libogc has 0x00 and 0x20, where 0x00 is tied to GX_ClearVCacheMetric and 0x20 related to
+  // cpPerfMode. 0x10 may be GX_SetVCacheMetric, but that function is empty. In any case, these all
+  // are probably for perf queries, and no title seems to actually need a full implementation.
+  UNKNOWN_00 = 0x00,
+  UNKNOWN_10 = 0x10,
+  UNKNOWN_20 = 0x20,
   // YAGCD says 0x30 only; LoadCPReg allows any
   MATINDEX_A = 0x30,
   // YAGCD says 0x40 only; LoadCPReg allows any
diff --git a/Source/Core/VideoCommon/VertexLoaderManager.cpp b/Source/Core/VideoCommon/VertexLoaderManager.cpp
index 8a2eba09b4..94a331a0d2 100644
--- a/Source/Core/VideoCommon/VertexLoaderManager.cpp
+++ b/Source/Core/VideoCommon/VertexLoaderManager.cpp
@@ -16,6 +16,8 @@
 #include "Common/Assert.h"
 #include "Common/CommonTypes.h"
 #include "Common/Logging/Log.h"
+
+#include "Core/DolphinAnalytics.h"
 #include "Core/HW/Memmap.h"
 
 #include "VideoCommon/BPMemory.h"
@@ -321,23 +323,71 @@ void LoadCPReg(u32 sub_cmd, u32 value, bool is_preprocess)
   CPState* state = is_preprocess ? &g_preprocess_cp_state : &g_main_cp_state;
   switch (sub_cmd & CP_COMMAND_MASK)
   {
+  case UNKNOWN_00:
+  case UNKNOWN_10:
+  case UNKNOWN_20:
+    if (!(sub_cmd == UNKNOWN_20 && value == 0))
+    {
+      // All titles using libogc or the official SDK issue 0x20 with value=0 on startup
+      DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_CP_PERF_COMMAND);
+      DEBUG_LOG_FMT(VIDEO, "Unknown CP command possibly relating to perf queries used: {:02x}",
+                    sub_cmd);
+    }
+    break;
+
   case MATINDEX_A:
+    if (sub_cmd != MATINDEX_A)
+    {
+      DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_MAYBE_INVALID_CP_COMMAND);
+      WARN_LOG_FMT(VIDEO,
+                   "CP MATINDEX_A: an exact value of {:02x} was expected "
+                   "but instead a value of {:02x} was seen",
+                   MATINDEX_A, sub_cmd);
+    }
+
     if (update_global_state)
       VertexShaderManager::SetTexMatrixChangedA(value);
     break;
 
   case MATINDEX_B:
+    if (sub_cmd != MATINDEX_B)
+    {
+      DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_MAYBE_INVALID_CP_COMMAND);
+      WARN_LOG_FMT(VIDEO,
+                   "CP MATINDEX_B: an exact value of {:02x} was expected "
+                   "but instead a value of {:02x} was seen",
+                   MATINDEX_B, sub_cmd);
+    }
+
     if (update_global_state)
       VertexShaderManager::SetTexMatrixChangedB(value);
     break;
 
   case VCD_LO:
+    if (sub_cmd != VCD_LO)  // Stricter than YAGCD
+    {
+      DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_MAYBE_INVALID_CP_COMMAND);
+      WARN_LOG_FMT(VIDEO,
+                   "CP VCD_LO: an exact value of {:02x} was expected "
+                   "but instead a value of {:02x} was seen",
+                   VCD_LO, sub_cmd);
+    }
+
     state->vtx_desc.low.Hex = value;
     state->attr_dirty = BitSet32::AllTrue(CP_NUM_VAT_REG);
     state->bases_dirty = true;
     break;
 
   case VCD_HI:
+    if (sub_cmd != VCD_HI)  // Stricter than YAGCD
+    {
+      DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_MAYBE_INVALID_CP_COMMAND);
+      WARN_LOG_FMT(VIDEO,
+                   "CP VCD_HI: an exact value of {:02x} was expected "
+                   "but instead a value of {:02x} was seen",
+                   VCD_HI, sub_cmd);
+    }
+
     state->vtx_desc.high.Hex = value;
     state->attr_dirty = BitSet32::AllTrue(CP_NUM_VAT_REG);
     state->bases_dirty = true;
@@ -345,21 +395,30 @@ void LoadCPReg(u32 sub_cmd, u32 value, bool is_preprocess)
 
   case CP_VAT_REG_A:
     if ((sub_cmd - CP_VAT_REG_A) >= CP_NUM_VAT_REG)
+    {
+      DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_MAYBE_INVALID_CP_COMMAND);
       WARN_LOG_FMT(VIDEO, "CP_VAT_REG_A: Invalid VAT {}", sub_cmd - CP_VAT_REG_A);
+    }
     state->vtx_attr[sub_cmd & CP_VAT_MASK].g0.Hex = value;
     state->attr_dirty[sub_cmd & CP_VAT_MASK] = true;
     break;
 
   case CP_VAT_REG_B:
     if ((sub_cmd - CP_VAT_REG_B) >= CP_NUM_VAT_REG)
+    {
+      DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_MAYBE_INVALID_CP_COMMAND);
       WARN_LOG_FMT(VIDEO, "CP_VAT_REG_B: Invalid VAT {}", sub_cmd - CP_VAT_REG_B);
+    }
     state->vtx_attr[sub_cmd & CP_VAT_MASK].g1.Hex = value;
     state->attr_dirty[sub_cmd & CP_VAT_MASK] = true;
     break;
 
   case CP_VAT_REG_C:
     if ((sub_cmd - CP_VAT_REG_C) >= CP_NUM_VAT_REG)
+    {
+      DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_MAYBE_INVALID_CP_COMMAND);
       WARN_LOG_FMT(VIDEO, "CP_VAT_REG_C: Invalid VAT {}", sub_cmd - CP_VAT_REG_C);
+    }
     state->vtx_attr[sub_cmd & CP_VAT_MASK].g2.Hex = value;
     state->attr_dirty[sub_cmd & CP_VAT_MASK] = true;
     break;
@@ -376,6 +435,7 @@ void LoadCPReg(u32 sub_cmd, u32 value, bool is_preprocess)
     break;
 
   default:
+    DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_UNKNOWN_CP_COMMAND);
     WARN_LOG_FMT(VIDEO, "Unknown CP register {:02x} set to {:08x}", sub_cmd, value);
   }
 }
diff --git a/Source/Core/VideoCommon/XFMemory.h b/Source/Core/VideoCommon/XFMemory.h
index 7efd3d2209..0d16c4f700 100644
--- a/Source/Core/VideoCommon/XFMemory.h
+++ b/Source/Core/VideoCommon/XFMemory.h
@@ -199,7 +199,8 @@ enum
   XFMEM_STATE1 = 0x1003,
   XFMEM_CLOCK = 0x1004,
   XFMEM_CLIPDISABLE = 0x1005,
-  XFMEM_SETGPMETRIC = 0x1006,
+  XFMEM_SETGPMETRIC = 0x1006,   // Perf0 according to YAGCD
+  XFMEM_UNKNOWN_1007 = 0x1007,  // Perf1 according to YAGCD
   XFMEM_VTXSPECS = 0x1008,
   XFMEM_SETNUMCHAN = 0x1009,
   XFMEM_SETCHAN0_AMBCOLOR = 0x100a,
@@ -211,21 +212,26 @@ enum
   XFMEM_SETCHAN0_ALPHA = 0x1010,
   XFMEM_SETCHAN1_ALPHA = 0x1011,
   XFMEM_DUALTEX = 0x1012,
+  XFMEM_UNKNOWN_GROUP_1_START = 0x1013,
+  XFMEM_UNKNOWN_GROUP_1_END = 0x1017,
   XFMEM_SETMATRIXINDA = 0x1018,
   XFMEM_SETMATRIXINDB = 0x1019,
   XFMEM_SETVIEWPORT = 0x101a,
   XFMEM_SETZSCALE = 0x101c,
   XFMEM_SETZOFFSET = 0x101f,
   XFMEM_SETPROJECTION = 0x1020,
-  // XFMEM_SETPROJECTIONB      = 0x1021,
-  // XFMEM_SETPROJECTIONC      = 0x1022,
-  // XFMEM_SETPROJECTIOND      = 0x1023,
-  // XFMEM_SETPROJECTIONE      = 0x1024,
-  // XFMEM_SETPROJECTIONF      = 0x1025,
-  // XFMEM_SETPROJECTIONORTHO1 = 0x1026,
-  // XFMEM_SETPROJECTIONORTHO2 = 0x1027,
+  // XFMEM_SETPROJECTIONB = 0x1021,
+  // XFMEM_SETPROJECTIONC = 0x1022,
+  // XFMEM_SETPROJECTIOND = 0x1023,
+  // XFMEM_SETPROJECTIONE = 0x1024,
+  // XFMEM_SETPROJECTIONF = 0x1025,
+  // XFMEM_SETPROJECTIONORTHO = 0x1026,
+  XFMEM_UNKNOWN_GROUP_2_START = 0x1027,
+  XFMEM_UNKNOWN_GROUP_2_END = 0x103e,
   XFMEM_SETNUMTEXGENS = 0x103f,
   XFMEM_SETTEXMTXINFO = 0x1040,
+  XFMEM_UNKNOWN_GROUP_3_START = 0x1048,
+  XFMEM_UNKNOWN_GROUP_3_END = 0x104f,
   XFMEM_SETPOSTMTXINFO = 0x1050,
   XFMEM_REGISTERS_END = 0x1058,
 };
diff --git a/Source/Core/VideoCommon/XFStructs.cpp b/Source/Core/VideoCommon/XFStructs.cpp
index 741680b826..9ceaf2edc1 100644
--- a/Source/Core/VideoCommon/XFStructs.cpp
+++ b/Source/Core/VideoCommon/XFStructs.cpp
@@ -9,6 +9,7 @@
 #include "Common/Logging/Log.h"
 #include "Common/Swap.h"
 
+#include "Core/DolphinAnalytics.h"
 #include "Core/HW/Memmap.h"
 
 #include "VideoCommon/CPMemory.h"
@@ -182,6 +183,7 @@ static void XFRegWritten(int transferSize, u32 baseAddress, DataReader src)
     case 0x104d:
     case 0x104e:
     case 0x104f:
+      DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_UNKNOWN_XF_COMMAND);
       DEBUG_LOG_FMT(VIDEO, "Possible Normal Mtx XF reg?: {:x}={:x}", address, newValue);
       break;
 
@@ -192,8 +194,8 @@ static void XFRegWritten(int transferSize, u32 baseAddress, DataReader src)
     case 0x1017:
 
     default:
-      if (newValue != 0)  // Ignore writes of zero.
-        WARN_LOG_FMT(VIDEO, "Unknown XF Reg: {:x}={:x}", address, newValue);
+      DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_UNKNOWN_XF_COMMAND);
+      WARN_LOG_FMT(VIDEO, "Unknown XF Reg: {:x}={:x}", address, newValue);
       break;
     }
 
@@ -211,6 +213,7 @@ void LoadXFReg(u32 transferSize, u32 baseAddress, DataReader src)
   if (baseAddress + transferSize > XFMEM_REGISTERS_END)
   {
     WARN_LOG_FMT(VIDEO, "XF load exceeds address space: {:x} {} bytes", baseAddress, transferSize);
+    DolphinAnalytics::Instance().ReportGameQuirk(GameQuirk::USES_UNKNOWN_XF_COMMAND);
 
     if (baseAddress >= XFMEM_REGISTERS_END)
       transferSize = 0;