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

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>,
	jsarha@ti.com, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v4] drm/omap: plane zpos/zorder management improvements
Date: Mon, 08 Jan 2018 12:43:57 +0200	[thread overview]
Message-ID: <4624608.KaQvfX4sdg@avalon> (raw)
In-Reply-To: <03b90f1a-a8c3-d0e7-786c-9f78d6ca49d2@ti.com>

Hello,

On Monday, 8 January 2018 10:59:29 EET Tomi Valkeinen wrote:
> On 08/01/18 10:20, Peter Ujfalusi wrote:
> > On 2018-01-05 16:04, Laurent Pinchart wrote:
> >> On Friday, 5 January 2018 13:30:37 EET Peter Ujfalusi wrote:
> >>> Use the plane index as default zpos for all planes. Even if the
> >>> application is not setting zpos/zorder explicitly we will have unique
> >>> zpos for each plane.
> >>> 
> >>> Enforce that all planes must have unique zpos on the given crtc.
> >> 
> >> Could you explain the rationale for that in the commit message, what's
> >> wrong with duplicate zpos values ?
> > 
> > Planes with identical zpos is only 'valid' _if_ they are not
> > overlapping, if they do overlap then it is - imho - not a valid
> > configuration anyway (which one should be on top?).
> 
> For DSS it's clear. It is an invalid HW configuration to have multiple
> planes with the same zpos in the same crtc. I believe the result is
> undefined HW behavior.
> 
> So we either return an error, or the kernel normalizes zpos'es.
> Normalizing means the kernel is guessing what the end result should be,
> so I like error better.

I wouldn't call that guessing :-) Duplicate zpos values result in plane being 
sorted based on the plane ID (this is obviously implementation-dependent, I 
mean this is the currently implemented behaviour), which I don't think is an 
issue in itself. A userspace zpos API that resolves conflicting zpos values 
that way isn't a broken API, even if its behaviour might be considered a bit 
complex or cumbersome.

I'm not against forbidding duplicate zpos values, but I think the rationale 
should be captured in the kernel message.

There's also a risk of breaking non-atomic userspace (as explained in my 
previous e-mail), as well as atomic userspace that doesn't set zpos explicitly 
if run after an application that changed the zpos values. True, that would 
today result in an undefined behaviour, so this might not be considered a 
problem.

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel