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

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3238 bytes --]

On Tue, Jun 16, 2020 at 06:54:35PM +0300, Dmitry Osipenko wrote:
> 16.06.2020 15:17, Thierry Reding пишет:
> > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > 
> > 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.
> > 
> > Changes in v2:
> > - hardcode cursor plane zpos to 255 instead of 0 (Ville)
> > 
> > Reported-by: Jonathan Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > ---
> >  drivers/gpu/drm/tegra/dc.c  | 9 +++++++--
> >  drivers/gpu/drm/tegra/hub.c | 2 +-
> >  2 files changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> > index 83f31c6e891c..85408eed4685 100644
> > --- a/drivers/gpu/drm/tegra/dc.c
> > +++ b/drivers/gpu/drm/tegra/dc.c
> > @@ -787,7 +787,7 @@ static struct drm_plane *tegra_primary_plane_create(struct drm_device *drm,
> >  	}
> >  
> >  	drm_plane_helper_add(&plane->base, &tegra_plane_helper_funcs);
> > -	drm_plane_create_zpos_property(&plane->base, plane->index, 0, 255);
> > +	drm_plane_create_zpos_property(&plane->base, plane->index, 0, 254);
> >  
> >  	err = drm_plane_create_rotation_property(&plane->base,
> >  						 DRM_MODE_ROTATE_0,
> > @@ -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;
> >  }
> > @@ -1074,7 +1075,11 @@ static struct drm_plane *tegra_dc_overlay_plane_create(struct drm_device *drm,
> >  	}
> >  
> >  	drm_plane_helper_add(&plane->base, &tegra_plane_helper_funcs);
> > -	drm_plane_create_zpos_property(&plane->base, plane->index, 0, 255);
> > +
> > +	if (!cursor)
> > +		drm_plane_create_zpos_property(&plane->base, plane->index, 0, 254);
> > +	else
> > +		drm_plane_create_zpos_immutable_property(&plane->base, 255);
> 
> On T20/30 we're are setting the plane's type to CURSOR because we want
> to use one overlay plane for the mouse cursor. Nevertheless, it's still
> a generic overlay plane that can change its z-position, and thus, it's
> wrong to make zpos immutable here.

But it doesn't really make sense for the cursor plane to change z-
position, even if it's technically possible. We do want it to always be
on top anyway. Doing it this way makes the cursor behave the same way
irrespective of the Tegra generation that we're running on.

Yes, that may not fully expose the capabilities of the hardware, but we
are already restricting the hardware capabilities by exposing the
overlay plane as a cursor plane in the first place.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]