[Mesa-dev] [PATCH v11 08/15] vulkan/wsi/x11: Consistently update swapchain status

Jason Ekstrand jason at jlekstrand.net
Wed Feb 21 19:30:22 UTC 2018


I'm going to squash this with the previous one and send review comments on
the combined patch.  It's terribly difficult to reason about the two
individually.

On Wed, Feb 21, 2018 at 6:05 AM, Daniel Stone <daniels at collabora.com> wrote:

> Use a helper function for updating the swapchain status. This will be
> used later to handle VK_SUBOPTIMAL_KHR, where we need to make a
> non-error status stick to the swapchain until recreation.
>
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> ---
>  src/vulkan/wsi/wsi_common_x11.c | 90 ++++++++++++++++++++++++++++++
> ++---------
>  1 file changed, 72 insertions(+), 18 deletions(-)
>
> diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_
> x11.c
> index 308b20c9f02..e84572810d3 100644
> --- a/src/vulkan/wsi/wsi_common_x11.c
> +++ b/src/vulkan/wsi/wsi_common_x11.c
> @@ -641,6 +641,36 @@ struct x11_swapchain {
>     struct x11_image                             images[0];
>  };
>
> +/**
> + * Update the swapchain status with the result of an operation, and return
> + * the combined status. The chain status will eventually be returned from
> + * AcquireNextImage and QueuePresent.
> + *
> + * We make sure to 'stick' more pessimistic statuses: an out-of-date error
> + * is permanent once seen, and every subsequent call will return this. If
> + * this has not been seen, success will be returned.
> + */
> +static VkResult
> +x11_swapchain_result(struct x11_swapchain *chain, VkResult result)
> +{
> +   /* Prioritise returning existing errors for consistency. */
> +   if (chain->status < 0)
> +      return chain->status;
> +
> +   /* If we have a new error, mark it as permanent on the chain and
> return. */
> +   if (result < 0) {
> +      chain->status = result;
> +      return result;
> +   }
> +
> +   /* Return temporary errors, but don't persist them. */
> +   if (result == VK_TIMEOUT || result == VK_NOT_READY)
> +      return result;
> +
> +   /* No changes, so return the last status. */
> +   return chain->status;
> +}
> +
>  static struct wsi_image *
>  x11_get_wsi_image(struct wsi_swapchain *wsi_chain, uint32_t image_index)
>  {
> @@ -648,6 +678,9 @@ x11_get_wsi_image(struct wsi_swapchain *wsi_chain,
> uint32_t image_index)
>     return &chain->images[image_index].base;
>  }
>
> +/**
> + * Process an X11 Present event. Does not update chain->status.
> + */
>  static VkResult
>  x11_handle_dri3_present_event(struct x11_swapchain *chain,
>                                xcb_present_generic_event_t *event)
> @@ -719,6 +752,8 @@ x11_acquire_next_image_poll_x11(struct x11_swapchain
> *chain,
>     xcb_generic_event_t *event;
>     struct pollfd pfds;
>     uint64_t atimeout;
> +   VkResult result = VK_SUCCESS;
> +
>     while (1) {
>        for (uint32_t i = 0; i < chain->base.image_count; i++) {
>           if (!chain->images[i].busy) {
> @@ -726,7 +761,8 @@ x11_acquire_next_image_poll_x11(struct x11_swapchain
> *chain,
>              xshmfence_await(chain->images[i].shm_fence);
>              *image_index = i;
>              chain->images[i].busy = true;
> -            return VK_SUCCESS;
> +            result = VK_SUCCESS;
> +           goto out;
>           }
>        }
>
> @@ -734,24 +770,31 @@ x11_acquire_next_image_poll_x11(struct
> x11_swapchain *chain,
>
>        if (timeout == UINT64_MAX) {
>           event = xcb_wait_for_special_event(chain->conn,
> chain->special_event);
> -         if (!event)
> -            return VK_ERROR_OUT_OF_DATE_KHR;
> +         if (!event) {
> +            result = VK_ERROR_OUT_OF_DATE_KHR;
> +            goto out;
> +         }
>        } else {
>           event = xcb_poll_for_special_event(chain->conn,
> chain->special_event);
>           if (!event) {
>              int ret;
> -            if (timeout == 0)
> -               return VK_NOT_READY;
> +            if (timeout == 0) {
> +               result = VK_NOT_READY;
> +               goto out;
> +            }
>
>              atimeout = wsi_get_absolute_timeout(timeout);
>
>              pfds.fd = xcb_get_file_descriptor(chain->conn);
>              pfds.events = POLLIN;
>              ret = poll(&pfds, 1, timeout / 1000 / 1000);
> -            if (ret == 0)
> -               return VK_TIMEOUT;
> -            if (ret == -1)
> -               return VK_ERROR_OUT_OF_DATE_KHR;
> +            if (ret == 0) {
> +               result = VK_TIMEOUT;
> +               goto out;
> +            } else if (ret == -1) {
> +               result = VK_ERROR_OUT_OF_DATE_KHR;
> +               goto out;
> +            }
>
>              /* If a non-special event happens, the fd will still
>               * poll. So recalculate the timeout now just in case.
> @@ -765,11 +808,18 @@ x11_acquire_next_image_poll_x11(struct
> x11_swapchain *chain,
>           }
>        }
>
> +      /* Update the swapchain status here. We may catch non-fatal errors
> here,
> +       * in which case we need to update the status and continue.
> +       */
>        VkResult result = x11_handle_dri3_present_event(chain, (void
> *)event);
> +      result = x11_swapchain_result(chain, result);
>        free(event);
>        if (result < 0)
> -         return result;
> +         goto out;
>     }
> +
> +out:
> +   return x11_swapchain_result(chain, result);
>  }
>
>  static VkResult
> @@ -781,11 +831,9 @@ x11_acquire_next_image_from_queue(struct
> x11_swapchain *chain,
>     uint32_t image_index;
>     VkResult result = wsi_queue_pull(&chain->acquire_queue,
>                                      &image_index, timeout);
> -   if (result < 0) {
> -      return result;
> -   } else if (chain->status != VK_SUCCESS) {
> -      return chain->status;
> -   }
> +   /* On error, the thread has shut down, so safe to update chain->status
> */
> +   if (result < 0)
> +      return x11_swapchain_result(chain, result);
>
>     assert(image_index < chain->base.image_count);
>     xshmfence_await(chain->images[image_index].shm_fence);
> @@ -859,13 +907,16 @@ 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);
>     }
> +
> +   return x11_swapchain_result(chain, result);
>  }
>
>  static void *
> @@ -888,6 +939,9 @@ x11_manage_fifo_queues(void *state)
>        if (result < 0) {
>           goto fail;
>        } else if (chain->status < 0) {
> +         /* The status can change underneath us if the swapchain is
> destroyed
> +          * from another thread.
> +          */
>           return NULL;
>        }
>
> @@ -910,7 +964,7 @@ x11_manage_fifo_queues(void *state)
>     }
>
>  fail:
> -   chain->status = result;
> +   result = x11_swapchain_result(chain, result);
>     wsi_queue_push(&chain->acquire_queue, UINT32_MAX);
>
>     return NULL;
> --
> 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/20180221/820a7632/attachment.html>


More information about the mesa-dev mailing list