From b6b0af727d0ec78339f9c705b64a19efc966cecb Mon Sep 17 00:00:00 2001
From: Xaver Hugl <xaver.hugl@gmail.com>
Date: Tue, 16 Nov 2021 11:41:15 +0100
Subject: [PATCH] backends/drm: don't take crtcs from dpms disabled outputs

Fixes a crash I have with dpms + suspend, which was caused by the udev
event for updating outputs being called before the output got enabled
again. When DrmGpu::updateOutputs got called it removed the crtc from
the inactive output and then disabled the output afterwards. Instead,
only remove crtcs if an output is really disabled.
This also allows to generalize the logic for lease outputs, and could
in the future allow for faster dpms on/off switching.
---
 src/backends/drm/drm_gpu.cpp    | 38 +++++++++++++++------------------
 src/backends/drm/drm_output.cpp |  1 +
 src/backends/drm/drm_pipeline.h |  3 ++-
 3 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/src/backends/drm/drm_gpu.cpp b/src/backends/drm/drm_gpu.cpp
index 16005c7323..28938e435e 100644
--- a/src/backends/drm/drm_gpu.cpp
+++ b/src/backends/drm/drm_gpu.cpp
@@ -268,6 +268,8 @@ bool DrmGpu::updateOutputs()
                 qCDebug(KWIN_DRM, "New %soutput on GPU %s: %s", conn->isNonDesktop() ? "non-desktop " : "", qPrintable(m_devNode), qPrintable(conn->modelName()));
                 m_pipelines << conn->pipeline();
                 if (conn->isNonDesktop()) {
+                    conn->pipeline()->pending.active = false;
+                    conn->pipeline()->applyPendingChanges();
                     auto leaseOutput = new DrmLeaseOutput(conn->pipeline(), m_leaseDevice);
                     m_leaseOutputs << leaseOutput;
                 } else {
@@ -311,6 +313,7 @@ bool DrmGpu::updateOutputs()
         for (const auto &pipeline : qAsConst(m_pipelines)) {
             pipeline->applyPendingChanges();
             if (!pipeline->pending.crtc && pipeline->output()) {
+                pipeline->pending.enabled = false;
                 pipeline->output()->setEnabled(false);
             }
         }
@@ -335,35 +338,28 @@ bool DrmGpu::checkCrtcAssignment(QVector<DrmConnector*> connectors, QVector<DrmC
             qCWarning(KWIN_DRM) << "disabling connector" << conn->modelName() << "without a crtc";
             conn->pipeline()->pending.crtc = nullptr;
         }
-        // non-desktop outputs need to be tested when they would be enabled so that they can be used if leased
-        QVector<DrmPipeline*> leasePipelines;
-        for (const auto &output : qAsConst(m_leaseOutputs)) {
-            if (!output->lease()) {
-                output->pipeline()->pending.active = true;
-                leasePipelines << output->pipeline();
+        // pipelines that are enabled but not active need to be activated for the test
+        QVector<DrmPipeline*> inactivePipelines;
+        for (const auto &pipeline : qAsConst(m_pipelines)) {
+            if (!pipeline->pending.active) {
+                pipeline->pending.active = true;
+                inactivePipelines << pipeline;
             }
         }
         const auto unused = unusedObjects();
         bool test = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::Test, unused);
-        if (!leasePipelines.isEmpty() && test) {
-            // non-desktop outputs should be disabled for normal usage
-            for (const auto &pipeline : qAsConst(leasePipelines)) {
-                pipeline->pending.active = false;
-            }
-            bool ret = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::Test, unused);
-            if (ret) {
-                for (const auto &pipeline : qAsConst(leasePipelines)) {
-                    pipeline->applyPendingChanges();
-                }
-            }
-            return ret;
-        } else {
-            return test;
+        // disable inactive pipelines again
+        for (const auto &pipeline : qAsConst(inactivePipelines)) {
+            pipeline->pending.active = false;
         }
+        if (!inactivePipelines.isEmpty() && test) {
+            test = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::Test, unused);
+        }
+        return test;
     }
     auto connector = connectors.takeFirst();
     auto pipeline = connector->pipeline();
-    if (const auto &output = pipeline->output(); output && !pipeline->pending.active) {
+    if (!pipeline->pending.enabled) {
         // disabled pipelines don't need CRTCs
         pipeline->pending.crtc = nullptr;
         return checkCrtcAssignment(connectors, crtcs);
diff --git a/src/backends/drm/drm_output.cpp b/src/backends/drm/drm_output.cpp
index ec8a25ba6d..ef0773f98c 100644
--- a/src/backends/drm/drm_output.cpp
+++ b/src/backends/drm/drm_output.cpp
@@ -453,6 +453,7 @@ bool DrmOutput::queueChanges(const WaylandOutputConfig &config)
     m_pipeline->pending.overscan = props->overscan;
     m_pipeline->pending.rgbRange = props->rgbRange;
     m_pipeline->pending.transformation = DrmPlane::Transformation::Rotate0;
+    m_pipeline->pending.enabled = props->enabled;
     return true;
 }
 
diff --git a/src/backends/drm/drm_pipeline.h b/src/backends/drm/drm_pipeline.h
index 42d0f9efc6..950d435ac9 100644
--- a/src/backends/drm/drm_pipeline.h
+++ b/src/backends/drm/drm_pipeline.h
@@ -92,7 +92,8 @@ public:
 
     struct State {
         DrmCrtc *crtc = nullptr;
-        bool active = true;
+        bool active = true; // whether or not the pipeline should be currently used
+        bool enabled = true;// whether or not the pipeline needs a crtc
         int modeIndex = 0;
         uint32_t overscan = 0;
         AbstractWaylandOutput::RgbRange rgbRange = AbstractWaylandOutput::RgbRange::Automatic;