On Tue, Apr 26, 2016 at 09:55:06PM +0300, Ville Syrjälä wrote:
On Tue, Apr 26, 2016 at 08:23:46PM +0200, Daniel Vetter wrote:
On Tue, Apr 26, 2016 at 08:40:45PM +0300, Ville Syrjälä wrote:
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.
We want it per-crtc I'd say, so that you could flip each crtc individually.
Then you could just issue multiple ioctls. For eg. those nasty 4k MST display (or just otherwise neatly synced displayes) you want to wait for all the fences upfront and the flip everything at once, otherwise you'll get nasty tears at the seams.
But really the reason for per-plane is hw composer from Android. I don't see any point in designing an api that's needlessly different from what the main user expects (even if it may be silly).
What are they doing that can't stuff the fences into an array instead of props?
The hw composer interface is one in-fence per plane. That's really the major reason why the kernel interface is built to match. And I really don't think we should diverge just because we have a slight different color preference ;-)
As long as you end up with a pile of fences somehow it'll work. -Daniel