nvservice: add a lock around NvHostEvent and remove release fence on SFv2 (#1197)

* nvservice: add a lock to NvHostEvent

* Disable surface flinger release fence and readd infinite timeout

* FenceAction: Add a timeout of 1 seconds as this shouldn't wait forever anyuway

* surfaceflinger: remove leftovers from the release fence

* Don't allow infinite timeout on syncpoint while printing all timeout for better debugging

Ryujinx.Graphics.Gpu/Synchronization/SynchronizationManager.cs

@@ -112,14 +112,10 @@ namespace Ryujinx.Graphics.Gpu.Synchronization
                 throw new ArgumentOutOfRangeException(nameof(id));
             }
 
-            bool warnAboutTimeout = false;
-
             // TODO: Remove this when GPU channel scheduling will be implemented.
             if (timeout == Timeout.InfiniteTimeSpan)
             {
                 timeout = TimeSpan.FromSeconds(1);
-
-                warnAboutTimeout = true;
             }
 
             using (ManualResetEvent waitEvent = new ManualResetEvent(false))
@@ -135,10 +131,7 @@ namespace Ryujinx.Graphics.Gpu.Synchronization
 
                 if (!signaled && info != null)
                 {
-                    if (warnAboutTimeout)
-                    {
-                        Logger.PrintError(LogClass.Gpu, $"Wait on syncpoint {id} for threshold {threshold} took more than {timeout.TotalMilliseconds}ms, resuming execution...");
-                    }
+                    Logger.PrintError(LogClass.Gpu, $"Wait on syncpoint {id} for threshold {threshold} took more than {timeout.TotalMilliseconds}ms, resuming execution...");
 
                     _syncpoints[id].UnregisterCallback(info);
                 }

Ryujinx.HLE/HOS/Services/Nv/NvDrvServices/NvHostCtrl/NvHostCtrlDeviceFile.cs

@@ -95,26 +95,29 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostCtrl
 
         private KEvent QueryEvent(uint eventId)
         {
-            uint eventSlot;
-            uint syncpointId;
-
-            if ((eventId >> 28) == 1)
-            {
-                eventSlot   = eventId & 0xFFFF;
-                syncpointId = (eventId >> 16) & 0xFFF;
-            }
-            else
+            lock (_events)
             {
-                eventSlot   = eventId & 0xFF;
-                syncpointId = eventId >> 4;
-            }
+                uint eventSlot;
+                uint syncpointId;
 
-            if (eventSlot >= EventsCount || _events[eventSlot] == null || _events[eventSlot].Fence.Id != syncpointId)
-            {
-                return null;
-            }
+                if ((eventId >> 28) == 1)
+                {
+                    eventSlot = eventId & 0xFFFF;
+                    syncpointId = (eventId >> 16) & 0xFFF;
+                }
+                else
+                {
+                    eventSlot = eventId & 0xFF;
+                    syncpointId = eventId >> 4;
+                }
+
+                if (eventSlot >= EventsCount || _events[eventSlot] == null || _events[eventSlot].Fence.Id != syncpointId)
+                {
+                    return null;
+                }
 
-            return _events[eventSlot].Event;
+                return _events[eventSlot].Event;
+            }
         }
 
         public override NvInternalResult QueryEvent(out int eventHandle, uint eventId)
@@ -226,61 +229,71 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostCtrl
 
         private NvInternalResult EventRegister(ref uint userEventId)
         {
-            NvInternalResult result = EventUnregister(ref userEventId);
-
-            if (result == NvInternalResult.Success)
+            lock (_events)
             {
-                _events[userEventId] = new NvHostEvent(_device.System.HostSyncpoint, userEventId, _device.System);
+                NvInternalResult result = EventUnregister(ref userEventId);
+
+                if (result == NvInternalResult.Success)
+                {
+                    _events[userEventId] = new NvHostEvent(_device.System.HostSyncpoint, userEventId, _device.System);
+                }
+
+                return result;
             }
 
-            return result;
         }
 
         private NvInternalResult EventUnregister(ref uint userEventId)
         {
-            if (userEventId >= EventsCount)
+            lock (_events)
             {
-                return NvInternalResult.InvalidInput;
-            }
+                if (userEventId >= EventsCount)
+                {
+                    return NvInternalResult.InvalidInput;
+                }
 
-            NvHostEvent hostEvent = _events[userEventId];
+                NvHostEvent hostEvent = _events[userEventId];
 
-            if (hostEvent == null)
-            {
-                return NvInternalResult.Success;
-            }
+                if (hostEvent == null)
+                {
+                    return NvInternalResult.Success;
+                }
 
-            if (hostEvent.State == NvHostEventState.Available ||
-                hostEvent.State == NvHostEventState.Cancelled ||
-                hostEvent.State == NvHostEventState.Signaled)
-            {
-                _events[userEventId].Dispose();
-                _events[userEventId] = null;
+                if (hostEvent.State == NvHostEventState.Available ||
+                    hostEvent.State == NvHostEventState.Cancelled ||
+                    hostEvent.State == NvHostEventState.Signaled)
+                {
+                    _events[userEventId].Dispose();
+                    _events[userEventId] = null;
 
-                return NvInternalResult.Success;
-            }
+                    return NvInternalResult.Success;
+                }
 
-            return NvInternalResult.Busy;
+                return NvInternalResult.Busy;
+            }
         }
 
         private NvInternalResult EventKill(ref ulong eventMask)
         {
-            NvInternalResult result = NvInternalResult.Success;
-
-            for (uint eventId = 0; eventId < EventsCount; eventId++)
+            lock (_events)
             {
-                if ((eventMask & (1UL << (int)eventId)) != 0)
-                {
-                    NvInternalResult tmp = EventUnregister(ref eventId);
+                NvInternalResult result = NvInternalResult.Success;
 
-                    if (tmp != NvInternalResult.Success)
+                for (uint eventId = 0; eventId < EventsCount; eventId++)
+                {
+                    if ((eventMask & (1UL << (int)eventId)) != 0)
                     {
-                        result = tmp;
+                        NvInternalResult tmp = EventUnregister(ref eventId);
+
+                        if (tmp != NvInternalResult.Success)
+                        {
+                            result = tmp;
+                        }
                     }
                 }
-            }
 
-            return result;
+                return result;
+            }
         }
 
         private NvInternalResult EventSignal(ref uint userEventId)
@@ -292,27 +305,34 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostCtrl
                 return NvInternalResult.InvalidInput;
             }
 
-            NvHostEvent hostEvent = _events[eventId];
-
-            if (hostEvent == null)
+            lock (_events)
             {
-                return NvInternalResult.InvalidInput;
-            }
+                NvHostEvent hostEvent = _events[eventId];
 
-            NvHostEventState oldState = hostEvent.State;
+                if (hostEvent == null)
+                {
+                    return NvInternalResult.InvalidInput;
+                }
 
-            if (oldState == NvHostEventState.Waiting)
-            {
-                hostEvent.State = NvHostEventState.Cancelling;
+                lock (hostEvent.Lock)
+                {
 
-                hostEvent.Cancel(_device.Gpu);
-            }
+                    NvHostEventState oldState = hostEvent.State;
 
-            hostEvent.State = NvHostEventState.Cancelled;
+                    if (oldState == NvHostEventState.Waiting)
+                    {
+                        hostEvent.State = NvHostEventState.Cancelling;
+
+                        hostEvent.Cancel(_device.Gpu);
+                    }
 
-            _device.System.HostSyncpoint.UpdateMin(hostEvent.Fence.Id);
+                    hostEvent.State = NvHostEventState.Cancelled;
 
-            return NvInternalResult.Success;
+                    _device.System.HostSyncpoint.UpdateMin(hostEvent.Fence.Id);
+
+                    return NvInternalResult.Success;
+                }
+            }
         }
 
         private NvInternalResult SyncptReadMinOrMax(ref NvFence arguments, bool max)
@@ -379,67 +399,81 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostCtrl
 
             uint eventIndex;
 
-            if (isWaitEventAsyncCmd)
+            lock (_events)
             {
-                eventIndex = value;
-
-                if (eventIndex >= EventsCount)
+                if (isWaitEventAsyncCmd)
                 {
-                    return NvInternalResult.InvalidInput;
-                }
+                    eventIndex = value;
 
-                hostEvent = _events[eventIndex];
-            }
-            else
-            {
-                hostEvent = GetFreeEvent(fence.Id, out eventIndex);
-            }
-
-            if (hostEvent != null &&
-               (hostEvent.State == NvHostEventState.Available ||
-                hostEvent.State == NvHostEventState.Signaled  ||
-                hostEvent.State == NvHostEventState.Cancelled))
-            {
-                bool timedOut = hostEvent.Wait(_device.Gpu, fence);
-
-                if (timedOut)
-                {
-                    if (isWaitEventCmd)
-                    {
-                        value = ((fence.Id & 0xfff) << 16) | 0x10000000;
-                    }
-                    else
+                    if (eventIndex >= EventsCount)
                     {
-                        value = fence.Id << 4;
+                        return NvInternalResult.InvalidInput;
                     }
 
-                    value |= eventIndex;
-
-                    result = NvInternalResult.TryAgain;
+                    hostEvent = _events[eventIndex];
                 }
                 else
                 {
-                    value = fence.Value;
-
-                    return NvInternalResult.Success;
+                    hostEvent = GetFreeEventLocked(fence.Id, out eventIndex);
                 }
-            }
-            else
-            {
-                Logger.PrintError(LogClass.ServiceNv, $"Invalid Event at index {eventIndex} (isWaitEventAsyncCmd: {isWaitEventAsyncCmd}, isWaitEventCmd: {isWaitEventCmd})");
 
                 if (hostEvent != null)
                 {
-                    Logger.PrintError(LogClass.ServiceNv, hostEvent.DumpState(_device.Gpu));
+                    lock (hostEvent.Lock)
+                    {
+                        if (hostEvent.State == NvHostEventState.Available ||
+                            hostEvent.State == NvHostEventState.Signaled ||
+                            hostEvent.State == NvHostEventState.Cancelled)
+                        {
+                            bool timedOut = hostEvent.Wait(_device.Gpu, fence);
+
+                            if (timedOut)
+                            {
+                                if (isWaitEventCmd)
+                                {
+                                    value = ((fence.Id & 0xfff) << 16) | 0x10000000;
+                                }
+                                else
+                                {
+                                    value = fence.Id << 4;
+                                }
+
+                                value |= eventIndex;
+
+                                result = NvInternalResult.TryAgain;
+                            }
+                            else
+                            {
+                                value = fence.Value;
+
+                                return NvInternalResult.Success;
+                            }
+                        }
+                        else
+                        {
+                            Logger.PrintError(LogClass.ServiceNv, $"Invalid Event at index {eventIndex} (isWaitEventAsyncCmd: {isWaitEventAsyncCmd}, isWaitEventCmd: {isWaitEventCmd})");
+
+                            if (hostEvent != null)
+                            {
+                                Logger.PrintError(LogClass.ServiceNv, hostEvent.DumpState(_device.Gpu));
+                            }
+
+                            result = NvInternalResult.InvalidInput;
+                        }
+                    }
                 }
+                else
+                {
+                    Logger.PrintError(LogClass.ServiceNv, $"Invalid Event at index {eventIndex} (isWaitEventAsyncCmd: {isWaitEventAsyncCmd}, isWaitEventCmd: {isWaitEventCmd})");
 
-                result = NvInternalResult.InvalidInput;
+                    result = NvInternalResult.InvalidInput;
+                }
             }
 
             return result;
         }
 
