[Mesa-dev] [PATCH 09/11] vulkan/wsi: Return VK_SUBOPTIMAL_KHR for X11

Jason Ekstrand jason at jlekstrand.net
Thu Feb 15 21:38:04 UTC 2018


On Thu, Feb 15, 2018 at 7:57 AM, Daniel Stone <daniels at collabora.com> wrote:

> From: Louis-Francis Ratté-Boulianne <lfrb at collabora.com>
>
> When it is detected that a window could have been flipped
> but has been copied because of suboptimal format/modifier.
> The Vulkan client should then re-create the swapchain.
>
> Signed-off-by: Louis-Francis Ratté-Boulianne <lfrb at collabora.com>
> Reviewed-by: Daniel Stone <daniels at collabora.com>
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> ---
>  src/vulkan/wsi/wsi_common_x11.c | 64 ++++++++++++++++++++++++++++++
> +++++++----
>  1 file changed, 58 insertions(+), 6 deletions(-)
>
> diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_
> x11.c
> index c569aa17187..a9929af338c 100644
> --- a/src/vulkan/wsi/wsi_common_x11.c
> +++ b/src/vulkan/wsi/wsi_common_x11.c
> @@ -130,6 +130,8 @@ wsi_x11_connection_create(const VkAllocationCallbacks
> *alloc,
>  {
>     xcb_query_extension_cookie_t dri3_cookie, pres_cookie, amd_cookie,
> nv_cookie;
>     xcb_query_extension_reply_t *dri3_reply, *pres_reply, *amd_reply,
> *nv_reply;
> +   bool has_dri3_v1_1 = false;
> +   bool has_present_v1_1 = false;
>
>     struct wsi_x11_connection *wsi_conn =
>        vk_alloc(alloc, sizeof(*wsi_conn), 8,
> @@ -138,7 +140,7 @@ wsi_x11_connection_create(const VkAllocationCallbacks
> *alloc,
>        return NULL;
>
>     dri3_cookie = xcb_query_extension(conn, 4, "DRI3");
> -   pres_cookie = xcb_query_extension(conn, 7, "PRESENT");
> +   pres_cookie = xcb_query_extension(conn, 7, "Present");
>

This seems a bit odd.  Did we just not use it before?  Looking through
things, it appears we didn't.


>     /* We try to be nice to users and emit a warning if they try to use a
>      * Vulkan application on a system without DRI3 enabled.  However, this
> ends
> @@ -173,13 +175,27 @@ wsi_x11_connection_create(const
> VkAllocationCallbacks *alloc,
>
>        ver_cookie = xcb_dri3_query_version(conn, 1, 1);
>        ver_reply = xcb_dri3_query_version_reply(conn, ver_cookie, NULL);
> -      wsi_conn->has_dri3_modifiers =
> +      has_dri3_v1_1 =
>           (ver_reply->major_version > 1 || ver_reply->minor_version >= 1);
>        free(ver_reply);
>     }
>  #endif
>
>     wsi_conn->has_present = pres_reply->present != 0;
> +#if XCB_PRESENT_MAJOR_VERSION > 1 || XCB_PRESENT_MINOR_VERSION >= 1
> +   if (wsi_conn->has_present) {
> +      xcb_present_query_version_cookie_t ver_cookie;
> +      xcb_present_query_version_reply_t *ver_reply;
> +
> +      ver_cookie = xcb_present_query_version(conn, 1, 1);
> +      ver_reply = xcb_present_query_version_reply(conn, ver_cookie,
> NULL);
> +      has_present_v1_1 =
> +        (ver_reply->major_version > 1 || ver_reply->minor_version >= 1);
> +      free(ver_reply);
> +   }
> +#endif
> +
> +   wsi_conn->has_dri3_modifiers = has_dri3_v1_1 && has_present_v1_1;
>     wsi_conn->is_proprietary_x11 = false;
>     if (amd_reply && amd_reply->present)
>        wsi_conn->is_proprietary_x11 = true;
> @@ -651,6 +667,8 @@ struct x11_swapchain {
>
>     bool                                         threaded;
>     VkResult                                     status;
> +   bool                                         suboptimal;
> +   bool                                         realloc_suboptimal;
>     struct wsi_queue                             present_queue;
>     struct wsi_queue                             acquire_queue;
>     pthread_t                                    queue_manager;
> @@ -699,6 +717,10 @@ x11_handle_dri3_present_event(struct x11_swapchain
> *chain,
>        xcb_present_complete_notify_event_t *complete = (void *) event;
>        if (complete->kind == XCB_PRESENT_COMPLETE_KIND_PIXMAP)
>           chain->last_present_msc = complete->msc;
> +#if XCB_PRESENT_MAJOR_VERSION > 1 || XCB_PRESENT_MINOR_VERSION >= 1
> +      if (complete->mode == XCB_PRESENT_COMPLETE_MODE_SUBOPTIMAL_COPY)
> +         chain->suboptimal = true;
>

I think I like the approach taken in GLX better.  Here, we'll properly
reallocate when we go from not flipping to flipping but, what happens if we
stop flipping?  In that case, we can do better if we reallocate again.

Also, I find "chain->suboptimal" and "chain->realloc_suboptimal" to be very
confusing.  chain->suboptimal has an obvious meaning but the other
doesn't.  At the very least we need better documentation as to what they
mean.


> +#endif
>        break;
>     }
>
> @@ -828,6 +850,11 @@ x11_present_to_x11(struct x11_swapchain *chain,
> uint32_t image_index,
>     if (chain->base.present_mode == VK_PRESENT_MODE_IMMEDIATE_KHR)
>        options |= XCB_PRESENT_OPTION_ASYNC;
>
> +#if XCB_PRESENT_MAJOR_VERSION > 1 || XCB_PRESENT_MINOR_VERSION >= 1
> +   if (chain->has_dri3_modifiers)
> +      options |= XCB_PRESENT_OPTION_SUBOPTIMAL;
> +#endif
> +
>     xshmfence_reset(image->shm_fence);
>
>     ++chain->send_sbc;
> @@ -862,11 +889,19 @@ x11_acquire_next_image(struct wsi_swapchain
> *anv_chain,
>                         uint32_t *image_index)
>  {
>     struct x11_swapchain *chain = (struct x11_swapchain *)anv_chain;
> +   VkResult result;
>
>     if (chain->threaded) {
> -      return x11_acquire_next_image_from_queue(chain, image_index,
> timeout);
> +      result = x11_acquire_next_image_from_queue(chain, image_index,
> timeout);
>     } else {
> -      return x11_acquire_next_image_poll_x11(chain, image_index,
> timeout);
> +      result = x11_acquire_next_image_poll_x11(chain, image_index,
> timeout);
> +   }
> +
> +   if (result != VK_SUCCESS) {
> +      return result;
> +   } else {
> +      return (chain->realloc_suboptimal && chain->suboptimal) ?
> VK_SUBOPTIMAL_KHR
> +                                                              :
> VK_SUCCESS;
>     }
>  }
>
> @@ -876,12 +911,20 @@ x11_queue_present(struct wsi_swapchain *anv_chain,
>                    const VkPresentRegionKHR *damage)
>  {
>     struct x11_swapchain *chain = (struct x11_swapchain *)anv_chain;
> +   VkResult result;
>
>     if (chain->threaded) {
>        wsi_queue_push(&chain->present_queue, image_index);
> -      return chain->status;
> +      result = chain->status;
>     } else {
> -      return x11_present_to_x11(chain, image_index, 0);
> +      result = x11_present_to_x11(chain, image_index, 0);
> +   }
> +
> +   if (result != VK_SUCCESS) {
> +      return result;
> +   } else {
> +      return (chain->realloc_suboptimal && chain->suboptimal) ?
> VK_SUBOPTIMAL_KHR
> +                                                              :
> VK_SUCCESS;
>     }
>  }
>
> @@ -1220,6 +1263,15 @@ x11_surface_create_swapchain(VkIcdSurfaceBase
> *icd_surface,
>     chain->status = VK_SUCCESS;
>     chain->has_dri3_modifiers = wsi_conn->has_dri3_modifiers;
>
> +   chain->suboptimal = false;
> +   chain->realloc_suboptimal = true;
> +   if (pCreateInfo->oldSwapchain) {
> +      struct x11_swapchain *old_chain = (void *)pCreateInfo->oldSwapchain;
> +
> +      if (old_chain->suboptimal)
> +         chain->realloc_suboptimal = false;
> +   }
> +
>     if (!wsi_x11_check_dri3_compatible(conn, local_fd))
>         chain->base.use_prime_blit = true;
>
> --
> 2.14.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180215/ccf576e4/attachment-0001.html>


More information about the mesa-dev mailing list