From 3f7abae2fc5e06fb3bd3ad6dbaf07bc473d55b5b Mon Sep 17 00:00:00 2001 From: Faith Ekstrand Date: Mon, 3 Mar 2025 16:35:50 -0600 Subject: [PATCH] zink: Don't present to Wayland surfaces asynchronously Wayland EGL has a driver invariant which requires that any `wl_surface` (or wp_linux_drm_syncobj_surface_v1) calls happen inside the client's call to eglSwapBuffers(). Submitting surface messages after eglSwapBuffers() returns causes serialization issues with the Wayland surface protocol and can lead to the compositor booting the app. Fixes: 8ade5588e39d ("zink: add kopper api") Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/12736 Reviewed-by: Mike Blumenkrantz Part-of: (cherry picked from commit b92117d9bb9167f51af8be3210163a042d3b1d44) --- .pick_status.json | 2 +- src/gallium/drivers/zink/zink_kopper.c | 29 ++++++++++++++++---------- src/gallium/drivers/zink/zink_kopper.h | 2 ++ 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 9c74cf5b9ec..2c59972af05 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -204,7 +204,7 @@ "description": "zink: Don't present to Wayland surfaces asynchronously", "nominated": true, "nomination_type": 2, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "8ade5588e39d736bdeab9bdd8ffa7cbfb6a5191e", "notes": null diff --git a/src/gallium/drivers/zink/zink_kopper.c b/src/gallium/drivers/zink/zink_kopper.c index 2c6f3838ab8..ef04ea1628c 100644 --- a/src/gallium/drivers/zink/zink_kopper.c +++ b/src/gallium/drivers/zink/zink_kopper.c @@ -259,9 +259,6 @@ kopper_CreateSwapchain(struct zink_screen *screen, struct kopper_displaytarget * bool has_alpha = cdt->info.has_alpha && (cdt->caps.supportedCompositeAlpha & VK_COMPOSITE_ALPHA_PRE_MULTIPLIED_BIT_KHR); if (cdt->swapchain) { cswap->scci = cdt->swapchain->scci; - /* avoid UAF if async present needs to-be-retired swapchain */ - if (cdt->type == KOPPER_WAYLAND && cdt->swapchain->swapchain) - util_queue_fence_wait(&cdt->swapchain->present_fence); cswap->scci.oldSwapchain = cdt->swapchain->swapchain; } else { cswap->scci.sType = VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR; @@ -331,7 +328,7 @@ kopper_CreateSwapchain(struct zink_screen *screen, struct kopper_displaytarget * error = VKSCR(CreateSwapchainKHR)(screen->dev, &cswap->scci, NULL, &cswap->swapchain); if (error == VK_ERROR_NATIVE_WINDOW_IN_USE_KHR) { - if (util_queue_is_initialized(&screen->flush_queue)) + if (cdt->async) util_queue_finish(&screen->flush_queue); simple_mtx_lock(&screen->queue_lock); VkResult wait_result = VKSCR(QueueWaitIdle)(screen->queue); @@ -503,6 +500,16 @@ zink_kopper_displaytarget_create(struct zink_screen *screen, unsigned tex_usage, } simple_mtx_unlock(&screen->dt_lock); + /* Wayland EGL has a driver invariant which requires that any `wl_surface` + * (or wp_linux_drm_syncobj_surface_v1) calls happen inside the client's + * call to eglSwapBuffers(). Submitting surface messages after + * eglSwapBuffers() returns causes serialization issues with the Wayland + * surface protocol and can lead to the compositor booting the app. This + * means we can't do async submit on Wayland. + */ + cdt->async = util_queue_is_initialized(&screen->flush_queue) && + cdt->type != KOPPER_WAYLAND; + *stride = cdt->stride; return cdt; @@ -549,7 +556,7 @@ kopper_acquire(struct zink_screen *screen, struct zink_resource *res, uint64_t t res->obj->access = 0; res->obj->access_stage = 0; } - if (timeout == UINT64_MAX && util_queue_is_initialized(&screen->flush_queue) && + if (timeout == UINT64_MAX && cdt->async && p_atomic_read_relaxed(&cdt->swapchain->num_acquires) >= cdt->swapchain->max_acquires) { util_queue_fence_wait(&cdt->swapchain->present_fence); /* With a sequence of @@ -877,13 +884,15 @@ zink_kopper_present_queue(struct zink_screen *screen, struct zink_resource *res, cdt->swapchain->images[i].age += 1; } } - if (util_queue_is_initialized(&screen->flush_queue)) { + if (cdt->async) { p_atomic_inc(&cpi->swapchain->async_presents); struct pipe_resource *pres = NULL; pipe_resource_reference(&pres, &res->base.b); util_queue_add_job(&screen->flush_queue, cpi, &cdt->swapchain->present_fence, kopper_present, NULL, 0); } else { + if (screen->threaded_submit) + util_queue_finish(&screen->flush_queue); kopper_present(cpi, screen, -1); } res->obj->indefinite_acquire = false; @@ -961,7 +970,7 @@ zink_kopper_acquire_readback(struct zink_context *ctx, struct zink_resource *res if (res->obj->dt_idx != UINT32_MAX) { if (!zink_kopper_present_readback(ctx, res)) break; - } else if (util_queue_is_initialized(&screen->flush_queue)) { + } else if (cdt->async) { /* AcquireNextImageKHR and QueuePresentKHR both access the swapchain, and * if res->obj->dt_idx == UINT32_MAX then zink_kopper_present_readback is * not called and we don't wait for the cdt->swapchain->present_fence. @@ -998,6 +1007,7 @@ bool zink_kopper_present_readback(struct zink_context *ctx, struct zink_resource *res) { struct zink_screen *screen = zink_screen(ctx->base.screen); + struct kopper_displaytarget *cdt = res->obj->dt; VkSubmitInfo si = {0}; assert(zink_is_swapchain(res)); if (res->obj->last_dt_idx == UINT32_MAX) @@ -1024,10 +1034,8 @@ zink_kopper_present_readback(struct zink_context *ctx, struct zink_resource *res return false; zink_kopper_present_queue(screen, res, 0, NULL); - if (util_queue_is_initialized(&screen->flush_queue)) { - struct kopper_displaytarget *cdt = res->obj->dt; + if (cdt->async) util_queue_fence_wait(&cdt->swapchain->present_fence); - } simple_mtx_lock(&screen->queue_lock); error = VKSCR(QueueWaitIdle)(screen->queue); @@ -1037,7 +1045,6 @@ zink_kopper_present_readback(struct zink_context *ctx, struct zink_resource *res util_dynarray_append(&screen->semaphores, VkSemaphore, acquire); simple_mtx_unlock(&screen->semaphores_lock); - struct kopper_displaytarget *cdt = res->obj->dt; cdt->age_locked = false; return zink_screen_handle_vkresult(screen, error); diff --git a/src/gallium/drivers/zink/zink_kopper.h b/src/gallium/drivers/zink/zink_kopper.h index 8b91f38f69e..e1d18ae7ef7 100644 --- a/src/gallium/drivers/zink/zink_kopper.h +++ b/src/gallium/drivers/zink/zink_kopper.h @@ -90,6 +90,8 @@ struct kopper_displaytarget struct kopper_loader_info info; + bool async; // True if submits should go through zink_screen::flush_queue + VkSurfaceCapabilitiesKHR caps; VkImageFormatListCreateInfo format_list; enum kopper_type type;