[Mesa-dev] [PATCH 11/16] anv: Stop setting BO flags in bo_init_new

[Nanley Chery]

On Thu, May 18, 2017 at 02:00:58PM -0700, Jason Ekstrand wrote:
> The idea behind doing this was to make it easier to set various flags.
> However, we have enough custom flag settings floating around the driver
> that this is more of a nuisance than a help.
> ---
>  src/intel/vulkan/anv_allocator.c | 17 ++++++++++-------
>  src/intel/vulkan/anv_device.c    | 12 ++++++------
>  src/intel/vulkan/anv_queue.c     |  4 ++--
>  3 files changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_allocator.c
> index b767542..d637867 100644
> --- a/src/intel/vulkan/anv_allocator.c
> +++ b/src/intel/vulkan/anv_allocator.c
> @@ -1025,6 +1025,12 @@ anv_bo_pool_alloc(struct anv_bo_pool *pool, struct anv_bo *bo, uint32_t size)
>     if (result != VK_SUCCESS)
>        return result;
>  
> +   if (pool->device->instance->physicalDevice.supports_48bit_addresses)
> +      new_bo.flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> +
> +   if (pool->device->instance->physicalDevice.has_exec_async)
> +      new_bo.flags |= EXEC_OBJECT_ASYNC;
> +
>     assert(new_bo.size == pow2_size);
>  
>     new_bo.map = anv_gem_mmap(pool->device, new_bo.gem_handle, 0, pow2_size, 0);
> @@ -1154,7 +1160,10 @@ anv_scratch_pool_alloc(struct anv_device *device, struct anv_scratch_pool *pool,
>      *
>      * so nothing will ever touch the top page.
>      */
> -   bo->bo.flags &= ~EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> +   assert(!(bo->bo.flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS));
> +
> +   if (device->instance->physicalDevice.has_exec_async)
> +      bo->bo.flags |= EXEC_OBJECT_ASYNC;
>  
>     /* Set the exists last because it may be read by other threads */
>     __sync_synchronize();
> @@ -1309,12 +1318,6 @@ anv_bo_cache_import(struct anv_device *device,
>  
>        anv_bo_init(&bo->bo, gem_handle, size);
>  
> -      if (device->instance->physicalDevice.supports_48bit_addresses)
> -         bo->bo.flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> -
> -      if (device->instance->physicalDevice.has_exec_async)
> -         bo->bo.flags |= EXEC_OBJECT_ASYNC;
> -
>        _mesa_hash_table_insert(cache->bo_map, (void *)(uintptr_t)gem_handle, bo);
>     }
>  
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index b0ccbbb..9444ff8 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -1453,12 +1453,6 @@ anv_bo_init_new(struct anv_bo *bo, struct anv_device *device, uint64_t size)
>  
>     anv_bo_init(bo, gem_handle, size);
>  
> -   if (device->instance->physicalDevice.supports_48bit_addresses)
> -      bo->flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> -
> -   if (device->instance->physicalDevice.has_exec_async)
> -      bo->flags |= EXEC_OBJECT_ASYNC;
> -

This patch introduces the following behavioral changes:

* The bo created in anv_CreateDmaBufImageINTEL loses EXEC_OBJECT_ASYNC.
* The workaround_bo created in anv_CreateDevice loses both flags.
* The bo created in genX(CreateQueryPool) loses both flags.

I don't see why we'd want to drop these flags. If it was intentional,
could mention why in the commit message?

-Nanley

>     return VK_SUCCESS;
>  }
>  
> @@ -1536,6 +1530,12 @@ VkResult anv_AllocateMemory(
>           goto fail;
>     }
>  
> +   if (pdevice->supports_48bit_addresses)
> +      mem->bo->flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> +
> +   if (pdevice->has_exec_async)
> +      mem->bo->flags |= EXEC_OBJECT_ASYNC;
> +
>     *pMem = anv_device_memory_to_handle(mem);
>  
>     return VK_SUCCESS;
> diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c
> index fac979a..86eee28 100644
> --- a/src/intel/vulkan/anv_queue.c
> +++ b/src/intel/vulkan/anv_queue.c
> @@ -553,7 +553,7 @@ VkResult anv_CreateSemaphore(
>        /* If we're going to use this as a fence, we need to *not* have the
>         * EXEC_OBJECT_ASYNC bit set.
>         */
> -      semaphore->permanent.bo->flags &= ~EXEC_OBJECT_ASYNC;
> +      assert(!(semaphore->permanent.bo->flags & EXEC_OBJECT_ASYNC));
>     } else {
>        assert(!"Unknown handle type");
>        vk_free2(&device->alloc, pAllocator, semaphore);
> @@ -644,7 +644,7 @@ VkResult anv_ImportSemaphoreFdKHX(
>        /* If we're going to use this as a fence, we need to *not* have the
>         * EXEC_OBJECT_ASYNC bit set.
>         */
> -      bo->flags &= ~EXEC_OBJECT_ASYNC;
> +      assert(!(bo->flags & EXEC_OBJECT_ASYNC));
>  
>        anv_semaphore_impl_cleanup(device, &semaphore->permanent);
>  
> -- 
> 2.5.0.400.gff86faf
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev