<div dir="ltr">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. </div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Feb 21, 2018 at 6:05 AM, Daniel Stone <<a href="mailto:daniels@collabora.com" target="_blank">daniels@collabora.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 <<a href="mailto:daniels@collabora.com">daniels@collabora.com</a>> --- 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 <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div>