From f1d2dac2133d64ddbebdc003d19fe088456b20cb Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Wed, 4 Jul 2018 19:45:54 +0200 Subject: [PATCH 1/2] Vulkan: Fix various stability issues with WSI. Added an ifdef to trigger "hardening" which will return spurious errors for critical WSI things like acquire next image, queue submit and surface dimension queries. --- gfx/common/vulkan_common.c | 157 ++++++++++++++++++++++++------------- 1 file changed, 101 insertions(+), 56 deletions(-) diff --git a/gfx/common/vulkan_common.c b/gfx/common/vulkan_common.c index 5725ed6883..19afaa2490 100644 --- a/gfx/common/vulkan_common.c +++ b/gfx/common/vulkan_common.c @@ -43,6 +43,29 @@ static VkInstance cached_instance_vk; static VkDevice cached_device_vk; static retro_vulkan_destroy_device_t cached_destroy_device_vk; +//#define WSI_HARDENING_TEST +#ifdef WSI_HARDENING_TEST +static unsigned wsi_harden_counter = 0; +static unsigned wsi_harden_counter2 = 0; + +static void trigger_spurious_error_vkresult(VkResult *res) +{ + ++wsi_harden_counter; + if ((wsi_harden_counter & 15) == 12) + *res = VK_ERROR_OUT_OF_DATE_KHR; + else if ((wsi_harden_counter & 31) == 13) + *res = VK_ERROR_OUT_OF_DATE_KHR; + else if ((wsi_harden_counter & 15) == 6) + *res = VK_ERROR_SURFACE_LOST_KHR; +} + +static bool trigger_spurious_error(void) +{ + ++wsi_harden_counter2; + return (wsi_harden_counter2 & 15) == 9; +} +#endif + #ifdef VULKAN_DEBUG static VKAPI_ATTR VkBool32 VKAPI_CALL vulkan_debug_cb( VkDebugReportFlagsEXT flags, @@ -2278,6 +2301,32 @@ bool vulkan_surface_create(gfx_ctx_vulkan_data_t *vk, return true; } +static void vulkan_destroy_swapchain(gfx_ctx_vulkan_data_t *vk) +{ + unsigned i; + + if (vk->swapchain != VK_NULL_HANDLE) + { + vkDeviceWaitIdle(vk->context.device); + vkDestroySwapchainKHR(vk->context.device, vk->swapchain, NULL); + memset(vk->context.swapchain_images, 0, sizeof(vk->context.swapchain_images)); + vk->swapchain = VK_NULL_HANDLE; + } + + for (i = 0; i < VULKAN_MAX_SWAPCHAIN_IMAGES; i++) + { + if (vk->context.swapchain_semaphores[i] != VK_NULL_HANDLE) + vkDestroySemaphore(vk->context.device, + vk->context.swapchain_semaphores[i], NULL); + if (vk->context.swapchain_fences[i] != VK_NULL_HANDLE) + vkDestroyFence(vk->context.device, + vk->context.swapchain_fences[i], NULL); + } + + memset(vk->context.swapchain_semaphores, 0, sizeof(vk->context.swapchain_semaphores)); + memset(vk->context.swapchain_fences, 0, sizeof(vk->context.swapchain_fences)); +} + void vulkan_present(gfx_ctx_vulkan_data_t *vk, unsigned index) { VkPresentInfoKHR present = { VK_STRUCTURE_TYPE_PRESENT_INFO_KHR }; @@ -2304,10 +2353,14 @@ void vulkan_present(gfx_ctx_vulkan_data_t *vk, unsigned index) #endif err = vkQueuePresentKHR(vk->context.queue, &present); +#ifdef WSI_HARDENING_TEST + trigger_spurious_error_vkresult(&err); +#endif + if (err != VK_SUCCESS || result != VK_SUCCESS) { - RARCH_LOG("[Vulkan]: QueuePresent failed, invalidating swapchain.\n"); - vk->context.invalid_swapchain = true; + RARCH_LOG("[Vulkan]: QueuePresent failed, destroying swapchain.\n"); + vulkan_destroy_swapchain(vk); } #ifdef HAVE_THREADS @@ -2325,12 +2378,8 @@ void vulkan_context_destroy(gfx_ctx_vulkan_data_t *vk, if (vk->context.device) vkDeviceWaitIdle(vk->context.device); - if (vk->swapchain) - { - vkDestroySwapchainKHR(vk->context.device, - vk->swapchain, NULL); - vk->swapchain = VK_NULL_HANDLE; - } + + vulkan_destroy_swapchain(vk); if (destroy_surface && vk->vk_surface != VK_NULL_HANDLE) { @@ -2339,16 +2388,6 @@ void vulkan_context_destroy(gfx_ctx_vulkan_data_t *vk, vk->vk_surface = VK_NULL_HANDLE; } - for (i = 0; i < VULKAN_MAX_SWAPCHAIN_IMAGES; i++) - { - if (vk->context.swapchain_semaphores[i] != VK_NULL_HANDLE) - vkDestroySemaphore(vk->context.device, - vk->context.swapchain_semaphores[i], NULL); - if (vk->context.swapchain_fences[i] != VK_NULL_HANDLE) - vkDestroyFence(vk->context.device, - vk->context.swapchain_fences[i], NULL); - } - #ifdef VULKAN_DEBUG if (vk->context.debug_callback) vkDestroyDebugReportCallbackEXT(vk->context.instance, vk->context.debug_callback, NULL); @@ -2423,8 +2462,10 @@ void vulkan_acquire_next_image(gfx_ctx_vulkan_data_t *vk) { VK_STRUCTURE_TYPE_FENCE_CREATE_INFO }; VkSemaphoreCreateInfo sem_info = { VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO }; - bool is_retrying = false; + bool is_retrying = false; + +retry: if (vk->swapchain == VK_NULL_HANDLE) { /* We don't have a swapchain, try to create one now. */ @@ -2445,48 +2486,58 @@ void vulkan_acquire_next_image(gfx_ctx_vulkan_data_t *vk) } } -retry: vkCreateFence(vk->context.device, &fence_info, NULL, &fence); err = vkAcquireNextImageKHR(vk->context.device, vk->swapchain, UINT64_MAX, VK_NULL_HANDLE, fence, &vk->context.current_swapchain_index); - index = vk->context.current_swapchain_index; - if (vk->context.swapchain_semaphores[index] == VK_NULL_HANDLE) - vkCreateSemaphore(vk->context.device, &sem_info, - NULL, &vk->context.swapchain_semaphores[index]); - if (err == VK_SUCCESS) vkWaitForFences(vk->context.device, 1, &fence, true, UINT64_MAX); + +#ifdef WSI_HARDENING_TEST + trigger_spurious_error_vkresult(&err); +#endif + vkDestroyFence(vk->context.device, fence, NULL); - vulkan_acquire_wait_fences(vk); - - if (err != VK_SUCCESS) + if (err == VK_ERROR_OUT_OF_DATE_KHR) { + /* Throw away the old swapchain and try again. */ + vulkan_destroy_swapchain(vk); + if (is_retrying) { - RARCH_ERR("[Vulkan]: Tried acquring next swapchain image after creating new one, but failed ...\n"); + RARCH_ERR("[Vulkan]: Swapchain is out of date, trying to create new one. Have tried multiple times ...\n"); + retro_sleep(10); } else - { - RARCH_LOG("[Vulkan]: AcquireNextImage failed, invalidating swapchain.\n"); - vk->context.invalid_swapchain = true; - - RARCH_LOG("[Vulkan]: AcquireNextImage failed, so trying to recreate swapchain.\n"); - if (!vulkan_create_swapchain(vk, vk->context.swapchain_width, - vk->context.swapchain_height, vk->context.swap_interval)) - { - RARCH_ERR("[Vulkan]: Failed to create new swapchain.\n"); - } - else - { - is_retrying = true; - goto retry; - } - } + RARCH_ERR("[Vulkan]: Swapchain is out of date, trying to create new one.\n"); + is_retrying = true; + vulkan_acquire_clear_fences(vk); + goto retry; } + else if (err != VK_SUCCESS) + { + /* We are screwed, don't try anymore. Maybe it will work later. */ + vulkan_destroy_swapchain(vk); + RARCH_ERR("[Vulkan]: Failed to acquire from swapchain (err = %d).\n", + (int)err); + if (err == VK_ERROR_SURFACE_LOST_KHR) + RARCH_ERR("[Vulkan]: Got VK_ERROR_SURFACE_LOST_KHR.\n"); + /* Force driver to reset swapchain image handles. */ + vk->context.invalid_swapchain = true; + vulkan_acquire_clear_fences(vk); + return; + } + + index = vk->context.current_swapchain_index; + if (vk->context.swapchain_semaphores[index] == VK_NULL_HANDLE) + { + vkCreateSemaphore(vk->context.device, &sem_info, + NULL, &vk->context.swapchain_semaphores[index]); + } + vulkan_acquire_wait_fences(vk); } bool vulkan_create_swapchain(gfx_ctx_vulkan_data_t *vk, @@ -2511,6 +2562,7 @@ bool vulkan_create_swapchain(gfx_ctx_vulkan_data_t *vk, VkCompositeAlphaFlagBitsKHR composite = VK_COMPOSITE_ALPHA_OPAQUE_BIT_KHR; vkDeviceWaitIdle(vk->context.device); + vulkan_acquire_clear_fences(vk); vk->created_new_swapchain = true; if (vk->swapchain != VK_NULL_HANDLE && @@ -2613,10 +2665,8 @@ bool vulkan_create_swapchain(gfx_ctx_vulkan_data_t *vk, else swapchain_size = surface_properties.currentExtent; -#if 0 - /* Tests for deferred creation. */ - static unsigned retry_count = 0; - if (++retry_count < 50) +#ifdef WSI_HARDENING_TEST + if (trigger_spurious_error()) { surface_properties.maxImageExtent.width = 0; surface_properties.maxImageExtent.height = 0; @@ -2657,11 +2707,7 @@ bool vulkan_create_swapchain(gfx_ctx_vulkan_data_t *vk, * We hard sync against the swapchain, so if we have 2 images, * we would be unable to overlap CPU and GPU, which can get very slow * for GPU-rendered cores. */ - desired_swapchain_images = 3; - - /* Limit latency. */ - if (desired_swapchain_images > settings->uints.video_max_swapchain_images) - desired_swapchain_images = settings->uints.video_max_swapchain_images; + desired_swapchain_images = settings->uints.video_max_swapchain_images; /* Clamp images requested to what is supported by the implementation. */ if (desired_swapchain_images < surface_properties.minImageCount) @@ -2763,8 +2809,7 @@ bool vulkan_create_swapchain(gfx_ctx_vulkan_data_t *vk, RARCH_LOG("[Vulkan]: Got %u swapchain images.\n", vk->context.num_swapchain_images); - vulkan_acquire_clear_fences(vk); + /* Force driver to reset swapchain image handles. */ vk->context.invalid_swapchain = true; - return true; } From 52cb0fe37576f4d399d3932508d4120394f1494f Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Wed, 4 Jul 2018 20:25:03 +0200 Subject: [PATCH 2/2] Vulkan: Fix crash when we get two create_swapchain errors in a row. --- gfx/common/vulkan_common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gfx/common/vulkan_common.c b/gfx/common/vulkan_common.c index 19afaa2490..31c7f3d620 100644 --- a/gfx/common/vulkan_common.c +++ b/gfx/common/vulkan_common.c @@ -62,7 +62,7 @@ static void trigger_spurious_error_vkresult(VkResult *res) static bool trigger_spurious_error(void) { ++wsi_harden_counter2; - return (wsi_harden_counter2 & 15) == 9; + return ((wsi_harden_counter2 & 15) == 9) || ((wsi_harden_counter2 & 15) == 10); } #endif @@ -2482,6 +2482,7 @@ retry: vk->context.current_swapchain_index = 0; vulkan_acquire_clear_fences(vk); vulkan_acquire_wait_fences(vk); + vk->context.invalid_swapchain = true; return; } }