Merge pull request #8992 from Morph1984/vi-vsync-event

service: vi: Retrieve vsync event once per display

src/core/hle/service/nvflinger/nvflinger.cpp

@@ -22,6 +22,7 @@
 #include "core/hle/service/nvflinger/ui/graphic_buffer.h"
 #include "core/hle/service/vi/display/vi_display.h"
 #include "core/hle/service/vi/layer/vi_layer.h"
+#include "core/hle/service/vi/vi_results.h"
 #include "video_core/gpu.h"
 
 namespace Service::NVFlinger {
@@ -163,15 +164,15 @@ std::optional<u32> NVFlinger::FindBufferQueueId(u64 display_id, u64 layer_id) {
     return layer->GetBinderId();
 }
 
-Kernel::KReadableEvent* NVFlinger::FindVsyncEvent(u64 display_id) {
+ResultVal<Kernel::KReadableEvent*> NVFlinger::FindVsyncEvent(u64 display_id) {
     const auto lock_guard = Lock();
     auto* const display = FindDisplay(display_id);
 
     if (display == nullptr) {
-        return nullptr;
+        return VI::ResultNotFound;
     }
 
-    return &display->GetVSyncEvent();
+    return display->GetVSyncEvent();
 }
 
 VI::Display* NVFlinger::FindDisplay(u64 display_id) {

src/core/hle/service/nvflinger/nvflinger.h

@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "common/common_types.h"
+#include "core/hle/result.h"
 #include "core/hle/service/kernel_helpers.h"
 
 namespace Common {
@@ -71,8 +72,9 @@ public:
 
     /// Gets the vsync event for the specified display.
     ///
-    /// If an invalid display ID is provided, then nullptr is returned.
-    [[nodiscard]] Kernel::KReadableEvent* FindVsyncEvent(u64 display_id);
+    /// If an invalid display ID is provided, then VI::ResultNotFound is returned.
+    /// If the vsync event has already been retrieved, then VI::ResultPermissionDenied is returned.
+    [[nodiscard]] ResultVal<Kernel::KReadableEvent*> FindVsyncEvent(u64 display_id);
 
     /// Performs a composition request to the emulated nvidia GPU and triggers the vsync events when
     /// finished.

src/core/hle/service/vi/display/vi_display.cpp

@@ -19,6 +19,7 @@
 #include "core/hle/service/nvflinger/hos_binder_driver_server.h"
 #include "core/hle/service/vi/display/vi_display.h"
 #include "core/hle/service/vi/layer/vi_layer.h"
+#include "core/hle/service/vi/vi_results.h"
 
 namespace Service::VI {
 
@@ -55,8 +56,18 @@ const Layer& Display::GetLayer(std::size_t index) const {
     return *layers.at(index);
 }
 
-Kernel::KReadableEvent& Display::GetVSyncEvent() {
-    return vsync_event->GetReadableEvent();
+ResultVal<Kernel::KReadableEvent*> Display::GetVSyncEvent() {
+    if (got_vsync_event) {
+        return ResultPermissionDenied;
+    }
+
+    got_vsync_event = true;
+
+    return GetVSyncEventUnchecked();
+}
+
+Kernel::KReadableEvent* Display::GetVSyncEventUnchecked() {
+    return &vsync_event->GetReadableEvent();
 }
 
 void Display::SignalVSyncEvent() {

src/core/hle/service/vi/display/vi_display.h

@@ -9,6 +9,7 @@
 
 #include "common/common_funcs.h"
 #include "common/common_types.h"
+#include "core/hle/result.h"
 
 namespace Kernel {
 class KEvent;
@@ -73,8 +74,16 @@ public:
         return layers.size();
     }
 
-    /// Gets the readable vsync event.
-    Kernel::KReadableEvent& GetVSyncEvent();
+    /**
+     * Gets the internal vsync event.
+     *
+     * @returns The internal Vsync event if it has not yet been retrieved,
+     *          VI::ResultPermissionDenied otherwise.
+     */
+    [[nodiscard]] ResultVal<Kernel::KReadableEvent*> GetVSyncEvent();
+
+    /// Gets the internal vsync event.
+    Kernel::KReadableEvent* GetVSyncEventUnchecked();
 
     /// Signals the internal vsync event.
     void SignalVSyncEvent();
@@ -118,6 +127,7 @@ private:
 
     std::vector<std::unique_ptr<Layer>> layers;
     Kernel::KEvent* vsync_event{};
+    bool got_vsync_event{false};
 };
 
 } // namespace Service::VI

src/core/hle/service/vi/vi.cpp

@@ -29,16 +29,12 @@
 #include "core/hle/service/service.h"
 #include "core/hle/service/vi/vi.h"
 #include "core/hle/service/vi/vi_m.h"
+#include "core/hle/service/vi/vi_results.h"
 #include "core/hle/service/vi/vi_s.h"
 #include "core/hle/service/vi/vi_u.h"
 
 namespace Service::VI {
 
-constexpr Result ERR_OPERATION_FAILED{ErrorModule::VI, 1};
-constexpr Result ERR_PERMISSION_DENIED{ErrorModule::VI, 5};
-constexpr Result ERR_UNSUPPORTED{ErrorModule::VI, 6};
-constexpr Result ERR_NOT_FOUND{ErrorModule::VI, 7};
-
 struct DisplayInfo {
     /// The name of this particular display.
     char display_name[0x40]{"Default"};
@@ -348,7 +344,7 @@ private:
         if (!layer_id) {
             LOG_ERROR(Service_VI, "Layer not found! display=0x{:016X}", display);
             IPC::ResponseBuilder rb{ctx, 2};
-            rb.Push(ERR_NOT_FOUND);
+            rb.Push(ResultNotFound);
             return;
         }
 
@@ -498,7 +494,7 @@ private:
         if (!display_id) {
             LOG_ERROR(Service_VI, "Display not found! display_name={}", name);
             IPC::ResponseBuilder rb{ctx, 2};
-            rb.Push(ERR_NOT_FOUND);
+            rb.Push(ResultNotFound);
             return;
         }
 
@@ -554,14 +550,14 @@ private:
 
         if (scaling_mode > NintendoScaleMode::PreserveAspectRatio) {
             LOG_ERROR(Service_VI, "Invalid scaling mode provided.");
-            rb.Push(ERR_OPERATION_FAILED);
+            rb.Push(ResultOperationFailed);
             return;
         }
 
         if (scaling_mode != NintendoScaleMode::ScaleToWindow &&
             scaling_mode != NintendoScaleMode::PreserveAspectRatio) {
             LOG_ERROR(Service_VI, "Unsupported scaling mode supplied.");
-            rb.Push(ERR_UNSUPPORTED);
+            rb.Push(ResultNotSupported);
             return;
         }
 
@@ -594,7 +590,7 @@ private:
         if (!display_id) {
             LOG_ERROR(Service_VI, "Layer not found! layer_id={}", layer_id);
             IPC::ResponseBuilder rb{ctx, 2};
-            rb.Push(ERR_NOT_FOUND);
+            rb.Push(ResultNotFound);
             return;
         }
 
@@ -602,7 +598,7 @@ private:
         if (!buffer_queue_id) {
             LOG_ERROR(Service_VI, "Buffer queue id not found! display_id={}", *display_id);
             IPC::ResponseBuilder rb{ctx, 2};
-            rb.Push(ERR_NOT_FOUND);
+            rb.Push(ResultNotFound);
             return;
         }
 
@@ -640,7 +636,7 @@ private:
         if (!layer_id) {
             LOG_ERROR(Service_VI, "Layer not found! display_id={}", display_id);
             IPC::ResponseBuilder rb{ctx, 2};
-            rb.Push(ERR_NOT_FOUND);
+            rb.Push(ResultNotFound);
             return;
         }
 
@@ -648,7 +644,7 @@ private:
         if (!buffer_queue_id) {
             LOG_ERROR(Service_VI, "Buffer queue id not found! display_id={}", display_id);
             IPC::ResponseBuilder rb{ctx, 2};
-            rb.Push(ERR_NOT_FOUND);
+            rb.Push(ResultNotFound);
             return;
         }
 
@@ -675,19 +671,23 @@ private:
         IPC::RequestParser rp{ctx};
         const u64 display_id = rp.Pop<u64>();
 
-        LOG_WARNING(Service_VI, "(STUBBED) called. display_id=0x{:016X}", display_id);
+        LOG_DEBUG(Service_VI, "called. display_id={}", display_id);
 
         const auto vsync_event = nv_flinger.FindVsyncEvent(display_id);
-        if (!vsync_event) {
-            LOG_ERROR(Service_VI, "Vsync event was not found for display_id={}", display_id);
+        if (vsync_event.Failed()) {
+            const auto result = vsync_event.Code();
+            if (result == ResultNotFound) {
+                LOG_ERROR(Service_VI, "Vsync event was not found for display_id={}", display_id);
+            }
+
             IPC::ResponseBuilder rb{ctx, 2};
-            rb.Push(ERR_NOT_FOUND);
+            rb.Push(result);
             return;
         }
 
         IPC::ResponseBuilder rb{ctx, 2, 1};
         rb.Push(ResultSuccess);
-        rb.PushCopyObjects(vsync_event);
+        rb.PushCopyObjects(*vsync_event);
     }
 
     void ConvertScalingMode(Kernel::HLERequestContext& ctx) {
@@ -764,7 +764,7 @@ private:
             return ConvertedScaleMode::PreserveAspectRatio;
         default:
             LOG_ERROR(Service_VI, "Invalid scaling mode specified, mode={}", mode);
-            return ERR_OPERATION_FAILED;
+            return ResultOperationFailed;
         }
     }
 
@@ -794,7 +794,7 @@ void detail::GetDisplayServiceImpl(Kernel::HLERequestContext& ctx, Core::System&
     if (!IsValidServiceAccess(permission, policy)) {
         LOG_ERROR(Service_VI, "Permission denied for policy {}", policy);
         IPC::ResponseBuilder rb{ctx, 2};
-        rb.Push(ERR_PERMISSION_DENIED);
+        rb.Push(ResultPermissionDenied);
         return;
     }
 

src/core/hle/service/vi/vi_results.h

@@ -0,0 +1,13 @@
+// SPDX-FileCopyrightText: Copyright 2022 yuzu Emulator Project
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include "core/hle/result.h"
+
+namespace Service::VI {
+
+constexpr Result ResultOperationFailed{ErrorModule::VI, 1};
+constexpr Result ResultPermissionDenied{ErrorModule::VI, 5};
+constexpr Result ResultNotSupported{ErrorModule::VI, 6};
+constexpr Result ResultNotFound{ErrorModule::VI, 7};
+
+} // namespace Service::VI