From 7283c98f27b774b358b7d4bbca1872cf815de6d9 Mon Sep 17 00:00:00 2001 From: Xaver Hugl Date: Fri, 8 Oct 2021 11:22:02 +0200 Subject: [PATCH] platforms/drm: disable unused resources on modesets When we switch CRTCs it can happen that a CRTC would stay enabled yet has no connectors anymore. In this case the kernel may reject our atomic commit, which would cause the modeset to fail. To counteract that, properly disable unused drm objects --- src/backends/drm/drm_gpu.cpp | 48 ++++++++++++++++++----- src/backends/drm/drm_gpu.h | 2 + src/backends/drm/drm_object.h | 5 +++ src/backends/drm/drm_object_connector.cpp | 5 +++ src/backends/drm/drm_object_connector.h | 1 + src/backends/drm/drm_object_crtc.cpp | 14 ++++++- src/backends/drm/drm_object_crtc.h | 6 ++- src/backends/drm/drm_object_plane.cpp | 6 +++ src/backends/drm/drm_object_plane.h | 1 + src/backends/drm/drm_pipeline.cpp | 25 +++++++++++- src/backends/drm/drm_pipeline.h | 2 +- 11 files changed, 101 insertions(+), 14 deletions(-) diff --git a/src/backends/drm/drm_gpu.cpp b/src/backends/drm/drm_gpu.cpp index a86c9bc8e5..a86f169b77 100644 --- a/src/backends/drm/drm_gpu.cpp +++ b/src/backends/drm/drm_gpu.cpp @@ -177,6 +177,7 @@ void DrmGpu::initDrmResources() DrmPlane *p = new DrmPlane(this, kplane->plane_id); if (p->init()) { m_planes << p; + m_allObjects << p; } else { delete p; } @@ -199,13 +200,19 @@ void DrmGpu::initDrmResources() } auto planes = m_planes; for (int i = 0; i < resources->count_crtcs; ++i) { + uint32_t crtcId = resources->crtcs[i]; DrmPlane *primary = nullptr; + DrmPlane *cursor = nullptr; for (const auto &plane : qAsConst(planes)) { - if (plane->type() == DrmPlane::TypeIndex::Primary - && plane->isCrtcSupported(i)) { - primary = plane; - if (plane->getProp(DrmPlane::PropertyIndex::CrtcId)->current() == resources->crtcs[i]) { - break; + if (plane->isCrtcSupported(i)) { + if (plane->type() == DrmPlane::TypeIndex::Primary) { + if (!primary || primary->getProp(DrmPlane::PropertyIndex::CrtcId)->pending() == crtcId) { + primary = plane; + } + } else if (plane->type() == DrmPlane::TypeIndex::Cursor) { + if (!cursor || cursor->getProp(DrmPlane::PropertyIndex::CrtcId)->pending() == crtcId) { + cursor = plane; + } } } } @@ -214,12 +221,13 @@ void DrmGpu::initDrmResources() continue; } planes.removeOne(primary); - auto c = new DrmCrtc(this, resources->crtcs[i], i, primary); + auto c = new DrmCrtc(this, crtcId, i, primary, cursor); if (!c->init()) { delete c; continue; } m_crtcs << c; + m_allObjects << c; } } @@ -263,6 +271,7 @@ bool DrmGpu::updateOutputs() continue; } m_connectors << conn; + m_allObjects << conn; } else { removedConnectors.removeOne(conn); conn->updateProperties(); @@ -299,6 +308,7 @@ bool DrmGpu::updateOutputs() removeLeaseOutput(leaseOutput); } m_connectors.removeOne(connector); + m_allObjects.removeOne(connector); delete connector; } @@ -348,13 +358,14 @@ bool DrmGpu::checkCrtcAssignment(QVector connectors, QVectorpipeline(); } } - bool test = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::Test); + 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); + bool ret = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::Test, unused); if (ret) { for (const auto &pipeline : qAsConst(leasePipelines)) { pipeline->applyPendingChanges(); @@ -728,6 +739,8 @@ 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(); }); } @@ -743,11 +756,28 @@ bool DrmGpu::maybeModeset() return pipeline->modesetPresentPending() || !pipeline->pending.active; }); if (presentPendingForAll) { - return DrmPipeline::commitPipelines(pipelines, DrmPipeline::CommitMode::CommitModeset); + return DrmPipeline::commitPipelines(pipelines, DrmPipeline::CommitMode::CommitModeset, unusedObjects()); } else { // commit only once all pipelines are ready for presentation return true; } } +QVector DrmGpu::unusedObjects() const +{ + if (!m_atomicModeSetting) { + return {}; + } + QVector ret = m_allObjects; + for (const auto &pipeline : m_pipelines) { + ret.removeOne(pipeline->connector()); + if (pipeline->pending.crtc) { + ret.removeOne(pipeline->pending.crtc); + ret.removeOne(pipeline->pending.crtc->primaryPlane()); + ret.removeOne(pipeline->pending.crtc->cursorPlane()); + } + } + return ret; +} + } diff --git a/src/backends/drm/drm_gpu.h b/src/backends/drm/drm_gpu.h index c15b839efa..09223744d7 100644 --- a/src/backends/drm/drm_gpu.h +++ b/src/backends/drm/drm_gpu.h @@ -101,6 +101,7 @@ private: void initDrmResources(); bool checkCrtcAssignment(QVector connectors, QVector crtcs); + QVector unusedObjects() const; void handleLeaseRequest(KWaylandServer::DrmLeaseV1Interface *leaseRequest); void handleLeaseRevoked(KWaylandServer::DrmLeaseV1Interface *lease); @@ -123,6 +124,7 @@ private: QVector m_planes; QVector m_crtcs; QVector m_connectors; + QVector m_allObjects; QVector m_pipelines; QVector m_drmOutputs; diff --git a/src/backends/drm/drm_object.h b/src/backends/drm/drm_object.h index 5ba450fa41..4d48d7df67 100644 --- a/src/backends/drm/drm_object.h +++ b/src/backends/drm/drm_object.h @@ -36,6 +36,11 @@ public: */ virtual bool init() = 0; + /** + * Set the properties in such a way that this resource won't be used anymore + */ + virtual void disable() = 0; + uint32_t id() const; DrmGpu *gpu() const; uint32_t type() const; diff --git a/src/backends/drm/drm_object_connector.cpp b/src/backends/drm/drm_object_connector.cpp index 20a02206a3..959942cb09 100644 --- a/src/backends/drm/drm_object_connector.cpp +++ b/src/backends/drm/drm_object_connector.cpp @@ -369,6 +369,11 @@ DrmPipeline *DrmConnector::pipeline() const return m_pipeline.data(); } +void DrmConnector::disable() +{ + setPending(PropertyIndex::CrtcId, 0); +} + QDebug& operator<<(QDebug& s, const KWin::DrmConnector *obj) { QDebugStateSaver saver(s); diff --git a/src/backends/drm/drm_object_connector.h b/src/backends/drm/drm_object_connector.h index 671021dff0..873ed46e0d 100644 --- a/src/backends/drm/drm_object_connector.h +++ b/src/backends/drm/drm_object_connector.h @@ -53,6 +53,7 @@ public: bool init() override; bool needsModeset() const override; bool updateProperties() override; + void disable() override; QVector encoders() const; bool isConnected() const; diff --git a/src/backends/drm/drm_object_crtc.cpp b/src/backends/drm/drm_object_crtc.cpp index c1475b1636..79da043223 100644 --- a/src/backends/drm/drm_object_crtc.cpp +++ b/src/backends/drm/drm_object_crtc.cpp @@ -18,7 +18,7 @@ namespace KWin { -DrmCrtc::DrmCrtc(DrmGpu *gpu, uint32_t crtcId, int pipeIndex, DrmPlane *primaryPlane) +DrmCrtc::DrmCrtc(DrmGpu *gpu, uint32_t crtcId, int pipeIndex, DrmPlane *primaryPlane, DrmPlane *cursorPlane) : DrmObject(gpu, crtcId, { PropertyDefinition(QByteArrayLiteral("MODE_ID"), Requirement::Required), PropertyDefinition(QByteArrayLiteral("ACTIVE"), Requirement::Required), @@ -29,6 +29,7 @@ DrmCrtc::DrmCrtc(DrmGpu *gpu, uint32_t crtcId, int pipeIndex, DrmPlane *primaryP , m_crtc(drmModeGetCrtc(gpu->fd(), crtcId)) , m_pipeIndex(pipeIndex) , m_primaryPlane(primaryPlane) + , m_cursorPlane(cursorPlane) { } @@ -147,4 +148,15 @@ DrmPlane *DrmCrtc::primaryPlane() const return m_primaryPlane; } +DrmPlane *DrmCrtc::cursorPlane() const +{ + return m_cursorPlane; +} + +void DrmCrtc::disable() +{ + setPending(PropertyIndex::Active, 0); + setPendingBlob(PropertyIndex::ModeId, nullptr, sizeof(drmModeModeInfo)); +} + } diff --git a/src/backends/drm/drm_object_crtc.h b/src/backends/drm/drm_object_crtc.h index ce285bd314..7bc5a73663 100644 --- a/src/backends/drm/drm_object_crtc.h +++ b/src/backends/drm/drm_object_crtc.h @@ -27,7 +27,7 @@ class DrmPlane; class DrmCrtc : public DrmObject { public: - DrmCrtc(DrmGpu *gpu, uint32_t crtcId, int pipeIndex, DrmPlane *primaryPlane); + DrmCrtc(DrmGpu *gpu, uint32_t crtcId, int pipeIndex, DrmPlane *primaryPlane, DrmPlane *cursorPlane); enum class PropertyIndex : uint32_t { ModeId = 0, @@ -40,10 +40,12 @@ public: bool init() override; bool needsModeset() const override; + void disable() override; int pipeIndex() const; int gammaRampSize() const; DrmPlane *primaryPlane() const; + DrmPlane *cursorPlane() const; drmModeModeInfo queryCurrentMode(); QSharedPointer current() const; @@ -64,6 +66,8 @@ private: QSharedPointer m_nextBuffer; int m_pipeIndex; DrmPlane *m_primaryPlane; + // currently unused but needs to be filtered out for modesets + DrmPlane *m_cursorPlane; struct { QPoint pos; diff --git a/src/backends/drm/drm_object_plane.cpp b/src/backends/drm/drm_object_plane.cpp index 03fbf51b30..c4cacc0f23 100644 --- a/src/backends/drm/drm_object_plane.cpp +++ b/src/backends/drm/drm_object_plane.cpp @@ -208,4 +208,10 @@ DrmPlane::Transformations DrmPlane::supportedTransformations() const return m_supportedTransformations; } +void DrmPlane::disable() +{ + setPending(PropertyIndex::CrtcId, 0); + setPending(PropertyIndex::FbId, 0); +} + } diff --git a/src/backends/drm/drm_object_plane.h b/src/backends/drm/drm_object_plane.h index a5a60631c2..e55492f71a 100644 --- a/src/backends/drm/drm_object_plane.h +++ b/src/backends/drm/drm_object_plane.h @@ -65,6 +65,7 @@ public: bool init() override; bool needsModeset() const override; + void disable() override; TypeIndex type(); bool isCrtcSupported(int pipeIndex) const; diff --git a/src/backends/drm/drm_pipeline.cpp b/src/backends/drm/drm_pipeline.cpp index 828b6a9244..73c26f6309 100644 --- a/src/backends/drm/drm_pipeline.cpp +++ b/src/backends/drm/drm_pipeline.cpp @@ -111,7 +111,7 @@ bool DrmPipeline::present(const QSharedPointer &buffer) return true; } -bool DrmPipeline::commitPipelines(const QVector &pipelines, CommitMode mode) +bool DrmPipeline::commitPipelines(const QVector &pipelines, CommitMode mode, const QVector &unusedObjects) { Q_ASSERT(!pipelines.isEmpty()); if (pipelines[0]->gpu()->atomicModeSetting()) { @@ -121,7 +121,7 @@ bool DrmPipeline::commitPipelines(const QVector &pipelines, Commit return false; } uint32_t flags = 0; - const auto &failed = [pipelines, req, mode](){ + const auto &failed = [pipelines, req, mode, unusedObjects](){ drmModeAtomicFree(req); for (const auto &pipeline : pipelines) { pipeline->printDebugInfo(); @@ -139,6 +139,9 @@ bool DrmPipeline::commitPipelines(const QVector &pipelines, Commit pipeline->output()->presentFailed(); } } + for (const auto &obj : unusedObjects) { + obj->rollbackPending(); + } return false; }; for (const auto &pipeline : pipelines) { @@ -151,6 +154,16 @@ bool DrmPipeline::commitPipelines(const QVector &pipelines, Commit return failed(); } } + for (const auto &unused : unusedObjects) { + unused->disable(); + if (unused->needsModeset()) { + flags |= DRM_MODE_ATOMIC_ALLOW_MODESET; + } + if (!unused->atomicPopulate(req)) { + qCWarning(KWIN_DRM) << "Populating atomic values failed for unused resource" << unused; + return failed(); + } + } bool modeset = flags & DRM_MODE_ATOMIC_ALLOW_MODESET; Q_ASSERT(!modeset || mode != CommitMode::Commit); if (modeset) { @@ -176,6 +189,7 @@ bool DrmPipeline::commitPipelines(const QVector &pipelines, Commit if (pipeline->pending.crtc) { pipeline->pending.crtc->commitPending(); pipeline->pending.crtc->primaryPlane()->commitPending(); + pipeline->pending.crtc->cursorPlane()->commitPending(); } if (mode != CommitMode::Test) { pipeline->m_modesetPresentPending = false; @@ -185,6 +199,7 @@ bool DrmPipeline::commitPipelines(const QVector &pipelines, Commit pipeline->pending.crtc->primaryPlane()->setNext(pipeline->m_primaryBuffer); pipeline->pending.crtc->commit(); pipeline->pending.crtc->primaryPlane()->commit(); + pipeline->pending.crtc->cursorPlane()->commit(); } pipeline->m_current = pipeline->pending; if (modeset && pipeline->activePending()) { @@ -192,6 +207,12 @@ bool DrmPipeline::commitPipelines(const QVector &pipelines, Commit } } } + for (const auto &obj : unusedObjects) { + obj->commitPending(); + if (mode != CommitMode::Test) { + obj->commit(); + } + } drmModeAtomicFree(req); return true; } else { diff --git a/src/backends/drm/drm_pipeline.h b/src/backends/drm/drm_pipeline.h index b8b0f3f4c1..a0829cc30d 100644 --- a/src/backends/drm/drm_pipeline.h +++ b/src/backends/drm/drm_pipeline.h @@ -101,7 +101,7 @@ public: CommitModeset }; Q_ENUM(CommitMode); - static bool commitPipelines(const QVector &pipelines, CommitMode mode); + static bool commitPipelines(const QVector &pipelines, CommitMode mode, const QVector &unusedObjects = {}); private: bool populateAtomicValues(drmModeAtomicReq *req, uint32_t &flags);