diff --git a/flutter-pi b/flutter-pi new file mode 100644 index 000000000..cd4e34650 Binary files /dev/null and b/flutter-pi differ diff --git a/src/modesetting.c b/src/modesetting.c index 8ad4a24de..4d3dc9cf5 100644 --- a/src/modesetting.c +++ b/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; } diff --git a/src/modesetting.h b/src/modesetting.h index 4afda0bd0..ca49dea38 100644 --- a/src/modesetting.h +++ b/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); diff --git a/src/window.c b/src/window.c index 1dec153af..bd9185b08 100644 --- a/src/window.c +++ b/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"); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 687746d24..1dfb1988d 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -20,4 +20,16 @@ target_link_libraries( Unity ) -add_test(flutterpi_test flutterpi_test) \ No newline at end of file +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) diff --git a/test/kms_present_test.c b/test/kms_present_test.c new file mode 100644 index 000000000..ef6504867 --- /dev/null +++ b/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; +}