-        public NvHostEvent GetFreeEvent(uint id, out uint eventIndex)
+        private NvHostEvent GetFreeEventLocked(uint id, out uint eventIndex)
         {
             eventIndex = EventsCount;
 
@@ -490,38 +524,44 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostCtrl
         {
             Logger.PrintWarning(LogClass.ServiceNv, "Closing channel");
 
-            // If the device file need to be closed, cancel all user events and dispose events.
-            for (int i = 0; i < _events.Length; i++)
+            lock (_events)
             {
-                NvHostEvent evnt = _events[i];
-
-                if (evnt != null)
+                // If the device file need to be closed, cancel all user events and dispose events.
+                for (int i = 0; i < _events.Length; i++)
                 {
-                    if (evnt.State == NvHostEventState.Waiting)
-                    {
-                        evnt.State = NvHostEventState.Cancelling;
+                    NvHostEvent evnt = _events[i];
 
-                        evnt.Cancel(_device.Gpu);
-                    }
-                    else if (evnt.State == NvHostEventState.Signaling)
+                    if (evnt != null)
                     {
-                        // Wait at max 9ms if the guest app is trying to signal the event while closing it..
-                        int retryCount = 0;
-                        do
+                        lock (evnt.Lock)
                         {
-                            if (retryCount++ > 9)
+                            if (evnt.State == NvHostEventState.Waiting)
                             {
-                                break;
-                            }
+                                evnt.State = NvHostEventState.Cancelling;
 
-                            // TODO: This should be handled by the kernel (reschedule the current thread ect), waiting for Kernel decoupling work.
-                            Thread.Sleep(1);
-                        } while (evnt.State != NvHostEventState.Signaled);
-                    }
+                                evnt.Cancel(_device.Gpu);
+                            }
+                            else if (evnt.State == NvHostEventState.Signaling)
+                            {
+                                // Wait at max 9ms if the guest app is trying to signal the event while closing it..
+                                int retryCount = 0;
+                                do
+                                {
+                                    if (retryCount++ > 9)
+                                    {
+                                        break;
+                                    }
+
+                                    // TODO: This should be handled by the kernel (reschedule the current thread ect), waiting for Kernel decoupling work.
+                                    Thread.Sleep(1);
+                                } while (evnt.State != NvHostEventState.Signaled);
+                            }
 
-                    evnt.Dispose();
+                            evnt.Dispose();
 
-                   _events[i] = null;
+                            _events[i] = null;
+                        }
+                    }
                 }
             }
         }

Ryujinx.HLE/HOS/Services/Nv/NvDrvServices/NvHostCtrl/Types/NvHostEvent.cs

@@ -21,6 +21,8 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostCtrl
         private NvFence _previousFailingFence;
         private uint    _failingCount;
 
+        public object Lock = new object();
+
         /// <summary>
         /// Max failing count until waiting on CPU.
         /// FIXME: This seems enough for most of the cases, reduce if needed.
@@ -49,83 +51,88 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostCtrl
             _failingCount               = 0;
         }
 
-        public void Reset()
-        {
-            Fence.Id    = 0;
-            Fence.Value = 0;
-            State       = NvHostEventState.Available;
-        }
-
         private void Signal()
         {
-            NvHostEventState oldState = State;
+            lock (Lock)
+            {
+                NvHostEventState oldState = State;
 
-            State = NvHostEventState.Signaling;
+                State = NvHostEventState.Signaling;
 
-            if (oldState == NvHostEventState.Waiting)
-            {
-                Event.WritableEvent.Signal();
-            }
+                if (oldState == NvHostEventState.Waiting)
+                {
+                    Event.WritableEvent.Signal();
+                }
 
-            State = NvHostEventState.Signaled;
+                State = NvHostEventState.Signaled;
+            }
         }
 
         private void GpuSignaled()
         {
-            ResetFailingState();
+            lock (Lock)
+            {
+                ResetFailingState();
 
-            Signal();
+                Signal();
+            }
         }
 
         public void Cancel(GpuContext gpuContext)
         {
-            if (_waiterInformation != null)
+            lock (Lock)
             {
-                gpuContext.Synchronization.UnregisterCallback(Fence.Id, _waiterInformation);
-
-                if (_previousFailingFence.Id == Fence.Id && _previousFailingFence.Value == Fence.Value)
+                if (_waiterInformation != null)
                 {
-                    _failingCount++;
-                }
-                else
-                {
-                    _failingCount = 1;
+                    gpuContext.Synchronization.UnregisterCallback(Fence.Id, _waiterInformation);
+
+                    if (_previousFailingFence.Id == Fence.Id && _previousFailingFence.Value == Fence.Value)
+                    {
+                        _failingCount++;
+                    }
+                    else
+                    {
+                        _failingCount = 1;
 
-                    _previousFailingFence = Fence;
+                        _previousFailingFence = Fence;
+                    }
+
+                    Signal();
                 }
 
-                Signal();
+                Event.WritableEvent.Clear();
             }
-
-            Event.WritableEvent.Clear();
         }
 
         public bool Wait(GpuContext gpuContext, NvFence fence)
         {
-            Fence = fence;
-            State = NvHostEventState.Waiting;
-
-            // 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).
-            //       The reason for this is that the NVN code will try to wait until giving up.
-            //       This is done by trying to wait and signal multiple times until aborting after you are past the timeout.
-            //       As such, if it fails too many time, we enforce a wait on the CPU side indefinitely.
-            //       This allows to keep GPU and CPU in sync when we are slow.
-            if (_failingCount == FailingCountMax)
+            lock (Lock)
             {
-                Logger.PrintWarning(LogClass.ServiceNv, "GPU processing thread is too slow, waiting on CPU...");
+                Fence = fence;
+                State = NvHostEventState.Waiting;
+
+                // 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).
+                //       The reason for this is that the NVN code will try to wait until giving up.
+                //       This is done by trying to wait and signal multiple times until aborting after you are past the timeout.
+                //       As such, if it fails too many time, we enforce a wait on the CPU side indefinitely.
+                //       This allows to keep GPU and CPU in sync when we are slow.
+                if (_failingCount == FailingCountMax)
+                {
+                    Logger.PrintWarning(LogClass.ServiceNv, "GPU processing thread is too slow, waiting on CPU...");
 
-                bool timedOut = Fence.Wait(gpuContext, Timeout.InfiniteTimeSpan);
+                    bool timedOut = Fence.Wait(gpuContext, Timeout.InfiniteTimeSpan);
 
-                GpuSignaled();
+                    GpuSignaled();
 
-                return timedOut;
-            }
-            else
-            {
-                _waiterInformation = gpuContext.Synchronization.RegisterCallbackOnSyncpoint(Fence.Id, Fence.Value, GpuSignaled);
+                    return timedOut;
+                }
+                else
+                {
+                    _waiterInformation = gpuContext.Synchronization.RegisterCallbackOnSyncpoint(Fence.Id, Fence.Value, GpuSignaled);
 
-                return true;
+                    return true;
+                }
             }
         }
 

