From 745282736eb8b1b3bc0ac9d45a77a40e502b887f Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Wed, 17 Jan 2024 13:13:35 +0100 Subject: [PATCH] wsi/x11: Compare modifiers before signalling SUBOPTIMAL. When receiving SUBOPTIMAL_COPY, we need to consider that it can be a false positive. Xwayland may send this suboptimal copy if there are pending DRM modifier feedbacks from compositor, but it's likely the modifier lists are identical. Hash the modifier lists and compare them against the newly queried modifier list when SUBOPTIMAL_COPY is received to work around false positives. This fixes crashes in games that cannot handle SUBOPTIMAL correctly, and avoid needless stutters when entering full-screen modes. Signed-off-by: Hans-Kristian Arntzen Tested-by: Yiwei Zhang Reviewed-by: Yiwei Zhang Part-of: --- src/vulkan/wsi/meson.build | 3 +- src/vulkan/wsi/wsi_common_x11.c | 76 ++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/src/vulkan/wsi/meson.build b/src/vulkan/wsi/meson.build index c8206eac996..be4886a1a9e 100644 --- a/src/vulkan/wsi/meson.build +++ b/src/vulkan/wsi/meson.build @@ -65,7 +65,8 @@ libvulkan_wsi = static_library( include_directories : [inc_include, inc_src], dependencies : [ vulkan_wsi_deps, dep_libdrm, dep_libudev, idep_vulkan_util_headers, - idep_vulkan_runtime_headers, idep_xmlconfig, idep_mesautil, platform_deps + idep_vulkan_runtime_headers, idep_xmlconfig, idep_mesautil, platform_deps, + idep_blake3 ], gnu_symbol_visibility : 'hidden', build_by_default : false, diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_x11.c index 6eba814bd3b..46733c932c8 100644 --- a/src/vulkan/wsi/wsi_common_x11.c +++ b/src/vulkan/wsi/wsi_common_x11.c @@ -46,6 +46,7 @@ #include #include "drm-uapi/drm_fourcc.h" #include "util/hash_table.h" +#include "util/mesa-blake3.h" #include "util/os_file.h" #include "util/os_time.h" #include "util/u_debug.h" @@ -1057,6 +1058,8 @@ struct x11_swapchain { uint32_t depth; VkExtent2D extent; + blake3_hash dri3_modifier_hash; + xcb_present_event_t event_id; xcb_special_event_t * special_event; uint64_t send_sbc; @@ -1218,6 +1221,9 @@ x11_get_wsi_image(struct wsi_swapchain *wsi_chain, uint32_t image_index) return &chain->images[image_index].base; } +static bool +wsi_x11_swapchain_query_dri3_modifiers_changed(struct x11_swapchain *chain); + /* XXX this belongs in presentproto */ #ifndef PresentWindowDestroyed #define PresentWindowDestroyed (1 << 0) @@ -1291,7 +1297,14 @@ x11_handle_dri3_present_event(struct x11_swapchain *chain, /* The winsys is now trying to flip directly and cannot due to our * configuration. Request the user reallocate. */ - result = VK_SUBOPTIMAL_KHR; + + /* Sometimes, this complete mode is spurious, and a false positive. + * Xwayland may report SUBOPTIMAL_COPY even if there are no changes in the modifiers. + * https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26616 for more details. */ + if (chain->status == VK_SUCCESS && + wsi_x11_swapchain_query_dri3_modifiers_changed(chain)) { + result = VK_SUBOPTIMAL_KHR; + } break; #endif default: @@ -2272,6 +2285,21 @@ x11_image_finish(struct x11_swapchain *chain, #endif } +static void +wsi_x11_recompute_dri3_modifier_hash(blake3_hash *hash, const struct wsi_drm_image_params *params) +{ + mesa_blake3 ctx; + _mesa_blake3_init(&ctx); + _mesa_blake3_update(&ctx, ¶ms->num_modifier_lists, sizeof(params->num_modifier_lists)); + for (uint32_t i = 0; i < params->num_modifier_lists; i++) { + _mesa_blake3_update(&ctx, &i, sizeof(i)); + _mesa_blake3_update(&ctx, params->modifiers[i], + params->num_modifiers[i] * sizeof(*params->modifiers[i])); + } + _mesa_blake3_update(&ctx, ¶ms->same_gpu, sizeof(params->same_gpu)); + _mesa_blake3_final(&ctx, *hash); +} + static void wsi_x11_get_dri3_modifiers(struct wsi_x11_connection *wsi_conn, xcb_connection_t *conn, xcb_window_t window, @@ -2348,6 +2376,50 @@ out: *num_tranches_in = 0; } +static bool +wsi_x11_swapchain_query_dri3_modifiers_changed(struct x11_swapchain *chain) +{ + const struct wsi_device *wsi_device = chain->base.wsi; + + if (wsi_device->sw || !wsi_device->supports_modifiers) + return false; + + struct wsi_drm_image_params drm_image_params; + uint64_t *modifiers[2] = {NULL, NULL}; + uint32_t num_modifiers[2] = {0, 0}; + + struct wsi_x11_connection *wsi_conn = + wsi_x11_get_connection((struct wsi_device*)chain->base.wsi, chain->conn); + + xcb_get_geometry_reply_t *geometry = + xcb_get_geometry_reply(chain->conn, xcb_get_geometry(chain->conn, chain->window), NULL); + if (geometry == NULL) + return false; + uint32_t bit_depth = geometry->depth; + free(geometry); + + drm_image_params = (struct wsi_drm_image_params){ + .base.image_type = WSI_IMAGE_TYPE_DRM, + .same_gpu = wsi_x11_check_dri3_compatible(wsi_device, chain->conn), + }; + + wsi_x11_get_dri3_modifiers(wsi_conn, chain->conn, chain->window, bit_depth, 32, + modifiers, num_modifiers, + &drm_image_params.num_modifier_lists, + &wsi_device->instance_alloc); + + drm_image_params.num_modifiers = num_modifiers; + drm_image_params.modifiers = (const uint64_t **)modifiers; + + blake3_hash hash; + wsi_x11_recompute_dri3_modifier_hash(&hash, &drm_image_params); + + for (int i = 0; i < ARRAY_SIZE(modifiers); i++) + vk_free(&wsi_device->instance_alloc, modifiers[i]); + + return memcmp(hash, chain->dri3_modifier_hash, sizeof(hash)) != 0; +} + static VkResult x11_swapchain_destroy(struct wsi_swapchain *anv_chain, const VkAllocationCallbacks *pAllocator) @@ -2702,6 +2774,8 @@ x11_surface_create_swapchain(VkIcdSurfaceBase *icd_surface, pAllocator); drm_image_params.num_modifiers = num_modifiers; drm_image_params.modifiers = (const uint64_t **)modifiers; + + wsi_x11_recompute_dri3_modifier_hash(&chain->dri3_modifier_hash, &drm_image_params); } image_params = &drm_image_params.base; }