<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, May 23, 2017 at 2:35 PM, Nanley Chery <span dir="ltr"><<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Thu, May 18, 2017 at 02:00:58PM -0700, Jason Ekstrand wrote:<br>
> The idea behind doing this was to make it easier to set various flags.<br>
> However, we have enough custom flag settings floating around the driver<br>
> that this is more of a nuisance than a help.<br>
> ---<br>
>  src/intel/vulkan/anv_<wbr>allocator.c | 17 ++++++++++-------<br>
>  src/intel/vulkan/anv_device.c    | 12 ++++++------<br>
>  src/intel/vulkan/anv_queue.c     |  4 ++--<br>
>  3 files changed, 18 insertions(+), 15 deletions(-)<br>
><br>
> diff --git a/src/intel/vulkan/anv_<wbr>allocator.c b/src/intel/vulkan/anv_<wbr>allocator.c<br>
> index b767542..d637867 100644<br>
> --- a/src/intel/vulkan/anv_<wbr>allocator.c<br>
> +++ b/src/intel/vulkan/anv_<wbr>allocator.c<br>
> @@ -1025,6 +1025,12 @@ anv_bo_pool_alloc(struct anv_bo_pool *pool, struct anv_bo *bo, uint32_t size)<br>
>     if (result != VK_SUCCESS)<br>
>        return result;<br>
><br>
> +   if (pool->device->instance-><wbr>physicalDevice.supports_48bit_<wbr>addresses)<br>
> +      new_bo.flags |= EXEC_OBJECT_SUPPORTS_48B_<wbr>ADDRESS;<br>
> +<br>
> +   if (pool->device->instance-><wbr>physicalDevice.has_exec_async)<br>
> +      new_bo.flags |= EXEC_OBJECT_ASYNC;<br>
> +<br>
>     assert(new_bo.size == pow2_size);<br>
><br>
>     new_bo.map = anv_gem_mmap(pool->device, new_bo.gem_handle, 0, pow2_size, 0);<br>
> @@ -1154,7 +1160,10 @@ anv_scratch_pool_alloc(struct anv_device *device, struct anv_scratch_pool *pool,<br>
>      *<br>
>      * so nothing will ever touch the top page.<br>
>      */<br>
> -   bo->bo.flags &= ~EXEC_OBJECT_SUPPORTS_48B_<wbr>ADDRESS;<br>
> +   assert(!(bo->bo.flags & EXEC_OBJECT_SUPPORTS_48B_<wbr>ADDRESS));<br>
> +<br>
> +   if (device->instance-><wbr>physicalDevice.has_exec_async)<br>
> +      bo->bo.flags |= EXEC_OBJECT_ASYNC;<br>
><br>
>     /* Set the exists last because it may be read by other threads */<br>
>     __sync_synchronize();<br>
> @@ -1309,12 +1318,6 @@ anv_bo_cache_import(struct anv_device *device,<br>
><br>
>        anv_bo_init(&bo->bo, gem_handle, size);<br>
><br>
> -      if (device->instance-><wbr>physicalDevice.supports_48bit_<wbr>addresses)<br>
> -         bo->bo.flags |= EXEC_OBJECT_SUPPORTS_48B_<wbr>ADDRESS;<br>
> -<br>
> -      if (device->instance-><wbr>physicalDevice.has_exec_async)<br>
> -         bo->bo.flags |= EXEC_OBJECT_ASYNC;<br>
> -<br>
>        _mesa_hash_table_insert(cache-<wbr>>bo_map, (void *)(uintptr_t)gem_handle, bo);<br>
>     }<br>
><br>
> diff --git a/src/intel/vulkan/anv_device.<wbr>c b/src/intel/vulkan/anv_device.<wbr>c<br>
> index b0ccbbb..9444ff8 100644<br>
> --- a/src/intel/vulkan/anv_device.<wbr>c<br>
> +++ b/src/intel/vulkan/anv_device.<wbr>c<br>
> @@ -1453,12 +1453,6 @@ anv_bo_init_new(struct anv_bo *bo, struct anv_device *device, uint64_t size)<br>
><br>
>     anv_bo_init(bo, gem_handle, size);<br>
><br>
> -   if (device->instance-><wbr>physicalDevice.supports_48bit_<wbr>addresses)<br>
> -      bo->flags |= EXEC_OBJECT_SUPPORTS_48B_<wbr>ADDRESS;<br>
> -<br>
> -   if (device->instance-><wbr>physicalDevice.has_exec_async)<br>
> -      bo->flags |= EXEC_OBJECT_ASYNC;<br>
> -<br>
<br>
</div></div>This patch introduces the following behavioral changes:<br>
<br>
* The bo created in anv_CreateDmaBufImageINTEL loses EXEC_OBJECT_ASYNC.<br></blockquote><div><br></div><div>This probably shouldn't have had it set anyway<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
* The workaround_bo created in anv_CreateDevice loses both flags.<br></blockquote><div><br></div><div>I knew this would happen but I don't particularly care.  EXEC_OBJECT_ASYNC doesn't matter since it's driver internal and 48bit doesn't matter because it's only 4K.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
* The bo created in genX(CreateQueryPool) loses both flags.<br></blockquote><div><br></div><div>That was unintentional.  I'll fix it.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I don't see why we'd want to drop these flags. If it was intentional,<br>
could mention why in the commit message?<br></blockquote><div><br></div><div>Yup.  I'll update the commit mesage.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
-Nanley<br>
<div><div class="h5"><br>
>     return VK_SUCCESS;<br>
>  }<br>
><br>
> @@ -1536,6 +1530,12 @@ VkResult anv_AllocateMemory(<br>
>           goto fail;<br>
>     }<br>
><br>
> +   if (pdevice->supports_48bit_<wbr>addresses)<br>
> +      mem->bo->flags |= EXEC_OBJECT_SUPPORTS_48B_<wbr>ADDRESS;<br>
> +<br>
> +   if (pdevice->has_exec_async)<br>
> +      mem->bo->flags |= EXEC_OBJECT_ASYNC;<br>
> +<br>
>     *pMem = anv_device_memory_to_handle(<wbr>mem);<br>
><br>
>     return VK_SUCCESS;<br>
> diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c<br>
> index fac979a..86eee28 100644<br>
> --- a/src/intel/vulkan/anv_queue.c<br>
> +++ b/src/intel/vulkan/anv_queue.c<br>
> @@ -553,7 +553,7 @@ VkResult anv_CreateSemaphore(<br>
>        /* If we're going to use this as a fence, we need to *not* have the<br>
>         * EXEC_OBJECT_ASYNC bit set.<br>
>         */<br>
> -      semaphore->permanent.bo->flags &= ~EXEC_OBJECT_ASYNC;<br>
> +      assert(!(semaphore->permanent.<wbr>bo->flags & EXEC_OBJECT_ASYNC));<br>
>     } else {<br>
>        assert(!"Unknown handle type");<br>
>        vk_free2(&device->alloc, pAllocator, semaphore);<br>
> @@ -644,7 +644,7 @@ VkResult anv_ImportSemaphoreFdKHX(<br>
>        /* If we're going to use this as a fence, we need to *not* have the<br>
>         * EXEC_OBJECT_ASYNC bit set.<br>
>         */<br>
> -      bo->flags &= ~EXEC_OBJECT_ASYNC;<br>
> +      assert(!(bo->flags & EXEC_OBJECT_ASYNC));<br>
><br>
>        anv_semaphore_impl_cleanup(<wbr>device, &semaphore->permanent);<br>
><br>
> --<br>
> 2.5.0.400.gff86faf<br>
><br>
</div></div>> ______________________________<wbr>_________________<br>
> mesa-dev mailing list<br>
> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</blockquote></div><br></div></div>