Merge pull request #339 from bunnei/fixup-gsp-synch

Fixup gsp synch

7 files changed, 59 insertions, 117 deletions

diff --git a/src/core/arm/arm_interface.h b/src/core/arm/arm_interface.h
index c5935533..3b720941 100644
--- a/src/core/arm/arm_interface.h
+++ b/src/core/arm/arm_interface.h
@@ -78,6 +78,12 @@ public:
     virtual u64 GetTicks() const = 0;
 
     /**
+     * Advance the CPU core by the specified number of ticks (e.g. to simulate CPU execution time)
+     * @param ticks Number of ticks to advance the CPU core
+     */
+    virtual void AddTicks(u64 ticks) = 0;
+
+    /**
      * Saves the current CPU context
      * @param ctx Thread context to save
      */
diff --git a/src/core/arm/dyncom/arm_dyncom.cpp b/src/core/arm/dyncom/arm_dyncom.cpp
index 6d4fb1b4..a838fd25 100644
--- a/src/core/arm/dyncom/arm_dyncom.cpp
+++ b/src/core/arm/dyncom/arm_dyncom.cpp
@@ -47,68 +47,38 @@ ARM_DynCom::ARM_DynCom() : ticks(0) {
 ARM_DynCom::~ARM_DynCom() {
 }
 
-/**
- * Set the Program Counter to an address
- * @param addr Address to set PC to
- */
 void ARM_DynCom::SetPC(u32 pc) {
     state->pc = state->Reg[15] = pc;
 }
 
-/*
- * Get the current Program Counter
- * @return Returns current PC
- */
 u32 ARM_DynCom::GetPC() const {
     return state->Reg[15];
 }
 
-/**
- * Get an ARM register
- * @param index Register index (0-15)
- * @return Returns the value in the register
- */
 u32 ARM_DynCom::GetReg(int index) const {
     return state->Reg[index];
 }
 
-/**
- * Set an ARM register
- * @param index Register index (0-15)
- * @param value Value to set register to
- */
 void ARM_DynCom::SetReg(int index, u32 value) {
     state->Reg[index] = value;
 }
 
-/**
- * Get the current CPSR register
- * @return Returns the value of the CPSR register
- */
 u32 ARM_DynCom::GetCPSR() const {
     return state->Cpsr;
 }
 
-/**
- * Set the current CPSR register
- * @param cpsr Value to set CPSR to
- */
 void ARM_DynCom::SetCPSR(u32 cpsr) {
     state->Cpsr = cpsr;
 }
 
-/**
- * Returns the number of clock ticks since the last reset
- * @return Returns number of clock ticks
- */
 u64 ARM_DynCom::GetTicks() const {
     return ticks;
 }
 
-/**
- * Executes the given number of instructions
- * @param num_instructions Number of instructions to executes
- */
+void ARM_DynCom::AddTicks(u64 ticks) {
+    this->ticks += ticks;
+}
+
 void ARM_DynCom::ExecuteInstructions(int num_instructions) {
     state->NumInstrsToExecute = num_instructions;
 
@@ -118,11 +88,6 @@ void ARM_DynCom::ExecuteInstructions(int num_instructions) {
     ticks += InterpreterMainLoop(state.get());
 }
 
-/**
- * Saves the current CPU context
- * @param ctx Thread context to save
- * @todo Do we need to save Reg[15] and NextInstr?
- */
 void ARM_DynCom::SaveContext(ThreadContext& ctx) {
     memcpy(ctx.cpu_registers, state->Reg, sizeof(ctx.cpu_registers));
     memcpy(ctx.fpu_registers, state->ExtReg, sizeof(ctx.fpu_registers));
@@ -139,11 +104,6 @@ void ARM_DynCom::SaveContext(ThreadContext& ctx) {
     ctx.mode = state->NextInstr;
 }
 
-/**
- * Loads a CPU context
- * @param ctx Thread context to load
- * @param Do we need to load Reg[15] and NextInstr?
- */
 void ARM_DynCom::LoadContext(const ThreadContext& ctx) {
     memcpy(state->Reg, ctx.cpu_registers, sizeof(ctx.cpu_registers));
     memcpy(state->ExtReg, ctx.fpu_registers, sizeof(ctx.fpu_registers));
@@ -160,7 +120,6 @@ void ARM_DynCom::LoadContext(const ThreadContext& ctx) {
     state->NextInstr = ctx.mode;
 }
 
-/// Prepare core for thread reschedule (if needed to correctly handle state)
 void ARM_DynCom::PrepareReschedule() {
     state->NumInstrsToExecute = 0;
 }
diff --git a/src/core/arm/dyncom/arm_dyncom.h b/src/core/arm/dyncom/arm_dyncom.h
index 6fa2a0ba..7284dcd0 100644
--- a/src/core/arm/dyncom/arm_dyncom.h
+++ b/src/core/arm/dyncom/arm_dyncom.h
@@ -27,14 +27,14 @@ public:
      * Get the current Program Counter
      * @return Returns current PC
      */
-    u32 GetPC() const;
+    u32 GetPC() const override;
 
     /**
      * Get an ARM register
      * @param index Register index (0-15)
      * @return Returns the value in the register
      */
-    u32 GetReg(int index) const;
+    u32 GetReg(int index) const override;
 
     /**
      * Set an ARM register
@@ -47,7 +47,7 @@ public:
      * Get the current CPSR register
      * @return Returns the value of the CPSR register
      */
-    u32 GetCPSR() const;
+    u32 GetCPSR() const override;
 
     /**
      * Set the current CPSR register
@@ -59,7 +59,13 @@ public:
      * Returns the number of clock ticks since the last reset
      * @return Returns number of clock ticks
      */
-    u64 GetTicks() const;
+    u64 GetTicks() const override;
+
+    /**
+    * Advance the CPU core by the specified number of ticks (e.g. to simulate CPU execution time)
+    * @param ticks Number of ticks to advance the CPU core
+    */
+    void AddTicks(u64 ticks) override;
 
     /**
      * Saves the current CPU context
diff --git a/src/core/arm/interpreter/arm_interpreter.cpp b/src/core/arm/interpreter/arm_interpreter.cpp
index be04fc1a..80ebc359 100644
--- a/src/core/arm/interpreter/arm_interpreter.cpp
+++ b/src/core/arm/interpreter/arm_interpreter.cpp
@@ -38,78 +38,43 @@ ARM_Interpreter::~ARM_Interpreter() {
     delete state;
 }
 
-/**
- * Set the Program Counter to an address
- * @param addr Address to set PC to
- */
 void ARM_Interpreter::SetPC(u32 pc) {
     state->pc = state->Reg[15] = pc;
 }
 
-/*
- * Get the current Program Counter
- * @return Returns current PC
- */
 u32 ARM_Interpreter::GetPC() const {
     return state->pc;
 }
 
-/**
- * Get an ARM register
- * @param index Register index (0-15)
- * @return Returns the value in the register
- */
 u32 ARM_Interpreter::GetReg(int index) const {
     return state->Reg[index];
 }
 
-/**
- * Set an ARM register
- * @param index Register index (0-15)
- * @param value Value to set register to
- */
 void ARM_Interpreter::SetReg(int index, u32 value) {
     state->Reg[index] = value;
 }
 
-/**
- * Get the current CPSR register
- * @return Returns the value of the CPSR register
- */
 u32 ARM_Interpreter::GetCPSR() const {
     return state->Cpsr;
 }
 
-/**
- * Set the current CPSR register
- * @param cpsr Value to set CPSR to
- */
 void ARM_Interpreter::SetCPSR(u32 cpsr) {
     state->Cpsr = cpsr;
 }
 
-/**
- * Returns the number of clock ticks since the last reset
- * @return Returns number of clock ticks
- */
 u64 ARM_Interpreter::GetTicks() const {
-    return ARMul_Time(state);
+    return state->NumInstrs;
+}
+
+void ARM_Interpreter::AddTicks(u64 ticks) {
+    state->NumInstrs += ticks;
 }
 
-/**
- * Executes the given number of instructions
- * @param num_instructions Number of instructions to executes
- */
 void ARM_Interpreter::ExecuteInstructions(int num_instructions) {
     state->NumInstrsToExecute = num_instructions - 1;
     ARMul_Emulate32(state);
 }
 
-/**
- * Saves the current CPU context
- * @param ctx Thread context to save
- * @todo Do we need to save Reg[15] and NextInstr?
- */
 void ARM_Interpreter::SaveContext(ThreadContext& ctx) {
     memcpy(ctx.cpu_registers, state->Reg, sizeof(ctx.cpu_registers));
     memcpy(ctx.fpu_registers, state->ExtReg, sizeof(ctx.fpu_registers));
@@ -126,11 +91,6 @@ void ARM_Interpreter::SaveContext(ThreadContext& ctx) {
     ctx.mode = state->NextInstr;
 }
 
-/**
- * Loads a CPU context
- * @param ctx Thread context to load
- * @param Do we need to load Reg[15] and NextInstr?
- */
 void ARM_Interpreter::LoadContext(const ThreadContext& ctx) {
     memcpy(state->Reg, ctx.cpu_registers, sizeof(ctx.cpu_registers));
     memcpy(state->ExtReg, ctx.fpu_registers, sizeof(ctx.fpu_registers));
@@ -147,7 +107,6 @@ void ARM_Interpreter::LoadContext(const ThreadContext& ctx) {
     state->NextInstr = ctx.mode;
 }
 
-/// Prepare core for thread reschedule (if needed to correctly handle state)
 void ARM_Interpreter::PrepareReschedule() {
     state->NumInstrsToExecute = 0;
 }
diff --git a/src/core/arm/interpreter/arm_interpreter.h b/src/core/arm/interpreter/arm_interpreter.h
index b685215a..019dad5d 100644
--- a/src/core/arm/interpreter/arm_interpreter.h
+++ b/src/core/arm/interpreter/arm_interpreter.h
@@ -61,6 +61,12 @@ public:
     u64 GetTicks() const override;
 
     /**
+    * Advance the CPU core by the specified number of ticks (e.g. to simulate CPU execution time)
+    * @param ticks Number of ticks to advance the CPU core
+    */
+    void AddTicks(u64 ticks) override;
+
+    /**
      * Saves the current CPU context
      * @param ctx Thread context to save
      */
diff --git a/src/core/hle/hle.cpp b/src/core/hle/hle.cpp
index 2d314a4c..33ac1250 100644
--- a/src/core/hle/hle.cpp
+++ b/src/core/hle/hle.cpp
@@ -43,7 +43,15 @@ void CallSVC(u32 opcode) {
 
 void Reschedule(const char *reason) {
     _dbg_assert_msg_(Kernel, reason != 0 && strlen(reason) < 256, "Reschedule: Invalid or too long reason.");
+
+    // TODO(bunnei): It seems that games depend on some CPU execution time elapsing during HLE
+    // routines. This simulates that time by artificially advancing the number of CPU "ticks".
+    // The value was chosen empirically, it seems to work well enough for everything tested, but
+    // is likely not ideal. We should find a more accurate way to simulate timing with HLE.
+    Core::g_app_core->AddTicks(4000);
+
     Core::g_app_core->PrepareReschedule();
+
     g_reschedule = true;
 }
 
diff --git a/src/core/hw/gpu.cpp b/src/core/hw/gpu.cpp
index 67a8bc32..7e70b34c 100644
--- a/src/core/hw/gpu.cpp
+++ b/src/core/hw/gpu.cpp
@@ -21,12 +21,10 @@ namespace GPU {
 
 Regs g_regs;
 
-u32 g_cur_line = 0;         ///< Current vertical screen line
-u64 g_last_line_ticks = 0;  ///< CPU tick count from last vertical screen line
-u64 g_last_frame_ticks = 0; ///< CPU tick count from last frame
-
-static u32 kFrameCycles = 0; ///< 268MHz / 60 frames per second
-static u32 kFrameTicks  = 0; ///< Approximate number of instructions/frame
+static u64 frame_ticks      = 0;    ///< 268MHz / 60 frames per second
+static u32 cur_line         = 0;    ///< Current vertical screen line
+static u64 last_frame_ticks = 0;    ///< CPU tick count from last frame
+static u64 last_update_tick = 0;    ///< CPU ticl count from last GPU update
 
 template <typename T>
 inline void Read(T &var, const u32 raw_addr) {
@@ -34,7 +32,7 @@ inline void Read(T &var, const u32 raw_addr) {
     u32 index = addr / 4;
 
     // Reads other than u32 are untested, so I'd rather have them abort than silently fail
-    if (index >= Regs::NumIds() || !std::is_same<T,u32>::value) {
+    if (index >= Regs::NumIds() || !std::is_same<T, u32>::value) {
         LOG_ERROR(HW_GPU, "unknown Read%lu @ 0x%08X", sizeof(var) * 8, addr);
         return;
     }
@@ -48,7 +46,7 @@ inline void Write(u32 addr, const T data) {
     u32 index = addr / 4;
 
     // Writes other than u32 are untested, so I'd rather have them abort than silently fail
-    if (index >= Regs::NumIds() || !std::is_same<T,u32>::value) {
+    if (index >= Regs::NumIds() || !std::is_same<T, u32>::value) {
         LOG_ERROR(HW_GPU, "unknown Write%lu 0x%08X @ 0x%08X", sizeof(data) * 8, (u32)data, addr);
         return;
     }
@@ -179,7 +177,6 @@ template void Write<u8>(u32 addr, const u8 data);
 /// Update hardware
 void Update() {
     auto& framebuffer_top = g_regs.framebuffer_config[0];
-    u64 current_ticks = Core::g_app_core->GetTicks();
 
     // Update the frame after a certain number of CPU ticks have elapsed. This assumes that the
     // active frame in memory is always complete to render. There also may be issues with this
@@ -189,9 +186,9 @@ void Update() {
     // primitive homebrew relies on a vertical blank interrupt to happen inevitably (regardless of a
     // threading reschedule).
 
-    if ((current_ticks - g_last_frame_ticks) > GPU::kFrameTicks) {
+    if ((Core::g_app_core->GetTicks() - last_frame_ticks) > (GPU::frame_ticks)) {
         VideoCore::g_renderer->SwapBuffers();
-        g_last_frame_ticks = current_ticks;
+        last_frame_ticks = Core::g_app_core->GetTicks();
     }
 
     // Synchronize GPU on a thread reschedule: Because we cannot accurately predict a vertical
@@ -199,17 +196,20 @@ void Update() {
     // accurately when this is signalled between thread switches.
 
     if (HLE::g_reschedule) {
+        u64 current_ticks = Core::g_app_core->GetTicks();
+        u64 line_ticks = (GPU::frame_ticks / framebuffer_top.height) * 16;
 
-        // Synchronize line...
-        if ((current_ticks - g_last_line_ticks) >= GPU::kFrameTicks / framebuffer_top.height) {
+        //// Synchronize line...
+        if ((current_ticks - last_update_tick) >= line_ticks) {
             GSP_GPU::SignalInterrupt(GSP_GPU::InterruptId::PDC0);
-            g_cur_line++;
-            g_last_line_ticks = current_ticks;
+            cur_line++;
+            last_update_tick += line_ticks;
         }
 
         // Synchronize frame...
-        if (g_cur_line >= framebuffer_top.height) {
-            g_cur_line = 0;
+        if (cur_line >= framebuffer_top.height) {
+            cur_line = 0;
+            VideoCore::g_renderer->SwapBuffers();
             GSP_GPU::SignalInterrupt(GSP_GPU::InterruptId::PDC1);
         }
     }
@@ -217,11 +217,9 @@ void Update() {
 
 /// Initialize hardware
 void Init() {
-    kFrameCycles = 268123480 / Settings::values.gpu_refresh_rate;
-    kFrameTicks  = kFrameCycles / 3;
-
-    g_cur_line = 0;
-    g_last_frame_ticks = g_last_line_ticks = Core::g_app_core->GetTicks();
+    frame_ticks = 268123480 / Settings::values.gpu_refresh_rate;
+    cur_line = 0;
+    last_update_tick = last_frame_ticks = Core::g_app_core->GetTicks();
 
     auto& framebuffer_top = g_regs.framebuffer_config[0];
     auto& framebuffer_sub = g_regs.framebuffer_config[1];