Re: [RFC v2 5/8] drm/fence: add in-fences support

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Apr 26, 2016 at 07:20:49PM +0200, Daniel Vetter wrote:
> On Tue, Apr 26, 2016 at 07:26:21PM +0300, Ville Syrjälä wrote:
> > On Tue, Apr 26, 2016 at 04:36:36PM +0200, Daniel Vetter wrote:
> > > On Tue, Apr 26, 2016 at 11:14:22AM -0300, Gustavo Padovan wrote:
> > > > 2016-04-26 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> > > > 
> > > > > On Mon, Apr 25, 2016 at 07:33:25PM -0300, Gustavo Padovan wrote:
> > > > > > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > > > > > 
> > > > > > There is now a new property called FENCE_FD attached to every plane
> > > > > > state that receives the sync_file fd from userspace via the atomic commit
> > > > > > IOCTL.
> > > > > 
> > > > > I still don't like this property abuse. Also with atomic, all passed
> > > > > fences must be waited upon before anything is done, so attaching them
> > > > > to planes seems like it might just give people the wrong idea.
> > > > 
> > > > I'm actually fine with this as property, but another solutions is use
> > > > an array of {plane, fence_fd} and extend drm_atomic_ioctl args just like
> > > > we have done for out fences. However the FENCE_FD property is easier to
> > > > handle in userspace than the array. Any other idea?
> > > 
> > > Imo FENCE_FD is perfectly fine. But what's the concern around giving
> > > people the wrong idea with attaching fences to planes? For nonblocking
> > > commits we need to store them somewhere for the worker, drm_plane_state
> > > seems like an as good place as any other.
> > 
> > It gives the impression that each plane might flip as soon as its fence
> > signals.
> 
> That wouldn't be atomic. Not sure how someone could come up with that
> idea.

What else would it mean? It's attached to a specific plane, so why would
it affect other planes?

> I mean we could move FENCE_FD to the crtc (fence fds can be merged),
> but that's just a needless difference to what hwc expects. I think
> aligning with the only real-world users in this case here makes sense.

Well it doesn't belong on the crtc either. I would just stick in the
ioctl as a separate thing, then it's clear it's related to the whole
operation rather than any kms object.

> 
> Plus docs in case someone has funny ideas.

Weren't you just quoting rusty's API manifesto recently? ;)

Maybe it was someone else.

-- 
Ville Syrjälä
Intel OTC