Merge pull request #1536 from skidau/fifo-overflow

Fifo overflow fix

The idea behind separating the CPU and GPU thread path is to prevent two threads executing the same function (SetCPStatusFromCPU) at the same time. The CPU thread gets to that function via GetherPipeBursted and the GPU thread gets there via SetCPStatusFromGPU. I wrote the original (factored) version as I like to keep code duplication to a minimum. This worked most of the time but not all of the time. It was a good move to separate it to a GPU version and a CPU version, but then the GPU version called the CPU version at the tail end. Removing the call (as in this PR) prevents the FIFO overflow in Starfox Adventures.

The "Updating the CPStatus before FIFO events" change is simply there to keep the DC code path and the SC code path consistent in the !GPLinked scenario. The SC code path does the same thing when !GPLinked via RunGPU. Putting the SetCPStatus call before the !GPLinked state changes the DC code path to do the same thing. The more we keep the DC and SC code paths consistent, the better.

Forcing the exception check on interrupts is the change for The Last Story. It makes the emulator process interrupts more responsively on CP interrupts.

The HiWatermark check is something that old (pre-2010) versions of Dolphin used to do. Games like Battalion Wars 2 do not expect the GPU to run so slowly and do not have the code to react to that situation, so this change makes the emulator extra careful in those situations.

Source/Core/VideoCommon/CommandProcessor.cpp

@@ -304,6 +304,9 @@ void RegisterMMIO(MMIO::Mapping* mmio, u32 base)
 
 void GatherPipeBursted()
 {
+	if (IsOnThread())
+		SetCPStatusFromCPU();
+
 	ProcessFifoEvents();
 	// if we aren't linked, we don't care about gather pipe data
 	if (!m_CPCtrlReg.GPLinkEnable)
@@ -326,11 +329,8 @@ void GatherPipeBursted()
 		return;
 	}
 
-	if (IsOnThread())
-		SetCPStatusFromCPU();
-
 	// update the fifo pointer
-	if (fifo.CPWritePointer >= fifo.CPEnd)
+	if (fifo.CPWritePointer == fifo.CPEnd)
 		fifo.CPWritePointer = fifo.CPBase;
 	else
 		fifo.CPWritePointer += GATHER_PIPE_SIZE;
@@ -342,6 +342,10 @@ void GatherPipeBursted()
 		ProcessorInterface::Fifo_CPUEnd = fifo.CPEnd;
 	}
 
+	// If the game is running close to overflowing, make the exception checking more frequent.
+	if (fifo.bFF_HiWatermark)
+		CoreTiming::ForceExceptionCheck(0);
+
 	Common::AtomicAdd(fifo.CPReadWriteDistance, GATHER_PIPE_SIZE);
 
 	RunGpu();
@@ -369,6 +373,7 @@ void UpdateInterrupts(u64 userdata)
 		INFO_LOG(COMMANDPROCESSOR,"Interrupt cleared");
 		ProcessorInterface::SetInterrupt(INT_CAUSE_CP, false);
 	}
+	CoreTiming::ForceExceptionCheck(0);
 	interruptWaiting = false;
 }
 
@@ -404,7 +409,34 @@ void SetCPStatusFromGPU()
 			INFO_LOG(COMMANDPROCESSOR, "Cleared breakpoint at %i", fifo.CPReadPointer);
 		fifo.bFF_Breakpoint = false;
 	}
-	SetCPStatusFromCPU();
+
+	// overflow & underflow check
+	fifo.bFF_HiWatermark = (fifo.CPReadWriteDistance > fifo.CPHiWatermark);
+	fifo.bFF_LoWatermark = (fifo.CPReadWriteDistance < fifo.CPLoWatermark);
+
+	bool bpInt = fifo.bFF_Breakpoint && fifo.bFF_BPInt;
+	bool ovfInt = fifo.bFF_HiWatermark && fifo.bFF_HiWatermarkInt;
+	bool undfInt = fifo.bFF_LoWatermark && fifo.bFF_LoWatermarkInt;
+
+	bool interrupt = (bpInt || ovfInt || undfInt) && m_CPCtrlReg.GPReadEnable;
+
+	if (interrupt != interruptSet && !interruptWaiting)
+	{
+		u64 userdata = interrupt ? 1 : 0;
+		if (IsOnThread())
+		{
+			if (!interrupt || bpInt || undfInt || ovfInt)
+			{
+				// Schedule the interrupt asynchronously
+				interruptWaiting = true;
+				CommandProcessor::UpdateInterruptsFromVideoBackend(userdata);
+			}
+		}
+		else
+		{
+			CommandProcessor::UpdateInterrupts(userdata);
+		}
+	}
 }
 
 void SetCPStatusFromCPU()
@@ -421,23 +453,14 @@ void SetCPStatusFromCPU()
 
 	if (interrupt != interruptSet && !interruptWaiting)
 	{
-		u64 userdata = interrupt?1:0;
+		u64 userdata = interrupt ? 1 : 0;
 		if (IsOnThread())
 		{
 			if (!interrupt || bpInt || undfInt || ovfInt)
 			{
-				if (Core::IsGPUThread())
-				{
-					// Schedule the interrupt asynchronously
-					interruptWaiting = true;
-					CommandProcessor::UpdateInterruptsFromVideoBackend(userdata);
-				}
-				else
-				{
-					interruptSet = interrupt;
-					INFO_LOG(COMMANDPROCESSOR,"Interrupt set");
-					ProcessorInterface::SetInterrupt(INT_CAUSE_CP, interrupt);
-				}
+				interruptSet = interrupt;
+				INFO_LOG(COMMANDPROCESSOR,"Interrupt set");
+				ProcessorInterface::SetInterrupt(INT_CAUSE_CP, interrupt);
 			}
 		}
 		else
@@ -451,8 +474,7 @@ void ProcessFifoAllDistance()
 {
 	if (IsOnThread())
 	{
-		while (!CommandProcessor::interruptWaiting && fifo.bFF_GPReadEnable &&
-			fifo.CPReadWriteDistance && !AtBreakpoint())
+		while (!interruptWaiting && fifo.bFF_GPReadEnable && fifo.CPReadWriteDistance && !AtBreakpoint())
 			Common::YieldCPU();
 	}
 }
@@ -489,13 +511,6 @@ void SetCpStatusRegister()
 
 void SetCpControlRegister()
 {
-	// If the new fifo is being attached, force an exception check
-	// This fixes the hang while booting Eternal Darkness
-	if (!fifo.bFF_GPReadEnable && m_CPCtrlReg.GPReadEnable && !m_CPCtrlReg.BPEnable)
-	{
-		CoreTiming::ForceExceptionCheck(0);
-	}
-
 	fifo.bFF_BPInt = m_CPCtrlReg.BPInt;
 	fifo.bFF_BPEnable = m_CPCtrlReg.BPEnable;
 	fifo.bFF_HiWatermarkInt = m_CPCtrlReg.FifoOverflowIntEnable;

Source/Core/VideoCommon/OpcodeDecoding.cpp

@@ -118,10 +118,19 @@ static void UnknownOpcode(u8 cmd_byte, void *buffer, bool preprocess)
 		    "bFF_BPEnable: %s\n"
 		    "bFF_BPInt: %s\n"
 		    "bFF_Breakpoint: %s\n"
+		    "bFF_GPLinkEnable: %s\n"
+		    "bFF_HiWatermarkInt: %s\n"
+		    "bFF_LoWatermarkInt: %s\n"
 		    ,cmd_byte, fifo.CPBase, fifo.CPEnd, fifo.CPHiWatermark, fifo.CPLoWatermark, fifo.CPReadWriteDistance
-		    ,fifo.CPWritePointer, fifo.CPReadPointer, fifo.CPBreakpoint, fifo.bFF_GPReadEnable ? "true" : "false"
-		    ,fifo.bFF_BPEnable ? "true" : "false" ,fifo.bFF_BPInt ? "true" : "false"
-		    ,fifo.bFF_Breakpoint ? "true" : "false");
+		    ,fifo.CPWritePointer, fifo.CPReadPointer, fifo.CPBreakpoint
+		    ,fifo.bFF_GPReadEnable ? "true" : "false"
+		    ,fifo.bFF_BPEnable ? "true" : "false"
+		    ,fifo.bFF_BPInt ? "true" : "false"
+		    ,fifo.bFF_Breakpoint ? "true" : "false"
+		    ,fifo.bFF_GPLinkEnable ? "true" : "false"
+		    ,fifo.bFF_HiWatermarkInt ? "true" : "false"
+		    ,fifo.bFF_LoWatermarkInt ? "true" : "false"
+		    );
 	}
 }