Merge pull request #8236 from booto/si_greatest_hits_vol_1

SI: Minor bugfixes

18 files changed, 82 insertions, 69 deletions

diff --git a/Source/Core/Core/HW/SI/SI.cpp b/Source/Core/Core/HW/SI/SI.cpp
index 3438a4c950..b82af48ce0 100644
--- a/Source/Core/Core/HW/SI/SI.cpp
+++ b/Source/Core/Core/HW/SI/SI.cpp
@@ -8,7 +8,9 @@
 #include <array>
 #include <atomic>
 #include <cstring>
+#include <iomanip>
 #include <memory>
+#include <sstream>
 
 #include "Common/ChunkFile.h"
 #include "Common/CommonTypes.h"
@@ -280,31 +282,51 @@ static void GenerateSIInterrupt(SIInterruptType type)
   UpdateInterrupts();
 }
 
+constexpr u32 SI_XFER_LENGTH_MASK = 0x7f;
+
+// Translate [0,1,2,...,126,127] to [128,1,2,...,126,127]
+constexpr u32 ConvertSILengthField(u32 field)
+{
+  return ((field - 1) & SI_XFER_LENGTH_MASK) + 1;
+}
+
 static void RunSIBuffer(u64 user_data, s64 cycles_late)
 {
   if (s_com_csr.TSTART)
   {
-    // Math in_length
-    int in_length = s_com_csr.INLNGTH;
-    if (in_length == 0)
-      in_length = 128;
-    else
-      in_length++;
-
-    // Math out_length
-    int out_length = s_com_csr.OUTLNGTH;
-    if (out_length == 0)
-      out_length = 128;
-    else
-      out_length++;
+    u32 request_length = ConvertSILengthField(s_com_csr.OUTLNGTH);
+    u32 expected_response_length = ConvertSILengthField(s_com_csr.INLNGTH);
+    std::vector<u8> request_copy(s_si_buffer.data(), s_si_buffer.data() + request_length);
 
     std::unique_ptr<ISIDevice>& device = s_channel[s_com_csr.CHANNEL].device;
-    int numOutput = device->RunBuffer(s_si_buffer.data(), in_length);
+    u32 actual_response_length = device->RunBuffer(s_si_buffer.data(), request_length);
 
-    DEBUG_LOG(SERIALINTERFACE, "RunSIBuffer  chan: %d  inLen: %i  outLen: %i  processed: %i",
-              s_com_csr.CHANNEL, in_length, out_length, numOutput);
+    DEBUG_LOG(SERIALINTERFACE,
+              "RunSIBuffer  chan: %d  request_length: %u  expected_response_length: %u  "
+              "actual_response_length: %u",
+              s_com_csr.CHANNEL, request_length, expected_response_length, actual_response_length);
+    if (expected_response_length != actual_response_length)
+    {
+      std::stringstream ss;
+      for (u8 b : request_copy)
+      {
+        ss << std::hex << std::setw(2) << std::setfill('0') << (int)b << ' ';
+      }
+      ERROR_LOG(
+          SERIALINTERFACE,
+          "RunSIBuffer: expected_response_length(%u) != actual_response_length(%u): request: %s",
+          expected_response_length, actual_response_length, ss.str().c_str());
+    }
 
-    if (numOutput != 0)
+    // TODO:
+    // 1) Wait a reasonable amount of time for the result to be available:
+    //    request is N bytes, ends with a stop bit
+    //    response in M bytes, ends with a stop bit
+    //    processing controller-side takes K us (investigate?)
+    //    each bit takes 4us ([3us low/1us high] for a 0, [1us low/3us high] for a 1)
+    //    time until response is available is at least: K + ((N*8 + 1) + (M*8 + 1)) * 4 us
+    // 2) Investigate the timeout period for NOREP0
+    if (actual_response_length != 0)
     {
       s_com_csr.TSTART = 0;
       GenerateSIInterrupt(INT_TCINT);
diff --git a/Source/Core/Core/HW/SI/SI_Device.cpp b/Source/Core/Core/HW/SI/SI_Device.cpp
index bd63f0ad76..0d5eb89289 100644
--- a/Source/Core/Core/HW/SI/SI_Device.cpp
+++ b/Source/Core/Core/HW/SI/SI_Device.cpp
@@ -37,15 +37,16 @@ SIDevices ISIDevice::GetDeviceType() const
   return m_device_type;
 }
 
-int ISIDevice::RunBuffer(u8* buffer, int length)
+int ISIDevice::RunBuffer(u8* buffer, int request_length)
 {
 #ifdef _DEBUG
-  DEBUG_LOG(SERIALINTERFACE, "Send Data Device(%i) - Length(%i)   ", m_device_number, length);
+  DEBUG_LOG(SERIALINTERFACE, "Send Data Device(%i) - Length(%i)   ", m_device_number,
+            request_length);
 
   std::string temp;
   int num = 0;
 
-  while (num < length)
+  while (num < request_length)
   {
     temp += StringFromFormat("0x%02x ", buffer[num]);
     num++;
diff --git a/Source/Core/Core/HW/SI/SI_Device.h b/Source/Core/Core/HW/SI/SI_Device.h
index eaf4f0486b..c666f98805 100644
--- a/Source/Core/Core/HW/SI/SI_Device.h
+++ b/Source/Core/Core/HW/SI/SI_Device.h
@@ -73,7 +73,7 @@ public:
   SIDevices GetDeviceType() const;
 
   // Run the SI Buffer
-  virtual int RunBuffer(u8* buffer, int length);
+  virtual int RunBuffer(u8* buffer, int request_length);
   virtual int TransferInterval();
 
   // Return true on new data
diff --git a/Source/Core/Core/HW/SI/SI_DeviceDanceMat.cpp b/Source/Core/Core/HW/SI/SI_DeviceDanceMat.cpp
index 2c76200559..08686a140e 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceDanceMat.cpp
+++ b/Source/Core/Core/HW/SI/SI_DeviceDanceMat.cpp
@@ -17,24 +17,19 @@ CSIDevice_DanceMat::CSIDevice_DanceMat(SIDevices device, int device_number)
 {
 }
 
-int CSIDevice_DanceMat::RunBuffer(u8* buffer, int length)
+int CSIDevice_DanceMat::RunBuffer(u8* buffer, int request_length)
 {
   // Read the command
   EBufferCommands command = static_cast<EBufferCommands>(buffer[0]);
-
   if (command == CMD_RESET)
   {
-    ISIDevice::RunBuffer(buffer, length);
+    ISIDevice::RunBuffer(buffer, request_length);
 
     u32 id = Common::swap32(SI_DANCEMAT);
     std::memcpy(buffer, &id, sizeof(id));
+    return sizeof(id);
   }
-  else
-  {
-    return CSIDevice_GCController::RunBuffer(buffer, length);
-  }
-
-  return length;
+  return CSIDevice_GCController::RunBuffer(buffer, request_length);
 }
 
 u32 CSIDevice_DanceMat::MapPadStatus(const GCPadStatus& pad_status)
diff --git a/Source/Core/Core/HW/SI/SI_DeviceDanceMat.h b/Source/Core/Core/HW/SI/SI_DeviceDanceMat.h
index 5e6fdda9b8..17f35cd59b 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceDanceMat.h
+++ b/Source/Core/Core/HW/SI/SI_DeviceDanceMat.h
@@ -16,7 +16,7 @@ class CSIDevice_DanceMat : public CSIDevice_GCController
 public:
   CSIDevice_DanceMat(SIDevices device, int device_number);
 
-  int RunBuffer(u8* buffer, int length) override;
+  int RunBuffer(u8* buffer, int request_length) override;
   u32 MapPadStatus(const GCPadStatus& pad_status) override;
   bool GetData(u32& hi, u32& low) override;
 };
diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp
index 8d28ed322f..a776dad587 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp
+++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp
@@ -290,7 +290,7 @@ CSIDevice_GBA::CSIDevice_GBA(SIDevices device, int device_number) : ISIDevice(de
 {
 }
 
-int CSIDevice_GBA::RunBuffer(u8* buffer, int length)
+int CSIDevice_GBA::RunBuffer(u8* buffer, int request_length)
 {
   switch (m_next_action)
   {
diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.h b/Source/Core/Core/HW/SI/SI_DeviceGBA.h
index 6e1467b88d..c08ab954e0 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceGBA.h
+++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.h
@@ -45,7 +45,7 @@ class CSIDevice_GBA : public ISIDevice
 public:
   CSIDevice_GBA(SIDevices device, int device_number);
 
-  int RunBuffer(u8* buffer, int length) override;
+  int RunBuffer(u8* buffer, int request_length) override;
   int TransferInterval() override;
   bool GetData(u32& hi, u32& low) override;
   void SendCommand(u32 command, u8 poll) override;
diff --git a/Source/Core/Core/HW/SI/SI_DeviceGCAdapter.cpp b/Source/Core/Core/HW/SI/SI_DeviceGCAdapter.cpp
index 3f3d70202c..80d4f22ca7 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceGCAdapter.cpp
+++ b/Source/Core/Core/HW/SI/SI_DeviceGCAdapter.cpp
@@ -49,7 +49,7 @@ GCPadStatus CSIDevice_GCAdapter::GetPadStatus()
   return pad_status;
 }
 
-int CSIDevice_GCAdapter::RunBuffer(u8* buffer, int length)
+int CSIDevice_GCAdapter::RunBuffer(u8* buffer, int request_length)
 {
   if (!Core::WantsDeterminism())
   {
@@ -66,7 +66,7 @@ int CSIDevice_GCAdapter::RunBuffer(u8* buffer, int length)
       return 4;
     }
   }
-  return CSIDevice_GCController::RunBuffer(buffer, length);
+  return CSIDevice_GCController::RunBuffer(buffer, request_length);
 }
 
 bool CSIDevice_GCAdapter::GetData(u32& hi, u32& low)
diff --git a/Source/Core/Core/HW/SI/SI_DeviceGCAdapter.h b/Source/Core/Core/HW/SI/SI_DeviceGCAdapter.h
index aa34ff5d84..25fc3a775a 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceGCAdapter.h
+++ b/Source/Core/Core/HW/SI/SI_DeviceGCAdapter.h
@@ -16,7 +16,7 @@ public:
   CSIDevice_GCAdapter(SIDevices device, int device_number);
 
   GCPadStatus GetPadStatus() override;
-  int RunBuffer(u8* buffer, int length) override;
+  int RunBuffer(u8* buffer, int request_length) override;
 
   bool GetData(u32& hi, u32& low) override;
 
diff --git a/Source/Core/Core/HW/SI/SI_DeviceGCController.cpp b/Source/Core/Core/HW/SI/SI_DeviceGCController.cpp
index 11494cae46..024b3d6047 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceGCController.cpp
+++ b/Source/Core/Core/HW/SI/SI_DeviceGCController.cpp
@@ -36,17 +36,17 @@ CSIDevice_GCController::CSIDevice_GCController(SIDevices device, int device_numb
   m_origin.substick_y = GCPadStatus::C_STICK_CENTER_Y;
 }
 
-int CSIDevice_GCController::RunBuffer(u8* buffer, int length)
+int CSIDevice_GCController::RunBuffer(u8* buffer, int request_length)
 {
   // For debug logging only
-  ISIDevice::RunBuffer(buffer, length);
+  ISIDevice::RunBuffer(buffer, request_length);
 
   GCPadStatus pad_status = GetPadStatus();
   if (!pad_status.isConnected)
   {
     u32 reply = Common::swap32(SI_ERROR_NO_RESPONSE);
     std::memcpy(buffer, &reply, sizeof(reply));
-    return 4;
+    return sizeof(reply);
   }
 
   // Read the command
@@ -60,21 +60,21 @@ int CSIDevice_GCController::RunBuffer(u8* buffer, int length)
   {
     u32 id = Common::swap32(SI_GC_CONTROLLER);
     std::memcpy(buffer, &id, sizeof(id));
-    break;
+    return sizeof(id);
   }
 
   case CMD_DIRECT:
   {
-    INFO_LOG(SERIALINTERFACE, "PAD - Direct (Length: %d)", length);
+    INFO_LOG(SERIALINTERFACE, "PAD - Direct (Request length: %d)", request_length);
     u32 high, low;
     GetData(high, low);
-    for (int i = 0; i < (length - 1) / 2; i++)
+    for (int i = 0; i < 4; i++)
     {
       buffer[i + 0] = (high >> (24 - (i * 8))) & 0xff;
       buffer[i + 4] = (low >> (24 - (i * 8))) & 0xff;
     }
+    return sizeof(high) + sizeof(low);
   }
-  break;
 
   case CMD_ORIGIN:
   {
@@ -85,8 +85,8 @@ int CSIDevice_GCController::RunBuffer(u8* buffer, int length)
     {
       buffer[i] = *calibration++;
     }
+    return sizeof(SOrigin);
   }
-  break;
 
   // Recalibrate (FiRES: i am not 100 percent sure about this)
   case CMD_RECALIBRATE:
@@ -98,8 +98,8 @@ int CSIDevice_GCController::RunBuffer(u8* buffer, int length)
     {
       buffer[i] = *calibration++;
     }
+    return sizeof(SOrigin);
   }
-  break;
 
   // DEFAULT
   default:
@@ -110,7 +110,7 @@ int CSIDevice_GCController::RunBuffer(u8* buffer, int length)
   break;
   }
 
-  return length;
+  return 0;
 }
 
 void CSIDevice_GCController::HandleMoviePadStatus(GCPadStatus* pad_status)
diff --git a/Source/Core/Core/HW/SI/SI_DeviceGCController.h b/Source/Core/Core/HW/SI/SI_DeviceGCController.h
index 218c715e51..9c09c50f14 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceGCController.h
+++ b/Source/Core/Core/HW/SI/SI_DeviceGCController.h
@@ -34,8 +34,6 @@ protected:
     u8 trigger_right;
     u8 unk_4;
     u8 unk_5;
-    u8 unk_6;
-    u8 unk_7;
   };
 
   enum EDirectCommands
@@ -86,7 +84,7 @@ public:
   CSIDevice_GCController(SIDevices device, int device_number);
 
   // Run the SI Buffer
-  int RunBuffer(u8* buffer, int length) override;
+  int RunBuffer(u8* buffer, int request_length) override;
 
   // Return true on new data
   bool GetData(u32& hi, u32& low) override;
diff --git a/Source/Core/Core/HW/SI/SI_DeviceGCSteeringWheel.cpp b/Source/Core/Core/HW/SI/SI_DeviceGCSteeringWheel.cpp
index 3fe0a3230f..e3563b73aa 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceGCSteeringWheel.cpp
+++ b/Source/Core/Core/HW/SI/SI_DeviceGCSteeringWheel.cpp
@@ -20,10 +20,10 @@ CSIDevice_GCSteeringWheel::CSIDevice_GCSteeringWheel(SIDevices device, int devic
 {
 }
 
-int CSIDevice_GCSteeringWheel::RunBuffer(u8* buffer, int length)
+int CSIDevice_GCSteeringWheel::RunBuffer(u8* buffer, int request_length)
 {
   // For debug logging only
-  ISIDevice::RunBuffer(buffer, length);
+  ISIDevice::RunBuffer(buffer, request_length);
 
   // Read the command
   EBufferCommands command = static_cast<EBufferCommands>(buffer[0]);
@@ -36,14 +36,11 @@ int CSIDevice_GCSteeringWheel::RunBuffer(u8* buffer, int length)
   {
     u32 id = Common::swap32(SI_GC_STEERING);
     std::memcpy(buffer, &id, sizeof(id));
-    break;
+    return sizeof(id);
   }
-
-  default:
-    return CSIDevice_GCController::RunBuffer(buffer, length);
   }
 
-  return length;
+  return CSIDevice_GCController::RunBuffer(buffer, request_length);
 }
 
 bool CSIDevice_GCSteeringWheel::GetData(u32& hi, u32& low)
diff --git a/Source/Core/Core/HW/SI/SI_DeviceGCSteeringWheel.h b/Source/Core/Core/HW/SI/SI_DeviceGCSteeringWheel.h
index c9afee2773..05bc6bea16 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceGCSteeringWheel.h
+++ b/Source/Core/Core/HW/SI/SI_DeviceGCSteeringWheel.h
@@ -13,7 +13,7 @@ class CSIDevice_GCSteeringWheel : public CSIDevice_GCController
 public:
   CSIDevice_GCSteeringWheel(SIDevices device, int device_number);
 
-  int RunBuffer(u8* buffer, int length) override;
+  int RunBuffer(u8* buffer, int request_length) override;
   bool GetData(u32& hi, u32& low) override;
   void SendCommand(u32 command, u8 poll) override;
 
diff --git a/Source/Core/Core/HW/SI/SI_DeviceKeyboard.cpp b/Source/Core/Core/HW/SI/SI_DeviceKeyboard.cpp
index 6e3cc00ce6..d32fac1455 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceKeyboard.cpp
+++ b/Source/Core/Core/HW/SI/SI_DeviceKeyboard.cpp
@@ -21,10 +21,10 @@ CSIDevice_Keyboard::CSIDevice_Keyboard(SIDevices device, int device_number)
 {
 }
 
-int CSIDevice_Keyboard::RunBuffer(u8* buffer, int length)
+int CSIDevice_Keyboard::RunBuffer(u8* buffer, int request_length)
 {
   // For debug logging only
-  ISIDevice::RunBuffer(buffer, length);
+  ISIDevice::RunBuffer(buffer, request_length);
 
   // Read the command
   EBufferCommands command = static_cast<EBufferCommands>(buffer[0]);
@@ -37,21 +37,21 @@ int CSIDevice_Keyboard::RunBuffer(u8* buffer, int length)
   {
     u32 id = Common::swap32(SI_GC_KEYBOARD);
     std::memcpy(buffer, &id, sizeof(id));
-    break;
+    return sizeof(id);
   }
 
   case CMD_DIRECT:
   {
-    INFO_LOG(SERIALINTERFACE, "Keyboard - Direct (Length: %d)", length);
+    INFO_LOG(SERIALINTERFACE, "Keyboard - Direct (Request Length: %d)", request_length);
     u32 high, low;
     GetData(high, low);
-    for (int i = 0; i < (length - 1) / 2; i++)
+    for (int i = 0; i < 4; i++)
     {
       buffer[i + 0] = (high >> (24 - (i * 8))) & 0xff;
       buffer[i + 4] = (low >> (24 - (i * 8))) & 0xff;
     }
+    return sizeof(high) + sizeof(low);
   }
-  break;
 
   default:
   {
@@ -60,7 +60,7 @@ int CSIDevice_Keyboard::RunBuffer(u8* buffer, int length)
   break;
   }
 
-  return length;
+  return 0;
 }
 
 KeyboardStatus CSIDevice_Keyboard::GetKeyboardStatus() const
diff --git a/Source/Core/Core/HW/SI/SI_DeviceKeyboard.h b/Source/Core/Core/HW/SI/SI_DeviceKeyboard.h
index d703f24fa9..d9bb029d99 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceKeyboard.h
+++ b/Source/Core/Core/HW/SI/SI_DeviceKeyboard.h
@@ -18,7 +18,7 @@ public:
   CSIDevice_Keyboard(SIDevices device, int device_number);
 
   // Run the SI Buffer
-  int RunBuffer(u8* buffer, int length) override;
+  int RunBuffer(u8* buffer, int request_length) override;
 
   // Return true on new data
   bool GetData(u32& hi, u32& low) override;
diff --git a/Source/Core/Core/HW/SI/SI_DeviceNull.cpp b/Source/Core/Core/HW/SI/SI_DeviceNull.cpp
index ee4a39c253..f4ed00e7ae 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceNull.cpp
+++ b/Source/Core/Core/HW/SI/SI_DeviceNull.cpp
@@ -14,11 +14,11 @@ CSIDevice_Null::CSIDevice_Null(SIDevices device, int device_number)
 {
 }
 
-int CSIDevice_Null::RunBuffer(u8* buffer, int length)
+int CSIDevice_Null::RunBuffer(u8* buffer, int request_length)
 {
   u32 reply = Common::swap32(SI_ERROR_NO_RESPONSE);
   std::memcpy(buffer, &reply, sizeof(reply));
-  return 4;
+  return sizeof(reply);
 }
 
 bool CSIDevice_Null::GetData(u32& hi, u32& low)
diff --git a/Source/Core/Core/HW/SI/SI_DeviceNull.h b/Source/Core/Core/HW/SI/SI_DeviceNull.h
index 3b5e20ab65..88fd16021c 100644
--- a/Source/Core/Core/HW/SI/SI_DeviceNull.h
+++ b/Source/Core/Core/HW/SI/SI_DeviceNull.h
@@ -15,7 +15,7 @@ class CSIDevice_Null final : public ISIDevice
 public:
   CSIDevice_Null(SIDevices device, int device_number);
 
-  int RunBuffer(u8* buffer, int length) override;
+  int RunBuffer(u8* buffer, int request_length) override;
   bool GetData(u32& hi, u32& low) override;
   void SendCommand(u32 command, u8 poll) override;
 };
diff --git a/Source/Core/Core/HW/VideoInterface.cpp b/Source/Core/Core/HW/VideoInterface.cpp
index db235404d2..44f6050bdf 100644
--- a/Source/Core/Core/HW/VideoInterface.cpp
+++ b/Source/Core/Core/HW/VideoInterface.cpp
@@ -742,7 +742,7 @@ void Update(u64 ticks)
     if (Config::Get(Config::MAIN_REDUCE_POLLING_RATE))
       s_half_line_of_next_si_poll += GetHalfLinesPerEvenField() / 2;
     else
-      s_half_line_of_next_si_poll += SerialInterface::GetPollXLines();
+      s_half_line_of_next_si_poll += 2 * SerialInterface::GetPollXLines();
   }
   if (s_half_line_count == s_even_field_first_hl)
   {