From 97c1d335e5f9b36574f7778a97e9ad4c616f9bf2 Mon Sep 17 00:00:00 2001 From: Xaver Hugl Date: Sun, 26 May 2024 13:21:03 +0200 Subject: [PATCH] backends/drm: delay atomic commits until the time they're meant for Otherwise, if render time prediction and actual render time are very different, a lot of frames get dropped and in some cases more frames than is useful are rendered, creating stutter instead of only unnecessary latency BUG: 487605 --- src/backends/drm/drm_commit.cpp | 25 ++++++++++++++++ src/backends/drm/drm_commit.h | 4 +++ src/backends/drm/drm_commit_thread.cpp | 40 ++++++++++++++------------ src/backends/drm/drm_commit_thread.h | 2 +- src/core/renderbackend.cpp | 6 ++++ src/core/renderbackend.h | 3 ++ 6 files changed, 61 insertions(+), 19 deletions(-) diff --git a/src/backends/drm/drm_commit.cpp b/src/backends/drm/drm_commit.cpp index d67a727dd6..963e4f9fed 100644 --- a/src/backends/drm/drm_commit.cpp +++ b/src/backends/drm/drm_commit.cpp @@ -19,6 +19,8 @@ #include #include +using namespace std::chrono_literals; + namespace KWin { @@ -65,6 +67,13 @@ void DrmAtomicCommit::addBuffer(DrmPlane *plane, const std::shared_ptrinFenceFd, buffer ? buffer->syncFd().get() : -1); } m_planes.emplace(plane); + if (frame) { + if (m_targetPageflipTime) { + m_targetPageflipTime = std::min(*m_targetPageflipTime, frame->targetPageflipTime()); + } else { + m_targetPageflipTime = frame->targetPageflipTime(); + } + } } void DrmAtomicCommit::setVrr(DrmCrtc *crtc, bool vrr) @@ -198,6 +207,11 @@ void DrmAtomicCommit::merge(DrmAtomicCommit *onTop) m_vrr = onTop->m_vrr; } m_cursorOnly &= onTop->isCursorOnly(); + if (!m_targetPageflipTime) { + m_targetPageflipTime = onTop->m_targetPageflipTime; + } else if (onTop->m_targetPageflipTime) { + *m_targetPageflipTime = std::min(*m_targetPageflipTime, *onTop->m_targetPageflipTime); + } } void DrmAtomicCommit::setCursorOnly(bool cursor) @@ -210,6 +224,17 @@ bool DrmAtomicCommit::isCursorOnly() const return m_cursorOnly; } +std::optional DrmAtomicCommit::targetPageflipTime() const +{ + return m_targetPageflipTime; +} + +bool DrmAtomicCommit::isReadyFor(std::chrono::steady_clock::time_point pageflipTarget) const +{ + static constexpr auto s_pageflipSlop = 500us; + return (!m_targetPageflipTime || pageflipTarget + s_pageflipSlop >= *m_targetPageflipTime) && areBuffersReadable(); +} + DrmLegacyCommit::DrmLegacyCommit(DrmPipeline *pipeline, const std::shared_ptr &buffer, const std::shared_ptr &frame) : DrmCommit(pipeline->gpu()) , m_pipeline(pipeline) diff --git a/src/backends/drm/drm_commit.h b/src/backends/drm/drm_commit.h index 8373ae032c..8469f5f789 100644 --- a/src/backends/drm/drm_commit.h +++ b/src/backends/drm/drm_commit.h @@ -82,10 +82,14 @@ public: void setCursorOnly(bool cursor); bool isCursorOnly() const; + std::optional targetPageflipTime() const; + bool isReadyFor(std::chrono::steady_clock::time_point pageflipTarget) const; + private: bool doCommit(uint32_t flags); const QList m_pipelines; + std::optional m_targetPageflipTime; std::unordered_map> m_blobs; std::unordered_map> m_buffers; std::unordered_map> m_frames; diff --git a/src/backends/drm/drm_commit_thread.cpp b/src/backends/drm/drm_commit_thread.cpp index 6e13852a7e..b765dd7284 100644 --- a/src/backends/drm/drm_commit_thread.cpp +++ b/src/backends/drm/drm_commit_thread.cpp @@ -60,8 +60,8 @@ DrmCommitThread::DrmCommitThread(DrmGpu *gpu, const QString &name) std::this_thread::sleep_until(m_targetPageflipTime - m_safetyMargin); lock.lock(); } - optimizeCommits(); - if (!m_commits.front()->areBuffersReadable()) { + optimizeCommits(m_targetPageflipTime); + if (!m_commits.front()->isReadyFor(m_targetPageflipTime)) { // no commit is ready yet, reschedule if (m_vrr) { m_targetPageflipTime += 50us; @@ -86,7 +86,7 @@ DrmCommitThread::DrmCommitThread(DrmGpu *gpu, const QString &name) bool timeout = true; while (std::chrono::steady_clock::now() < cursorTarget && timeout && m_commits.front()->isCursorOnly()) { timeout = m_commitPending.wait_for(lock, 50us) == std::cv_status::timeout; - optimizeCommits(); + optimizeCommits(cursorTarget); } if (!timeout) { // some new commit was added, process that @@ -144,20 +144,20 @@ static std::unique_ptr mergeCommits(std::spanareBuffersReadable()) { - const auto firstNotReadable = std::find_if(m_commits.begin() + 1, m_commits.end(), [](const auto &commit) { - return !commit->areBuffersReadable(); + const auto firstNotReady = std::find_if(m_commits.begin() + 1, m_commits.end(), [pageflipTarget](const auto &commit) { + return !commit->isReadyFor(pageflipTarget); }); - if (firstNotReadable != m_commits.begin() + 1) { - auto merged = mergeCommits(std::span(m_commits.begin(), firstNotReadable)); - std::move(m_commits.begin(), firstNotReadable, std::back_inserter(m_commitsToDelete)); - m_commits.erase(m_commits.begin() + 1, firstNotReadable); + if (firstNotReady != m_commits.begin() + 1) { + auto merged = mergeCommits(std::span(m_commits.begin(), firstNotReady)); + std::move(m_commits.begin(), firstNotReady, std::back_inserter(m_commitsToDelete)); + m_commits.erase(m_commits.begin() + 1, firstNotReady); m_commits.front() = std::move(merged); } } @@ -165,24 +165,24 @@ void DrmCommitThread::optimizeCommits() for (auto it = m_commits.begin(); it != m_commits.end();) { const auto startIt = it; auto &startCommit = *startIt; - const auto firstNotSamePlaneReadable = std::find_if(startIt + 1, m_commits.end(), [&startCommit](const auto &commit) { - return startCommit->modifiedPlanes() != commit->modifiedPlanes() || !commit->areBuffersReadable(); + const auto firstNotSamePlaneNotReady = std::find_if(startIt + 1, m_commits.end(), [&startCommit, pageflipTarget](const auto &commit) { + return startCommit->modifiedPlanes() != commit->modifiedPlanes() || !commit->isReadyFor(pageflipTarget); }); - if (firstNotSamePlaneReadable == startIt + 1) { + if (firstNotSamePlaneNotReady == startIt + 1) { it++; continue; } - auto merged = mergeCommits(std::span(startIt, firstNotSamePlaneReadable)); - std::move(startIt, firstNotSamePlaneReadable, std::back_inserter(m_commitsToDelete)); + auto merged = mergeCommits(std::span(startIt, firstNotSamePlaneNotReady)); + std::move(startIt, firstNotSamePlaneNotReady, std::back_inserter(m_commitsToDelete)); startCommit = std::move(merged); - it = m_commits.erase(startIt + 1, firstNotSamePlaneReadable); + it = m_commits.erase(startIt + 1, firstNotSamePlaneNotReady); } if (m_commits.size() == 1) { // already done return; } std::unique_ptr front; - if (m_commits.front()->areBuffersReadable()) { + if (m_commits.front()->isReadyFor(pageflipTarget)) { // can't just move the commit, or merging might drop the last reference // to an OutputFrame, which should only happen in the main thread front = std::make_unique(*m_commits.front()); @@ -192,6 +192,10 @@ void DrmCommitThread::optimizeCommits() // try to move commits that are ready to the front for (auto it = m_commits.begin() + 1; it != m_commits.end();) { auto &commit = *it; + if (!commit->isReadyFor(pageflipTarget)) { + it++; + continue; + } // commits that target the same plane(s) need to stay in the same order const auto &planes = commit->modifiedPlanes(); const bool skipping = std::any_of(m_commits.begin(), it, [&planes](const auto &other) { @@ -199,7 +203,7 @@ void DrmCommitThread::optimizeCommits() return other->modifiedPlanes().contains(plane); }); }); - if (skipping || !commit->areBuffersReadable()) { + if (skipping) { it++; continue; } diff --git a/src/backends/drm/drm_commit_thread.h b/src/backends/drm/drm_commit_thread.h index 8fc684708c..159628f068 100644 --- a/src/backends/drm/drm_commit_thread.h +++ b/src/backends/drm/drm_commit_thread.h @@ -46,7 +46,7 @@ public: private: void clearDroppedCommits(); TimePoint estimateNextVblank(TimePoint now) const; - void optimizeCommits(); + void optimizeCommits(TimePoint pageflipTarget); void submit(); std::unique_ptr m_committed; diff --git a/src/core/renderbackend.cpp b/src/core/renderbackend.cpp index 1809a3ee6c..5c5f60b1e0 100644 --- a/src/core/renderbackend.cpp +++ b/src/core/renderbackend.cpp @@ -47,6 +47,7 @@ std::optional CpuRenderTimeQuery::query() OutputFrame::OutputFrame(RenderLoop *loop, std::chrono::nanoseconds refreshDuration) : m_loop(loop) , m_refreshDuration(refreshDuration) + , m_targetPageflipTime(loop->nextPresentationTimestamp()) { } @@ -128,6 +129,11 @@ void OutputFrame::addRenderTimeQuery(std::unique_ptr &&query) m_renderTimeQueries.push_back(std::move(query)); } +std::chrono::steady_clock::time_point OutputFrame::targetPageflipTime() const +{ + return m_targetPageflipTime; +} + RenderBackend::RenderBackend(QObject *parent) : QObject(parent) { diff --git a/src/core/renderbackend.h b/src/core/renderbackend.h index 65da4ffee8..f7dbf9b7c7 100644 --- a/src/core/renderbackend.h +++ b/src/core/renderbackend.h @@ -92,11 +92,14 @@ public: QRegion damage() const; void addRenderTimeQuery(std::unique_ptr &&query); + std::chrono::steady_clock::time_point targetPageflipTime() const; + private: std::optional queryRenderTime() const; RenderLoop *const m_loop; const std::chrono::nanoseconds m_refreshDuration; + const std::chrono::steady_clock::time_point m_targetPageflipTime; std::vector> m_feedbacks; std::optional m_contentType; PresentationMode m_presentationMode = PresentationMode::VSync;