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

@@ -19,6 +19,8 @@
 #include <QApplication>
 #include <QThread>
 
+using namespace std::chrono_literals;
+
 namespace KWin
 {
 
@@ -65,6 +67,13 @@ void DrmAtomicCommit::addBuffer(DrmPlane *plane, const std::shared_ptr<DrmFrameb
         addProperty(plane->inFenceFd, 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<std::chrono::steady_clock::time_point> 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<DrmFramebuffer> &buffer, const std::shared_ptr<OutputFrame> &frame)
     : DrmCommit(pipeline->gpu())
     , m_pipeline(pipeline)

src/backends/drm/drm_commit.h

@@ -82,10 +82,14 @@ public:
     void setCursorOnly(bool cursor);
     bool isCursorOnly() const;
 
+    std::optional<std::chrono::steady_clock::time_point> targetPageflipTime() const;
+    bool isReadyFor(std::chrono::steady_clock::time_point pageflipTarget) const;
+
 private:
     bool doCommit(uint32_t flags);
 
     const QList<DrmPipeline *> m_pipelines;
+    std::optional<std::chrono::steady_clock::time_point> m_targetPageflipTime;
     std::unordered_map<const DrmProperty *, std::shared_ptr<DrmBlob>> m_blobs;
     std::unordered_map<DrmPlane *, std::shared_ptr<DrmFramebuffer>> m_buffers;
     std::unordered_map<DrmPlane *, std::shared_ptr<OutputFrame>> m_frames;

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<DrmAtomicCommit> mergeCommits(std::span<const std::unique
     return ret;
 }
 
-void DrmCommitThread::optimizeCommits()
+void DrmCommitThread::optimizeCommits(TimePoint pageflipTarget)
 {
     if (m_commits.size() <= 1) {
         return;
     }
     // merge commits in the front that are already ready (regardless of which planes they modify)
     if (m_commits.front()->areBuffersReadable()) {
-        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<DrmAtomicCommit> 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<DrmAtomicCommit>(*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;
         }

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<DrmCommit> m_committed;

src/core/renderbackend.cpp

@@ -47,6 +47,7 @@ std::optional<RenderTimeSpan> 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<RenderTimeQuery> &&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)
 {

src/core/renderbackend.h

@@ -92,11 +92,14 @@ public:
     QRegion damage() const;
     void addRenderTimeQuery(std::unique_ptr<RenderTimeQuery> &&query);
 
+    std::chrono::steady_clock::time_point targetPageflipTime() const;
+
 private:
     std::optional<std::chrono::nanoseconds> queryRenderTime() const;
 
     RenderLoop *const m_loop;
     const std::chrono::nanoseconds m_refreshDuration;
+    const std::chrono::steady_clock::time_point m_targetPageflipTime;
     std::vector<std::unique_ptr<PresentationFeedback>> m_feedbacks;
     std::optional<ContentType> m_contentType;
     PresentationMode m_presentationMode = PresentationMode::VSync;