Re: [Mesa-dev] [PATCH v2 2/9] anv: Submit a dummy batch when only semaphores are provided.

2017-08-04 Thread Jason Ekstrand 
On Fri, Aug 4, 2017 at 1:43 AM, Chris Wilson 
wrote:

> Quoting Jason Ekstrand (2017-08-04 02:25:21)
> > Vulkan allows you to do a submit whose only job is to wait on and
> > trigger semaphores.  The easiest way for us to support that right
> > now is to insert a dummy execbuf.
> > ---
> > diff --git a/src/intel/vulkan/anv_device.c
> b/src/intel/vulkan/anv_device.c
> > index 793e519..0f0aa22 100644
> > --- a/src/intel/vulkan/anv_device.c
> > +++ b/src/intel/vulkan/anv_device.c
> > @@ -1014,6 +1014,28 @@ anv_device_init_border_colors(struct anv_device
> *device)
> >  border_colors);
> >  }
> >
> > +static void
> > +anv_device_init_trivial_batch(struct anv_device *device)
> > +{
> > +   anv_bo_init_new(>trivial_batch_bo, device, 4096);
>
> Is this created with ASYNC?


No, it isn't but I'm happy to set the flag.  This patch predates the ASYNC
stuff, I believe.

Just thinking that you only want the
> external ordering constraints on this bo, and not accidentally serialize
> between contexts.
>

Is this really an issue?  No other process will ever see this BO.  I
suppose the kernel is still doing unneeded flushing but this shouldn't
cause cross-context synchronization.


> > +   void *map = anv_gem_mmap(device, device->trivial_batch_bo.gem_
> handle,
> > +0, 4096, 0);
> > +
> > +   struct anv_batch batch = {
> > +  .start = map,
> > +  .next = map,
> > +  .end = map + 4096,
> > +   };
> > +
> > +   anv_batch_emit(, GEN7_MI_BATCH_BUFFER_END, bbe);
> > +   anv_batch_emit(, GEN7_MI_NOOP, noop);
> > +
> > +   if (!device->info.has_llc)
> > +  gen_clflush_range(map, batch.next - map);
> > +
> > +   anv_gem_munmap(map, device->trivial_batch_bo.size);
> > +}
>
> > diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> private.h
> > index b599db3..bc67bb6 100644
> > --- a/src/intel/vulkan/anv_private.h
> > +++ b/src/intel/vulkan/anv_private.h
> > @@ -745,6 +745,7 @@ struct anv_device {
> >  struct anv_state_pool   surface_state_pool;
> >
> >  struct anv_bo   workaround_bo;
> > +struct anv_bo   trivial_batch_bo;
>
> Do you use all 4096 bytes of the workaround_bo, or could you spare 64?
> ;)
>

I could... Then again, I can also easily spare a single 4K page per context
and prevent myself from accidentally overwriting my little batch. :-)


> > diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c
> > index 446c3de..9934fef 100644
> > --- a/src/intel/vulkan/anv_queue.c
> > +++ b/src/intel/vulkan/anv_queue.c
> > @@ -159,6 +159,23 @@ VkResult anv_QueueSubmit(
> > pthread_mutex_lock(>mutex);
> >
> > for (uint32_t i = 0; i < submitCount; i++) {
> > +  if (pSubmits[i].commandBufferCount == 0) {
> > + /* If we don't have any command buffers, we need to submit a
> dummy
> > +  * batch to give GEM something to wait on.  We could,
> potentially,
> > +  * come up with something more efficient but this shouldn't be
> a
> > +  * common case.
> > +  */
> > + result = anv_cmd_buffer_execbuf(device, NULL,
> > + pSubmits[i].pWaitSemaphores,
> > + pSubmits[i].
> waitSemaphoreCount,
> > + pSubmits[i].pSignalSemaphores,
> > + pSubmits[i].
> signalSemaphoreCount);
>
> Might as well just pass [i] along?
>

I can't.  See below where we only pass the wait semaphores to the first
execbuf in the batch and only pass the signal semaphores to the last.

--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 2/9] anv: Submit a dummy batch when only semaphores are provided.

2017-08-04 Thread Chris Wilson 
Quoting Jason Ekstrand (2017-08-04 02:25:21)
> Vulkan allows you to do a submit whose only job is to wait on and
> trigger semaphores.  The easiest way for us to support that right
> now is to insert a dummy execbuf.
> ---
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index 793e519..0f0aa22 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -1014,6 +1014,28 @@ anv_device_init_border_colors(struct anv_device 
> *device)
>  border_colors);
>  }
>  
> +static void
> +anv_device_init_trivial_batch(struct anv_device *device)
> +{
> +   anv_bo_init_new(>trivial_batch_bo, device, 4096);

Is this created with ASYNC? Just thinking that you only want the
external ordering constraints on this bo, and not accidentally serialize
between contexts.

> +   void *map = anv_gem_mmap(device, device->trivial_batch_bo.gem_handle,
> +0, 4096, 0);
> +
> +   struct anv_batch batch = {
> +  .start = map,
> +  .next = map,
> +  .end = map + 4096,
> +   };
> +
> +   anv_batch_emit(, GEN7_MI_BATCH_BUFFER_END, bbe);
> +   anv_batch_emit(, GEN7_MI_NOOP, noop);
> +
> +   if (!device->info.has_llc)
> +  gen_clflush_range(map, batch.next - map);
> +
> +   anv_gem_munmap(map, device->trivial_batch_bo.size);
> +}

> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> index b599db3..bc67bb6 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -745,6 +745,7 @@ struct anv_device {
>  struct anv_state_pool   surface_state_pool;
>  
>  struct anv_bo   workaround_bo;
> +struct anv_bo   trivial_batch_bo;

Do you use all 4096 bytes of the workaround_bo, or could you spare 64?
;)

> diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c
> index 446c3de..9934fef 100644
> --- a/src/intel/vulkan/anv_queue.c
> +++ b/src/intel/vulkan/anv_queue.c
> @@ -159,6 +159,23 @@ VkResult anv_QueueSubmit(
> pthread_mutex_lock(>mutex);
>  
> for (uint32_t i = 0; i < submitCount; i++) {
> +  if (pSubmits[i].commandBufferCount == 0) {
> + /* If we don't have any command buffers, we need to submit a dummy
> +  * batch to give GEM something to wait on.  We could, potentially,
> +  * come up with something more efficient but this shouldn't be a
> +  * common case.
> +  */
> + result = anv_cmd_buffer_execbuf(device, NULL,
> + pSubmits[i].pWaitSemaphores,
> + pSubmits[i].waitSemaphoreCount,
> + pSubmits[i].pSignalSemaphores,
> + pSubmits[i].signalSemaphoreCount);

Might as well just pass [i] along?
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2 2/9] anv: Submit a dummy batch when only semaphores are provided.

2017-08-03 Thread Jason Ekstrand 
Vulkan allows you to do a submit whose only job is to wait on and
trigger semaphores.  The easiest way for us to support that right
now is to insert a dummy execbuf.
---
 src/intel/vulkan/anv_batch_chain.c | 28 +---
 src/intel/vulkan/anv_device.c  | 26 ++
 src/intel/vulkan/anv_private.h |  1 +
 src/intel/vulkan/anv_queue.c   | 17 +
 4 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/src/intel/vulkan/anv_batch_chain.c 
b/src/intel/vulkan/anv_batch_chain.c
index 94e7a7d..65fe366 100644
--- a/src/intel/vulkan/anv_batch_chain.c
+++ b/src/intel/vulkan/anv_batch_chain.c
@@ -1388,6 +1388,23 @@ setup_execbuf_for_cmd_buffer(struct anv_execbuf *execbuf,
return VK_SUCCESS;
 }
 
+static void
+setup_empty_execbuf(struct anv_execbuf *execbuf, struct anv_device *device)
+{
+   anv_execbuf_add_bo(execbuf, >trivial_batch_bo, NULL, 0,
+  >alloc);
+
+   execbuf->execbuf = (struct drm_i915_gem_execbuffer2) {
+  .buffers_ptr = (uintptr_t) execbuf->objects,
+  .buffer_count = execbuf->bo_count,
+  .batch_start_offset = 0,
+  .batch_len = 8, /* GEN8_MI_BATCH_BUFFER_END and NOOP */
+  .flags = I915_EXEC_HANDLE_LUT | I915_EXEC_RENDER,
+  .rsvd1 = device->context_id,
+  .rsvd2 = 0,
+   };
+}
+
 VkResult
 anv_cmd_buffer_execbuf(struct anv_device *device,
struct anv_cmd_buffer *cmd_buffer,
@@ -1447,9 +1464,14 @@ anv_cmd_buffer_execbuf(struct anv_device *device,
   }
}
 
-   result = setup_execbuf_for_cmd_buffer(, cmd_buffer);
-   if (result != VK_SUCCESS)
-  return result;
+   if (cmd_buffer) {
+  result = setup_execbuf_for_cmd_buffer(, cmd_buffer);
+  if (result != VK_SUCCESS)
+ return result;
+   } else {
+  setup_empty_execbuf(, device);
+   }
+
 
result = anv_device_execbuf(device, , execbuf.bos);
 
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index 793e519..0f0aa22 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -1014,6 +1014,28 @@ anv_device_init_border_colors(struct anv_device *device)
 border_colors);
 }
 
+static void
+anv_device_init_trivial_batch(struct anv_device *device)
+{
+   anv_bo_init_new(>trivial_batch_bo, device, 4096);
+   void *map = anv_gem_mmap(device, device->trivial_batch_bo.gem_handle,
+0, 4096, 0);
+
+   struct anv_batch batch = {
+  .start = map,
+  .next = map,
+  .end = map + 4096,
+   };
+
+   anv_batch_emit(, GEN7_MI_BATCH_BUFFER_END, bbe);
+   anv_batch_emit(, GEN7_MI_NOOP, noop);
+
+   if (!device->info.has_llc)
+  gen_clflush_range(map, batch.next - map);
+
+   anv_gem_munmap(map, device->trivial_batch_bo.size);
+}
+
 VkResult anv_CreateDevice(
 VkPhysicalDevicephysicalDevice,
 const VkDeviceCreateInfo*   pCreateInfo,
@@ -1131,6 +1153,8 @@ VkResult anv_CreateDevice(
if (result != VK_SUCCESS)
   goto fail_surface_state_pool;
 
+   anv_device_init_trivial_batch(device);
+
anv_scratch_pool_init(device, >scratch_pool);
 
anv_queue_init(device, >queue);
@@ -1220,6 +1244,8 @@ void anv_DestroyDevice(
anv_gem_munmap(device->workaround_bo.map, device->workaround_bo.size);
anv_gem_close(device, device->workaround_bo.gem_handle);
 
+   anv_gem_close(device, device->trivial_batch_bo.gem_handle);
+
anv_state_pool_finish(>surface_state_pool);
anv_state_pool_finish(>instruction_state_pool);
anv_state_pool_finish(>dynamic_state_pool);
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index b599db3..bc67bb6 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -745,6 +745,7 @@ struct anv_device {
 struct anv_state_pool   surface_state_pool;
 
 struct anv_bo   workaround_bo;
+struct anv_bo   trivial_batch_bo;
 
 struct anv_pipeline_cache   blorp_shader_cache;
 struct blorp_contextblorp;
diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c
index 446c3de..9934fef 100644
--- a/src/intel/vulkan/anv_queue.c
+++ b/src/intel/vulkan/anv_queue.c
@@ -159,6 +159,23 @@ VkResult anv_QueueSubmit(
pthread_mutex_lock(>mutex);
 
for (uint32_t i = 0; i < submitCount; i++) {
+  if (pSubmits[i].commandBufferCount == 0) {
+ /* If we don't have any command buffers, we need to submit a dummy
+  * batch to give GEM something to wait on.  We could, potentially,
+  * come up with something more efficient but this shouldn't be a
+  * common case.
+  */
+ result = anv_cmd_buffer_execbuf(device, NULL,
+ pSubmits[i].pWaitSemaphores,
+