EventWait should not signal the event when it returns Success (#2739)

* Fix race when EventWait is called and a wait is done on the CPU

* This is useless now

* Fix EventSignal

* Ensure the signal belongs to the current fence, to avoid stale signals
This commit is contained in:
gdkchan 2021-10-19 17:25:32 -03:00 committed by GitHub
parent 63f1663fa9
commit 0d174cbd45
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 35 additions and 35 deletions

View file

@ -70,7 +70,7 @@ namespace Ryujinx.Graphics.Gpu.Synchronization
/// <param name="callback">The callback to call when the threshold is reached</param> /// <param name="callback">The callback to call when the threshold is reached</param>
/// <exception cref="System.ArgumentOutOfRangeException">Thrown when id >= MaxHardwareSyncpoints</exception> /// <exception cref="System.ArgumentOutOfRangeException">Thrown when id >= MaxHardwareSyncpoints</exception>
/// <returns>The created SyncpointWaiterHandle object or null if already past threshold</returns> /// <returns>The created SyncpointWaiterHandle object or null if already past threshold</returns>
public SyncpointWaiterHandle RegisterCallbackOnSyncpoint(uint id, uint threshold, Action callback) public SyncpointWaiterHandle RegisterCallbackOnSyncpoint(uint id, uint threshold, Action<SyncpointWaiterHandle> callback)
{ {
if (id >= MaxHardwareSyncpoints) if (id >= MaxHardwareSyncpoints)
{ {
@ -120,7 +120,7 @@ namespace Ryujinx.Graphics.Gpu.Synchronization
using (ManualResetEvent waitEvent = new ManualResetEvent(false)) using (ManualResetEvent waitEvent = new ManualResetEvent(false))
{ {
var info = _syncpoints[id].RegisterCallback(threshold, () => waitEvent.Set()); var info = _syncpoints[id].RegisterCallback(threshold, (x) => waitEvent.Set());
if (info == null) if (info == null)
{ {

View file

@ -34,13 +34,13 @@ namespace Ryujinx.Graphics.Gpu.Synchronization
/// <param name="threshold">The target threshold</param> /// <param name="threshold">The target threshold</param>
/// <param name="callback">The callback to call when the threshold is reached</param> /// <param name="callback">The callback to call when the threshold is reached</param>
/// <returns>The created SyncpointWaiterHandle object or null if already past threshold</returns> /// <returns>The created SyncpointWaiterHandle object or null if already past threshold</returns>
public SyncpointWaiterHandle RegisterCallback(uint threshold, Action callback) public SyncpointWaiterHandle RegisterCallback(uint threshold, Action<SyncpointWaiterHandle> callback)
{ {
lock (_waiters) lock (_waiters)
{ {
if (Value >= threshold) if (Value >= threshold)
{ {
callback(); callback(null);
return null; return null;
} }
@ -111,13 +111,13 @@ namespace Ryujinx.Graphics.Gpu.Synchronization
// we can't call it inside the lock. // we can't call it inside the lock.
if (expired != null) if (expired != null)
{ {
expired.Callback(); expired.Callback(expired);
if (expiredList != null) if (expiredList != null)
{ {
for (int i = 0; i < expiredList.Count; i++) for (int i = 0; i < expiredList.Count; i++)
{ {
expiredList[i].Callback(); expiredList[i].Callback(expiredList[i]);
} }
} }
} }

View file

@ -5,6 +5,6 @@ namespace Ryujinx.Graphics.Gpu.Synchronization
public class SyncpointWaiterHandle public class SyncpointWaiterHandle
{ {
internal uint Threshold; internal uint Threshold;
internal Action Callback; internal Action<SyncpointWaiterHandle> Callback;
} }
} }

View file

@ -314,26 +314,13 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostCtrl
return NvInternalResult.InvalidInput; return NvInternalResult.InvalidInput;
} }
lock (hostEvent.Lock)
{
NvHostEventState oldState = hostEvent.State;
if (oldState == NvHostEventState.Waiting)
{
hostEvent.State = NvHostEventState.Cancelling;
hostEvent.Cancel(_device.Gpu); hostEvent.Cancel(_device.Gpu);
}
hostEvent.State = NvHostEventState.Cancelled;
_device.System.HostSyncpoint.UpdateMin(hostEvent.Fence.Id); _device.System.HostSyncpoint.UpdateMin(hostEvent.Fence.Id);
return NvInternalResult.Success; return NvInternalResult.Success;
} }
} }
}
private NvInternalResult SyncptReadMinOrMax(ref NvFence arguments, bool max) private NvInternalResult SyncptReadMinOrMax(ref NvFence arguments, bool max)
{ {

View file

@ -68,10 +68,17 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostCtrl
} }
} }
private void GpuSignaled() private void GpuSignaled(SyncpointWaiterHandle waiterInformation)
{ {
lock (Lock) lock (Lock)
{ {
// If the signal does not match our current waiter,
// then it is from a past fence and we should just ignore it.
if (waiterInformation != null && waiterInformation != _waiterInformation)
{
return;
}
ResetFailingState(); ResetFailingState();
Signal(); Signal();
@ -82,9 +89,14 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostCtrl
{ {
lock (Lock) lock (Lock)
{ {
if (_waiterInformation != null) NvHostEventState oldState = State;
State = NvHostEventState.Cancelling;
if (oldState == NvHostEventState.Waiting && _waiterInformation != null)
{ {
gpuContext.Synchronization.UnregisterCallback(Fence.Id, _waiterInformation); gpuContext.Synchronization.UnregisterCallback(Fence.Id, _waiterInformation);
_waiterInformation = null;
if (_previousFailingFence.Id == Fence.Id && _previousFailingFence.Value == Fence.Value) if (_previousFailingFence.Id == Fence.Id && _previousFailingFence.Value == Fence.Value)
{ {
@ -96,10 +108,10 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostCtrl
_previousFailingFence = Fence; _previousFailingFence = Fence;
} }
Signal();
} }
State = NvHostEventState.Cancelled;
Event.WritableEvent.Clear(); Event.WritableEvent.Clear();
} }
} }
@ -108,9 +120,6 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostCtrl
{ {
lock (Lock) lock (Lock)
{ {
Fence = fence;
State = NvHostEventState.Waiting;
// NOTE: nvservices code should always wait on the GPU side. // NOTE: nvservices code should always wait on the GPU side.
// If we do this, we may get an abort or undefined behaviour when the GPU processing thread is blocked for a long period (for example, during shader compilation). // If we do this, we may get an abort or undefined behaviour when the GPU processing thread is blocked for a long period (for example, during shader compilation).
// The reason for this is that the NVN code will try to wait until giving up. // The reason for this is that the NVN code will try to wait until giving up.
@ -123,12 +132,15 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostCtrl
bool timedOut = Fence.Wait(gpuContext, Timeout.InfiniteTimeSpan); bool timedOut = Fence.Wait(gpuContext, Timeout.InfiniteTimeSpan);
GpuSignaled(); ResetFailingState();
return timedOut; return timedOut;
} }
else else
{ {
Fence = fence;
State = NvHostEventState.Waiting;
_waiterInformation = gpuContext.Synchronization.RegisterCallbackOnSyncpoint(Fence.Id, Fence.Value, GpuSignaled); _waiterInformation = gpuContext.Synchronization.RegisterCallbackOnSyncpoint(Fence.Id, Fence.Value, GpuSignaled);
return true; return true;

View file

@ -392,7 +392,7 @@ namespace Ryujinx.HLE.HOS.Services.SurfaceFlinger
} }
else else
{ {
item.Fence.RegisterCallback(_device.Gpu, () => item.Fence.RegisterCallback(_device.Gpu, (x) =>
{ {
_device.Gpu.Window.SignalFrameReady(); _device.Gpu.Window.SignalFrameReady();
_device.Gpu.GPFifo.Interrupt(); _device.Gpu.GPFifo.Interrupt();

View file

@ -1,5 +1,6 @@
using Ryujinx.Common.Logging; using Ryujinx.Common.Logging;
using Ryujinx.Graphics.Gpu; using Ryujinx.Graphics.Gpu;
using Ryujinx.Graphics.Gpu.Synchronization;
using Ryujinx.HLE.HOS.Services.Nv.Types; using Ryujinx.HLE.HOS.Services.Nv.Types;
using System; using System;
using System.Runtime.CompilerServices; using System.Runtime.CompilerServices;
@ -66,7 +67,7 @@ namespace Ryujinx.HLE.HOS.Services.SurfaceFlinger
return false; return false;
} }
public void RegisterCallback(GpuContext gpuContext, Action callback) public void RegisterCallback(GpuContext gpuContext, Action<SyncpointWaiterHandle> callback)
{ {
ref NvFence fence = ref NvFences[FenceCount - 1]; ref NvFence fence = ref NvFences[FenceCount - 1];
@ -76,7 +77,7 @@ namespace Ryujinx.HLE.HOS.Services.SurfaceFlinger
} }
else else
{ {
callback(); callback(null);
} }
} }