From 656313455bb4ea95a10f20681189db85cc18f555 Mon Sep 17 00:00:00 2001
From: Xaver Hugl <xaver.hugl@gmail.com>
Date: Sat, 9 Mar 2024 01:17:57 +0100
Subject: [PATCH] backends/drm: work around amdgpu vrr cursor bug differently

Instead of not-delaying cursor updates with adaptive sync, this forces a
software cursor instead. That way, the functionality works the same on all
the vendors.

For testing potential driver fixes, the environment variable
KWIN_DRM_DONT_FORCE_AMD_SW_CURSOR=1 can be used to disable this workaround
---
 src/backends/drm/drm_commit_thread.cpp    | 10 ++--------
 src/backends/drm/drm_egl_cursor_layer.cpp |  3 +++
 src/backends/drm/drm_pipeline.cpp         |  9 +++++++++
 src/backends/drm/drm_pipeline.h           |  5 +++++
 4 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/backends/drm/drm_commit_thread.cpp b/src/backends/drm/drm_commit_thread.cpp
index 89c0ca2908..3f6c0acae8 100644
--- a/src/backends/drm/drm_commit_thread.cpp
+++ b/src/backends/drm/drm_commit_thread.cpp
@@ -28,13 +28,7 @@ DrmCommitThread::DrmCommitThread(DrmGpu *gpu, const QString &name)
         return;
     }
 
-    static bool delayVrrCursorUpdatesEnvSet = false;
-    static const bool delayVrrCursorUpdatesEnv = qEnvironmentVariableIntValue("KWIN_DRM_DELAY_VRR_CURSOR_UPDATES", &delayVrrCursorUpdatesEnvSet) == 1;
-    // amdgpu doesn't handle this correctly, so it's off by default for amd gpus
-    // See https://gitlab.freedesktop.org/drm/amd/-/issues/2186 for more details
-    const bool delayVrrCursorUpdates = (!delayVrrCursorUpdatesEnvSet && !gpu->isAmdgpu()) || delayVrrCursorUpdatesEnv;
-
-    m_thread.reset(QThread::create([this, delayVrrCursorUpdates]() {
+    m_thread.reset(QThread::create([this]() {
         const auto thread = QThread::currentThread();
         gainRealTime();
         while (true) {
@@ -74,7 +68,7 @@ DrmCommitThread::DrmCommitThread(DrmGpu *gpu, const QString &name)
                 }
                 continue;
             }
-            if (m_commits.front()->isCursorOnly() && m_vrr && delayVrrCursorUpdates) {
+            if (m_commits.front()->isCursorOnly() && m_vrr) {
                 // wait for a primary plane commit to be in, while still enforcing
                 // a minimum cursor refresh rate of 30Hz
                 const auto cursorTarget = m_lastPageflip + std::chrono::duration_cast<std::chrono::nanoseconds>(1s) / 30;
diff --git a/src/backends/drm/drm_egl_cursor_layer.cpp b/src/backends/drm/drm_egl_cursor_layer.cpp
index c20886e0b4..36fd86957e 100644
--- a/src/backends/drm/drm_egl_cursor_layer.cpp
+++ b/src/backends/drm/drm_egl_cursor_layer.cpp
@@ -48,6 +48,9 @@ EglGbmCursorLayer::EglGbmCursorLayer(EglGbmBackend *eglBackend, DrmPipeline *pip
 
 std::optional<OutputLayerBeginFrameInfo> EglGbmCursorLayer::beginFrame()
 {
+    if (m_pipeline->amdgpuVrrWorkaroundActive()) {
+        return std::nullopt;
+    }
     // note that this allows blending to happen in sRGB or PQ encoding.
     // That's technically incorrect, but it looks okay and is intentionally allowed
     // as the hardware cursor is more important than an incorrectly blended cursor edge
diff --git a/src/backends/drm/drm_pipeline.cpp b/src/backends/drm/drm_pipeline.cpp
index 789050d438..eb0cab84ad 100644
--- a/src/backends/drm/drm_pipeline.cpp
+++ b/src/backends/drm/drm_pipeline.cpp
@@ -406,6 +406,9 @@ bool DrmPipeline::updateCursor()
     if (needsModeset() || !m_pending.crtc || !m_pending.active) {
         return false;
     }
+    if (amdgpuVrrWorkaroundActive() && m_cursorLayer->isEnabled()) {
+        return false;
+    }
     // explicitly check for the cursor plane and not for AMS, as we might not always have one
     if (m_pending.crtc->cursorPlane()) {
         // test the full state, to take pending commits into account
@@ -428,6 +431,12 @@ bool DrmPipeline::updateCursor()
     }
 }
 
+bool DrmPipeline::amdgpuVrrWorkaroundActive() const
+{
+    static const bool s_env = qEnvironmentVariableIntValue("KWIN_DRM_DONT_FORCE_AMD_SW_CURSOR") == 1;
+    return !s_env && gpu()->isAmdgpu() && (m_pending.presentationMode == PresentationMode::AdaptiveSync || m_pending.presentationMode == PresentationMode::AdaptiveAsync);
+}
+
 void DrmPipeline::applyPendingChanges()
 {
     m_next = m_pending;
diff --git a/src/backends/drm/drm_pipeline.h b/src/backends/drm/drm_pipeline.h
index 9ede68c249..a2bc843156 100644
--- a/src/backends/drm/drm_pipeline.h
+++ b/src/backends/drm/drm_pipeline.h
@@ -134,6 +134,11 @@ public:
     void setColorDescription(const ColorDescription &description);
     void setIccProfile(const std::shared_ptr<IccProfile> &profile);
 
+    /**
+     * amdgpu drops cursor updates with adaptive sync: https://gitlab.freedesktop.org/drm/amd/-/issues/2186
+     */
+    bool amdgpuVrrWorkaroundActive() const;
+
     enum class CommitMode {
         Test,
         TestAllowModeset,