From ff2bc0798fe659f6388cf54b6bf3ed0a26785b40 Mon Sep 17 00:00:00 2001
From: Xaver Hugl <xaver.hugl@gmail.com>
Date: Wed, 11 Oct 2023 17:33:24 +0200
Subject: [PATCH] backends/drm: fix properties not being applied when a modeset
 is not needed

When testing whether or not a modeset is needed, all that gets tested is
whether or not the kernel would allow the commit to happen without the ALLOW_MODESET
flag. If there's properties that are only changed in DrmPipeline::prepareAtomicModeset,
we need to apply those in the next commit, regardless of whether or not
the ALLOW_MODESET flag is necessary.

BUG: 476060
---
 src/backends/drm/drm_pipeline.cpp | 52 ++++++++++++++++++++-----------
 src/backends/drm/drm_pipeline.h   |  2 ++
 2 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/src/backends/drm/drm_pipeline.cpp b/src/backends/drm/drm_pipeline.cpp
index 266babee9f..f4d2669b8a 100644
--- a/src/backends/drm/drm_pipeline.cpp
+++ b/src/backends/drm/drm_pipeline.cpp
@@ -80,12 +80,9 @@ DrmPipeline::Error DrmPipeline::present()
     if (gpu()->atomicModeSetting()) {
         // test the full state, to take pending commits into account
         auto fullState = std::make_unique<DrmAtomicCommit>(QList<DrmPipeline *>{this});
-        if (Error err = prepareAtomicPresentation(fullState.get()); err != Error::None) {
+        if (Error err = prepareAtomicCommit(fullState.get(), CommitMode::Test); err != Error::None) {
             return err;
         }
-        if (m_pending.crtc->cursorPlane()) {
-            prepareAtomicCursor(fullState.get());
-        }
         if (!fullState->test()) {
             return errnoToError();
         }
@@ -94,6 +91,10 @@ DrmPipeline::Error DrmPipeline::present()
         if (Error err = prepareAtomicPresentation(primaryPlaneUpdate.get()); err != Error::None) {
             return err;
         }
+        if (m_pending.needsModesetProperties && !prepareAtomicModeset(primaryPlaneUpdate.get())) {
+            return Error::InvalidArguments;
+        }
+        atomicCommitSuccessful();
         m_commitThread->addCommit(std::move(primaryPlaneUpdate));
         return Error::None;
     } else {
@@ -135,20 +136,8 @@ DrmPipeline::Error DrmPipeline::commitPipelinesAtomic(const QList<DrmPipeline *>
         }
     }
     for (const auto &pipeline : pipelines) {
-        if (pipeline->activePending()) {
-            if (pipeline->prepareAtomicPresentation(commit.get()) != Error::None) {
-                return Error::InvalidArguments;
-            }
-            if (pipeline->m_pending.crtc->cursorPlane()) {
-                pipeline->prepareAtomicCursor(commit.get());
-            }
-            if (mode == CommitMode::TestAllowModeset || mode == CommitMode::CommitModeset) {
-                if (!pipeline->prepareAtomicModeset(commit.get())) {
-                    return Error::InvalidArguments;
-                }
-            }
-        } else {
-            pipeline->prepareAtomicDisable(commit.get());
+        if (Error err = pipeline->prepareAtomicCommit(commit.get(), mode); err != Error::None) {
+            return err;
         }
     }
     for (const auto &unused : unusedObjects) {
@@ -160,9 +149,13 @@ DrmPipeline::Error DrmPipeline::commitPipelinesAtomic(const QList<DrmPipeline *>
             qCDebug(KWIN_DRM) << "Atomic modeset test failed!" << strerror(errno);
             return errnoToError();
         }
-        const bool withoutModeset = commit->test();
+        const bool withoutModeset = std::all_of(pipelines.begin(), pipelines.end(), [](DrmPipeline *pipeline) {
+            auto commit = std::make_unique<DrmAtomicCommit>(QVector<DrmPipeline *>{pipeline});
+            return pipeline->prepareAtomicCommit(commit.get(), CommitMode::TestAllowModeset) == Error::None && commit->test();
+        });
         for (const auto &pipeline : pipelines) {
             pipeline->m_pending.needsModeset = !withoutModeset;
+            pipeline->m_pending.needsModesetProperties = true;
         }
         return Error::None;
     }
@@ -191,6 +184,26 @@ DrmPipeline::Error DrmPipeline::commitPipelinesAtomic(const QList<DrmPipeline *>
     }
 }
 
+DrmPipeline::Error DrmPipeline::prepareAtomicCommit(DrmAtomicCommit *commit, CommitMode mode)
+{
+    if (activePending()) {
+        if (Error err = prepareAtomicPresentation(commit); err != Error::None) {
+            return err;
+        }
+        if (m_pending.crtc->cursorPlane()) {
+            prepareAtomicCursor(commit);
+        }
+        if (mode == CommitMode::TestAllowModeset || mode == CommitMode::CommitModeset || m_pending.needsModesetProperties) {
+            if (!prepareAtomicModeset(commit)) {
+                return Error::InvalidArguments;
+            }
+        }
+    } else {
+        prepareAtomicDisable(commit);
+    }
+    return Error::None;
+}
+
 static QRect centerBuffer(const QSize &bufferSize, const QSize &modeSize)
 {
     const double widthScale = bufferSize.width() / double(modeSize.width());
@@ -365,6 +378,7 @@ DrmPipeline::Error DrmPipeline::errnoToError()
 void DrmPipeline::atomicCommitSuccessful()
 {
     m_pending.needsModeset = false;
+    m_pending.needsModesetProperties = false;
     m_current = m_pending;
 }
 
diff --git a/src/backends/drm/drm_pipeline.h b/src/backends/drm/drm_pipeline.h
index ec843a01b4..9d34f4807d 100644
--- a/src/backends/drm/drm_pipeline.h
+++ b/src/backends/drm/drm_pipeline.h
@@ -157,6 +157,7 @@ private:
 
     // atomic modesetting only
     void atomicCommitSuccessful();
+    Error prepareAtomicCommit(DrmAtomicCommit *commit, CommitMode mode);
     bool prepareAtomicModeset(DrmAtomicCommit *commit);
     Error prepareAtomicPresentation(DrmAtomicCommit *commit);
     void prepareAtomicCursor(DrmAtomicCommit *commit);
@@ -176,6 +177,7 @@ private:
         bool active = true; // whether or not the pipeline should be currently used
         bool enabled = true; // whether or not the pipeline needs a crtc
         bool needsModeset = false;
+        bool needsModesetProperties = false;
         std::shared_ptr<DrmConnectorMode> mode;
         uint32_t overscan = 0;
         Output::RgbRange rgbRange = Output::RgbRange::Automatic;