Re: [PATCH v5] drm/omap: plane zpos/zorder management improvements

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jan 10, 2018 at 06:28:51PM +0200, Peter Ujfalusi wrote:
> On 01/09/2018 04:40 PM, Daniel Vetter wrote:
> > On Tue, Jan 09, 2018 at 04:18:48PM +0200, Peter Ujfalusi wrote:
> >> Hi,
> >>
> >> On 2018-01-09 14:44, Daniel Vetter wrote:
> >>>> Changes since v4:
> >>>> - further simplify the zpos checking by using a mask and a single loop
> >>>> - Commit message has been extended
> >>>>
> >>>> Changes since v3:
> >>>> - Use drm_plane_index() instead of storing the same index wothin omap_plane
> >>>>   struct
> >>>> - Optimize the zpos validation loop so we avoid extra checks.
> >>>>
> >>>> Changes since v2:
> >>>> - The check for duplicate zpos is moved to omap_crtc
> >>>>
> >>>> Changes since v1:
> >>>> - Dropped the zpos normalization related patches
> >>>> - New patch based on the discussion, see commit message.
> >>>
> >>> Sorry for jumping in late to the party, but except when you have a really,
> >>> really, really good reason for why omapdrm has to not normalize zpos like
> >>> the other drivers do in this case, then I think we should be consistent.
> >>>
> >>> An inconsistent kms uapi is a lot worse than an uapi with some design
> >>> issues: The latter just means we eventually need v2, the former guarantees
> >>> we need v2.
> >>
> >> Even if the v2 contains the "drm/blend: Account also the primary plane
> >> of the crtc for normalized_zpos"?
> >> It is to ensure that the crtc->primary plane is going to have zpos = 0,
> >> even if it's plane_id is higher.
> > 
> > It's a bit a hack, but imo makes sense, given where we are with uapi.> Except it sounds like we have more bikesheds going on about what exactly
> > zpos is supposed to do.
> 
> As https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html have this to say
> about zpos:
> "priority of the given plane on crtc (optional) Note that multiple active
> planes on the same crtc can have an identical zpos value. The rule to solving
> the conflict is to compare the plane object IDs; the plane with a higher ID
> must be stacked on top of a plane with a lower ID."
> 
> It implies that the driver should not try to be clever about the normalization
> of the zpos. Even if it make sense.
> 
> Considering only crtc->primary is a bit flowed anyway as for the sake of
> completeness the crtc->cursor plane should have been kept on top at the same time.
> 
> >> As it was discussed we have use case when we have two (or more) crtcs,
> >> each with one plane (they are the primary planes for the given crtc) and
> >> user moves one of the plane from one crtc to another, where it is no
> >> longer the primary plane, but still holds zpos = 0.
> >>
> >> In this case we prefer to keep the actual primary plane of the crtc at
> >> the bottom and stack the new planes on top.
> > 
> > Yeah that all sounds reasonable. Or we state that userspace in that case
> > better readjust zpos to make it non-ambiguous. Or something else.
> > 
> > Just something that's consistent across drivers. I'm totally fine with
> > "organically grown uapi with lots of cruds and hacks".
> 
> I'm fine with using the current normalization as it is and refer to the UAPI
> doc if user space is not complying with it.
> But then, should the normalization be forced in the core for all drivers to
> get consistency?

We had that, but then removed it again for reasons I no longer entirely
understand. I guess we can keep it as-is for now, or if you want you can
float a patch to move it back into the main helpers.
-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