On Tue, May 23, 2017 at 2:35 PM, Nanley Chery <nanleych...@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
