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

[Tomi Valkeinen]

On 08/01/18 12:43, Laurent Pinchart wrote:
> 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 

I think they are broken already (on omapdrm): the driver doesn't deal
with this at the moment in any way, which leads to undefined behavior. I
may remember wrong, but I think I have seen sync losts connected to bad
z setup.

But you are right, it's also possible that nothing bad seems to happen,
if the only side effect is just that a plane disappears for a moment.

And if this behavior for non-atocic apps is normal, and other drivers
allow it, then I do agree that we have to normalize instead of returning
an error.

> 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.

Yes, this is a subject I have complained a few times: DRM keeping the
state. I think that will be a problem with many other properties too. An
app could change a platform specific property, which no other app is
aware of, and after that no other app would work correctly.

I believe each app has to know all the DRM properties and set them
accordingly on startup, and/or each app has to reset the properties when
exiting. Unfortunately I think both cases are not realistic.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki