| Message ID | 20200616121713.2983627-1-thierry.reding@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/tegra: Add zpos property for cursor planes | expand |
16.06.2020 15:17, Thierry Reding пишет: > From: Thierry Reding <treding@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. > > Changes in v2: > - hardcode cursor plane zpos to 255 instead of 0 (Ville) > > Reported-by: Jonathan Hunter <jonathanh@nvidia.com> > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > 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.
On Tue, Jun 16, 2020 at 06:54:35PM +0300, Dmitry Osipenko wrote: > 16.06.2020 15:17, Thierry Reding пишет: > > From: Thierry Reding <treding@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. > > > > Changes in v2: > > - hardcode cursor plane zpos to 255 instead of 0 (Ville) > > > > Reported-by: Jonathan Hunter <jonathanh@nvidia.com> > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > --- > > 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
16.06.2020 19:13, Thierry Reding пишет: > On Tue, Jun 16, 2020 at 06:54:35PM +0300, Dmitry Osipenko wrote: >> 16.06.2020 15:17, Thierry Reding пишет: >>> From: Thierry Reding <treding@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. >>> >>> Changes in v2: >>> - hardcode cursor plane zpos to 255 instead of 0 (Ville) >>> >>> Reported-by: Jonathan Hunter <jonathanh@nvidia.com> >>> Signed-off-by: Thierry Reding <treding@nvidia.com> >>> --- >>> 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. Userspace is free to reuse cursor's plane for different purposes. For example, cursor may be not needed at all and then cursor plane could be used as a regular overlay plane. The patch's title and commit's description says "*Add* zpos property for cursor planes", but in this case zpos property already existed before this patch, hence you're changing the old behavior here.
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); err = drm_plane_create_rotation_property(&plane->base, DRM_MODE_ROTATE_0, diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c index 22a03f7ffdc1..f8afc05bceb3 100644 --- a/drivers/gpu/drm/tegra/hub.c +++ b/drivers/gpu/drm/tegra/hub.c @@ -590,7 +590,7 @@ struct drm_plane *tegra_shared_plane_create(struct drm_device *drm, } drm_plane_helper_add(p, &tegra_shared_plane_helper_funcs); - drm_plane_create_zpos_property(p, 0, 0, 255); + drm_plane_create_zpos_property(p, index, 0, 254); return p; }