From 9f93358bc157b3d7fc2c931c3d7c9c8cba70fc40 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Thu, 25 Nov 2021 11:15:50 +0200 Subject: [PATCH] backends/drm: Notify about failed frames if there are actually pending frames In case a modeset needs to be performed, the drm backend will test all pipelines to ensure that new mode won't cause any bandwidth issues on other outputs, etc. To do that, it may delay presenting frames. If the new configuration doesn't work, it needs to notify about failed frames. However, the relevant code that notifies the RenderLoop about failed atomic commits doesn't check if there's actually a pending modeset present. When switching between VTs, systemd can revoke master permissions from kwin. To make things even more trickier, kwin can try to present a frame in that short time span. --- src/backends/drm/drm_gpu.cpp | 14 +++++++++++--- src/backends/drm/drm_pipeline.cpp | 13 +++++-------- src/backends/drm/drm_pipeline.h | 1 + 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/backends/drm/drm_gpu.cpp b/src/backends/drm/drm_gpu.cpp index 4c0d52fdb2..bfd747469c 100644 --- a/src/backends/drm/drm_gpu.cpp +++ b/src/backends/drm/drm_gpu.cpp @@ -727,12 +727,20 @@ bool DrmGpu::maybeModeset() bool presentPendingForAll = std::all_of(pipelines.constBegin(), pipelines.constEnd(), [](const auto &pipeline) { return pipeline->modesetPresentPending() || !pipeline->pending.active; }); - if (presentPendingForAll) { - return DrmPipeline::commitPipelines(pipelines, DrmPipeline::CommitMode::CommitModeset, unusedObjects()); - } else { + if (!presentPendingForAll) { // commit only once all pipelines are ready for presentation return true; } + const bool ok = DrmPipeline::commitPipelines(pipelines, DrmPipeline::CommitMode::CommitModeset, unusedObjects()); + for (DrmPipeline *pipeline : qAsConst(pipelines)) { + if (pipeline->modesetPresentPending() && pipeline->output()) { + pipeline->resetModesetPresentPending(); + if (!ok) { + pipeline->output()->presentFailed(); + } + } + } + return ok; } QVector DrmGpu::unusedObjects() const diff --git a/src/backends/drm/drm_pipeline.cpp b/src/backends/drm/drm_pipeline.cpp index 5ac5ccf65e..003fd0bf2b 100644 --- a/src/backends/drm/drm_pipeline.cpp +++ b/src/backends/drm/drm_pipeline.cpp @@ -126,10 +126,6 @@ bool DrmPipeline::commitPipelines(const QVector &pipelines, Commit pipeline->pending.crtc->rollbackPending(); pipeline->pending.crtc->primaryPlane()->rollbackPending(); } - if (mode != CommitMode::Test && pipeline->activePending() && pipeline->output()) { - pipeline->m_modesetPresentPending = false; - pipeline->output()->presentFailed(); - } } for (const auto &obj : unusedObjects) { printProps(obj, PrintMode::OnlyChanged); @@ -187,7 +183,6 @@ bool DrmPipeline::commitPipelines(const QVector &pipelines, Commit } } if (mode != CommitMode::Test) { - pipeline->m_modesetPresentPending = false; pipeline->m_pageflipPending = true; pipeline->m_connector->commit(); if (pipeline->pending.crtc) { @@ -225,9 +220,6 @@ bool DrmPipeline::commitPipelines(const QVector &pipelines, Commit for (const auto &pipeline : pipelines) { pipeline->revertPendingChanges(); pipeline->applyPendingChangesLegacy(); - if (mode == CommitMode::CommitModeset && pipeline->output() && pipeline->activePending()) { - pipeline->output()->presentFailed(); - } } return false; } else { @@ -510,6 +502,11 @@ bool DrmPipeline::modesetPresentPending() const return m_modesetPresentPending; } +void DrmPipeline::resetModesetPresentPending() +{ + m_modesetPresentPending = false; +} + DrmCrtc *DrmPipeline::currentCrtc() const { return m_current.crtc; diff --git a/src/backends/drm/drm_pipeline.h b/src/backends/drm/drm_pipeline.h index 7850c86530..126fdc3e94 100644 --- a/src/backends/drm/drm_pipeline.h +++ b/src/backends/drm/drm_pipeline.h @@ -80,6 +80,7 @@ public: void pageFlipped(std::chrono::nanoseconds timestamp); bool pageflipPending() const; bool modesetPresentPending() const; + void resetModesetPresentPending(); void printDebugInfo() const; QSize sourceSize() const;