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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 16, 2020 at 09:39:19PM +0300, Dmitry Osipenko wrote:
> 16.06.2020 21:14, Thierry Reding пишет:
> > From: Thierry Reding <treding@xxxxxxxxxx>
> > 
> > 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@xxxxxxxxxx>
> > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> > ---
> >  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.

Thierry

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux