Re: [PATCH 1/2] drm/blend: Account also the primary plane of the crtc for normalized_zpos

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jan 09, 2018 at 03:40:36PM +0200, Laurent Pinchart wrote:
> Hi Daniel,
> 
> On Tuesday, 9 January 2018 14:42:55 EET Daniel Vetter wrote:
> > On Fri, Dec 22, 2017 at 05:15:05PM +0200, Peter Ujfalusi wrote:
> > > On 2017-12-22 12:12, Ville Syrjälä wrote:
> > > > On Fri, Dec 22, 2017 at 11:16:47AM +0200, Tomi Valkeinen wrote:
> > > >> On 21/12/17 17:12, Ville Syrjälä wrote:
> > > >>>> If the userspace knows this, then it knows which primary plane is for
> > > >>>> which crtc, right?
> > > >>>> 
> > > >>>> And if it doesn't know this, then the userspace cannot associate any
> > > >>>> plane to any crtc (except what it configures itself).
> > > >>>> 
> > > >>>> So... If using legacy modesetting (and non-universal planes), there's
> > > >>>> no
> > > >>>> problem, primary planes are fixed and at low zpos. If using atomic
> > > >>>> modesetting and universal planes, there's really no (default)
> > > >>>> association between planes and crtcs, and "primary plane" doesn't
> > > >>>> have
> > > >>>> much meaning. Is that correct?
> > > >>>> 
> > > >>>> If so... Then I guess the atomic modesetting client essentially
> > > >>>> randomly
> > > >>>> picks which plane it uses as a "root plane" (if it even needs such),
> > > >>>> and
> > > >>>> which planes as overlay planes? If that's the case, then this patch
> > > >>>> doesn't quite fix the issue...
> > > >>> 
> > > >>> I'm not sure anyone has really thought how userspace would/should
> > > >>> assign
> > > >>> planes to crtcs. My only idea so far has been the crtc and their
> > > >>> preferred primary planes should be registered in the same order. But
> > > >>> someone should then also implement that same policy in userspace when
> > > >>> it's trying to figure out which plane to use. There are other
> > > >>> heuristics it should probably use, like preferring to pick a primary
> > > >>> plane for any fullscreen surface.
> > > >>> 
> > > >>> I guess from functional point of view it shouldn't matter which plane
> > > >>> you pick as long as the plane's user visible capabilities are
> > > >>> sufficient. But there might be user invisible power saving features
> > > >>> and
> > > >>> whatnot that only work with specific planes. So from that point of
> > > >>> view
> > > >>> maybe the order in which the planes get registered is important. Or we
> > > >>> could maybe come up with some kind of plane usage hints in the uapi
> > > >>> which could help userspace decide?
> > > >> 
> > > >> I was thinking about this, and... maybe there isn't even any problem
> > > >> (at
> > > >> least for OMAP). So lets say we set the default plane zpos to the plane
> > > >> index, and that the primary planes are reserved first (i.e. have lower
> > > >> zpos than overlay planes).
> > > >> 
> > > >> We have three different cases:
> > > >> 
> > > >> Legacy modesetting: No problems, primary plane is always at the bottom.
> > > >> If the userspace uses 2+ overlay planes, it can expect the zpos to be
> > > >> based on the plane id, or it can set the zpos.
> > > >> 
> > > >> Atomic+Universal, the application uses one primary plane, and zero or
> > > >> more overlay planes: No problems here, the situation is the same as for
> > > >> legacy.
> > > >> 
> > > >> Atomic+Universal, the application uses more than one primary plane, and
> > > >> zero or more overlay planes: in this case the app _has_ to manage zpos,
> > > >> because using two primary planes in a single screen, and expecting it
> > > >> to
> > > >> work by default, doesn't make sense.
> > > >> 
> > > >> If the above "rules" are valid, then there's no need for this patch.
> > > >> 
> > > >> But one question I don't have a good answer is that why would we want
> > > >> to
> > > >> normalize the zpos, instead of returning an error? If the above rules
> > > >> are valid, I think returning an error is better than normalizing and
> > > >> hiding the bad configuration.
> > > > 
> > > > IIRC I argued against the normalization, but some people really
> > > > wanted it for whatever reason.
> > > 
> > > OK, please ignore this series, I'll send a patch instead next year.
> > 
> > So now we end up with a bunch of kms drivers that normalize zpos, and a
> > bunch of others which rejects duplicated zpos.
> > 
> > That sounds even worse. Can we pls try to be consistent (even if it turns
> > out to be a not-so-great uapi decision, it's uapi, so let's not make
> > things worse by making it inconsistent).
> 
> For what it's worth, I'd tend to disallow duplicate zpos values. That forces 
> userspace to user atomic and to handle zpos explicitly, and that's exactly why 
> it's my preference as not handling zpos explicitly in userspace will lead to 
> random behaviour at best.

I don't care what we're going with tbf, except it should be consistent
across drivers ... Everyone just implementing their flavour of bikeshed in
their driver is kinda uncool.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel