backends/drm: make modeset tests explicit

Instead of checking for properties needing a modeset, do atomic tests with ALLOW_MODESET where it makes sense, and do a second atomic test afterwards without ALLOW_MODESET to check if the modeset can be skipped. This should ensure that KWin always does a modeset when it needs to do one, and not do a modeset when it's not necessary. Doing this also allows reducing the complexity of the drm backend a bit.
2022-06-25 21:14:09 +02:00 · 2022-06-25 21:14:09 +02:00 · 4be81e0176
commit 4be81e0176
parent bd8d65a861
12 changed files with 161 additions and 186 deletions
--- a/src/backends/drm/drm_gpu.cpp
+++ b/src/backends/drm/drm_gpu.cpp
@ -448,14 +448,14 @@ DrmPipeline::Error DrmGpu::testPipelines()
    std::copy_if(m_pipelines.constBegin(), m_pipelines.constEnd(), std::back_inserter(inactivePipelines), [](const auto pipeline) {
        return pipeline->enabled() && !pipeline->active();
    });
-    DrmPipeline::Error test = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::Test, unusedObjects());
+    DrmPipeline::Error test = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::TestAllowModeset, unusedObjects());
    if (!inactivePipelines.isEmpty() && test == DrmPipeline::Error::None) {
        // ensure that pipelines that are set as enabled but currently inactive
        // still work when they need to be set active again
        for (const auto pipeline : qAsConst(inactivePipelines)) {
            pipeline->setActive(true);
        }
-        test = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::Test, unusedObjects());
+        test = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::TestAllowModeset, unusedObjects());
        for (const auto pipeline : qAsConst(inactivePipelines)) {
            pipeline->setActive(false);
        }
@ -710,11 +710,8 @@ bool DrmGpu::isNVidia() const
 bool DrmGpu::needsModeset() const
 {
    return std::any_of(m_pipelines.constBegin(), m_pipelines.constEnd(), [](const auto &pipeline) {
-               return pipeline->needsModeset();
-           })
-        || std::any_of(m_allObjects.constBegin(), m_allObjects.constEnd(), [](const auto &object) {
-               return object->needsModeset();
-           });
+        return pipeline->needsModeset();
+    });
 }

 bool DrmGpu::maybeModeset()
--- a/src/backends/drm/drm_object.h
+++ b/src/backends/drm/drm_object.h
@ -51,7 +51,6 @@ public:
    void rollbackPending();
    bool atomicPopulate(drmModeAtomicReq *req) const;
    bool needsCommit() const;
-    virtual bool needsModeset() const = 0;
    virtual bool updateProperties();

    template<typename T>
--- a/src/backends/drm/drm_object_connector.cpp
+++ b/src/backends/drm/drm_object_connector.cpp
@ -250,21 +250,6 @@ bool DrmConnector::vrrCapable() const
    return false;
 }

-bool DrmConnector::needsModeset() const
-{
-    if (!gpu()->atomicModeSetting()) {
-        return false;
-    }
-    if (getProp(PropertyIndex::CrtcId)->needsCommit()) {
-        return true;
-    }
-    if (const auto &prop = getProp(PropertyIndex::MaxBpc); prop && prop->needsCommit()) {
-        return true;
-    }
-    const auto &rgb = getProp(PropertyIndex::Broadcast_RGB);
-    return rgb && rgb->needsCommit();
-}
-
 bool DrmConnector::hasRgbRange() const
 {
    const auto &rgb = getProp(PropertyIndex::Broadcast_RGB);
--- a/src/backends/drm/drm_object_connector.h
+++ b/src/backends/drm/drm_object_connector.h
@ -78,7 +78,6 @@ public:
    };

    bool init() override;
-    bool needsModeset() const override;
    bool updateProperties() override;
    void disable() override;

--- a/src/backends/drm/drm_object_crtc.cpp
+++ b/src/backends/drm/drm_object_crtc.cpp
@ -47,15 +47,6 @@ drmModeModeInfo DrmCrtc::queryCurrentMode()
    return m_crtc->mode;
 }

