Re: [RFC v2 1/8] dma-buf/fence: add fence_collection fences

[Daniel Vetter]

On Tue, Apr 26, 2016 at 12:02:08PM -0300, Gustavo Padovan wrote:
> 2016-04-26 Daniel Vetter <daniel@xxxxxxxx>:
> 
> > On Mon, Apr 25, 2016 at 07:33:21PM -0300, Gustavo Padovan wrote:
> > > From: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
> > > 
> > > struct fence_collection inherits from struct fence and carries a
> > > collection of fences that needs to be waited together.
> > > 
> > > It is useful to translate a sync_file to a fence to remove the complexity
> > > of dealing with sync_files on DRM drivers. So even if there are many
> > > fences in the sync_file that needs to waited for a commit to happen,
> > > they all get added to the fence_collection and passed for DRM use as
> > > a standard struct fence.
> > > 
> > > That means that no changes needed to any driver besides supporting fences.
> > > 
> > > fence_collection's fence doesn't belong to any timeline context, so
> > > fence_is_later() and fence_later() are not meant to be called with
> > > fence_collections fences.
> > > 
> > > v2: Comments by Daniel Vetter:
> > > 	- merge fence_collection_init() and fence_collection_add()
> > > 	- only add callbacks at ->enable_signalling()
> > > 	- remove fence_collection_put()
> > > 	- check for type on to_fence_collection()
> > > 	- adjust fence_is_later() and fence_later() to WARN_ON() if they
> > > 	are used with collection fences.
> > > 
> > > Signed-off-by: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
> > 
> > FENCE_NO_CONTEXT semantics needs an ack from amdgpu maintainers. I'm not
> > entirely sure they might not hit the new WARN_ON by accident now. Please
> > cc Alex Deucher & Christian König.
> 
> Sure, I'll Cc then in the revision. But if they use
> fence_context_alloc() to get the context they should never hit any
> WARN_ON as context numbers now starts at 1. 0 is reserved for
> FENCE_NO_CONTEXT.

I was more concerned whether the codepaths could accidentally walk over
non-amdgpu fences (through prime buffer sharing for example). Otoh that
would be a preexisting bug I think ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch