From 42c5e6bcf6dd29d00fb61a61a63e419fe56befc8 Mon Sep 17 00:00:00 2001 From: Xaver Hugl Date: Mon, 20 Jun 2022 19:35:25 +0200 Subject: [PATCH] backends/drm: handle failing commits better It can happen that the drm backend temporarily lacks permission to do atomic commits, or that the cached drm property values become out of sync with the real values held by the kernel. Instead of failing with both, attempt to update property values and try the commits again at a later time. --- src/backends/drm/drm_backend.cpp | 2 +- src/backends/drm/drm_backend.h | 2 +- src/backends/drm/drm_gpu.cpp | 63 ++++++++++++++------- src/backends/drm/drm_gpu.h | 8 +-- src/backends/drm/drm_output.cpp | 24 +++++--- src/backends/drm/drm_pipeline.cpp | 71 +++++++++++++++--------- src/backends/drm/drm_pipeline.h | 30 +++++++--- src/backends/drm/drm_pipeline_legacy.cpp | 51 +++++++++-------- 8 files changed, 158 insertions(+), 93 deletions(-) diff --git a/src/backends/drm/drm_backend.cpp b/src/backends/drm/drm_backend.cpp index 91e5a07363..2dd3aa9a64 100644 --- a/src/backends/drm/drm_backend.cpp +++ b/src/backends/drm/drm_backend.cpp @@ -706,7 +706,7 @@ bool DrmBackend::applyOutputChanges(const OutputConfiguration &config) toBeDisabled << output; } } - if (!gpu->testPendingConfiguration()) { + if (gpu->testPendingConfiguration() != DrmPipeline::Error::None) { for (const auto &output : qAsConst(toBeEnabled)) { output->revertQueuedChanges(); } diff --git a/src/backends/drm/drm_backend.h b/src/backends/drm/drm_backend.h index fc1bc73797..fc32eede4d 100644 --- a/src/backends/drm/drm_backend.h +++ b/src/backends/drm/drm_backend.h @@ -76,6 +76,7 @@ public: DrmRenderBackend *renderBackend() const; void releaseBuffers(); + void updateOutputs(); public Q_SLOTS: void turnOutputsOn(); @@ -96,7 +97,6 @@ private: void activate(bool active); void reactivate(); void deactivate(); - void updateOutputs(); bool readOutputsConfiguration(const QVector &outputs); void handleUdevEvent(); DrmGpu *addGpu(const QString &fileName); diff --git a/src/backends/drm/drm_gpu.cpp b/src/backends/drm/drm_gpu.cpp index 9879e1d05b..a69c3e0506 100644 --- a/src/backends/drm/drm_gpu.cpp +++ b/src/backends/drm/drm_gpu.cpp @@ -316,8 +316,8 @@ bool DrmGpu::updateOutputs() for (const auto &plane : qAsConst(m_planes)) { plane->updateProperties(); } - - if (testPendingConfiguration()) { + DrmPipeline::Error err = testPendingConfiguration(); + if (err == DrmPipeline::Error::None) { for (const auto &pipeline : qAsConst(m_pipelines)) { pipeline->applyPendingChanges(); if (pipeline->output() && !pipeline->crtc()) { @@ -325,6 +325,15 @@ bool DrmGpu::updateOutputs() pipeline->output()->setEnabled(false); } } + } else if (err == DrmPipeline::Error::NoPermission) { + for (const auto &pipeline : qAsConst(m_pipelines)) { + pipeline->revertPendingChanges(); + } + for (const auto &output : qAsConst(addedOutputs)) { + m_connectors.removeOne(output->connector()); + removeOutput(output); + } + QTimer::singleShot(50, m_platform, &DrmBackend::updateOutputs); } else { qCWarning(KWIN_DRM, "Failed to find a working setup for new outputs!"); for (const auto &pipeline : qAsConst(m_pipelines)) { @@ -340,12 +349,12 @@ bool DrmGpu::updateOutputs() return true; } -bool DrmGpu::checkCrtcAssignment(QVector connectors, const QVector &crtcs) +DrmPipeline::Error DrmGpu::checkCrtcAssignment(QVector connectors, const QVector &crtcs) { if (connectors.isEmpty() || crtcs.isEmpty()) { if (m_pipelines.isEmpty()) { // nothing to do - return true; + return DrmPipeline::Error::None; } // remaining connectors can't be powered for (const auto &conn : qAsConst(connectors)) { @@ -374,8 +383,9 @@ bool DrmGpu::checkCrtcAssignment(QVector connectors, const QVect crtcsLeft.removeOne(currentCrtc); pipeline->setCrtc(currentCrtc); do { - if (checkCrtcAssignment(connectors, crtcsLeft)) { - return true; + DrmPipeline::Error err = checkCrtcAssignment(connectors, crtcsLeft); + if (err == DrmPipeline::Error::None || err == DrmPipeline::Error::NoPermission || err == DrmPipeline::Error::FramePending) { + return err; } } while (pipeline->pruneModifier()); } @@ -386,16 +396,17 @@ bool DrmGpu::checkCrtcAssignment(QVector connectors, const QVect crtcsLeft.removeOne(crtc); pipeline->setCrtc(crtc); do { - if (checkCrtcAssignment(connectors, crtcsLeft)) { - return true; + DrmPipeline::Error err = checkCrtcAssignment(connectors, crtcsLeft); + if (err == DrmPipeline::Error::None || err == DrmPipeline::Error::NoPermission || err == DrmPipeline::Error::FramePending) { + return err; } } while (pipeline->pruneModifier()); } } - return false; + return DrmPipeline::Error::InvalidArguments; } -bool DrmGpu::testPendingConfiguration() +DrmPipeline::Error DrmGpu::testPendingConfiguration() { QVector connectors; for (const auto &conn : qAsConst(m_connectors)) { @@ -417,8 +428,9 @@ bool DrmGpu::testPendingConfiguration() return c1->getProp(DrmConnector::PropertyIndex::CrtcId)->current() > c2->getProp(DrmConnector::PropertyIndex::CrtcId)->current(); }); } - if (checkCrtcAssignment(connectors, crtcs)) { - return true; + DrmPipeline::Error err = checkCrtcAssignment(connectors, crtcs); + if (err == DrmPipeline::Error::None || err == DrmPipeline::Error::NoPermission || err == DrmPipeline::Error::FramePending) { + return err; } else { // try again without hw rotation bool hwRotationUsed = false; @@ -426,25 +438,27 @@ bool DrmGpu::testPendingConfiguration() hwRotationUsed |= (pipeline->bufferOrientation() != DrmPlane::Transformations(DrmPlane::Transformation::Rotate0)); pipeline->setBufferOrientation(DrmPlane::Transformation::Rotate0); } - return hwRotationUsed ? checkCrtcAssignment(connectors, crtcs) : false; + if (hwRotationUsed) { + err = checkCrtcAssignment(connectors, crtcs); + } + return err; } } -bool DrmGpu::testPipelines() +DrmPipeline::Error DrmGpu::testPipelines() { QVector inactivePipelines; std::copy_if(m_pipelines.constBegin(), m_pipelines.constEnd(), std::back_inserter(inactivePipelines), [](const auto pipeline) { return pipeline->enabled() && !pipeline->active(); }); - const auto unused = unusedObjects(); - bool test = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::Test, unused); - if (!inactivePipelines.isEmpty() && test) { + DrmPipeline::Error test = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::Test, 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, unused); + test = DrmPipeline::commitPipelines(m_pipelines, DrmPipeline::CommitMode::Test, unusedObjects()); for (const auto pipeline : qAsConst(inactivePipelines)) { pipeline->setActive(false); } @@ -750,16 +764,23 @@ bool DrmGpu::maybeModeset() } // make sure there's no pending pageflips waitIdle(); - const bool ok = DrmPipeline::commitPipelines(pipelines, DrmPipeline::CommitMode::CommitModeset, unusedObjects()); + const DrmPipeline::Error err = DrmPipeline::commitPipelines(pipelines, DrmPipeline::CommitMode::CommitModeset, unusedObjects()); for (DrmPipeline *pipeline : qAsConst(pipelines)) { if (pipeline->modesetPresentPending()) { pipeline->resetModesetPresentPending(); - if (!ok) { + if (err != DrmPipeline::Error::None) { pipeline->output()->frameFailed(); } } } - return ok; + if (err == DrmPipeline::Error::None) { + return true; + } else { + if (err != DrmPipeline::Error::FramePending) { + QTimer::singleShot(0, m_platform, &DrmBackend::updateOutputs); + } + return false; + } } QVector DrmGpu::unusedObjects() const diff --git a/src/backends/drm/drm_gpu.h b/src/backends/drm/drm_gpu.h index f683635cd8..8f8b670ab0 100644 --- a/src/backends/drm/drm_gpu.h +++ b/src/backends/drm/drm_gpu.h @@ -10,6 +10,7 @@ #ifndef DRM_GPU_H #define DRM_GPU_H +#include "drm_pipeline.h" #include "drm_virtual_output.h" #include @@ -39,7 +40,6 @@ class DrmConnector; class DrmPlane; class DrmBackend; class EglGbmBackend; -class DrmPipeline; class DrmAbstractOutput; class DrmLeaseOutput; class DrmRenderBackend; @@ -78,7 +78,7 @@ public: DrmVirtualOutput *createVirtualOutput(const QString &name, const QSize &size, double scale, DrmVirtualOutput::Type type); void removeVirtualOutput(DrmVirtualOutput *output); - bool testPendingConfiguration(); + DrmPipeline::Error testPendingConfiguration(); bool needsModeset() const; bool maybeModeset(); @@ -100,8 +100,8 @@ private: void initDrmResources(); void waitIdle(); - bool checkCrtcAssignment(QVector connectors, const QVector &crtcs); - bool testPipelines(); + DrmPipeline::Error checkCrtcAssignment(QVector connectors, const QVector &crtcs); + DrmPipeline::Error testPipelines(); QVector unusedObjects() const; void handleLeaseRequest(KWaylandServer::DrmLeaseV1Interface *leaseRequest); diff --git a/src/backends/drm/drm_output.cpp b/src/backends/drm/drm_output.cpp index 8c87b572f6..e24c890815 100644 --- a/src/backends/drm/drm_output.cpp +++ b/src/backends/drm/drm_output.cpp @@ -231,7 +231,7 @@ bool DrmOutput::setDrmDpmsMode(DpmsMode mode) return true; } m_pipeline->setActive(active); - if (DrmPipeline::commitPipelines({m_pipeline}, active ? DrmPipeline::CommitMode::Test : DrmPipeline::CommitMode::CommitModeset)) { + if (DrmPipeline::commitPipelines({m_pipeline}, active ? DrmPipeline::CommitMode::Test : DrmPipeline::CommitMode::CommitModeset) == DrmPipeline::Error::None) { m_pipeline->applyPendingChanges(); setDpmsModeInternal(mode); if (active) { @@ -289,7 +289,7 @@ void DrmOutput::updateModes() if (currentMode != m_pipeline->mode()) { // DrmConnector::findCurrentMode might fail m_pipeline->setMode(currentMode ? currentMode : m_pipeline->connector()->modes().constFirst()); - if (m_gpu->testPendingConfiguration()) { + if (m_gpu->testPendingConfiguration() == DrmPipeline::Error::None) { m_pipeline->applyPendingChanges(); m_renderLoop->setRefreshRate(m_pipeline->mode()->refreshRate()); } else { @@ -312,17 +312,27 @@ bool DrmOutput::present() RenderLoopPrivate *renderLoopPrivate = RenderLoopPrivate::get(m_renderLoop.get()); if (m_pipeline->syncMode() != renderLoopPrivate->presentMode) { m_pipeline->setSyncMode(renderLoopPrivate->presentMode); - if (DrmPipeline::commitPipelines({m_pipeline}, DrmPipeline::CommitMode::Test)) { + if (DrmPipeline::commitPipelines({m_pipeline}, DrmPipeline::CommitMode::Test) == DrmPipeline::Error::None) { m_pipeline->applyPendingChanges(); } else { m_pipeline->revertPendingChanges(); } } - bool modeset = gpu()->needsModeset(); - if (modeset ? m_pipeline->maybeModeset() : m_pipeline->present()) { + const bool needsModeset = gpu()->needsModeset(); + bool success; + if (needsModeset) { + success = m_pipeline->maybeModeset(); + } else { + DrmPipeline::Error err = m_pipeline->present(); + success = err == DrmPipeline::Error::None; + if (err == DrmPipeline::Error::InvalidArguments) { + QTimer::singleShot(0, m_gpu->platform(), &DrmBackend::updateOutputs); + } + } + if (success) { Q_EMIT outputChange(m_pipeline->primaryLayer()->currentDamage()); return true; - } else if (!modeset) { + } else if (!needsModeset) { qCWarning(KWIN_DRM) << "Presentation failed!" << strerror(errno); frameFailed(); } @@ -413,7 +423,7 @@ DrmOutputLayer *DrmOutput::outputLayer() const void DrmOutput::setColorTransformation(const std::shared_ptr &transformation) { m_pipeline->setColorTransformation(transformation); - if (DrmPipeline::commitPipelines({m_pipeline}, DrmPipeline::CommitMode::Test)) { + if (DrmPipeline::commitPipelines({m_pipeline}, DrmPipeline::CommitMode::Test) == DrmPipeline::Error::None) { m_pipeline->applyPendingChanges(); m_renderLoop->scheduleRepaint(); } else { diff --git a/src/backends/drm/drm_pipeline.cpp b/src/backends/drm/drm_pipeline.cpp index 2a44dc870c..be3ec02199 100644 --- a/src/backends/drm/drm_pipeline.cpp +++ b/src/backends/drm/drm_pipeline.cpp @@ -53,15 +53,15 @@ bool DrmPipeline::testScanout() return false; } if (gpu()->atomicModeSetting()) { - return commitPipelines({this}, CommitMode::Test); + return commitPipelines({this}, CommitMode::Test) == Error::None; } else { // no other way to test than to do it. // As we only have a maximum of one test per scanout cycle, this is fine - return presentLegacy(); + return presentLegacy() == Error::None; } } -bool DrmPipeline::present() +DrmPipeline::Error DrmPipeline::present() { Q_ASSERT(m_pending.crtc); if (gpu()->atomicModeSetting()) { @@ -69,13 +69,10 @@ bool DrmPipeline::present() } else { if (m_pending.layer->hasDirectScanoutBuffer()) { // already presented - return true; - } - if (!presentLegacy()) { - return false; + return Error::None; } + return presentLegacy(); } - return true; } bool DrmPipeline::maybeModeset() @@ -84,7 +81,7 @@ bool DrmPipeline::maybeModeset() return gpu()->maybeModeset(); } -bool DrmPipeline::commitPipelines(const QVector &pipelines, CommitMode mode, const QVector &unusedObjects) +DrmPipeline::Error DrmPipeline::commitPipelines(const QVector &pipelines, CommitMode mode, const QVector &unusedObjects) { Q_ASSERT(!pipelines.isEmpty()); if (pipelines[0]->gpu()->atomicModeSetting()) { @@ -94,12 +91,12 @@ bool DrmPipeline::commitPipelines(const QVector &pipelines, Commi } } -bool DrmPipeline::commitPipelinesAtomic(const QVector &pipelines, CommitMode mode, const QVector &unusedObjects) +DrmPipeline::Error DrmPipeline::commitPipelinesAtomic(const QVector &pipelines, CommitMode mode, const QVector &unusedObjects) { drmModeAtomicReq *req = drmModeAtomicAlloc(); if (!req) { qCCritical(KWIN_DRM) << "Failed to allocate drmModeAtomicReq!" << strerror(errno); - return false; + return Error::OutofMemory; } uint32_t flags = 0; const auto &failed = [pipelines, req, &flags, unusedObjects]() { @@ -113,16 +110,17 @@ bool DrmPipeline::commitPipelinesAtomic(const QVector &pipelines, printProps(obj, PrintMode::OnlyChanged); obj->rollbackPending(); } - return false; }; for (const auto &pipeline : pipelines) { if (pipeline->activePending() && !pipeline->m_pending.layer->checkTestBuffer()) { qCWarning(KWIN_DRM) << "Checking test buffer failed for" << mode; - return failed(); + failed(); + return Error::TestBufferFailed; } - if (!pipeline->populateAtomicValues(req, flags)) { + if (Error err = pipeline->populateAtomicValues(req, flags); err != Error::None) { qCWarning(KWIN_DRM) << "Populating atomic values failed for" << mode; - return failed(); + failed(); + return err; } } for (const auto &unused : unusedObjects) { @@ -132,7 +130,8 @@ bool DrmPipeline::commitPipelinesAtomic(const QVector &pipelines, } if (!unused->atomicPopulate(req)) { qCWarning(KWIN_DRM) << "Populating atomic values failed for unused resource" << unused; - return failed(); + failed(); + return errnoToError(); } } bool modeset = flags & DRM_MODE_ATOMIC_ALLOW_MODESET; @@ -148,11 +147,13 @@ bool DrmPipeline::commitPipelinesAtomic(const QVector &pipelines, } if (drmModeAtomicCommit(pipelines[0]->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); - return failed(); + failed(); + return errnoToError(); } if (mode != CommitMode::Test && drmModeAtomicCommit(pipelines[0]->gpu()->fd(), req, flags, nullptr) != 0) { qCCritical(KWIN_DRM) << "Atomic commit failed! This should never happen!" << strerror(errno); - return failed(); + failed(); + return errnoToError(); } for (const auto &pipeline : pipelines) { pipeline->atomicCommitSuccessful(mode); @@ -164,10 +165,10 @@ bool DrmPipeline::commitPipelinesAtomic(const QVector &pipelines, } } drmModeAtomicFree(req); - return true; + return Error::None; } -bool DrmPipeline::populateAtomicValues(drmModeAtomicReq *req, uint32_t &flags) +DrmPipeline::Error DrmPipeline::populateAtomicValues(drmModeAtomicReq *req, uint32_t &flags) { if (needsModeset()) { prepareAtomicModeset(); @@ -193,20 +194,20 @@ bool DrmPipeline::populateAtomicValues(drmModeAtomicReq *req, uint32_t &flags) } } if (!m_connector->atomicPopulate(req)) { - return false; + return errnoToError(); } if (m_pending.crtc) { if (!m_pending.crtc->atomicPopulate(req)) { - return false; + return errnoToError(); } if (!m_pending.crtc->primaryPlane()->atomicPopulate(req)) { - return false; + return errnoToError(); } if (m_pending.crtc->cursorPlane() && !m_pending.crtc->cursorPlane()->atomicPopulate(req)) { - return false; + return errnoToError(); } } - return true; + return Error::None; } void DrmPipeline::prepareAtomicModeset() @@ -262,6 +263,22 @@ uint32_t DrmPipeline::calculateUnderscan() return hborder; } +DrmPipeline::Error DrmPipeline::errnoToError() +{ + switch (errno) { + case EINVAL: + return Error::InvalidArguments; + case EBUSY: + return Error::FramePending; + case ENOMEM: + return Error::OutofMemory; + case EACCES: + return Error::NoPermission; + default: + return Error::Unknown; + } +} + void DrmPipeline::atomicCommitFailed() { m_connector->rollbackPending(); @@ -311,7 +328,7 @@ bool DrmPipeline::setCursor(const QPoint &hotspot) m_pending.cursorHotspot = hotspot; // explicitly check for the cursor plane and not for AMS, as we might not always have one if (m_pending.crtc->cursorPlane()) { - result = commitPipelines({this}, CommitMode::Test); + result = commitPipelines({this}, CommitMode::Test) == Error::None; if (result && m_output) { m_output->renderLoop()->scheduleRepaint(); } @@ -331,7 +348,7 @@ bool DrmPipeline::moveCursor() bool result; // explicitly check for the cursor plane and not for AMS, as we might not always have one if (m_pending.crtc->cursorPlane()) { - result = commitPipelines({this}, CommitMode::Test); + result = commitPipelines({this}, CommitMode::Test) == Error::None; } else { result = moveCursorLegacy(); } diff --git a/src/backends/drm/drm_pipeline.h b/src/backends/drm/drm_pipeline.h index 055b482c64..3ca5e5397d 100644 --- a/src/backends/drm/drm_pipeline.h +++ b/src/backends/drm/drm_pipeline.h @@ -53,11 +53,22 @@ public: DrmPipeline(DrmConnector *conn); ~DrmPipeline(); + enum class Error { + None, + OutofMemory, + InvalidArguments, + NoPermission, + FramePending, + TestBufferFailed, + Unknown, + }; + Q_ENUM(Error); + /** * tests the pending commit first and commits it if the test passes * if the test fails, there is a guarantee for no lasting changes */ - bool present(); + Error present(); bool testScanout(); bool maybeModeset(); @@ -118,28 +129,29 @@ public: Commit, CommitModeset }; - Q_ENUM(CommitMode) - static bool commitPipelines(const QVector &pipelines, CommitMode mode, const QVector &unusedObjects = {}); + Q_ENUM(CommitMode); + static Error commitPipelines(const QVector &pipelines, CommitMode mode, const QVector &unusedObjects = {}); private: bool activePending() const; bool isBufferForDirectScanout() const; uint32_t calculateUnderscan(); + static Error errnoToError(); // legacy only - bool presentLegacy(); - bool legacyModeset(); - bool applyPendingChangesLegacy(); + Error presentLegacy(); + Error legacyModeset(); + Error applyPendingChangesLegacy(); bool setCursorLegacy(); bool moveCursorLegacy(); - static bool commitPipelinesLegacy(const QVector &pipelines, CommitMode mode); + static Error commitPipelinesLegacy(const QVector &pipelines, CommitMode mode); // atomic modesetting only - bool populateAtomicValues(drmModeAtomicReq *req, uint32_t &flags); + Error populateAtomicValues(drmModeAtomicReq *req, uint32_t &flags); void atomicCommitFailed(); void atomicCommitSuccessful(CommitMode mode); void prepareAtomicModeset(); - static bool commitPipelinesAtomic(const QVector &pipelines, CommitMode mode, const QVector &unusedObjects); + static Error commitPipelinesAtomic(const QVector &pipelines, CommitMode mode, const QVector &unusedObjects); // logging helpers enum class PrintMode { diff --git a/src/backends/drm/drm_pipeline_legacy.cpp b/src/backends/drm/drm_pipeline_legacy.cpp index f44fc32e6d..3e4f54e9cf 100644 --- a/src/backends/drm/drm_pipeline_legacy.cpp +++ b/src/backends/drm/drm_pipeline_legacy.cpp @@ -21,31 +21,34 @@ namespace KWin { -bool DrmPipeline::presentLegacy() +DrmPipeline::Error DrmPipeline::presentLegacy() { - if (!m_pending.crtc->current() && !legacyModeset()) { - return false; + if (!m_pending.crtc->current()) { + Error err = legacyModeset(); + if (err != Error::None) { + return err; + } } const auto buffer = m_pending.layer->currentBuffer(); if (drmModePageFlip(gpu()->fd(), m_pending.crtc->id(), buffer->framebufferId(), DRM_MODE_PAGE_FLIP_EVENT, nullptr) != 0) { qCWarning(KWIN_DRM) << "Page flip failed:" << strerror(errno); - return false; + return errnoToError(); } m_pageflipPending = true; m_pending.crtc->setNext(buffer); - return true; + return Error::None; } -bool DrmPipeline::legacyModeset() +DrmPipeline::Error DrmPipeline::legacyModeset() { uint32_t connId = m_connector->id(); if (!m_pending.layer->checkTestBuffer()) { - return false; + return Error::TestBufferFailed; } const auto buffer = m_pending.layer->currentBuffer(); if (drmModeSetCrtc(gpu()->fd(), m_pending.crtc->id(), buffer->framebufferId(), 0, 0, &connId, 1, m_pending.mode->nativeMode()) != 0) { qCWarning(KWIN_DRM) << "Modeset failed!" << strerror(errno); - return false; + return errnoToError(); } // make sure the buffer gets kept alive, or the modeset gets reverted by the kernel if (m_pending.crtc->current()) { @@ -53,25 +56,24 @@ bool DrmPipeline::legacyModeset() } else { m_pending.crtc->setCurrent(buffer); } - return true; + return Error::None; } -bool DrmPipeline::commitPipelinesLegacy(const QVector &pipelines, CommitMode mode) +DrmPipeline::Error DrmPipeline::commitPipelinesLegacy(const QVector &pipelines, CommitMode mode) { - bool failure = false; + Error err = Error::None; for (const auto &pipeline : pipelines) { - if (!pipeline->applyPendingChangesLegacy()) { - failure = true; + err = pipeline->applyPendingChangesLegacy(); + if (err != Error::None) { break; } } - if (failure) { + if (err != Error::None) { // at least try to revert the config for (const auto &pipeline : pipelines) { pipeline->revertPendingChanges(); pipeline->applyPendingChangesLegacy(); } - return false; } else { for (const auto &pipeline : pipelines) { pipeline->applyPendingChanges(); @@ -80,11 +82,11 @@ bool DrmPipeline::commitPipelinesLegacy(const QVector &pipelines, pipeline->pageFlipped(std::chrono::steady_clock::now().time_since_epoch()); } } - return true; } + return err; } -bool DrmPipeline::applyPendingChangesLegacy() +DrmPipeline::Error DrmPipeline::applyPendingChangesLegacy() { if (!m_pending.active && m_pending.crtc) { drmModeSetCursor(gpu()->fd(), m_pending.crtc->id(), 0, 0, 0); @@ -93,7 +95,7 @@ bool DrmPipeline::applyPendingChangesLegacy() auto vrr = m_pending.crtc->getProp(DrmCrtc::PropertyIndex::VrrEnabled); if (vrr && !vrr->setPropertyLegacy(m_pending.syncMode == RenderLoopPrivate::SyncMode::Adaptive)) { qCWarning(KWIN_DRM) << "Setting vrr failed!" << strerror(errno); - return false; + return errnoToError(); } if (const auto &rgbRange = m_connector->getProp(DrmConnector::PropertyIndex::Broadcast_RGB)) { rgbRange->setEnumLegacy(m_pending.rgbRange); @@ -106,21 +108,24 @@ bool DrmPipeline::applyPendingChangesLegacy() m_connector->getProp(DrmConnector::PropertyIndex::Underscan_vborder)->setPropertyLegacy(m_pending.overscan); m_connector->getProp(DrmConnector::PropertyIndex::Underscan_hborder)->setPropertyLegacy(hborder); } - if (needsModeset() && !legacyModeset()) { - return false; + if (needsModeset()) { + Error err = legacyModeset(); + if (err != Error::None) { + return err; + } } if (m_pending.gamma && drmModeCrtcSetGamma(gpu()->fd(), m_pending.crtc->id(), m_pending.gamma->lut().size(), m_pending.gamma->lut().red(), m_pending.gamma->lut().green(), m_pending.gamma->lut().blue()) != 0) { qCWarning(KWIN_DRM) << "Setting gamma failed!" << strerror(errno); - return false; + return errnoToError(); } setCursorLegacy(); moveCursorLegacy(); } if (!m_connector->getProp(DrmConnector::PropertyIndex::Dpms)->setPropertyLegacy(activePending() ? DRM_MODE_DPMS_ON : DRM_MODE_DPMS_OFF)) { qCWarning(KWIN_DRM) << "Setting legacy dpms failed!" << strerror(errno); - return false; + return errnoToError(); } - return true; + return Error::None; } bool DrmPipeline::setCursorLegacy()