Ryujinx.HLE/HOS/Services/SurfaceFlinger/SurfaceFlinger.cs

@@ -26,8 +26,6 @@ namespace Ryujinx.HLE.HOS.Services.SurfaceFlinger
 
         private Stopwatch _chrono;
 
-        private AndroidFence _vblankFence;
-
         private long _ticks;
         private long _ticksPerFrame;
 
@@ -49,7 +47,6 @@ namespace Ryujinx.HLE.HOS.Services.SurfaceFlinger
         {
             public Layer        Layer;
             public BufferItem   Item;
-            public AndroidFence Fence;
         }
 
         public SurfaceFlinger(Switch device)
@@ -69,13 +66,6 @@ namespace Ryujinx.HLE.HOS.Services.SurfaceFlinger
 
             UpdateSwapInterval(1);
 
-            _vblankFence = AndroidFence.NoFence;
-            _vblankFence.AddFence(new NvFence
-            {
-                Id    = NvHostSyncpt.VBlank0SyncpointId,
-                Value = 0
-            });
-
             _composerThread.Start();
         }
 
@@ -222,8 +212,6 @@ namespace Ryujinx.HLE.HOS.Services.SurfaceFlinger
         {
             lock (Lock)
             {
-                _vblankFence.NvFences[0].Increment(_device.Gpu);
-
                 // TODO: support multilayers (& multidisplay ?)
                 if (_layers.Count == 0)
                 {
@@ -298,14 +286,10 @@ namespace Ryujinx.HLE.HOS.Services.SurfaceFlinger
                 flipX,
                 flipY);
 
-            // Enforce that dequeueBuffer wait for the next vblank
-            _vblankFence.NvFences[0].Value++;
-
             TextureCallbackInformation textureCallbackInformation = new TextureCallbackInformation
             {
                 Layer = layer,
                 Item  = item,
-                Fence = _vblankFence
             };
 
             _device.Gpu.Window.EnqueueFrameThreadSafe(
@@ -330,7 +314,9 @@ namespace Ryujinx.HLE.HOS.Services.SurfaceFlinger
 
         private void ReleaseBuffer(TextureCallbackInformation information)
         {
-            information.Layer.Consumer.ReleaseBuffer(information.Item, ref information.Fence);
+            AndroidFence fence = AndroidFence.NoFence;
+
+            information.Layer.Consumer.ReleaseBuffer(information.Item, ref fence);
         }
 
         private void AcquireBuffer(GpuContext ignored, object obj)