On 20/04/11 11:50, Lionel Landwerlin wrote:
> On 10/04/2020 19:51, Venkata Sandeep Dhanalakota wrote:
> > From: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
> >
> > To allow faster engine to engine synchronization, peel the layer of
> > dma-fence-chain to expose potential i915 fences so that the
> > i915-request code can emit HW semaphore wait/signal operations in the
> > ring which is faster than waking up the host to submit unblocked
> > workloads after interrupt notification.
> >
> > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
> > ---
> > .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 39 +++++++++++++++++--
> > 1 file changed, 35 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > index 8dd651cdca39..e43b76d7e9fd 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > @@ -2508,6 +2508,7 @@ await_fence_array(struct i915_execbuffer *eb,
> > for (n = 0; n < nfences; n++) {
> > struct drm_syncobj *syncobj;
> > + struct dma_fence_chain *chain;
> > unsigned int flags;
> > syncobj = ptr_unpack_bits(fences[n].syncobj, &flags, 2);
> > @@ -2515,10 +2516,40 @@ await_fence_array(struct i915_execbuffer *eb,
> > if (!fences[n].dma_fence)
> > continue;
> > - err = i915_request_await_dma_fence(eb->request,
> > - fences[n].dma_fence);
> > - if (err < 0)
> > - return err;
> > + /*
> > + * If we're dealing with a dma-fence-chain, peel the chain by
> > + * adding all of the unsignaled fences
> > + * (dma_fence_chain_for_each does that for us) the chain
> > + * points to.
> > + *
> > + * This enables us to identify waits on i915 fences and allows
> > + * for faster engine-to-engine synchronization using HW
> > + * semaphores.
> > + */
> > + chain = to_dma_fence_chain(fences[n].dma_fence);
> > + if (chain) {
> > + struct dma_fence *iter;
> > +
> > + dma_fence_chain_for_each(iter, fences[n].dma_fence) {
>
>
> The kbuild bot made me think of an interesting case.
>
> It is possible to build a chain where the first element isn't a
> dma_fence_chain.
>
Yes agreed, we could have a valid fence-chain with first element as normal
dma_fence and so iter_chain can be null. Will address this in next
revision of the patch.
>
> We should handle this here like this :
>
>
> if (iter_chain)
>
> err = i915_request_await_dma_fence(eb->request, iter_chain->fence);
>
> else
>
> err = i915_request_await_dma_fence(eb->request, iter);
>
> if (err < 0) {
>
> dma_fence_put(iter);
>
> return err;
>
> }
>
>
> > + struct dma_fence_chain *iter_chain =
> > + to_dma_fence_chain(iter);
> > +
> > + GEM_BUG_ON(!iter_chain);
> > +
> > + err = i915_request_await_dma_fence(eb->request,
> > + iter_chain->fence);
> > + if (err < 0) {
> > + dma_fence_put(iter);
> > + return err;
> > + }
> > + }
> > +
> > + } else {
> > + err = i915_request_await_dma_fence(eb->request,
> > + fences[n].dma_fence);
> > + if (err < 0)
> > + return err;
> > + }
> > }
> > return 0;
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
