Test DrmPipelines for outputs

Not all combinations of connectors, crtcs and planes will work
on all hardware, so we need to test the pipelines before using
them.

BUG: 433107
CCBUG: 435265

src/plugins/platforms/drm/drm_gpu.cpp

@@ -82,7 +82,7 @@ DrmGpu::DrmGpu(DrmBackend *backend, const QString &devNode, int fd, dev_t device
     // trying to activate Atomic Mode Setting (this means also Universal Planes)
     static const bool atomicModesetting = !qEnvironmentVariableIsSet("KWIN_DRM_NO_AMS");
     if (atomicModesetting) {
-        tryAMS();
+        initDrmResources();
     }
 }
 
@@ -117,216 +117,242 @@ clockid_t DrmGpu::presentationClock() const
     return m_presentationClock;
 }
 
-void DrmGpu::tryAMS()
+void DrmGpu::initDrmResources()
 {
-    m_atomicModeSetting = false;
+    // try atomic mode setting
     if (drmSetClientCap(m_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0) {
-        m_atomicModeSetting = true;
         DrmScopedPointer<drmModePlaneRes> planeResources(drmModeGetPlaneResources(m_fd));
-        if (!planeResources) {
+        if (planeResources) {
+            qCDebug(KWIN_DRM) << "Using Atomic Mode Setting on gpu" << m_devNode;
+            qCDebug(KWIN_DRM) << "Number of planes on GPU" << m_devNode << ":" << planeResources->count_planes;
+            // create the plane objects
+            for (unsigned int i = 0; i < planeResources->count_planes; ++i) {
+                DrmScopedPointer<drmModePlane> kplane(drmModeGetPlane(m_fd, planeResources->planes[i]));
+                DrmPlane *p = new DrmPlane(this, kplane->plane_id);
+                if (p->init()) {
+                    m_planes << p;
+                } else {
+                    delete p;
+                }
+            }
+            if (m_planes.isEmpty()) {
+                qCWarning(KWIN_DRM) << "Failed to create any plane. Falling back to legacy mode on GPU " << m_devNode;
+                m_atomicModeSetting = false;
+            } else {
+                m_atomicModeSetting = true;
+            }
+        } else {
             qCWarning(KWIN_DRM) << "Failed to get plane resources. Falling back to legacy mode on GPU " << m_devNode;
             m_atomicModeSetting = false;
-            return;
         }
-        qCDebug(KWIN_DRM) << "Using Atomic Mode Setting on gpu" << m_devNode;
-        qCDebug(KWIN_DRM) << "Number of planes on GPU" << m_devNode << ":" << planeResources->count_planes;
-        // create the plane objects
-        for (unsigned int i = 0; i < planeResources->count_planes; ++i) {
-            DrmScopedPointer<drmModePlane> kplane(drmModeGetPlane(m_fd, planeResources->planes[i]));
-            DrmPlane *p = new DrmPlane(this, kplane->plane_id);
-            if (p->init()) {
-                m_planes << p;
-            } else {
-                delete p;
-            }
-        }
-        if (m_planes.isEmpty()) {
-            qCWarning(KWIN_DRM) << "Failed to create any plane. Falling back to legacy mode on GPU " << m_devNode;
-            m_atomicModeSetting = false;
-        }
-        m_unusedPlanes = m_planes;
     } else {
         qCWarning(KWIN_DRM) << "drmSetClientCap for Atomic Mode Setting failed. Using legacy mode on GPU" << m_devNode;
+        m_atomicModeSetting = false;
+    }
+
+    DrmScopedPointer<drmModeRes> resources(drmModeGetResources(m_fd));
+    if (!resources) {
+        qCCritical(KWIN_DRM) << "drmModeGetResources for getting CRTCs failed on GPU" << m_devNode;
+        return;
+    }
+    for (int i = 0; i < resources->count_crtcs; ++i) {
+        auto c = new DrmCrtc(this, resources->crtcs[i], i);
+        if (!c->init()) {
+            delete c;
+            continue;
+        }
+        m_crtcs << c;
     }
 }
 
 bool DrmGpu::updateOutputs()
 {
-    auto oldConnectors = m_connectors;
-    auto oldCrtcs = m_crtcs;
     DrmScopedPointer<drmModeRes> resources(drmModeGetResources(m_fd));
     if (!resources) {
         qCWarning(KWIN_DRM) << "drmModeGetResources failed";
         return false;
     }
 
+    // check for added and removed connectors
+    QVector<DrmConnector *> removedConnectors = m_connectors;
     for (int i = 0; i < resources->count_connectors; ++i) {
         const uint32_t currentConnector = resources->connectors[i];
         auto it = std::find_if(m_connectors.constBegin(), m_connectors.constEnd(), [currentConnector] (DrmConnector *c) { return c->id() == currentConnector; });
         if (it == m_connectors.constEnd()) {
             auto c = new DrmConnector(this, currentConnector);
-            if (!c->init()) {
-                delete c;
-                continue;
-            }
-            if (c->isNonDesktop()) {
-                delete c;
-                continue;
-            }
-            if (!c->isConnected()) {
+            if (!c->init() || !c->isConnected() || c->isNonDesktop()) {
                 delete c;
                 continue;
             }
             m_connectors << c;
         } else {
             (*it)->updateProperties();
-            oldConnectors.removeOne(*it);
-        }
-    }
-
-    for (int i = 0; i < resources->count_crtcs; ++i) {
-        const uint32_t currentCrtc = resources->crtcs[i];
-        auto it = std::find_if(m_crtcs.constBegin(), m_crtcs.constEnd(), [currentCrtc] (DrmCrtc *c) { return c->id() == currentCrtc; });
-        if (it == m_crtcs.constEnd()) {
-            auto c = new DrmCrtc(this, currentCrtc, i);
-            if (!c->init()) {
-                delete c;
-                continue;
+            if ((*it)->isConnected()) {
+                removedConnectors.removeOne(*it);
             }
-            m_crtcs << c;
-        } else {
-            oldCrtcs.removeOne(*it);
+        }
+    }
+    for (const auto &connector : qAsConst(removedConnectors)) {
+        if (auto output = findOutput(connector->id())) {
+            removeOutput(output);
+        }
+        m_connectors.removeOne(connector);
+    }
+
+    // find unused and connected connectors
+    bool hasUnusedConnectors = false;
+    QVector<DrmConnector *> connectedConnectors;
+    for (const auto &conn : qAsConst(m_connectors)) {
+        auto output = findOutput(conn->id());
+        if (conn->isConnected() && !conn->isNonDesktop()) {
+            connectedConnectors << conn;
+            hasUnusedConnectors |= output == nullptr;
+            if (output) {
+                output->updateModes();
+            }
+        } else if (output) {
+            removeOutput(output);
         }
     }
 
-    for (auto c : qAsConst(oldConnectors)) {
-        m_connectors.removeOne(c);
+    // update crtc properties
+    for (const auto &crtc : qAsConst(m_crtcs)) {
+        crtc->updateProperties();
     }
-    for (auto c : qAsConst(oldCrtcs)) {
-        m_crtcs.removeOne(c);
-    }
-
-    QVector<DrmOutput*> connectedOutputs;
-    QVector<DrmConnector*> pendingConnectors;
-
-    // split up connected connectors in already or not yet assigned ones
-    for (DrmConnector *con : qAsConst(m_connectors)) {
-        if (!con->isConnected()) {
-            continue;
-        }
-
-        if (DrmOutput *o = findOutput(con->id())) {
-            connectedOutputs << o;
-
-            o->updateModes();
-        } else {
-            pendingConnectors << con;
-        }
-    }
-
-    // check for outputs which got removed
-    QVector<DrmOutput*> removedOutputs;
-    auto it = m_drmOutputs.begin();
-    while (it != m_drmOutputs.end()) {
-        if (connectedOutputs.contains(*it)) {
-            it++;
-            continue;
-        }
-        DrmOutput *removed = *it;
-        it = m_drmOutputs.erase(it);
-        removedOutputs.append(removed);
-    }
-
-    // before testing output configurations, update all the plane properties as they might have changed
+    // update plane properties
     for (const auto &plane : qAsConst(m_planes)) {
         plane->updateProperties();
     }
 
-    for (DrmConnector *con : qAsConst(pendingConnectors)) {
-        DrmScopedPointer<drmModeConnector> connector(drmModeGetConnector(m_fd, con->id()));
-        if (!connector) {
-            continue;
+    if (hasUnusedConnectors) {
+        // delete current pipelines of active outputs
+        for (const auto &output : qAsConst(m_drmOutputs)) {
+            m_pipelines.removeOne(output->pipeline());
+            delete output->pipeline();
+            output->setPipeline(nullptr);
         }
-        if (connector->count_modes == 0) {
-            continue;
+
+        if (m_atomicModeSetting) {
+            // sort outputs by being already connected (to any CRTC) so that already working outputs get preferred
+            std::sort(connectedConnectors.begin(), connectedConnectors.end(), [](auto c1, auto c2){
+                return c1->getProp(DrmConnector::PropertyIndex::CrtcId)->current() > c2->getProp(DrmConnector::PropertyIndex::CrtcId)->current();
+            });
         }
-        bool outputDone = false;
+        auto config = findWorkingCombination({}, connectedConnectors, m_crtcs, m_planes);
 
-        QVector<uint32_t> encoders = con->encoders();
-        for (auto encId : qAsConst(encoders)) {
-            DrmScopedPointer<drmModeEncoder> encoder(drmModeGetEncoder(m_fd, encId));
-            if (!encoder) {
-                continue;
-            }
-            for (DrmCrtc *crtc : qAsConst(m_crtcs)) {
-                if (!(encoder->possible_crtcs & (1 << crtc->pipeIndex()))) {
-                    continue;
-                }
-
-                // check if crtc isn't used yet -- currently we don't allow multiple outputs on one crtc (cloned mode)
-                auto it = std::find_if(connectedOutputs.constBegin(), connectedOutputs.constEnd(),
-                    [crtc] (DrmOutput *o) {
-                        return o->m_pipeline->crtc() == crtc;
-                    }
-                );
-                if (it != connectedOutputs.constEnd()) {
-                    continue;
-                }
-
-                auto primary = getCompatiblePlane(DrmPlane::TypeIndex::Primary, crtc);
-                if (m_atomicModeSetting && !primary) {
-                    continue;
-                }
-
-                auto pipeline = new DrmPipeline(this, con, crtc, primary);
-                DrmOutput *output = new DrmOutput(this, pipeline);
-
-                // outputEnabled only adds it to the render backend but not to the platform
-                // this is necessary for accurate pipeline tests
-                Q_EMIT outputEnabled(output);
-
-                // test
-                if (!pipeline->test(m_pipelines)) {
-                    Q_EMIT outputDisabled(output);
-                    delete output;
-                    delete pipeline;
-                    continue;
-                }
-                m_pipelines << pipeline;
-
-                // add the output to the platform and create wayland objects
-                Q_EMIT outputAdded(output);
+        DrmPipeline::commitPipelines(config, DrmPipeline::CommitMode::Commit);
+        m_pipelines << config;
 
+        for (const auto &pipeline : qAsConst(config)) {
+            auto output = pipeline->output();
+            if (m_outputs.contains(output)) {
+                // try setting hardware rotation
+                output->updateTransform(output->transform());
+            } else {
+                qCDebug(KWIN_DRM).nospace() << "New output on GPU " << m_devNode << ": " << pipeline->connector()->modelName();
                 if (!output->initCursor(m_cursorSize)) {
                     m_backend->setSoftwareCursorForced(true);
                 }
-                qCDebug(KWIN_DRM, "For new output %s on GPU %s use mode %dx%d@%d", qPrintable(output->name()), qPrintable(m_devNode), output->modeSize().width(), output->modeSize().height(), output->refreshRate());
-
-                connectedOutputs << output;
                 m_outputs << output;
-                outputDone = true;
-                break;
-            }
-            if (outputDone) {
-                break;
+                m_drmOutputs << output;
+                Q_EMIT outputAdded(output);
             }
         }
     }
-    m_drmOutputs = connectedOutputs;
 
-    for(DrmOutput *removedOutput : removedOutputs) {
-        removeOutput(removedOutput);
+    return true;
+}
+
+QVector<DrmPipeline *> DrmGpu::findWorkingCombination(const QVector<DrmPipeline *> &pipelines, QVector<DrmConnector *> connectors, QVector<DrmCrtc *> crtcs, const QVector<DrmPlane *> &planes)
+{
+    if (connectors.isEmpty() || crtcs.isEmpty()) {
+        // no further pipelines can be added -> test configuration
+        if (testCombination(pipelines)) {
+            return pipelines;
+        } else {
+            return {};
+        }
+    }
+    auto connector = connectors.takeFirst();
+    const auto encoders = connector->encoders();
+
+    if (m_atomicModeSetting) {
+        // try the crtc that this connector is already connected to first
+        std::sort(crtcs.begin(), crtcs.end(), [connector](auto c1, auto c2){
+            Q_UNUSED(c2)
+            if (connector->getProp(DrmConnector::PropertyIndex::CrtcId)->current() == c1->id()) {
+                return true;
+            } else {
+                return false;
+            }
+        });
     }
 
-    qDeleteAll(oldConnectors);
-    qDeleteAll(oldCrtcs);
-    return true;
+    auto recurse = [this, connector, connectors, crtcs, planes, pipelines] (DrmCrtc *crtc, DrmPlane *primaryPlane) {
+        auto pipeline = new DrmPipeline(this, connector, crtc, primaryPlane);
+        auto crtcsLeft = crtcs;
+        crtcsLeft.removeOne(crtc);
+        auto planesLeft = planes;
+        planesLeft.removeOne(primaryPlane);
+        auto allPipelines = pipelines;
+        allPipelines << pipeline;
+        auto ret = findWorkingCombination(allPipelines, connectors, crtcsLeft, planesLeft);
+        if (ret.isEmpty()) {
+            delete pipeline;
+        }
+        return ret;
+    };
+    for (const auto &encoderId : encoders) {
+        DrmScopedPointer<drmModeEncoder> encoder(drmModeGetEncoder(m_fd, encoderId));
+        for (const auto &crtc : qAsConst(crtcs)) {
+            if (m_atomicModeSetting) {
+                for (const auto &plane : qAsConst(planes)) {
+                    if (plane->type() == DrmPlane::TypeIndex::Primary
+                        && plane->isCrtcSupported(crtc->pipeIndex())) {
+                        if (auto workingPipelines = recurse(crtc, plane); !workingPipelines.isEmpty()) {
+                            return workingPipelines;
+                        }
+                    }
+                }
+            } else {
+                if (auto workingPipelines = recurse(crtc, nullptr); !workingPipelines.isEmpty()) {
+                    return workingPipelines;
+                }
+            }
+        }
+    }
+    return {};
+}
+
+bool DrmGpu::testCombination(const QVector<DrmPipeline *> &pipelines)
+{
+    for (const auto &pipeline : pipelines) {
+        auto output = findOutput(pipeline->connector()->id());
+        if (output) {
+            output->setPipeline(pipeline);
+            pipeline->setOutput(output);
+        } else {
+            output = new DrmOutput(this, pipeline);
+            Q_EMIT outputEnabled(output);// create render resources for the test
+        }
+        pipeline->setup();
+    }
+
+    if (DrmPipeline::commitPipelines(pipelines, DrmPipeline::CommitMode::Test)) {
+        return true;
+    } else {
+        for (const auto &pipeline : qAsConst(pipelines)) {
+            Q_EMIT outputDisabled(pipeline->output());
+            delete pipeline->output();
+        }
+        return false;
+    }
 }
 
 DrmOutput *DrmGpu::findOutput(quint32 connector)
 {
     auto it = std::find_if(m_drmOutputs.constBegin(), m_drmOutputs.constEnd(), [connector] (DrmOutput *o) {
-        return o->m_pipeline->connector()->id() == connector;
+        return o->connector()->id() == connector;
     });
     if (it != m_drmOutputs.constEnd()) {
         return *it;
@@ -334,20 +360,6 @@ DrmOutput *DrmGpu::findOutput(quint32 connector)
     return nullptr;
 }
 
-DrmPlane *DrmGpu::getCompatiblePlane(DrmPlane::TypeIndex typeIndex, DrmCrtc *crtc)
-{
-    for (auto plane : qAsConst(m_unusedPlanes)) {
-        if (plane->type() != typeIndex) {
-            continue;
-        }
-        if (plane->isCrtcSupported(crtc->pipeIndex())) {
-            m_unusedPlanes.removeOne(plane);
-            return plane;
-        }
-    }
-    return nullptr;
-}
-
 void DrmGpu::waitIdle()
 {
     m_socketNotifier->setEnabled(false);
@@ -456,10 +468,6 @@ void DrmGpu::removeOutput(DrmOutput *output)
     Q_EMIT outputRemoved(output);
     auto pipeline = output->m_pipeline;
     delete output;
-    m_connectors.removeOne(pipeline->connector());
-    if (pipeline->primaryPlane()) {
-        m_unusedPlanes << pipeline->primaryPlane();
-    }
     m_pipelines.removeOne(pipeline);
     delete pipeline;
 }

src/plugins/platforms/drm/drm_gpu.h

@@ -115,10 +115,12 @@ protected:
 
 private:
     void dispatchEvents();
-    DrmPlane *getCompatiblePlane(DrmPlane::TypeIndex typeIndex, DrmCrtc *crtc);
     DrmOutput *findOutput(quint32 connector);
     void removeOutput(DrmOutput *output);
-    void tryAMS();
+    void initDrmResources();
+
+    QVector<DrmPipeline *> findWorkingCombination(const QVector<DrmPipeline *> &pipelines, QVector<DrmConnector *> connectors, QVector<DrmCrtc *> crtcs, const QVector<DrmPlane *> &planes);
+    bool testCombination(const QVector<DrmPipeline *> &pipelines);
 
     DrmBackend* const m_backend;
     QPointer<AbstractEglDrmBackend> m_eglBackend;
@@ -135,15 +137,11 @@ private:
     QSocketNotifier *m_socketNotifier = nullptr;
     bool m_addFB2ModifiersSupported = false;
 
-    // all planes: primarys, cursors and overlays
     QVector<DrmPlane*> m_planes;
-    QVector<DrmPlane*> m_unusedPlanes;
-    // crtcs
     QVector<DrmCrtc*> m_crtcs;
-    // connectors
     QVector<DrmConnector*> m_connectors;
-    // pipelines
     QVector<DrmPipeline*> m_pipelines;
+
     QVector<DrmOutput*> m_drmOutputs;
     // includes virtual outputs
     QVector<DrmAbstractOutput*> m_outputs;

src/plugins/platforms/drm/drm_object_connector.h

@@ -112,7 +112,6 @@ private:
     QSize m_physicalSize = QSize(-1, -1);
     QVector<Mode> m_modes;
     int m_modeIndex = 0;
-
 };
 
 }

src/plugins/platforms/drm/drm_output.cpp

@@ -40,6 +40,7 @@ namespace KWin
 DrmOutput::DrmOutput(DrmGpu *gpu, DrmPipeline *pipeline)
     : DrmAbstractOutput(gpu)
     , m_pipeline(pipeline)
+    , m_connector(pipeline->connector())
 {
     m_pipeline->setOutput(this);
     auto conn = m_pipeline->connector();
@@ -74,6 +75,7 @@ DrmOutput::~DrmOutput()
     if (m_pageFlipPending) {
         pageFlipped();
     }
+    m_pipeline->setOutput(nullptr);
 }
 
 bool DrmOutput::initCursor(const QSize &cursorSize)
@@ -417,6 +419,11 @@ void DrmOutput::setOverscan(uint32_t overscan)
     }
 }
 
+DrmConnector *DrmOutput::connector() const
+{
+    return m_connector;
+}
+
 DrmPipeline *DrmOutput::pipeline() const
 {
     return m_pipeline;
@@ -459,4 +466,10 @@ void DrmOutput::setRgbRange(RgbRange range)
     }
 }
 
+void DrmOutput::setPipeline(DrmPipeline *pipeline)
+{
+    Q_ASSERT_X(!pipeline || pipeline->connector() == m_connector, "DrmOutput::setPipeline", "Pipeline with wrong connector set!");
+    m_pipeline = pipeline;
+}
+
 }

src/plugins/platforms/drm/drm_output.h

@@ -50,7 +50,10 @@ public:
     void pageFlipped();
     bool isDpmsEnabled() const override;
 
+    DrmConnector *connector() const;
     DrmPipeline *pipeline() const;
+    void setPipeline(DrmPipeline *pipeline);
+
     GbmBuffer *currentBuffer() const override;
     QSize sourceSize() const override;
     bool isFormatSupported(uint32_t drmFormat) const override;
@@ -80,6 +83,7 @@ private:
     void setRgbRange(RgbRange range) override;
 
     DrmPipeline *m_pipeline;
+    DrmConnector *m_connector;
 
     QSharedPointer<DrmDumbBuffer> m_cursor;
     bool m_pageFlipPending = false;

src/plugins/platforms/drm/drm_pipeline.cpp

@@ -41,17 +41,8 @@ DrmPipeline::DrmPipeline(DrmGpu *gpu, DrmConnector *conn, DrmCrtc *crtc, DrmPlan
     , m_crtc(crtc)
     , m_primaryPlane(primaryPlane)
 {
-    if (m_gpu->atomicModeSetting()) {
-        m_connector->findCurrentMode(m_crtc->queryCurrentMode());
-        m_connector->setPending(DrmConnector::PropertyIndex::CrtcId, m_crtc->id());
-        auto mode = m_connector->currentMode();
-        m_crtc->setPending(DrmCrtc::PropertyIndex::Active, 1);
-        m_crtc->setPendingBlob(DrmCrtc::PropertyIndex::ModeId, &mode.mode, sizeof(drmModeModeInfo));
-    }
     m_allObjects << m_connector << m_crtc;
     if (m_primaryPlane) {
-        m_primaryPlane->setPending(DrmPlane::PropertyIndex::CrtcId, m_crtc->id());
-        m_primaryPlane->set(QPoint(0, 0), sourceSize(), QPoint(0, 0), m_connector->currentMode().size);
         m_allObjects << m_primaryPlane;
     }
 }
@@ -60,12 +51,29 @@ DrmPipeline::~DrmPipeline()
 {
 }
 
+void DrmPipeline::setup()
+{
+    if (!m_gpu->atomicModeSetting()) {
+        return;
+    }
+    if (m_connector->getProp(DrmConnector::PropertyIndex::CrtcId)->current() == m_crtc->id()) {
+        m_connector->findCurrentMode(m_crtc->queryCurrentMode());
+    }
+    m_connector->setPending(DrmConnector::PropertyIndex::CrtcId, m_crtc->id());
+    m_crtc->setPending(DrmCrtc::PropertyIndex::Active, 1);
+    auto mode = m_connector->currentMode();
+    m_crtc->setPendingBlob(DrmCrtc::PropertyIndex::ModeId, &mode.mode, sizeof(drmModeModeInfo));
+    m_primaryPlane->setPending(DrmPlane::PropertyIndex::CrtcId, m_crtc->id());
+    m_primaryPlane->set(QPoint(0, 0), sourceSize(), QPoint(0, 0), mode.size);
+    m_primaryPlane->setTransformation(DrmPlane::Transformation::Rotate0);
+}
+
 bool DrmPipeline::test(const QVector<DrmPipeline*> &pipelines)
 {
     if (m_gpu->atomicModeSetting()) {
-        return checkTestBuffer() && atomicTest(pipelines);
+        return checkTestBuffer() && commitPipelines(pipelines, CommitMode::Test);
     } else {
-        return m_legacyNeedsModeset ? modeset(0) : true;
+        return true;
     }
 }
 
@@ -106,86 +114,72 @@ bool DrmPipeline::present(const QSharedPointer<DrmBuffer> &buffer)
 
 bool DrmPipeline::atomicCommit()
 {
-    drmModeAtomicReq *req = drmModeAtomicAlloc();
-    if (!req) {
-        qCDebug(KWIN_DRM) << "Failed to allocate drmModeAtomicReq!" << strerror(errno);
-        return false;
-    }
-    bool result = doAtomicCommit(req, 0, false);
-    drmModeAtomicFree(req);
-    return result;
+    return commitPipelines({this}, CommitMode::CommitWithPageflipEvent);
 }
 
-bool DrmPipeline::atomicTest(const QVector<DrmPipeline*> &pipelines)
+bool DrmPipeline::commitPipelines(const QVector<DrmPipeline*> &pipelines, CommitMode mode)
 {
-    drmModeAtomicReq *req = drmModeAtomicAlloc();
-    if (!req) {
-        qCDebug(KWIN_DRM) << "Failed to allocate drmModeAtomicReq!" << strerror(errno);
-        return false;
-    }
-    uint32_t flags = 0;
-    bool result = true;
-    // the whole batch needs to be tested in one atomic commit to consider not-yet-applied changes of other pipelines
-    for (const auto &pipeline : pipelines) {
-        if (pipeline != this) {
-            result &= pipeline->populateAtomicValues(req, flags);
-            if (!result) {
+    Q_ASSERT(!pipelines.isEmpty());
+
+    if (pipelines[0]->m_gpu->atomicModeSetting()) {
+        drmModeAtomicReq *req = drmModeAtomicAlloc();
+        if (!req) {
+            qCDebug(KWIN_DRM) << "Failed to allocate drmModeAtomicReq!" << strerror(errno);
+            return false;
+        }
+        uint32_t flags = 0;
+        bool result = true;
+        for (const auto &pipeline : pipelines) {
+            if (!pipeline->checkTestBuffer() || !pipeline->populateAtomicValues(req, flags)) {
+                result = false;
                 break;
             }
         }
-    }
-    if (result) {
-        result &= doAtomicCommit(req, flags, true);
-    } else {
-        if (m_oldTestBuffer) {
-            m_primaryBuffer = m_oldTestBuffer;
-            m_oldTestBuffer = nullptr;
+        if (mode != CommitMode::CommitWithPageflipEvent) {
+            flags &= ~DRM_MODE_PAGE_FLIP_EVENT;
         }
-        for (const auto &obj : qAsConst(m_allObjects)) {
-            obj->rollbackPending();
-        }
-    }
-    drmModeAtomicFree(req);
-    return result;
-}
-
-bool DrmPipeline::doAtomicCommit(drmModeAtomicReq *req, uint32_t flags, bool testOnly)
-{
-    bool result = populateAtomicValues(req, flags);
-
-    // test
-    if (result && drmModeAtomicCommit(m_gpu->fd(), req, (flags & (~DRM_MODE_PAGE_FLIP_EVENT)) | DRM_MODE_ATOMIC_TEST_ONLY, m_output) != 0) {
-        qCWarning(KWIN_DRM) << "Atomic test failed!" << strerror(errno);
-        printDebugInfo();
-        result = false;
-    }
-    // commit
-    if (!testOnly && result && drmModeAtomicCommit(m_gpu->fd(), req, flags, m_output) != 0) {
-        qCCritical(KWIN_DRM) << "Atomic commit failed! This never should've happened!" << strerror(errno);
-        printDebugInfo();
-        result = false;
-    }
-    if (result) {
-        m_oldTestBuffer = nullptr;
-        for (const auto &obj : qAsConst(m_allObjects)) {
-            obj->commitPending();
-        }
-        if (!testOnly) {
-            for (const auto &obj : qAsConst(m_allObjects)) {
-                obj->commit();
+        if (result) {
+            result = drmModeAtomicCommit(pipelines[0]->m_gpu->fd(), req, (flags & (~DRM_MODE_PAGE_FLIP_EVENT)) | DRM_MODE_ATOMIC_TEST_ONLY, pipelines[0]->m_output) == 0;
+            if (result && mode != CommitMode::Test) {
+                result = drmModeAtomicCommit(pipelines[0]->m_gpu->fd(), req, flags, pipelines[0]->m_output) == 0;
             }
-            m_primaryPlane->setNext(m_primaryBuffer);
         }
+        if (result) {
+            for (const auto &pipeline : pipelines) {
+                pipeline->m_oldTestBuffer = nullptr;
+                for (const auto &obj : qAsConst(pipeline->m_allObjects)) {
+                    obj->commitPending();
+                }
+                if (mode != CommitMode::Test) {
+                    pipeline->m_primaryPlane->setNext(pipeline->m_primaryBuffer);
+                    for (const auto &obj : qAsConst(pipeline->m_allObjects)) {
+                        obj->commit();
+                    }
+                }
+            }
+        } else {
+            qCWarning(KWIN_DRM) << (mode == CommitMode::Test ? "Atomic test" : "Atomic commit") << "failed!" << strerror(errno);
+            for (const auto &pipeline : pipelines) {
+                pipeline->printDebugInfo();
+                if (pipeline->m_oldTestBuffer) {
+                    pipeline->m_primaryBuffer = pipeline->m_oldTestBuffer;
+                    pipeline->m_oldTestBuffer = nullptr;
+                }
+                for (const auto &obj : qAsConst(pipeline->m_allObjects)) {
+                    obj->rollbackPending();
+                }
+            }
+        }
+        drmModeAtomicFree(req);
+        return result;
     } else {
-        if (m_oldTestBuffer) {
-            m_primaryBuffer = m_oldTestBuffer;
-            m_oldTestBuffer = nullptr;
-        }
-        for (const auto &obj : qAsConst(m_allObjects)) {
-            obj->rollbackPending();
+        for (const auto &pipeline : pipelines) {
+            if (pipeline->m_legacyNeedsModeset && !pipeline->modeset(0)) {
+                return false;
+            }
         }
+        return true;
     }
-    return result;
 }
 
 bool DrmPipeline::populateAtomicValues(drmModeAtomicReq *req, uint32_t &flags)
@@ -264,6 +258,12 @@ bool DrmPipeline::modeset(int modeIndex)
         }
         m_oldTestBuffer = nullptr;
         m_legacyNeedsModeset = false;
+        // make sure the buffer gets kept alive, or the modeset gets reverted by the kernel
+        if (m_crtc->current()) {
+            m_crtc->setNext(m_primaryBuffer);
+        } else {
+            m_crtc->setCurrent(m_primaryBuffer);
+        }
     }
     return true;
 }
@@ -273,6 +273,9 @@ bool DrmPipeline::checkTestBuffer()
     if (m_primaryBuffer && m_primaryBuffer->size() == sourceSize()) {
         return true;
     }
+    if (!m_active) {
+        return true;
+    }
 #if HAVE_GBM
     auto backend = m_gpu->eglBackend();
     if (backend && m_output) {

src/plugins/platforms/drm/drm_pipeline.h

@@ -36,6 +36,11 @@ public:
     DrmPipeline(DrmGpu *gpu, DrmConnector *conn, DrmCrtc *crtc, DrmPlane *primaryPlane);
     ~DrmPipeline();
 
+    /**
+     * Sets the necessary initial drm properties for the pipeline to work
+     */
+    void setup();
+
     /**
      * checks if the connector(s) and plane(s) are set to the CRTC(s)
      * always returns false in legacy mode
@@ -86,13 +91,19 @@ public:
     void setOutput(DrmOutput *output);
     DrmOutput *output() const;
 
+    enum class CommitMode {
+        Test,
+        Commit,
+        CommitWithPageflipEvent
+    };
+
+    static bool commitPipelines(const QVector<DrmPipeline*> &pipelines, CommitMode mode);
+
 private:
-    bool atomicCommit();
-    bool atomicTest(const QVector<DrmPipeline*> &pipelines);
-    bool doAtomicCommit(drmModeAtomicReq *req, uint32_t flags, bool testOnly);
     bool populateAtomicValues(drmModeAtomicReq *req, uint32_t &flags);
-    bool presentLegacy();
     bool test();
+    bool atomicCommit();
+    bool presentLegacy();
     bool checkTestBuffer();
 
     bool setPendingTransformation(const DrmPlane::Transformations &transformation);