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

[Jason Ekstrand]

On Tue, May 23, 2017 at 2:35 PM, Nanley Chery <nanleychery at gmail.com> wrote:

> 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.
>

This probably shouldn't have had it set anyway


> * The workaround_bo created in anv_CreateDevice loses both flags.
>

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.


> * The bo created in genX(CreateQueryPool) loses both flags.
>

That was unintentional.  I'll fix it.


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

Yup.  I'll update the commit mesage.


> -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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170523/58827bb8/attachment.html>