[PATCH 1/3] drm/etnaviv: submit support for in-fences

[Daniel Vetter]

On Fri, Mar 17, 2017 at 11:00:44AM -0300, Gustavo Padovan wrote:
> 2017-03-16 Philipp Zabel <p.zabel at pengutronix.de>:
> 
> > Hi Gustavo,
> > 
> > On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote:
> > [...]
> > > I was thinking on some function that would iterate over all fences in
> > > the fence_array and check their context. The if we find our own gpu
> > > context in there we fail the submit.
> > 
> > Why would we have to fail if somebody feeds us our own fences? Wouldn't
> > it be enough to just wait if there are foreign fences in the array?
> 
> You don't need to fail necessarily. In my mind I had the use case that
> maybe some driver could deadlock waiting for his own fence. What you
> do with the information that the array has it a fence with the driver's
> context is entirely up to the driver to decide.

With the current infrastructure you can't have future fences (i.e. a fence
for something that might happen somewhen in the future, but for which no
driver is yet committed to execute - i.e. it's not yet queued). And
without future fences you can't ever have deadlocks.

The "are these all my own fences" check is more useful just to import the
fences into your own internal objects and use your driver-internal depency
handling (which usually means, more hw, less cpu waiting). But that's
entirely orthogonal to "can we deadlock", which atm is "no"[1].
-Daniel

1: You can deadlock when e.g. one driver holds a lock while waiting for a
fence, and another driver needs that lock before it is willing to signal
said fence. But that's not any different from waiting for anything else in
the kernel, and will be checked by the cross-release lockdep stuff rsn.

> 
> > 
> > How about something like this:
> > 
> > ----------8<----------
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > index 364fe50d020de..0b0bdaf4406d4 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > @@ -296,6 +296,22 @@ static void submit_cleanup(struct etnaviv_gem_submit *submit)
> >  	kfree(submit);
> >  }
> >  
> > +static bool dma_fence_all_in_context(struct dma_fence *fence, u64 context)
> > +{
> > +	if (dma_fence_is_array(fence)) {
> > +		struct dma_fence_array *array = to_dma_fence_array(fence);
> > +		int i;
> > +
> > +		for (i = 0; i < array->num_fences; i++) {
> > +			if (array->fences[i]->context != context)
> > +				return false;
> > +		}
> > +		return true;
> > +	}
> > +
> > +	return fence->context == context;
> > +}
> 
> If we don't mind having fences with our own context, why should we check
> them?
> 
> Gustavo
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch