vulkan/wsi/x11: Consistently update and return swapchain status

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.  Instead of
direct comparisons to VK_SUCCESS to check for error, test for negative
numbers meaning an error status, and positive numbers indicating
non-error statuses.

v2 (Jason Ekstrand):
 - Use a pattern of "return x11_swapchain_result(chain, VK_WHATEVER)"
 - Handle wsi_queue_pull returning VK_TIMEOUT
 - Call x11_swapchain_result in x11_present_to_x11

Signed-off-by: Daniel Stone <[email protected]>
Reviewed-by: Jason Ekstrand <[email protected]>

1 files changed, 63 insertions, 19 deletions

diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_x11.c
index 15d291463e9..e5e719e1d50 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)
@@ -726,7 +759,7 @@ 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;
+            return x11_swapchain_result(chain, VK_SUCCESS);
          }
       }
 
@@ -735,13 +768,13 @@ 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;
+            return x11_swapchain_result(chain, VK_ERROR_OUT_OF_DATE_KHR);
       } else {
          event = xcb_poll_for_special_event(chain->conn, chain->special_event);
          if (!event) {
             int ret;
             if (timeout == 0)
-               return VK_NOT_READY;
+               return x11_swapchain_result(chain, VK_NOT_READY);
 
             atimeout = wsi_get_absolute_timeout(timeout);
 
@@ -749,9 +782,9 @@ x11_acquire_next_image_poll_x11(struct x11_swapchain *chain,
             pfds.events = POLLIN;
             ret = poll(&pfds, 1, timeout / 1000 / 1000);
             if (ret == 0)
-               return VK_TIMEOUT;
+               return x11_swapchain_result(chain, VK_TIMEOUT);
             if (ret == -1)
-               return VK_ERROR_OUT_OF_DATE_KHR;
+               return x11_swapchain_result(chain, VK_ERROR_OUT_OF_DATE_KHR);
 
             /* If a non-special event happens, the fd will still
              * poll. So recalculate the timeout now just in case.
@@ -765,10 +798,13 @@ 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);
       free(event);
-      if (result != VK_SUCCESS)
-         return result;
+      if (result < 0)
+         return x11_swapchain_result(chain, result);
    }
 }
 
@@ -781,9 +817,13 @@ 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 != VK_SUCCESS) {
-      return result;
-   } else if (chain->status != VK_SUCCESS) {
+   if (result < 0 || result == VK_TIMEOUT) {
+      /* On error, the thread has shut down, so safe to update chain->status.
+       * Calling x11_swapchain_result with VK_TIMEOUT won't modify
+       * chain->status so that is also safe.
+       */
+      return x11_swapchain_result(chain, result);
+   } else if (chain->status < 0) {
       return chain->status;
    }
 
@@ -792,7 +832,7 @@ x11_acquire_next_image_from_queue(struct x11_swapchain *chain,
 
    *image_index_out = image_index;
 
-   return VK_SUCCESS;
+   return chain->status;
 }
 
 static VkResult
@@ -835,7 +875,7 @@ x11_present_to_x11(struct x11_swapchain *chain, uint32_t image_index,
 
    xcb_flush(chain->conn);
 
-   return VK_SUCCESS;
+   return x11_swapchain_result(chain, VK_SUCCESS);
 }
 
 static VkResult
@@ -876,7 +916,7 @@ x11_manage_fifo_queues(void *state)
 
    assert(chain->base.present_mode == VK_PRESENT_MODE_FIFO_KHR);
 
-   while (chain->status == VK_SUCCESS) {
+   while (chain->status >= 0) {
       /* It should be safe to unconditionally block here.  Later in the loop
        * we blocks until the previous present has landed on-screen.  At that
        * point, we should have received IDLE_NOTIFY on all images presented
@@ -885,15 +925,19 @@ x11_manage_fifo_queues(void *state)
        */
       uint32_t image_index;
       result = wsi_queue_pull(&chain->present_queue, &image_index, INT64_MAX);
-      if (result != VK_SUCCESS) {
+      assert(result != VK_TIMEOUT);
+      if (result < 0) {
          goto fail;
-      } else if (chain->status != VK_SUCCESS) {
+      } else if (chain->status < 0) {
+         /* The status can change underneath us if the swapchain is destroyed
+          * from another thread.
+          */
          return NULL;
       }
 
       uint64_t target_msc = chain->last_present_msc + 1;
       result = x11_present_to_x11(chain, image_index, target_msc);
-      if (result != VK_SUCCESS)
+      if (result < 0)
          goto fail;
 
       while (chain->last_present_msc < target_msc) {
@@ -906,13 +950,13 @@ x11_manage_fifo_queues(void *state)
 
          result = x11_handle_dri3_present_event(chain, (void *)event);
          free(event);
-         if (result != VK_SUCCESS)
+         if (result < 0)
             goto fail;
       }
    }
 
 fail:
-   chain->status = result;
+   result = x11_swapchain_result(chain, result);
    wsi_queue_push(&chain->acquire_queue, UINT32_MAX);
 
    return NULL;
@@ -934,7 +978,7 @@ x11_image_init(VkDevice device_h, struct x11_swapchain *chain,
       result = wsi_create_native_image(&chain->base, pCreateInfo,
                                        0, NULL, NULL, &image->base);
    }
-   if (result != VK_SUCCESS)
+   if (result < 0)
       return result;
 
    image->pixmap = xcb_generate_id(chain->conn);