Remove the synchronous DRM atomic page-flip wait from Flutter’s raster presentation path

src/modesetting.c

@@ -119,6 +119,8 @@ struct drmdev {
         void_callback_t destroy_callback;
 
         struct kms_req *last_flipped;
+        bool flip_pending;
+        struct kms_req *queued;
     } per_crtc_state[32];
 
     int master_fd;
@@ -1331,6 +1333,10 @@ void drmdev_unmap_dumb_buffer(struct drmdev *drmdev, void *map, size_t size) {
     }
 }
 
+static int kms_req_commit_locked(
+    struct kms_req *req, bool blocking, kms_scanout_cb_t scanout_cb, void *userdata, void_callback_t destroy_cb
+);
+
 static void
 drmdev_on_page_flip_locked(int fd, unsigned int sequence, unsigned int tv_sec, unsigned int tv_usec, unsigned int crtc_id, void *userdata) {
     struct kms_req_builder *builder;
@@ -1375,6 +1381,19 @@ drmdev_on_page_flip_locked(int fd, unsigned int sequence, unsigned int tv_sec, u
 
     kms_req_swap_ptrs(last_flipped, req);
     kms_req_unref(req);
+
+    drmdev->per_crtc_state[crtc->index].flip_pending = false;
+
+    // Release the previous scanout before submitting the newest queued frame.
+    req = drmdev->per_crtc_state[crtc->index].queued;
+    drmdev->per_crtc_state[crtc->index].queued = NULL;
+    if (req != NULL) {
+        int ok = kms_req_commit_locked(req, false, NULL, NULL, NULL);
+        if (ok != 0) {
+            LOG_ERROR("Could not commit queued frame: %s\n", strerror(ok));
+        }
+        kms_req_unref(req);
+    }
 }
 
 static int drmdev_on_modesetting_fd_ready_locked(struct drmdev *drmdev) {
@@ -1867,6 +1886,21 @@ void drmdev_suspend(struct drmdev *drmdev) {
         return;
     }
 
+    // Do not leave queued buffers or unread page flips behind when closing
+    // the event fd. Waiting here is only needed when relinquishing the display.
+    for (size_t i = 0; i < drmdev->n_crtcs; i++) {
+        kms_req_swap_ptrs(&drmdev->per_crtc_state[i].queued, NULL);
+    }
+    for (size_t i = 0; i < drmdev->n_crtcs; i++) {
+        while (drmdev->per_crtc_state[i].flip_pending) {
+            if (drmdev_on_modesetting_fd_ready_locked(drmdev) != 0) {
+                LOG_ERROR("Could not drain page flips before suspending.\n");
+                drmdev_unlock(drmdev);
+                return;
+            }
+        }
+    }
+
     drmdev->interface.close(drmdev->master_fd, drmdev->master_fd_metadata, drmdev->userdata);
     drmdev->master_fd = -1;
     drmdev->master_fd_metadata = NULL;
@@ -2755,7 +2789,7 @@ static bool drm_plane_is_active(struct drm_plane *plane) {
 }
 
 static int
-kms_req_commit_common(struct kms_req *req, bool blocking, kms_scanout_cb_t scanout_cb, void *userdata, void_callback_t destroy_cb) {
+kms_req_commit_locked(struct kms_req *req, bool blocking, kms_scanout_cb_t scanout_cb, void *userdata, void_callback_t destroy_cb) {
     struct kms_req_builder *builder;
     struct drm_mode_blob *mode_blob;
     uint32_t flags;
@@ -2770,7 +2804,9 @@ kms_req_commit_common(struct kms_req *req, bool blocking, kms_scanout_cb_t scano
     ASSERT_NOT_NULL(req);
     builder = (struct kms_req_builder *) req;
 
-    drmdev_lock(builder->drmdev);
+    if (builder->drmdev->per_crtc_state[builder->crtc->index].flip_pending) {
+        return EBUSY;
+    }
 
     if (builder->drmdev->master_fd < 0) {
         LOG_ERROR("Commit requested, but drmdev doesn't have a DRM master fd right now.\n");
@@ -3011,6 +3047,7 @@ kms_req_commit_common(struct kms_req *req, bool blocking, kms_scanout_cb_t scano
     // builder->connector->committed_state.encoder_id = 0;
 
     drmdev_set_scanout_callback_locked(builder->drmdev, builder->crtc->id, scanout_cb, userdata, destroy_cb);
+    builder->drmdev->per_crtc_state[builder->crtc->index].flip_pending = !internally_blocking;
 
     if (internally_blocking) {
         uint64_t sequence = 0;
@@ -3041,8 +3078,6 @@ kms_req_commit_common(struct kms_req *req, bool blocking, kms_scanout_cb_t scano
         }
     }
 
-    drmdev_unlock(builder->drmdev);
-
     return 0;
 
 fail_unset_scanout_callback:
@@ -3059,8 +3094,41 @@ kms_req_commit_common(struct kms_req *req, bool blocking, kms_scanout_cb_t scano
         drm_mode_blob_destroy(mode_blob);
 
 fail_unlock:
-    drmdev_unlock(builder->drmdev);
+    return ok;
+}
 
+static int
+kms_req_commit_common(struct kms_req *req, bool blocking, kms_scanout_cb_t scanout_cb, void *userdata, void_callback_t destroy_cb) {
+    struct drmdev *drmdev;
+    int ok;
+
+    ASSERT_NOT_NULL(req);
+    drmdev = ((struct kms_req_builder *) req)->drmdev;
+    drmdev_lock(drmdev);
+    ok = kms_req_commit_locked(req, blocking, scanout_cb, userdata, destroy_cb);
+    drmdev_unlock(drmdev);
+    return ok;
+}
+
+int kms_req_present(struct kms_req *req) {
+    struct kms_req_builder *builder;
+    struct drmdev *drmdev;
+    int ok;
+
+    ASSERT_NOT_NULL(req);
+    builder = (struct kms_req_builder *) req;
+    drmdev = builder->drmdev;
+    drmdev_lock(drmdev);
+    if (drmdev->master_fd < 0) {
+        ok = EBUSY;
+    } else if (!builder->use_legacy && drmdev->per_crtc_state[builder->crtc->index].flip_pending) {
+        // At most one request waits behind the kernel's outstanding page flip.
+        kms_req_swap_ptrs(&drmdev->per_crtc_state[builder->crtc->index].queued, req);
+        ok = 0;
+    } else {
+        ok = kms_req_commit_locked(req, builder->use_legacy, NULL, NULL, NULL);
+    }
+    drmdev_unlock(drmdev);
     return ok;
 }
 

src/modesetting.h

@@ -924,6 +924,15 @@ int kms_req_commit_blocking(struct kms_req *req, uint64_t *vblank_ns_out);
 
 int kms_req_commit_nonblocking(struct kms_req *req, kms_scanout_cb_t scanout_cb, void *userdata, void_callback_t destroy_cb);
 
+/**
+ * Present without waiting for an atomic page flip. If a flip is outstanding,
+ * retain only the newest queued request and submit it when that flip completes.
+ * Takes an internal reference; the caller must still unref its own request.
+ * Returns submission errors immediately; errors for queued frames are logged.
+ * Legacy modesetting retains blocking presentation.
+ */
+int kms_req_present(struct kms_req *req);
+
 struct drm_connector *__next_connector(const struct drmdev *drmdev, const struct drm_connector *connector);
 
 struct drm_encoder *__next_encoder(const struct drmdev *drmdev, const struct drm_encoder *encoder);

src/window.c

@@ -1109,9 +1109,9 @@ static void on_present_frame(void *userdata) {
 
     frame = userdata;
 
-    TRACER_BEGIN(frame->tracer, "kms_req_commit_nonblocking");
-    ok = kms_req_commit_blocking(frame->req, NULL);
-    TRACER_END(frame->tracer, "kms_req_commit_nonblocking");
+    TRACER_BEGIN(frame->tracer, "kms_req_present");
+    ok = kms_req_present(frame->req);
+    TRACER_END(frame->tracer, "kms_req_present");
 
     if (ok != 0) {
         LOG_ERROR("Could not commit frame request.\n");

test/CMakeLists.txt

@@ -20,4 +20,16 @@ target_link_libraries(
     Unity
 )
 
-add_test(flutterpi_test flutterpi_test)
+add_test(flutterpi_test flutterpi_test)
+
+# This test includes modesetting.c with a mocked DRM submission boundary.
+add_executable(kms_present_test kms_present_test.c)
+target_include_directories(kms_present_test PRIVATE
+    ${CMAKE_SOURCE_DIR}/src
+    ${CMAKE_SOURCE_DIR}/third_party/flutter_embedder_header/include
+    ${CMAKE_BINARY_DIR}
+)
+target_compile_options(kms_present_test PRIVATE -UNDEBUG -ffunction-sections -fdata-sections)
+target_link_options(kms_present_test PRIVATE -Wl,--gc-sections)
+target_link_libraries(kms_present_test PRIVATE PkgConfig::DRM PkgConfig::GBM Threads::Threads m)
+add_test(kms_present_test kms_present_test)

test/kms_present_test.c

@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: MIT
+// Standalone test: compile with -ffunction-sections -fdata-sections,
+// -Wl,--gc-sections, the project include paths, and pkg-config libdrm gbm.
+// Only the DRM submission/event boundary is mocked; no display is accessed.
+#define drmModeAtomicCommit test_atomic_commit
+#define drmAuthMagic test_auth_magic
+#define drmHandleEvent test_handle_event
+#include "../src/modesetting.c"
+
+static void *submitted;
+static unsigned int submissions;
+static int submit_error;
+static unsigned int releases[10];
+static bool draining;
+
+int test_auth_magic(int fd, drm_magic_t magic) {
+    (void) fd;
+    (void) magic;
+    return 0;
+}
+
+int test_atomic_commit(int fd, drmModeAtomicReq *req, uint32_t flags, void *userdata) {
+    (void) fd;
+    (void) req;
+    assert(flags & DRM_MODE_ATOMIC_NONBLOCK);
+    assert(flags & DRM_MODE_PAGE_FLIP_EVENT);
+    assert(submitted == NULL);
+    if (submit_error) {
+        errno = submit_error;
+        return -1;
+    }
+    submitted = userdata;
+    submissions++;
+    return 0;
+}
+
+int test_handle_event(int fd, drmEventContextPtr ctx) {
+    // Asynchronous presentation must never read/wait for an event inline.
+    assert(draining && submitted != NULL);
+    void *req = submitted;
+    submitted = NULL;
+    ctx->page_flip_handler2(fd, 1, 1, 0, 1, req);
+    return 0;
+}
+
+static void close_device(int fd, void *metadata, void *userdata) {
+    (void) fd;
+    (void) metadata;
+    struct drmdev *dev = userdata;
+    assert(!dev->per_crtc_state[0].flip_pending);
+    assert(dev->per_crtc_state[0].queued == NULL);
+}
+
+static void release_layer(void *userdata) {
+    (*(unsigned int *) userdata)++;
+}
+
+static struct kms_req *request(struct drmdev *dev, unsigned int index) {
+    struct kms_req_builder *builder = calloc(1, sizeof *builder);
+    assert(builder != NULL);
+    builder->n_refs = REFCOUNT_INIT_1;
+    builder->drmdev = drmdev_ref(dev);
+    builder->crtc = dev->crtcs;
+    builder->connector = dev->connectors;
+    builder->req = drmModeAtomicAlloc();
+    assert(builder->req != NULL);
+    builder->n_layers = 1;
+    builder->layers[0].plane = dev->planes;
+    builder->layers[0].release_callback = release_layer;
+    builder->layers[0].release_callback_userdata = &releases[index];
+    return (struct kms_req *) builder;
+}
+
+static void present(struct drmdev *dev, unsigned int index, int expected) {
+    struct kms_req *req = request(dev, index);
+    assert(kms_req_present(req) == expected);
+    kms_req_unref(req);
+}
+
+static void flip(struct drmdev *dev) {
+    void *req = submitted;
+    assert(req != NULL);
+    submitted = NULL;
+    drmdev_lock(dev);
+    drmdev_on_page_flip_locked(0, 1, 1, 0, dev->crtcs->id, req);
+    drmdev_unlock(dev);
+}
+
+int main(void) {
+    struct drm_crtc crtc = { .id = 1, .index = 0 };
+    struct drm_connector connector = {0};
+    struct drm_plane plane = {0};
+    struct drmdev dev = {
+        .n_refs = REFCOUNT_INIT_1,
+        .mutex = PTHREAD_MUTEX_INITIALIZER,
+        .master_fd = 1,
+        .n_crtcs = 1,
+        .crtcs = &crtc,
+        .connectors = &connector,
+        .planes = &plane,
+        .interface.close = close_device,
+    };
+    dev.userdata = &dev;
+
+    present(&dev, 0, 0);
+    assert(submissions == 1 && releases[0] == 0);
+    present(&dev, 1, 0);
+    present(&dev, 2, 0);
+    assert(submissions == 1 && releases[1] == 1 && releases[2] == 0);
+    struct kms_req *extra = request(&dev, 9);
+    assert(kms_req_commit_nonblocking(extra, NULL, NULL, NULL) == EBUSY);
+    kms_req_unref(extra);
+    flip(&dev);
+    assert(submissions == 2 && releases[0] == 0);
+    flip(&dev);
+    assert(releases[0] == 1 && releases[2] == 0);
+    assert(!dev.per_crtc_state[0].flip_pending && dev.per_crtc_state[0].queued == NULL);
+
+    // A failed submission must release its buffers and allow the next frame.
+    submit_error = EINVAL;
+    present(&dev, 3, EINVAL);
+    assert(releases[3] == 1 && !dev.per_crtc_state[0].flip_pending);
+    submit_error = 0;
+    present(&dev, 4, 0);
+    present(&dev, 5, 0);
+    submit_error = EBUSY;
+    flip(&dev);
+    assert(releases[2] == 1 && releases[5] == 1);
+    assert(!dev.per_crtc_state[0].flip_pending && dev.per_crtc_state[0].queued == NULL);
+    submit_error = 0;
+    present(&dev, 6, 0);
+    flip(&dev);
+    assert(releases[4] == 1 && releases[6] == 0);
+
+    // Suspending cancels the queued frame and consumes the outstanding flip.
+    present(&dev, 7, 0);
+    present(&dev, 8, 0);
+    draining = true;
+    drmdev_suspend(&dev);
+    draining = false;
+    assert(dev.master_fd == -1 && submitted == NULL);
+    assert(releases[6] == 1 && releases[7] == 0 && releases[8] == 1);
+    present(&dev, 9, EBUSY);
+    assert(releases[9] == 2);
+
+    kms_req_swap_ptrs(&dev.per_crtc_state[0].last_flipped, NULL);
+    assert(releases[7] == 1 && refcount_is_one(&dev.n_refs));
+    pthread_mutex_destroy(&dev.mutex);
+    puts("KMS presentation tests passed");
+    return 0;
+}