-bool DrmCrtc::needsModeset() const
-{
-    if (!gpu()->atomicModeSetting()) {
-        return false;
-    }
-    return getProp(PropertyIndex::Active)->needsCommit()
-        || getProp(PropertyIndex::ModeId)->needsCommit();
-}
-
 int DrmCrtc::pipeIndex() const
 {
    return m_pipeIndex;
--- a/src/backends/drm/drm_object_crtc.h
+++ b/src/backends/drm/drm_object_crtc.h
@ -39,7 +39,6 @@ public:
    };

    bool init() override;
-    bool needsModeset() const override;
    void disable() override;

    int pipeIndex() const;
--- a/src/backends/drm/drm_object_plane.cpp
+++ b/src/backends/drm/drm_object_plane.cpp
@ -153,18 +153,6 @@ void DrmPlane::setBuffer(DrmFramebuffer *buffer)
    setPending(PropertyIndex::FbId, buffer ? buffer->framebufferId() : 0);
 }

-bool DrmPlane::needsModeset() const
-{
-    if (!gpu()->atomicModeSetting() || type() == TypeIndex::Cursor) {
-        return false;
-    }
-    auto rotation = getProp(PropertyIndex::Rotation);
-    if (rotation && rotation->needsCommit()) {
-        return true;
-    }
-    return getProp(PropertyIndex::CrtcId)->needsCommit();
-}
-
 bool DrmPlane::isCrtcSupported(int pipeIndex) const
 {
    return (m_possibleCrtcs & (1 << pipeIndex));
--- a/src/backends/drm/drm_object_plane.h
+++ b/src/backends/drm/drm_object_plane.h
@ -66,7 +66,6 @@ public:
    Q_DECLARE_FLAGS(Transformations, Transformation)

    bool init() override;
-    bool needsModeset() const override;
    void disable() override;
    TypeIndex type() const;

--- a/src/backends/drm/drm_output.cpp
+++ b/src/backends/drm/drm_output.cpp
@ -273,7 +273,7 @@ bool DrmOutput::setDrmDpmsMode(DpmsMode mode)
        return true;
    }
    m_pipeline->setActive(active);
-    if (DrmPipeline::commitPipelines({m_pipeline}, active ? DrmPipeline::CommitMode::Test : DrmPipeline::CommitMode::CommitModeset) == DrmPipeline::Error::None) {
+    if (DrmPipeline::commitPipelines({m_pipeline}, active ? DrmPipeline::CommitMode::TestAllowModeset : DrmPipeline::CommitMode::CommitModeset) == DrmPipeline::Error::None) {
        m_pipeline->applyPendingChanges();
        setDpmsModeInternal(mode);
        if (active) {
--- a/src/backends/drm/drm_pipeline.cpp
+++ b/src/backends/drm/drm_pipeline.cpp
@ -93,15 +93,7 @@ DrmPipeline::Error DrmPipeline::commitPipelines(const QVector<DrmPipeline *> &pi

 DrmPipeline::Error DrmPipeline::commitPipelinesAtomic(const QVector<DrmPipeline *> &pipelines, CommitMode mode, const QVector<DrmObject *> &unusedObjects)
 {
-    drmModeAtomicReq *req = drmModeAtomicAlloc();
-    if (!req) {
-        qCCritical(KWIN_DRM) << "Failed to allocate drmModeAtomicReq!" << strerror(errno);
-        return Error::OutofMemory;
-    }
-    uint32_t flags = 0;
-    const auto &failed = [pipelines, req, &flags, unusedObjects]() {
-        drmModeAtomicFree(req);
-        printFlags(flags);
+    const auto &failed = [&pipelines, &unusedObjects]() {
        for (const auto &pipeline : pipelines) {
            pipeline->printDebugInfo();
            pipeline->atomicCommitFailed();
@ -111,118 +103,131 @@ DrmPipeline::Error DrmPipeline::commitPipelinesAtomic(const QVector<DrmPipeline
            obj->rollbackPending();
        }
    };
+
+    DrmUniquePtr<drmModeAtomicReq> req{drmModeAtomicAlloc()};
+    if (!req) {
+        qCCritical(KWIN_DRM) << "Failed to allocate drmModeAtomicReq!" << strerror(errno);
+        return Error::OutofMemory;
+    }
    for (const auto &pipeline : pipelines) {
-        if (pipeline->activePending() && !pipeline->m_pending.layer->checkTestBuffer()) {
-            qCWarning(KWIN_DRM) << "Checking test buffer failed for" << mode;
-            failed();
-            return Error::TestBufferFailed;
+        if (pipeline->activePending()) {
+            if (!pipeline->m_pending.layer->checkTestBuffer()) {
+                qCWarning(KWIN_DRM) << "Checking test buffer failed for" << mode;
+                failed();
+                return Error::TestBufferFailed;
+            }
+            pipeline->prepareAtomicPresentation();
+            if (mode == CommitMode::TestAllowModeset || mode == CommitMode::CommitModeset) {
+                pipeline->prepareAtomicModeset();
+            }
+        } else {
+            pipeline->prepareAtomicDisable();
        }
-        if (Error err = pipeline->populateAtomicValues(req, flags); err != Error::None) {
-            qCWarning(KWIN_DRM) << "Populating atomic values failed for" << mode;
+        if (!pipeline->populateAtomicValues(req.get())) {
            failed();
-            return err;
+            return errnoToError();
        }
    }
    for (const auto &unused : unusedObjects) {
        unused->disable();
-        if (unused->needsModeset()) {
-            flags |= DRM_MODE_ATOMIC_ALLOW_MODESET;
-        }
-        if (!unused->atomicPopulate(req)) {
+        if (!unused->atomicPopulate(req.get())) {
            qCWarning(KWIN_DRM) << "Populating atomic values failed for unused resource" << unused;
            failed();
            return errnoToError();
        }
    }
-    bool modeset = flags & DRM_MODE_ATOMIC_ALLOW_MODESET;
-    Q_ASSERT(!modeset || mode != CommitMode::Commit);
-    if (modeset) {
+    const auto gpu = pipelines[0]->gpu();
+    switch (mode) {
+    case CommitMode::TestAllowModeset: {
+        bool withModeset = drmModeAtomicCommit(gpu->fd(), req.get(), DRM_MODE_ATOMIC_ALLOW_MODESET | DRM_MODE_ATOMIC_TEST_ONLY, nullptr) == 0;
+        if (!withModeset) {
+            qCDebug(KWIN_DRM) << "Atomic modeset test failed!" << strerror(errno);
+            failed();
+            return errnoToError();
+        }
+        bool withoutModeset = drmModeAtomicCommit(gpu->fd(), req.get(), DRM_MODE_ATOMIC_TEST_ONLY, nullptr) == 0;
+        for (const auto &pipeline : pipelines) {
+            pipeline->m_pending.needsModeset = !withoutModeset;
+        }
+        std::for_each(pipelines.begin(), pipelines.end(), std::mem_fn(&DrmPipeline::atomicTestSuccessful));
+        std::for_each(unusedObjects.begin(), unusedObjects.end(), std::mem_fn(&DrmObject::commitPending));
+        return Error::None;
+    }
+    case CommitMode::CommitModeset: {
        // The kernel fails commits with DRM_MODE_PAGE_FLIP_EVENT when a crtc is disabled in the commit
        // and already was disabled before, to work around some quirks in old userspace.
        // Instead of using DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK, do the modeset in a blocking
        // fashion without page flip events and directly call the pageFlipped method afterwards
-        flags = flags & (~DRM_MODE_PAGE_FLIP_EVENT);
-    } else {
-        flags |= DRM_MODE_ATOMIC_NONBLOCK;
-    }
-    DrmGpu *gpu = pipelines[0]->gpu();
-    if (drmModeAtomicCommit(gpu->fd(), req, (flags & (~DRM_MODE_PAGE_FLIP_EVENT)) | DRM_MODE_ATOMIC_TEST_ONLY, nullptr) != 0) {
-        qCDebug(KWIN_DRM) << "Atomic test for" << mode << "failed!" << strerror(errno);
-        failed();
-        return errnoToError();
-    }
-    if (mode != CommitMode::Test && drmModeAtomicCommit(gpu->fd(), req, flags, gpu) != 0) {
-        qCCritical(KWIN_DRM) << "Atomic commit failed! This should never happen!" << strerror(errno);
-        failed();
-        return errnoToError();
-    }
-    for (const auto &pipeline : pipelines) {
-        pipeline->atomicCommitSuccessful(mode);
-    }
-    for (const auto &obj : unusedObjects) {
-        obj->commitPending();
-        if (mode != CommitMode::Test) {
-            obj->commit();
+        bool commit = drmModeAtomicCommit(gpu->fd(), req.get(), DRM_MODE_ATOMIC_ALLOW_MODESET, nullptr) == 0;
+        if (!commit) {
+            qCCritical(KWIN_DRM) << "Atomic modeset commit failed!" << strerror(errno);
+            failed();
+            return errnoToError();
        }
+        std::for_each(pipelines.begin(), pipelines.end(), std::mem_fn(&DrmPipeline::atomicModesetSuccessful));
+        std::for_each(unusedObjects.begin(), unusedObjects.end(), std::mem_fn(&DrmObject::commitPending));
+        std::for_each(unusedObjects.begin(), unusedObjects.end(), std::mem_fn(&DrmObject::commit));
+        return Error::None;
+    }
+    case CommitMode::Test: {
+        bool test = drmModeAtomicCommit(pipelines[0]->gpu()->fd(), req.get(), DRM_MODE_ATOMIC_TEST_ONLY, nullptr) == 0;
+        if (!test) {
+            qCDebug(KWIN_DRM) << "Atomic test failed!" << strerror(errno);
+            failed();
+            return errnoToError();
+        }
+        std::for_each(pipelines.begin(), pipelines.end(), std::mem_fn(&DrmPipeline::atomicTestSuccessful));
+        Q_ASSERT(unusedObjects.isEmpty());
+        return Error::None;
+    }
+    case CommitMode::Commit: {
+        bool commit = drmModeAtomicCommit(pipelines[0]->gpu()->fd(), req.get(), DRM_MODE_ATOMIC_NONBLOCK | DRM_MODE_PAGE_FLIP_EVENT, gpu) == 0;
+        if (!commit) {
+            qCCritical(KWIN_DRM) << "Atomic commit failed!" << strerror(errno);
+            failed();
+            return errnoToError();
+        }
+        std::for_each(pipelines.begin(), pipelines.end(), std::mem_fn(&DrmPipeline::atomicCommitSuccessful));
+        Q_ASSERT(unusedObjects.isEmpty());
+        return Error::None;
+    }
+    default:
+        Q_UNREACHABLE();
    }
-    drmModeAtomicFree(req);
-    return Error::None;
 }

-DrmPipeline::Error DrmPipeline::populateAtomicValues(drmModeAtomicReq *req, uint32_t &flags)
+void DrmPipeline::prepareAtomicPresentation()
 {
-    bool modeset = needsModeset();
-    if (modeset) {
-        flags |= DRM_MODE_ATOMIC_ALLOW_MODESET;
-        m_pending.needsModeset = true;
-    }
-    if (activePending()) {
-        flags |= DRM_MODE_PAGE_FLIP_EVENT;
-    }
-    if (m_pending.needsModeset) {
-        prepareAtomicModeset();
-    }
-    if (m_pending.crtc) {
-        m_pending.crtc->setPending(DrmCrtc::PropertyIndex::VrrEnabled, m_pending.syncMode == RenderLoopPrivate::SyncMode::Adaptive);
-        m_pending.crtc->setPending(DrmCrtc::PropertyIndex::Gamma_LUT, m_pending.gamma ? m_pending.gamma->blobId() : 0);
-        const auto modeSize = m_pending.mode->size();
-        const auto fb = m_pending.layer->currentBuffer().get();
-        m_pending.crtc->primaryPlane()->set(QPoint(0, 0), fb ? fb->buffer()->size() : bufferSize(), QPoint(0, 0), modeSize);
-        m_pending.crtc->primaryPlane()->setBuffer(activePending() ? fb : nullptr);
+    m_pending.crtc->setPending(DrmCrtc::PropertyIndex::VrrEnabled, m_pending.syncMode == RenderLoopPrivate::SyncMode::Adaptive);
+    m_pending.crtc->setPending(DrmCrtc::PropertyIndex::Gamma_LUT, m_pending.gamma ? m_pending.gamma->blobId() : 0);
+    const auto modeSize = m_pending.mode->size();
+    const auto fb = m_pending.layer->currentBuffer().get();
+    m_pending.crtc->primaryPlane()->set(QPoint(0, 0), fb->buffer()->size(), QPoint(0, 0), modeSize);
+    m_pending.crtc->primaryPlane()->setBuffer(fb);

-        if (m_pending.crtc->cursorPlane()) {
-            const auto layer = cursorLayer();
-            bool active = activePending() && layer->isVisible();
-            m_pending.crtc->cursorPlane()->set(QPoint(0, 0), gpu()->cursorSize(), layer->position(), gpu()->cursorSize());
-            m_pending.crtc->cursorPlane()->setBuffer(active ? layer->currentBuffer().get() : nullptr);
-            m_pending.crtc->cursorPlane()->setPending(DrmPlane::PropertyIndex::CrtcId, active ? m_pending.crtc->id() : 0);
-        }
-    }
-    if (!m_connector->atomicPopulate(req)) {
-        return errnoToError();
+    if (m_pending.crtc->cursorPlane()) {
+        const auto layer = cursorLayer();
+        m_pending.crtc->cursorPlane()->set(QPoint(0, 0), gpu()->cursorSize(), layer->position(), gpu()->cursorSize());
+        m_pending.crtc->cursorPlane()->setBuffer(layer->isVisible() ? layer->currentBuffer().get() : nullptr);
+        m_pending.crtc->cursorPlane()->setPending(DrmPlane::PropertyIndex::CrtcId, layer->isVisible() ? m_pending.crtc->id() : 0);
    }
+}
+
+void DrmPipeline::prepareAtomicDisable()
+{
+    m_connector->disable();
    if (m_pending.crtc) {
-        if (!m_pending.crtc->atomicPopulate(req)) {
-            return errnoToError();
-        }
-        if (!m_pending.crtc->primaryPlane()->atomicPopulate(req)) {
-            return errnoToError();
-        }
-        if (m_pending.crtc->cursorPlane() && !m_pending.crtc->cursorPlane()->atomicPopulate(req)) {
-            return errnoToError();
+        m_pending.crtc->disable();
+        m_pending.crtc->primaryPlane()->disable();
+        if (auto cursor = m_pending.crtc->cursorPlane()) {
+            cursor->disable();
        }
    }
-    return Error::None;
 }

 void DrmPipeline::prepareAtomicModeset()
 {
-    if (!m_pending.crtc) {
-        m_connector->setPending(DrmConnector::PropertyIndex::CrtcId, 0);
-        return;
-    }
-
-    m_connector->setPending(DrmConnector::PropertyIndex::CrtcId, activePending() ? m_pending.crtc->id() : 0);
+    m_connector->setPending(DrmConnector::PropertyIndex::CrtcId, m_pending.crtc->id());
    if (const auto &prop = m_connector->getProp(DrmConnector::PropertyIndex::Broadcast_RGB)) {
        prop->setEnum(m_pending.rgbRange);
    }
@ -245,16 +250,35 @@ void DrmPipeline::prepareAtomicModeset()
        bpc->setPending(std::min(bpc->maxValue(), preferred));
    }

-    m_pending.crtc->setPending(DrmCrtc::PropertyIndex::Active, activePending());
-    m_pending.crtc->setPending(DrmCrtc::PropertyIndex::ModeId, activePending() ? m_pending.mode->blobId() : 0);
+    m_pending.crtc->setPending(DrmCrtc::PropertyIndex::Active, 1);
+    m_pending.crtc->setPending(DrmCrtc::PropertyIndex::ModeId, m_pending.mode->blobId());

-    m_pending.crtc->primaryPlane()->setPending(DrmPlane::PropertyIndex::CrtcId, activePending() ? m_pending.crtc->id() : 0);
+    m_pending.crtc->primaryPlane()->setPending(DrmPlane::PropertyIndex::CrtcId, m_pending.crtc->id());
    m_pending.crtc->primaryPlane()->setTransformation(m_pending.bufferOrientation);
    if (m_pending.crtc->cursorPlane()) {
        m_pending.crtc->cursorPlane()->setTransformation(DrmPlane::Transformation::Rotate0);
    }
 }

+bool DrmPipeline::populateAtomicValues(drmModeAtomicReq *req)
+{
+    if (!m_connector->atomicPopulate(req)) {
+        return false;
+    }
+    if (m_pending.crtc) {
+        if (!m_pending.crtc->atomicPopulate(req)) {
+            return false;
+        }
+        if (!m_pending.crtc->primaryPlane()->atomicPopulate(req)) {
+            return false;
+        }
+        if (m_pending.crtc->cursorPlane() && !m_pending.crtc->cursorPlane()->atomicPopulate(req)) {
+            return false;
+        }
+    }
+    return true;
+}
+
 uint32_t DrmPipeline::calculateUnderscan()
 {
    const auto size = m_pending.mode->size();
@ -296,7 +320,7 @@ void DrmPipeline::atomicCommitFailed()
    }
 }

-void DrmPipeline::atomicCommitSuccessful(CommitMode mode)
+void DrmPipeline::atomicTestSuccessful()
 {
    m_connector->commitPending();
    if (m_pending.crtc) {
@ -306,26 +330,35 @@ void DrmPipeline::atomicCommitSuccessful(CommitMode mode)
            m_pending.crtc->cursorPlane()->commitPending();
        }
    }
-    if (mode != CommitMode::Test) {
-        m_pending.needsModeset = false;
-        if (activePending()) {
-            m_pageflipPending = true;
-        }
-        m_connector->commit();
-        if (m_pending.crtc) {
-            m_pending.crtc->commit();
-            m_pending.crtc->primaryPlane()->setNext(m_pending.layer->currentBuffer());
-            m_pending.crtc->primaryPlane()->commit();
-            if (m_pending.crtc->cursorPlane()) {
-                m_pending.crtc->cursorPlane()->setNext(cursorLayer()->currentBuffer());
-                m_pending.crtc->cursorPlane()->commit();
-            }
-        }
-        m_current = m_pending;
-        if (mode == CommitMode::CommitModeset && activePending()) {
-            pageFlipped(std::chrono::steady_clock::now().time_since_epoch());
+}
+
+void DrmPipeline::atomicCommitSuccessful()
+{
+    atomicTestSuccessful();
+    m_pending.needsModeset = false;
+    if (activePending()) {
+        m_pageflipPending = true;
+    }
+    m_connector->commit();
+    if (m_pending.crtc) {
+        m_pending.crtc->commit();
+        m_pending.crtc->primaryPlane()->setNext(m_pending.layer->currentBuffer());
+        m_pending.crtc->primaryPlane()->commit();
+        if (m_pending.crtc->cursorPlane()) {
+            m_pending.crtc->cursorPlane()->setNext(cursorLayer()->currentBuffer());
+            m_pending.crtc->cursorPlane()->commit();
        }
    }
+    m_current = m_pending;
+}
+
+void DrmPipeline::atomicModesetSuccessful()
+{
+    atomicCommitSuccessful();
+    m_pending.needsModeset = false;
+    if (activePending()) {
+        pageFlipped(std::chrono::steady_clock::now().time_since_epoch());
+    }
 }

 bool DrmPipeline::setCursor(const QPoint &hotspot)
@ -449,27 +482,7 @@ bool DrmPipeline::pruneModifier()

 bool DrmPipeline::needsModeset() const
 {
-    if (m_connector->needsModeset()) {
-        return true;
-    }
-    if (m_pending.crtc) {
-        if (m_pending.crtc->needsModeset()) {
-            return true;
-        }
-        if (auto primary = m_pending.crtc->primaryPlane(); primary && primary->needsModeset()) {
-            return true;
-        }
-        if (auto cursor = m_pending.crtc->cursorPlane(); cursor && cursor->needsModeset()) {
-            return true;
-        }
-    }
-    return m_pending.crtc != m_current.crtc
-        || m_pending.active != m_current.active
-        || m_pending.mode != m_current.mode
-        || m_pending.rgbRange != m_current.rgbRange
-        || m_pending.bufferOrientation != m_current.bufferOrientation
-        || m_connector->linkStatus() == DrmConnector::LinkStatus::Bad
-        || m_modesetPresentPending;
+    return m_pending.needsModeset;
 }

 bool DrmPipeline::activePending() const
--- a/src/backends/drm/drm_pipeline.h
+++ b/src/backends/drm/drm_pipeline.h
@ -127,6 +127,7 @@ public:

    enum class CommitMode {
        Test,
+        TestAllowModeset,
        Commit,
        CommitModeset
    };
@ -147,10 +148,14 @@ private:
    static Error commitPipelinesLegacy(const QVector<DrmPipeline *> &pipelines, CommitMode mode);

    // atomic modesetting only
-    Error populateAtomicValues(drmModeAtomicReq *req, uint32_t &flags);
+    bool populateAtomicValues(drmModeAtomicReq *req);
    void atomicCommitFailed();
-    void atomicCommitSuccessful(CommitMode mode);
+    void atomicTestSuccessful();
+    void atomicCommitSuccessful();
+    void atomicModesetSuccessful();
    void prepareAtomicModeset();
+    void prepareAtomicPresentation();
+    void prepareAtomicDisable();
    static Error commitPipelinesAtomic(const QVector<DrmPipeline *> &pipelines, CommitMode mode, const QVector<DrmObject *> &unusedObjects);

    // logging helpers
--- a/src/backends/drm/drm_pipeline_legacy.cpp
+++ b/src/backends/drm/drm_pipeline_legacy.cpp
@ -78,7 +78,7 @@ DrmPipeline::Error DrmPipeline::commitPipelinesLegacy(const QVector<DrmPipeline
        for (const auto &pipeline : pipelines) {
            pipeline->applyPendingChanges();
            pipeline->m_current = pipeline->m_pending;
-            if (mode == CommitMode::CommitModeset && mode != CommitMode::Test && pipeline->activePending()) {
+            if (mode == CommitMode::CommitModeset && pipeline->activePending()) {
                pipeline->pageFlipped(std::chrono::steady_clock::now().time_since_epoch());
            }
        }
@ -108,7 +108,7 @@ DrmPipeline::Error DrmPipeline::applyPendingChangesLegacy()
            m_connector->getProp(DrmConnector::PropertyIndex::Underscan_vborder)->setPropertyLegacy(m_pending.overscan);
            m_connector->getProp(DrmConnector::PropertyIndex::Underscan_hborder)->setPropertyLegacy(hborder);
        }
-        if (needsModeset()) {
+        if (m_pending.crtc != m_current.crtc || m_pending.mode != m_current.mode) {
            Error err = legacyModeset();
            if (err != Error::None) {
                return err;