Re: [Intel-gfx] [PATCH 3/4] drm/i915: peel dma-fence-chains wait fences

[Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com>]

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@intel.com>
> > 
> > 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@intel.com>
> > ---
> >   .../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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx