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

[Jason Ekstrand]

On Fri, Aug 4, 2017 at 1:43 AM, Chris Wilson <chris at chris-wilson.co.uk>
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(&device->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(&batch, GEN7_MI_BATCH_BUFFER_END, bbe);
> > +   anv_batch_emit(&batch, 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(&device->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 &pSubmits[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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170804/fb0f9abe/attachment.html>