From f9a3d9738b12883e268b81731f8e231df3e376c3 Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Wed, 4 Dec 2019 13:19:23 -0600 Subject: [PATCH] anv: Use BO fences/semaphores for AcquireNextImage Instead of doing a dummy submit on the command buffer for the fence or a dummy semaphore and trusting in implicit sync, this commit moves us to take advantage of implicit sync and just use the WSI image BO as the fence. Both semaphores and fences require a tiny bit of extra plumbing to do this but the result is that we can get rid of a bunch of the extra synchronization we're doing today. Reviewed-by: Lionel Landwerlin --- src/intel/vulkan/anv_private.h | 18 ++++++-- src/intel/vulkan/anv_queue.c | 31 +++++++++++--- src/intel/vulkan/anv_wsi.c | 78 +++++++++++++++++++++------------- 3 files changed, 86 insertions(+), 41 deletions(-) diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index 96254f620d8..32bb027142e 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -671,6 +671,13 @@ struct anv_bo { bool has_client_visible_address:1; }; +static inline struct anv_bo * +anv_bo_ref(struct anv_bo *bo) +{ + p_atomic_inc(&bo->refcount); + return bo; +} + static inline struct anv_bo * anv_bo_unwrap(struct anv_bo *bo) { @@ -2804,6 +2811,7 @@ void anv_cmd_emit_conditional_render_predicate(struct anv_cmd_buffer *cmd_buffer enum anv_fence_type { ANV_FENCE_TYPE_NONE = 0, ANV_FENCE_TYPE_BO, + ANV_FENCE_TYPE_WSI_BO, ANV_FENCE_TYPE_SYNCOBJ, ANV_FENCE_TYPE_WSI, }; @@ -2875,6 +2883,7 @@ enum anv_semaphore_type { ANV_SEMAPHORE_TYPE_NONE = 0, ANV_SEMAPHORE_TYPE_DUMMY, ANV_SEMAPHORE_TYPE_BO, + ANV_SEMAPHORE_TYPE_WSI_BO, ANV_SEMAPHORE_TYPE_SYNC_FILE, ANV_SEMAPHORE_TYPE_DRM_SYNCOBJ, ANV_SEMAPHORE_TYPE_TIMELINE, @@ -2909,10 +2918,11 @@ struct anv_semaphore_impl { enum anv_semaphore_type type; union { - /* A BO representing this semaphore when type == ANV_SEMAPHORE_TYPE_BO. - * This BO will be added to the object list on any execbuf2 calls for - * which this semaphore is used as a wait or signal fence. When used as - * a signal fence, the EXEC_OBJECT_WRITE flag will be set. + /* A BO representing this semaphore when type == ANV_SEMAPHORE_TYPE_BO + * or type == ANV_SEMAPHORE_TYPE_WSI_BO. This BO will be added to the + * object list on any execbuf2 calls for which this semaphore is used as + * a wait or signal fence. When used as a signal fence or when type == + * ANV_SEMAPHORE_TYPE_WSI_BO, the EXEC_OBJECT_WRITE flag will be set. */ struct anv_bo *bo; diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c index 7bdcb5e2b7f..caa3b50049a 100644 --- a/src/intel/vulkan/anv_queue.c +++ b/src/intel/vulkan/anv_queue.c @@ -732,6 +732,18 @@ anv_queue_submit(struct anv_queue *queue, goto error; break; + case ANV_SEMAPHORE_TYPE_WSI_BO: + /* When using a window-system buffer as a semaphore, always enable + * EXEC_OBJECT_WRITE. This gives us a WaR hazard with the display or + * compositor's read of the buffer and enforces that we don't start + * rendering until they are finished. This is exactly the + * synchronization we want with vkAcquireNextImage. + */ + result = anv_queue_submit_add_fence_bo(submit, impl->bo, true /* signal */); + if (result != VK_SUCCESS) + goto error; + break; + case ANV_SEMAPHORE_TYPE_SYNC_FILE: assert(!pdevice->has_syncobj); if (submit->in_fence == -1) { @@ -1108,6 +1120,10 @@ anv_fence_impl_cleanup(struct anv_device *device, anv_bo_pool_free(&device->batch_bo_pool, impl->bo.bo); break; + case ANV_FENCE_TYPE_WSI_BO: + anv_device_release_bo(device, impl->bo.bo); + break; + case ANV_FENCE_TYPE_SYNCOBJ: anv_gem_syncobj_destroy(device, impl->syncobj); break; @@ -1204,6 +1220,7 @@ VkResult anv_GetFenceStatus( switch (impl->type) { case ANV_FENCE_TYPE_BO: + case ANV_FENCE_TYPE_WSI_BO: /* BO fences don't support import/export */ assert(fence->temporary.type == ANV_FENCE_TYPE_NONE); switch (impl->bo.state) { @@ -1311,13 +1328,11 @@ anv_wait_for_bo_fences(struct anv_device *device, for (uint32_t i = 0; i < fenceCount; i++) { ANV_FROM_HANDLE(anv_fence, fence, pFences[i]); - /* This function assumes that all fences are BO fences and that they - * have no temporary state. Since BO fences will never be exported, - * this should be a safe assumption. - */ - assert(fence->permanent.type == ANV_FENCE_TYPE_BO); - assert(fence->temporary.type == ANV_FENCE_TYPE_NONE); - struct anv_fence_impl *impl = &fence->permanent; + struct anv_fence_impl *impl = + fence->temporary.type != ANV_FENCE_TYPE_NONE ? + &fence->temporary : &fence->permanent; + assert(impl->type == ANV_FENCE_TYPE_BO || + impl->type == ANV_FENCE_TYPE_WSI_BO); switch (impl->bo.state) { case ANV_BO_FENCE_STATE_RESET: @@ -1435,6 +1450,7 @@ anv_wait_for_fences(struct anv_device *device, ANV_FROM_HANDLE(anv_fence, fence, pFences[i]); switch (fence->permanent.type) { case ANV_FENCE_TYPE_BO: + case ANV_FENCE_TYPE_WSI_BO: result = anv_wait_for_bo_fences(device, 1, &pFences[i], true, abs_timeout); break; @@ -1798,6 +1814,7 @@ anv_semaphore_impl_cleanup(struct anv_device *device, break; case ANV_SEMAPHORE_TYPE_BO: + case ANV_SEMAPHORE_TYPE_WSI_BO: anv_device_release_bo(device, impl->bo); break; diff --git a/src/intel/vulkan/anv_wsi.c b/src/intel/vulkan/anv_wsi.c index 0ba25b1f847..ed5d077b4ab 100644 --- a/src/intel/vulkan/anv_wsi.c +++ b/src/intel/vulkan/anv_wsi.c @@ -40,6 +40,48 @@ anv_wsi_image_get_modifier(VkImage _image) return image->drm_format_mod; } +static void +anv_wsi_signal_semaphore_for_memory(VkDevice _device, + VkSemaphore _semaphore, + VkDeviceMemory _memory) +{ + ANV_FROM_HANDLE(anv_device, device, _device); + ANV_FROM_HANDLE(anv_semaphore, semaphore, _semaphore); + ANV_FROM_HANDLE(anv_device_memory, memory, _memory); + + /* Put a BO semaphore with the image BO in the temporary. For BO binary + * semaphores, we always set EXEC_OBJECT_WRITE so this creates a WaR + * hazard with the display engine's read to ensure that no one writes to + * the image before the read is complete. + */ + anv_semaphore_reset_temporary(device, semaphore); + + struct anv_semaphore_impl *impl = &semaphore->temporary; + impl->type = ANV_SEMAPHORE_TYPE_WSI_BO; + impl->bo = anv_bo_ref(memory->bo); +} + +static void +anv_wsi_signal_fence_for_memory(VkDevice _device, + VkFence _fence, + VkDeviceMemory _memory) +{ + ANV_FROM_HANDLE(anv_device, device, _device); + ANV_FROM_HANDLE(anv_fence, fence, _fence); + ANV_FROM_HANDLE(anv_device_memory, memory, _memory); + + /* Put a BO fence with the image BO in the temporary. For BO fences, we + * always just wait until the BO isn't busy and reads from the BO should + * count as busy. + */ + anv_fence_reset_temporary(device, fence); + + struct anv_fence_impl *impl = &fence->temporary; + impl->type = ANV_FENCE_TYPE_WSI_BO; + impl->bo.bo = anv_bo_ref(memory->bo); + impl->bo.state = ANV_BO_FENCE_STATE_SUBMITTED; +} + VkResult anv_init_wsi(struct anv_physical_device *physical_device) { @@ -56,6 +98,10 @@ anv_init_wsi(struct anv_physical_device *physical_device) physical_device->wsi_device.supports_modifiers = true; physical_device->wsi_device.image_get_modifier = anv_wsi_image_get_modifier; + physical_device->wsi_device.signal_semaphore_for_memory = + anv_wsi_signal_semaphore_for_memory; + physical_device->wsi_device.signal_fence_for_memory = + anv_wsi_signal_fence_for_memory; return VK_SUCCESS; } @@ -242,36 +288,8 @@ VkResult anv_AcquireNextImage2KHR( ANV_FROM_HANDLE(anv_device, device, _device); struct anv_physical_device *pdevice = &device->instance->physicalDevice; - VkResult result = wsi_common_acquire_next_image2(&pdevice->wsi_device, - _device, - pAcquireInfo, - pImageIndex); - - /* Thanks to implicit sync, the image is ready immediately. However, we - * should wait for the current GPU state to finish. Regardless of the - * result of the presentation, we need to signal the semaphore & fence. - */ - - if (pAcquireInfo->semaphore != VK_NULL_HANDLE) { - /* Put a dummy semaphore in temporary, this is the fastest way to avoid - * any kind of work yet still provide some kind of synchronization. This - * only works because the Mesa WSI code always returns an image - * immediately if available. - */ - ANV_FROM_HANDLE(anv_semaphore, semaphore, pAcquireInfo->semaphore); - anv_semaphore_reset_temporary(device, semaphore); - - struct anv_semaphore_impl *impl = &semaphore->temporary; - - impl->type = ANV_SEMAPHORE_TYPE_DUMMY; - } - - if (pAcquireInfo->fence != VK_NULL_HANDLE) { - result = anv_QueueSubmit(anv_queue_to_handle(&device->queue), - 0, NULL, pAcquireInfo->fence); - } - - return result; + return wsi_common_acquire_next_image2(&pdevice->wsi_device, _device, + pAcquireInfo, pImageIndex); } VkResult anv_QueuePresentKHR( -- 2.30.2