[PATCH v3] drm/tegra: Add zpos property for cursor planes

[Dmitry Osipenko]

17.06.2020 17:10, Thierry Reding пишет:
> On Tue, Jun 16, 2020 at 09:39:19PM +0300, Dmitry Osipenko wrote:
>> 16.06.2020 21:14, Thierry Reding пишет:
>>> From: Thierry Reding <treding at nvidia.com>
>>>
>>> As of commit 4dc55525b095 ("drm: plane: Verify that no or all planes
>>> have a zpos property") a warning is emitted if there's a mix of planes
>>> with and without a zpos property.
>>>
>>> On Tegra, cursor planes are always composited on top of all other
>>> planes, which is why they never had a zpos property attached to them.
>>> However, since the composition order is fixed, this is trivial to
>>> remedy by simply attaching an immutable zpos property to them.
>>>
>>> v3: do not hardcode zpos for overlay planes used as cursor (Dmitry)
>>> v2: hardcode cursor plane zpos to 255 instead of 0 (Ville)
>>>
>>> Reported-by: Jonathan Hunter <jonathanh at nvidia.com>
>>> Signed-off-by: Thierry Reding <treding at nvidia.com>
>>> ---
>>>  drivers/gpu/drm/tegra/dc.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>>> index 83f31c6e891c..04d6848d19fc 100644
>>> --- a/drivers/gpu/drm/tegra/dc.c
>>> +++ b/drivers/gpu/drm/tegra/dc.c
>>> @@ -957,6 +957,7 @@ static struct drm_plane *tegra_dc_cursor_plane_create(struct drm_device *drm,
>>>  	}
>>>  
>>>  	drm_plane_helper_add(&plane->base, &tegra_cursor_plane_helper_funcs);
>>> +	drm_plane_create_zpos_immutable_property(&plane->base, 255);
>>>  
>>>  	return &plane->base;
>>>  }
>>>
>>
>> Looks nice, thanks! Since you dropped all other zpos changes for other
>> planes and given that the other planes have 255 for the max zpos, what
>> about to set the cursor's zpos to 256?
> 
> I'd prefer to have all of the planes' zpos within the same range. By
> default the other planes will be on the very bottom end of that range
> and the atomic core will normalize the zpos for all planes anyway, so
> the cursor plane will end up with a very small normalized zpos in the
> end.
> 
> The zpos documentation already mentions that the behaviour is undefined
> if two planes have the same zpos value, so I think userspace is going to
> know how to set these anyway.
> 
> It might be worth to do a follow-up patch that is smarter about setting
> the range of valid values. 0-255 is true on later chips where we
> actually have a proper "layer depth" register field and I wanted this to
> be uniform across all generations. Other drivers seem to set the upper
> limit on the zpos range to be equal to the number of planes available,
> so that there aren't potentially big gaps in the numbering. That said,
> since the core already normalizes the zpos for us, I don't see a big
> benefit in explicitly clipping the range.

But the cursor plane doesn't use the "layer depth" register, doesn't it?
So the zpos over 255 shouldn't matter in this case.

I know that DRM should normalizes the given zpos, but still it makes me
a bit uncomfortable knowing that there are the ranges overlap visible to
userspace :)