[PATCH 00/23] drm: Fill in default values for plane properties

[PATCH 00/23] drm: Fill in default values for plane properties

[Maxime Ripard]

Hi,

We have a bunch of functions that create planes properties that will take a
default value, but it isn't actually enforced in the plane state.

This leads to drivers having multiple strategies to work around that issue,
most of them being a variation of forcing a value at plane state reset time.
Others work fine by luck, or have entirely ignored the issue.

This series aims at making sure the default value set by the call to the
function isn't ignored, and then making sure all drivers behave consistently.

Let me know what you think,
Maxime

Dave Stevenson (3):
  drm/object: Add drm_object_property_get_default_value() function
  drm/object: Add default zpos value at reset
  drm/object: Add default color encoding and range value at reset

Maxime Ripard (20):
  drm/komeda: plane: switch to plane reset helper
  drm/tegra: plane: switch to plane reset helper
  drm/tegra: hub: Fix zpos initial value mismatch
  drm/msm/mdp5: Fix zpos initial value mismatch
  drm/amd/display: Fix color encoding mismatch
  drm/tegra: plane: Remove redundant zpos initialisation
  drm/komeda: plane: Remove redundant zpos initialisation
  drm/exynos: plane: Remove redundant zpos initialisation
  drm/imx: ipuv3-plane: Remove redundant zpos initialisation
  drm/msm/mdp5: Remove redundant zpos initialisation
  drm/nouveau/kms: Remove redundant zpos initialisation
  drm/omap: plane: Fix zpos initial value mismatch
  drm/omap: plane: Remove redundant zpos initialisation
  drm/rcar: plane: Remove redundant zpos initialisation
  drm/sti: plane: Remove redundant zpos initialisation
  drm/sun4i: layer: Remove redundant zpos initialisation
  drm/komeda: plane: Remove redundant color encoding and range
    initialisation
  drm/armada: overlay: Remove redundant color encoding and range
    initialisation
  drm/imx: ipuv3-plane: Remove redundant color encoding and range
    initialisation
  drm/omap: plane: Remove redundant color encoding and range
    initialisation

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  2 +-
 .../gpu/drm/arm/display/komeda/komeda_plane.c | 13 +----
 drivers/gpu/drm/armada/armada_overlay.c       |  2 -
 drivers/gpu/drm/drm_atomic_state_helper.c     | 25 +++++++++
 drivers/gpu/drm/drm_mode_object.c             | 53 +++++++++++++++----
 drivers/gpu/drm/exynos/exynos_drm_plane.c     |  5 +-
 drivers/gpu/drm/imx/ipuv3-plane.c             |  8 +--
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c    | 16 +++---
 drivers/gpu/drm/nouveau/dispnv50/wndw.c       |  2 -
 drivers/gpu/drm/omapdrm/omap_plane.c          | 22 ++++----
 drivers/gpu/drm/rcar-du/rcar_du_plane.c       |  1 -
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c         |  1 -
 drivers/gpu/drm/sti/sti_cursor.c              |  2 +-
 drivers/gpu/drm/sti/sti_gdp.c                 |  2 +-
 drivers/gpu/drm/sti/sti_hqvdp.c               |  2 +-
 drivers/gpu/drm/sti/sti_plane.c               |  6 ---
 drivers/gpu/drm/sti/sti_plane.h               |  1 -
 drivers/gpu/drm/sun4i/sun4i_layer.c           | 16 +++---
 drivers/gpu/drm/tegra/hub.c                   |  2 +-
 drivers/gpu/drm/tegra/plane.c                 |  6 +--
 include/drm/drm_mode_object.h                 |  7 +++
 21 files changed, 111 insertions(+), 83 deletions(-)

-- 
2.34.1

[PATCH 01/23] drm/komeda: plane: switch to plane reset helper

[Maxime Ripard]

komeda_plane_reset() does the state initialisation by copying a lot of
the code found in the __drm_atomic_helper_plane_reset(). Let's switch to
that helper and reduce the boilerplate.

Cc: Brian Starkey <brian.starkey@arm.com>
Cc: "James (Qian) Wang" <james.qian.wang@arm.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Mihail Atanassov <mihail.atanassov@arm.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
index d63d83800a8a..1778f6e1ea56 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
@@ -145,14 +145,10 @@ static void komeda_plane_reset(struct drm_plane *plane)
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
 	if (state) {
-		state->base.rotation = DRM_MODE_ROTATE_0;
-		state->base.pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
-		state->base.alpha = DRM_BLEND_ALPHA_OPAQUE;
+		__drm_atomic_helper_plane_reset(plane, &state->base);
 		state->base.zpos = kplane->layer->base.id;
 		state->base.color_encoding = DRM_COLOR_YCBCR_BT601;
 		state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE;
-		plane->state = &state->base;
-		plane->state->plane = plane;
 	}
 }
 
-- 
2.34.1

[PATCH 02/23] drm/tegra: plane: switch to plane reset helper

tegra_plane_reset() does the state initialisation by copying a lot of
the code found in the __drm_atomic_helper_plane_reset(). Let's switch to
that helper and reduce the boilerplate.

Cc: linux-tegra@vger.kernel.org
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/tegra/plane.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index 321cb1f13da6..ec0822c86926 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -37,8 +37,7 @@ static void tegra_plane_reset(struct drm_plane *plane)
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
 	if (state) {
-		plane->state = &state->base;
-		plane->state->plane = plane;
+		__drm_atomic_helper_plane_reset(plane, &state->base);
 		plane->state->zpos = p->index;
 		plane->state->normalized_zpos = p->index;
 
-- 
2.34.1

[PATCH 02/23] drm/tegra: plane: switch to plane reset helper

[Maxime Ripard]

tegra_plane_reset() does the state initialisation by copying a lot of
the code found in the __drm_atomic_helper_plane_reset(). Let's switch to
that helper and reduce the boilerplate.

Cc: linux-tegra@vger.kernel.org
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/tegra/plane.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index 321cb1f13da6..ec0822c86926 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -37,8 +37,7 @@ static void tegra_plane_reset(struct drm_plane *plane)
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
 	if (state) {
-		plane->state = &state->base;
-		plane->state->plane = plane;
+		__drm_atomic_helper_plane_reset(plane, &state->base);
 		plane->state->zpos = p->index;
 		plane->state->normalized_zpos = p->index;
 
-- 
2.34.1

[PATCH 03/23] drm/tegra: hub: Fix zpos initial value mismatch

[Maxime Ripard]

While the tegra_shared_plane_create() function calls
drm_plane_create_zpos_property() with an initial value of 0,
tegra_plane_reset() will force it to the plane index.

Fix the discrepancy by setting the initial zpos value to the plane index
in the function call.

Cc: linux-tegra@vger.kernel.org
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/tegra/hub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
index b910155f80c4..7f68a142d922 100644
--- a/drivers/gpu/drm/tegra/hub.c
+++ b/drivers/gpu/drm/tegra/hub.c
@@ -788,7 +788,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, 255);
 
 	return p;
 }
-- 
2.34.1

[PATCH 03/23] drm/tegra: hub: Fix zpos initial value mismatch

[Maxime Ripard]

While the tegra_shared_plane_create() function calls
drm_plane_create_zpos_property() with an initial value of 0,
tegra_plane_reset() will force it to the plane index.

Fix the discrepancy by setting the initial zpos value to the plane index
in the function call.

Cc: linux-tegra@vger.kernel.org
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/tegra/hub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
index b910155f80c4..7f68a142d922 100644
--- a/drivers/gpu/drm/tegra/hub.c
+++ b/drivers/gpu/drm/tegra/hub.c
@@ -788,7 +788,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, 255);
 
 	return p;
 }
-- 
2.34.1

[PATCH 04/23] drm/msm/mdp5: Fix zpos initial value mismatch

[Maxime Ripard]

While the mdp5_plane_install_properties() function calls
drm_plane_create_zpos_property() with an initial value of 1,
mdp5_plane_reset() will force it to another value depending on the plane
type.

Fix the discrepancy by setting the initial zpos value to the same value
in the drm_plane_create_zpos_property() call.

Cc: freedreno@lists.freedesktop.org
Cc: linux-arm-msm@vger.kernel.org
Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Sean Paul <sean@poorly.run>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index c6b69afcbac8..d60982f4bd11 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -48,6 +48,8 @@ static void mdp5_plane_destroy(struct drm_plane *plane)
 static void mdp5_plane_install_properties(struct drm_plane *plane,
 		struct drm_mode_object *obj)
 {
+	unsigned int zpos;
+
 	drm_plane_create_rotation_property(plane,
 					   DRM_MODE_ROTATE_0,
 					   DRM_MODE_ROTATE_0 |
@@ -59,7 +61,12 @@ static void mdp5_plane_install_properties(struct drm_plane *plane,
 			BIT(DRM_MODE_BLEND_PIXEL_NONE) |
 			BIT(DRM_MODE_BLEND_PREMULTI) |
 			BIT(DRM_MODE_BLEND_COVERAGE));
-	drm_plane_create_zpos_property(plane, 1, 1, 255);
+
+	if (plane->type == DRM_PLANE_TYPE_PRIMARY)
+		zpos = STAGE_BASE;
+	else
+		zpos = STAGE0 + drm_plane_index(plane);
+	drm_plane_create_zpos_property(plane, zpos, 1, 255);
 }
 
 static void
-- 
2.34.1

[PATCH 04/23] drm/msm/mdp5: Fix zpos initial value mismatch

[Maxime Ripard]

While the mdp5_plane_install_properties() function calls
drm_plane_create_zpos_property() with an initial value of 1,
mdp5_plane_reset() will force it to another value depending on the plane
type.

Fix the discrepancy by setting the initial zpos value to the same value
in the drm_plane_create_zpos_property() call.

Cc: freedreno@lists.freedesktop.org
Cc: linux-arm-msm@vger.kernel.org
Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Sean Paul <sean@poorly.run>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index c6b69afcbac8..d60982f4bd11 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -48,6 +48,8 @@ static void mdp5_plane_destroy(struct drm_plane *plane)
 static void mdp5_plane_install_properties(struct drm_plane *plane,
 		struct drm_mode_object *obj)
 {
+	unsigned int zpos;
+
 	drm_plane_create_rotation_property(plane,
 					   DRM_MODE_ROTATE_0,
 					   DRM_MODE_ROTATE_0 |
@@ -59,7 +61,12 @@ static void mdp5_plane_install_properties(struct drm_plane *plane,
 			BIT(DRM_MODE_BLEND_PIXEL_NONE) |
 			BIT(DRM_MODE_BLEND_PREMULTI) |
 			BIT(DRM_MODE_BLEND_COVERAGE));
-	drm_plane_create_zpos_property(plane, 1, 1, 255);
+
+	if (plane->type == DRM_PLANE_TYPE_PRIMARY)
+		zpos = STAGE_BASE;
+	else
+		zpos = STAGE0 + drm_plane_index(plane);
+	drm_plane_create_zpos_property(plane, zpos, 1, 255);
 }
 
 static void
-- 
2.34.1

Re: [PATCH 04/23] drm/msm/mdp5: Fix zpos initial value mismatch

[Dmitry Baryshkov]

On Mon, 7 Feb 2022 at 19:56, Maxime Ripard <maxime@cerno.tech> wrote:
>
> While the mdp5_plane_install_properties() function calls
> drm_plane_create_zpos_property() with an initial value of 1,
> mdp5_plane_reset() will force it to another value depending on the plane
> type.
>
> Fix the discrepancy by setting the initial zpos value to the same value
> in the drm_plane_create_zpos_property() call.

Could you please squash two msm/mdp5 patches into a single patch,
making it clear that the code is moved.

Also please add:
Fixes: 7d36db0be3b9 ("drm/msm/mdp5: switch to standard zpos property")

With that in place:
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

>
> Cc: freedreno@lists.freedesktop.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Sean Paul <sean@poorly.run>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
> index c6b69afcbac8..d60982f4bd11 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
> @@ -48,6 +48,8 @@ static void mdp5_plane_destroy(struct drm_plane *plane)
>  static void mdp5_plane_install_properties(struct drm_plane *plane,
>                 struct drm_mode_object *obj)
>  {
> +       unsigned int zpos;
> +
>         drm_plane_create_rotation_property(plane,
>                                            DRM_MODE_ROTATE_0,
>                                            DRM_MODE_ROTATE_0 |
> @@ -59,7 +61,12 @@ static void mdp5_plane_install_properties(struct drm_plane *plane,
>                         BIT(DRM_MODE_BLEND_PIXEL_NONE) |
>                         BIT(DRM_MODE_BLEND_PREMULTI) |
>                         BIT(DRM_MODE_BLEND_COVERAGE));
> -       drm_plane_create_zpos_property(plane, 1, 1, 255);
> +
> +       if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> +               zpos = STAGE_BASE;
> +       else
> +               zpos = STAGE0 + drm_plane_index(plane);
> +       drm_plane_create_zpos_property(plane, zpos, 1, 255);
>  }
>
>  static void
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 04/23] drm/msm/mdp5: Fix zpos initial value mismatch

[Dmitry Baryshkov]

On Mon, 7 Feb 2022 at 19:56, Maxime Ripard <maxime@cerno.tech> wrote:
>
> While the mdp5_plane_install_properties() function calls
> drm_plane_create_zpos_property() with an initial value of 1,
> mdp5_plane_reset() will force it to another value depending on the plane
> type.
>
> Fix the discrepancy by setting the initial zpos value to the same value
> in the drm_plane_create_zpos_property() call.

Could you please squash two msm/mdp5 patches into a single patch,
making it clear that the code is moved.

Also please add:
Fixes: 7d36db0be3b9 ("drm/msm/mdp5: switch to standard zpos property")

With that in place:
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

>
> Cc: freedreno@lists.freedesktop.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Sean Paul <sean@poorly.run>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
> index c6b69afcbac8..d60982f4bd11 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
> @@ -48,6 +48,8 @@ static void mdp5_plane_destroy(struct drm_plane *plane)
>  static void mdp5_plane_install_properties(struct drm_plane *plane,
>                 struct drm_mode_object *obj)
>  {
> +       unsigned int zpos;
> +
>         drm_plane_create_rotation_property(plane,
>                                            DRM_MODE_ROTATE_0,
>                                            DRM_MODE_ROTATE_0 |
> @@ -59,7 +61,12 @@ static void mdp5_plane_install_properties(struct drm_plane *plane,
>                         BIT(DRM_MODE_BLEND_PIXEL_NONE) |
>                         BIT(DRM_MODE_BLEND_PREMULTI) |
>                         BIT(DRM_MODE_BLEND_COVERAGE));
> -       drm_plane_create_zpos_property(plane, 1, 1, 255);
> +
> +       if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> +               zpos = STAGE_BASE;
> +       else
> +               zpos = STAGE0 + drm_plane_index(plane);
> +       drm_plane_create_zpos_property(plane, zpos, 1, 255);
>  }
>
>  static void
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 04/23] drm/msm/mdp5: Fix zpos initial value mismatch

[Maxime Ripard]

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

Hi,

On Mon, Feb 07, 2022 at 10:27:24PM +0300, Dmitry Baryshkov wrote:
> On Mon, 7 Feb 2022 at 19:56, Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > While the mdp5_plane_install_properties() function calls
> > drm_plane_create_zpos_property() with an initial value of 1,
> > mdp5_plane_reset() will force it to another value depending on the plane
> > type.
> >
> > Fix the discrepancy by setting the initial zpos value to the same value
> > in the drm_plane_create_zpos_property() call.
> 
> Could you please squash two msm/mdp5 patches into a single patch,
> making it clear that the code is moved.
> 
> Also please add:
> Fixes: 7d36db0be3b9 ("drm/msm/mdp5: switch to standard zpos property")

If we are to merge both patches, we can't have a fixes tag, since it
relies on the other framework patches that won't get backported.

Maxime

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

Re: [PATCH 04/23] drm/msm/mdp5: Fix zpos initial value mismatch

[Maxime Ripard]

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

Hi,

On Mon, Feb 07, 2022 at 10:27:24PM +0300, Dmitry Baryshkov wrote:
> On Mon, 7 Feb 2022 at 19:56, Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > While the mdp5_plane_install_properties() function calls
> > drm_plane_create_zpos_property() with an initial value of 1,
> > mdp5_plane_reset() will force it to another value depending on the plane
> > type.
> >
> > Fix the discrepancy by setting the initial zpos value to the same value
> > in the drm_plane_create_zpos_property() call.
> 
> Could you please squash two msm/mdp5 patches into a single patch,
> making it clear that the code is moved.
> 
> Also please add:
> Fixes: 7d36db0be3b9 ("drm/msm/mdp5: switch to standard zpos property")

If we are to merge both patches, we can't have a fixes tag, since it
relies on the other framework patches that won't get backported.

Maxime

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

[PATCH 05/23] drm/amd/display: Fix color encoding mismatch

[Maxime Ripard]

The amdgpu KMS driver calls drm_plane_create_color_properties() with a
default encoding set to BT709.

However, the core will ignore it and the driver doesn't force it in its
plane state reset hook, so the initial value will be 0, which represents
BT601.

Fix the mismatch by using an initial value of BT601 in
drm_plane_create_color_properties().

Cc: amd-gfx@lists.freedesktop.org
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index feccf2b555d2..86b27a355e90 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7914,7 +7914,7 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
 			BIT(DRM_COLOR_YCBCR_BT2020),
 			BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
 			BIT(DRM_COLOR_YCBCR_FULL_RANGE),
-			DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
+			DRM_COLOR_YCBCR_BT601, DRM_COLOR_YCBCR_LIMITED_RANGE);
 	}
 
 	supported_rotations =
-- 
2.34.1

[PATCH 05/23] drm/amd/display: Fix color encoding mismatch

[Maxime Ripard]

The amdgpu KMS driver calls drm_plane_create_color_properties() with a
default encoding set to BT709.

However, the core will ignore it and the driver doesn't force it in its
plane state reset hook, so the initial value will be 0, which represents
BT601.

Fix the mismatch by using an initial value of BT601 in
drm_plane_create_color_properties().

Cc: amd-gfx@lists.freedesktop.org
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index feccf2b555d2..86b27a355e90 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7914,7 +7914,7 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
 			BIT(DRM_COLOR_YCBCR_BT2020),
 			BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
 			BIT(DRM_COLOR_YCBCR_FULL_RANGE),
-			DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
+			DRM_COLOR_YCBCR_BT601, DRM_COLOR_YCBCR_LIMITED_RANGE);
 	}
 
 	supported_rotations =
-- 
2.34.1

Re: [PATCH 05/23] drm/amd/display: Fix color encoding mismatch

[Harry Wentland]

On 2022-02-07 11:34, Maxime Ripard wrote:
> The amdgpu KMS driver calls drm_plane_create_color_properties() with a
> default encoding set to BT709.
> 
> However, the core will ignore it and the driver doesn't force it in its
> plane state reset hook, so the initial value will be 0, which represents
> BT601.
> 

Isn't this a core issue? Should __drm_atomic_helper_plane_state_reset
reset all plane_state members to their properties' default values?

The amdgpu KMS driver currently doesn't respect the color_encoding
property but I would expect that a call to drm_plane_create_color_properties
with a default of BT709 means we're getting BT709 as color_encoding 
as part of atomic commits.

Harry

> Fix the mismatch by using an initial value of BT601 in
> drm_plane_create_color_properties().
> 
> Cc: amd-gfx@lists.freedesktop.org
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index feccf2b555d2..86b27a355e90 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -7914,7 +7914,7 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
>  			BIT(DRM_COLOR_YCBCR_BT2020),
>  			BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
>  			BIT(DRM_COLOR_YCBCR_FULL_RANGE),
> -			DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
> +			DRM_COLOR_YCBCR_BT601, DRM_COLOR_YCBCR_LIMITED_RANGE);
>  	}
>  
>  	supported_rotations =

Re: [PATCH 05/23] drm/amd/display: Fix color encoding mismatch

[Harry Wentland]

On 2022-02-07 11:34, Maxime Ripard wrote:
> The amdgpu KMS driver calls drm_plane_create_color_properties() with a
> default encoding set to BT709.
> 
> However, the core will ignore it and the driver doesn't force it in its
> plane state reset hook, so the initial value will be 0, which represents
> BT601.
> 

Isn't this a core issue? Should __drm_atomic_helper_plane_state_reset
reset all plane_state members to their properties' default values?

The amdgpu KMS driver currently doesn't respect the color_encoding
property but I would expect that a call to drm_plane_create_color_properties
with a default of BT709 means we're getting BT709 as color_encoding 
as part of atomic commits.

Harry

> Fix the mismatch by using an initial value of BT601 in
> drm_plane_create_color_properties().
> 
> Cc: amd-gfx@lists.freedesktop.org
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index feccf2b555d2..86b27a355e90 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -7914,7 +7914,7 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
>  			BIT(DRM_COLOR_YCBCR_BT2020),
>  			BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
>  			BIT(DRM_COLOR_YCBCR_FULL_RANGE),
> -			DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
> +			DRM_COLOR_YCBCR_BT601, DRM_COLOR_YCBCR_LIMITED_RANGE);
>  	}
>  
>  	supported_rotations =

Re: [PATCH 05/23] drm/amd/display: Fix color encoding mismatch

[Harry Wentland]

On 2022-02-07 13:57, Harry Wentland wrote:
> On 2022-02-07 11:34, Maxime Ripard wrote:
>> The amdgpu KMS driver calls drm_plane_create_color_properties() with a
>> default encoding set to BT709.
>>
>> However, the core will ignore it and the driver doesn't force it in its
>> plane state reset hook, so the initial value will be 0, which represents
>> BT601.
>>
> 
> Isn't this a core issue? Should __drm_atomic_helper_plane_state_reset
> reset all plane_state members to their properties' default values?
> 

Ah, looks like that's exactly what you do in the later patches, which is
perfect. With that, I don't think you'll need this patch anymore.

Harry

> The amdgpu KMS driver currently doesn't respect the color_encoding
> property but I would expect that a call to drm_plane_create_color_properties
> with a default of BT709 means we're getting BT709 as color_encoding 
> as part of atomic commits.
> 
> Harry
> 
>> Fix the mismatch by using an initial value of BT601 in
>> drm_plane_create_color_properties().
>>
>> Cc: amd-gfx@lists.freedesktop.org
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: "Christian König" <christian.koenig@amd.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Cc: Leo Li <sunpeng.li@amd.com>
>> Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
>> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
>> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
>> ---
>>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index feccf2b555d2..86b27a355e90 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -7914,7 +7914,7 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
>>  			BIT(DRM_COLOR_YCBCR_BT2020),
>>  			BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
>>  			BIT(DRM_COLOR_YCBCR_FULL_RANGE),
>> -			DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
>> +			DRM_COLOR_YCBCR_BT601, DRM_COLOR_YCBCR_LIMITED_RANGE);
>>  	}
>>  
>>  	supported_rotations =
> 

Re: [PATCH 05/23] drm/amd/display: Fix color encoding mismatch

[Harry Wentland]

On 2022-02-07 13:57, Harry Wentland wrote:
> On 2022-02-07 11:34, Maxime Ripard wrote:
>> The amdgpu KMS driver calls drm_plane_create_color_properties() with a
>> default encoding set to BT709.
>>
>> However, the core will ignore it and the driver doesn't force it in its
>> plane state reset hook, so the initial value will be 0, which represents
>> BT601.
>>
> 
> Isn't this a core issue? Should __drm_atomic_helper_plane_state_reset
> reset all plane_state members to their properties' default values?
> 

Ah, looks like that's exactly what you do in the later patches, which is
perfect. With that, I don't think you'll need this patch anymore.

Harry

> The amdgpu KMS driver currently doesn't respect the color_encoding
> property but I would expect that a call to drm_plane_create_color_properties
> with a default of BT709 means we're getting BT709 as color_encoding 
> as part of atomic commits.
> 
> Harry
> 
>> Fix the mismatch by using an initial value of BT601 in
>> drm_plane_create_color_properties().
>>
>> Cc: amd-gfx@lists.freedesktop.org
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: "Christian König" <christian.koenig@amd.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Cc: Leo Li <sunpeng.li@amd.com>
>> Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
>> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
>> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
>> ---
>>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index feccf2b555d2..86b27a355e90 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -7914,7 +7914,7 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
>>  			BIT(DRM_COLOR_YCBCR_BT2020),
>>  			BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
>>  			BIT(DRM_COLOR_YCBCR_FULL_RANGE),
>> -			DRM_COLOR_YCBCR_BT709, DRM_COLOR_YCBCR_LIMITED_RANGE);
>> +			DRM_COLOR_YCBCR_BT601, DRM_COLOR_YCBCR_LIMITED_RANGE);
>>  	}
>>  
>>  	supported_rotations =
> 

Re: [PATCH 05/23] drm/amd/display: Fix color encoding mismatch

[Maxime Ripard]

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

Hi Harry,

On Mon, Feb 07, 2022 at 01:59:38PM -0500, Harry Wentland wrote:
> On 2022-02-07 13:57, Harry Wentland wrote:
> > On 2022-02-07 11:34, Maxime Ripard wrote:
> >> The amdgpu KMS driver calls drm_plane_create_color_properties() with a
> >> default encoding set to BT709.
> >>
> >> However, the core will ignore it and the driver doesn't force it in its
> >> plane state reset hook, so the initial value will be 0, which represents
> >> BT601.
> >>
> > 
> > Isn't this a core issue? Should __drm_atomic_helper_plane_state_reset
> > reset all plane_state members to their properties' default values?
> > 
> 
> Ah, looks like that's exactly what you do in the later patches, which is
> perfect. With that, I don't think you'll need this patch anymore.

Ok, I'll squash it into the patch that removes the reset code.

Thanks!
Maxime

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

Re: [PATCH 05/23] drm/amd/display: Fix color encoding mismatch

[Maxime Ripard]

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

Hi Harry,

On Mon, Feb 07, 2022 at 01:59:38PM -0500, Harry Wentland wrote:
> On 2022-02-07 13:57, Harry Wentland wrote:
> > On 2022-02-07 11:34, Maxime Ripard wrote:
> >> The amdgpu KMS driver calls drm_plane_create_color_properties() with a
> >> default encoding set to BT709.
> >>
> >> However, the core will ignore it and the driver doesn't force it in its
> >> plane state reset hook, so the initial value will be 0, which represents
> >> BT601.
> >>
> > 
> > Isn't this a core issue? Should __drm_atomic_helper_plane_state_reset
> > reset all plane_state members to their properties' default values?
> > 
> 
> Ah, looks like that's exactly what you do in the later patches, which is
> perfect. With that, I don't think you'll need this patch anymore.

Ok, I'll squash it into the patch that removes the reset code.

Thanks!
Maxime

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

Re: [PATCH 05/23] drm/amd/display: Fix color encoding mismatch

[Harry Wentland]

On 2022-02-10 03:42, Maxime Ripard wrote:
> Hi Harry,
> 
> On Mon, Feb 07, 2022 at 01:59:38PM -0500, Harry Wentland wrote:
>> On 2022-02-07 13:57, Harry Wentland wrote:
>>> On 2022-02-07 11:34, Maxime Ripard wrote:
>>>> The amdgpu KMS driver calls drm_plane_create_color_properties() with a
>>>> default encoding set to BT709.
>>>>
>>>> However, the core will ignore it and the driver doesn't force it in its
>>>> plane state reset hook, so the initial value will be 0, which represents
>>>> BT601.
>>>>
>>>
>>> Isn't this a core issue? Should __drm_atomic_helper_plane_state_reset
>>> reset all plane_state members to their properties' default values?
>>>
>>
>> Ah, looks like that's exactly what you do in the later patches, which is
>> perfect. With that, I don't think you'll need this patch anymore.
> 
> Ok, I'll squash it into the patch that removes the reset code.
> 

I don't think that's right. I think we can just drop this patch.
The amdgpu display driver is not doing BT601 by default.

Harry

> Thanks!
> Maxime

Re: [PATCH 05/23] drm/amd/display: Fix color encoding mismatch

[Harry Wentland]

On 2022-02-10 03:42, Maxime Ripard wrote:
> Hi Harry,
> 
> On Mon, Feb 07, 2022 at 01:59:38PM -0500, Harry Wentland wrote:
>> On 2022-02-07 13:57, Harry Wentland wrote:
>>> On 2022-02-07 11:34, Maxime Ripard wrote:
>>>> The amdgpu KMS driver calls drm_plane_create_color_properties() with a
>>>> default encoding set to BT709.
>>>>
>>>> However, the core will ignore it and the driver doesn't force it in its
>>>> plane state reset hook, so the initial value will be 0, which represents
>>>> BT601.
>>>>
>>>
>>> Isn't this a core issue? Should __drm_atomic_helper_plane_state_reset
>>> reset all plane_state members to their properties' default values?
>>>
>>
>> Ah, looks like that's exactly what you do in the later patches, which is
>> perfect. With that, I don't think you'll need this patch anymore.
> 
> Ok, I'll squash it into the patch that removes the reset code.
> 

I don't think that's right. I think we can just drop this patch.
The amdgpu display driver is not doing BT601 by default.

Harry

> Thanks!
> Maxime

Re: [PATCH 05/23] drm/amd/display: Fix color encoding mismatch

[Maxime Ripard]

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

Hi Harry,

On Thu, Feb 10, 2022 at 09:38:24AM -0500, Harry Wentland wrote:
> On 2022-02-10 03:42, Maxime Ripard wrote:
> > On Mon, Feb 07, 2022 at 01:59:38PM -0500, Harry Wentland wrote:
> >> On 2022-02-07 13:57, Harry Wentland wrote:
> >>> On 2022-02-07 11:34, Maxime Ripard wrote:
> >>>> The amdgpu KMS driver calls drm_plane_create_color_properties() with a
> >>>> default encoding set to BT709.
> >>>>
> >>>> However, the core will ignore it and the driver doesn't force it in its
> >>>> plane state reset hook, so the initial value will be 0, which represents
> >>>> BT601.
> >>>>
> >>>
> >>> Isn't this a core issue? Should __drm_atomic_helper_plane_state_reset
> >>> reset all plane_state members to their properties' default values?
> >>>
> >>
> >> Ah, looks like that's exactly what you do in the later patches, which is
> >> perfect. With that, I don't think you'll need this patch anymore.
> > 
> > Ok, I'll squash it into the patch that removes the reset code.
> > 
> 
> I don't think that's right. I think we can just drop this patch.
> The amdgpu display driver is not doing BT601 by default.

My understanding from the code currently in tree is that:

1) amdgpu_dm_plane_init() will call drm_plane_create_color_properties()
   with an initial value set to BT709.

2) dm_drm_plane_reset() will use kzalloc and then just rely on
   __drm_atomic_helper_plane_reset(), which will not set the color encoding
   at all. It's thus 0 in the initial state.

3) the drm_color_encoding enum will have BT601 associated to 0

So it does look like the default for amdgpu at the moment is BT601?

Maxime

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

Re: [PATCH 05/23] drm/amd/display: Fix color encoding mismatch

[Maxime Ripard]

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

Hi Harry,

On Thu, Feb 10, 2022 at 09:38:24AM -0500, Harry Wentland wrote:
> On 2022-02-10 03:42, Maxime Ripard wrote:
> > On Mon, Feb 07, 2022 at 01:59:38PM -0500, Harry Wentland wrote:
> >> On 2022-02-07 13:57, Harry Wentland wrote:
> >>> On 2022-02-07 11:34, Maxime Ripard wrote:
> >>>> The amdgpu KMS driver calls drm_plane_create_color_properties() with a
> >>>> default encoding set to BT709.
> >>>>
> >>>> However, the core will ignore it and the driver doesn't force it in its
> >>>> plane state reset hook, so the initial value will be 0, which represents
> >>>> BT601.
> >>>>
> >>>
> >>> Isn't this a core issue? Should __drm_atomic_helper_plane_state_reset
> >>> reset all plane_state members to their properties' default values?
> >>>
> >>
> >> Ah, looks like that's exactly what you do in the later patches, which is
> >> perfect. With that, I don't think you'll need this patch anymore.
> > 
> > Ok, I'll squash it into the patch that removes the reset code.
> > 
> 
> I don't think that's right. I think we can just drop this patch.
> The amdgpu display driver is not doing BT601 by default.

My understanding from the code currently in tree is that:

1) amdgpu_dm_plane_init() will call drm_plane_create_color_properties()
   with an initial value set to BT709.

2) dm_drm_plane_reset() will use kzalloc and then just rely on
   __drm_atomic_helper_plane_reset(), which will not set the color encoding
   at all. It's thus 0 in the initial state.

3) the drm_color_encoding enum will have BT601 associated to 0

So it does look like the default for amdgpu at the moment is BT601?

Maxime

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

[PATCH 06/23] drm/object: Add drm_object_property_get_default_value() function

[Maxime Ripard]

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

Some functions to create properties (drm_plane_create_zpos_property or
drm_plane_create_color_properties for example) will ask for a range of
acceptable value and an initial one.

This initial value is then stored in the values array for that property.

Let's provide an helper to access this property.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/drm_mode_object.c | 53 +++++++++++++++++++++++++------
 include/drm/drm_mode_object.h     |  7 ++++
 2 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index 86d9e907c0b2..ba1608effc0f 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -297,11 +297,26 @@ int drm_object_property_set_value(struct drm_mode_object *obj,
 }
 EXPORT_SYMBOL(drm_object_property_set_value);
 
+static int __drm_object_property_get_prop_value(struct drm_mode_object *obj,
+						struct drm_property *property,
+						uint64_t *val)
+{
+	int i;
+
+	for (i = 0; i < obj->properties->count; i++) {
+		if (obj->properties->properties[i] == property) {
+			*val = obj->properties->values[i];
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
 static int __drm_object_property_get_value(struct drm_mode_object *obj,
 					   struct drm_property *property,
 					   uint64_t *val)
 {
-	int i;
 
 	/* read-only properties bypass atomic mechanism and still store
 	 * their value in obj->properties->values[].. mostly to avoid
@@ -311,15 +326,7 @@ static int __drm_object_property_get_value(struct drm_mode_object *obj,
 			!(property->flags & DRM_MODE_PROP_IMMUTABLE))
 		return drm_atomic_get_property(obj, property, val);
 
-	for (i = 0; i < obj->properties->count; i++) {
-		if (obj->properties->properties[i] == property) {
-			*val = obj->properties->values[i];
-			return 0;
-		}
-
-	}
-
-	return -EINVAL;
+	return __drm_object_property_get_prop_value(obj, property, val);
 }
 
 /**
@@ -348,6 +355,32 @@ int drm_object_property_get_value(struct drm_mode_object *obj,
 }
 EXPORT_SYMBOL(drm_object_property_get_value);
 
+/**
+ * drm_object_property_get_default_value - retrieve the default value of a
+ * property when in atomic mode.
+ * @obj: drm mode object to get property value from
+ * @property: property to retrieve
+ * @val: storage for the property value
+ *
+ * This function retrieves the default state of the given property as passed in
+ * to drm_object_attach_property
+ *
+ * Only atomic drivers should call this function directly, as for non-atomic
+ * drivers it will return the current value.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_object_property_get_default_value(struct drm_mode_object *obj,
+					  struct drm_property *property,
+					  uint64_t *val)
+{
+	WARN_ON(!drm_drv_uses_atomic_modeset(property->dev));
+
+	return __drm_object_property_get_prop_value(obj, property, val);
+}
+EXPORT_SYMBOL(drm_object_property_get_default_value);
+
 /* helper for getconnector and getproperties ioctls */
 int drm_mode_object_get_properties(struct drm_mode_object *obj, bool atomic,
 				   uint32_t __user *prop_ptr,
diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
index c34a3e8030e1..912f1e415685 100644
--- a/include/drm/drm_mode_object.h
+++ b/include/drm/drm_mode_object.h
@@ -98,6 +98,10 @@ struct drm_object_properties {
 	 * Hence atomic drivers should not use drm_object_property_set_value()
 	 * and drm_object_property_get_value() on mutable objects, i.e. those
 	 * without the DRM_MODE_PROP_IMMUTABLE flag set.
+	 *
+	 * For atomic drivers the default value of properties is stored in this
+	 * array, so drm_object_property_get_default_value can be used to
+	 * retrieve it.
 	 */
 	uint64_t values[DRM_OBJECT_MAX_PROPERTY];
 };
@@ -126,6 +130,9 @@ int drm_object_property_set_value(struct drm_mode_object *obj,
 int drm_object_property_get_value(struct drm_mode_object *obj,
 				  struct drm_property *property,
 				  uint64_t *value);
+int drm_object_property_get_default_value(struct drm_mode_object *obj,
+					  struct drm_property *property,
+					  uint64_t *val);
 
 void drm_object_attach_property(struct drm_mode_object *obj,
 				struct drm_property *property,
-- 
2.34.1

Re: [PATCH 06/23] drm/object: Add drm_object_property_get_default_value() function

[Laurent Pinchart]

Hi Maxime and Dave,

Thank you for the patch.

On Mon, Feb 07, 2022 at 05:34:58PM +0100, Maxime Ripard wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> Some functions to create properties (drm_plane_create_zpos_property or
> drm_plane_create_color_properties for example) will ask for a range of
> acceptable value and an initial one.
> 
> This initial value is then stored in the values array for that property.
> 
> Let's provide an helper to access this property.
> 
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/drm_mode_object.c | 53 +++++++++++++++++++++++++------
>  include/drm/drm_mode_object.h     |  7 ++++
>  2 files changed, 50 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index 86d9e907c0b2..ba1608effc0f 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -297,11 +297,26 @@ int drm_object_property_set_value(struct drm_mode_object *obj,
>  }
>  EXPORT_SYMBOL(drm_object_property_set_value);
>  
> +static int __drm_object_property_get_prop_value(struct drm_mode_object *obj,
> +						struct drm_property *property,
> +						uint64_t *val)
> +{
> +	int i;
> +
> +	for (i = 0; i < obj->properties->count; i++) {
> +		if (obj->properties->properties[i] == property) {
> +			*val = obj->properties->values[i];
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  static int __drm_object_property_get_value(struct drm_mode_object *obj,
>  					   struct drm_property *property,
>  					   uint64_t *val)
>  {
> -	int i;
>  
>  	/* read-only properties bypass atomic mechanism and still store
>  	 * their value in obj->properties->values[].. mostly to avoid
> @@ -311,15 +326,7 @@ static int __drm_object_property_get_value(struct drm_mode_object *obj,
>  			!(property->flags & DRM_MODE_PROP_IMMUTABLE))
>  		return drm_atomic_get_property(obj, property, val);
>  
> -	for (i = 0; i < obj->properties->count; i++) {
> -		if (obj->properties->properties[i] == property) {
> -			*val = obj->properties->values[i];
> -			return 0;
> -		}
> -
> -	}
> -
> -	return -EINVAL;
> +	return __drm_object_property_get_prop_value(obj, property, val);
>  }
>  
>  /**
> @@ -348,6 +355,32 @@ int drm_object_property_get_value(struct drm_mode_object *obj,
>  }
>  EXPORT_SYMBOL(drm_object_property_get_value);
>  
> +/**
> + * drm_object_property_get_default_value - retrieve the default value of a
> + * property when in atomic mode.
> + * @obj: drm mode object to get property value from
> + * @property: property to retrieve
> + * @val: storage for the property value
> + *
> + * This function retrieves the default state of the given property as passed in
> + * to drm_object_attach_property
> + *
> + * Only atomic drivers should call this function directly, as for non-atomic
> + * drivers it will return the current value.
> + *
> + * Returns:
> + * Zero on success, error code on failure.
> + */
> +int drm_object_property_get_default_value(struct drm_mode_object *obj,
> +					  struct drm_property *property,
> +					  uint64_t *val)
> +{
> +	WARN_ON(!drm_drv_uses_atomic_modeset(property->dev));
> +
> +	return __drm_object_property_get_prop_value(obj, property, val);
> +}
> +EXPORT_SYMBOL(drm_object_property_get_default_value);
> +
>  /* helper for getconnector and getproperties ioctls */
>  int drm_mode_object_get_properties(struct drm_mode_object *obj, bool atomic,
>  				   uint32_t __user *prop_ptr,
> diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
> index c34a3e8030e1..912f1e415685 100644
> --- a/include/drm/drm_mode_object.h
> +++ b/include/drm/drm_mode_object.h
> @@ -98,6 +98,10 @@ struct drm_object_properties {
>  	 * Hence atomic drivers should not use drm_object_property_set_value()
>  	 * and drm_object_property_get_value() on mutable objects, i.e. those
>  	 * without the DRM_MODE_PROP_IMMUTABLE flag set.
> +	 *
> +	 * For atomic drivers the default value of properties is stored in this
> +	 * array, so drm_object_property_get_default_value can be used to
> +	 * retrieve it.
>  	 */
>  	uint64_t values[DRM_OBJECT_MAX_PROPERTY];
>  };
> @@ -126,6 +130,9 @@ int drm_object_property_set_value(struct drm_mode_object *obj,
>  int drm_object_property_get_value(struct drm_mode_object *obj,
>  				  struct drm_property *property,
>  				  uint64_t *value);
> +int drm_object_property_get_default_value(struct drm_mode_object *obj,
> +					  struct drm_property *property,
> +					  uint64_t *val);
>  
>  void drm_object_attach_property(struct drm_mode_object *obj,
>  				struct drm_property *property,
> -- 
> 2.34.1
> 

-- 
Regards,

Laurent Pinchart

[PATCH 07/23] drm/object: Add default zpos value at reset

[Maxime Ripard]

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

The drm_plane_create_zpos_property() function asks for an initial value,
and will set the associated plane state variable with that value if a
state is present.

However, that function is usually called at a time where there's no
state yet. Since the drm_plane_state reset helper doesn't take care of
reading that value when it's called, it means that in most cases the
initial value will be 0, or the drivers will have to work around it.

Let's add some code in __drm_atomic_helper_plane_state_reset() to get
the initial zpos value if the property has been attached in order to fix
this.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index ddcf5c2c8e6a..1412cee404ca 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -243,11 +243,22 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
 void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state,
 					   struct drm_plane *plane)
 {
+	u64 val;
+
 	plane_state->plane = plane;
 	plane_state->rotation = DRM_MODE_ROTATE_0;
 
 	plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
 	plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
+
+	if (plane->zpos_property) {
+		if (!drm_object_property_get_default_value(&plane->base,
+							   plane->zpos_property,
+							   &val)) {
+			plane_state->zpos = val;
+			plane_state->normalized_zpos = val;
+		}
+	}
 }
 EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset);
 
-- 
2.34.1

Re: [PATCH 07/23] drm/object: Add default zpos value at reset

[Harry Wentland]

On 2022-02-07 11:34, Maxime Ripard wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> The drm_plane_create_zpos_property() function asks for an initial value,
> and will set the associated plane state variable with that value if a
> state is present.
> 
> However, that function is usually called at a time where there's no
> state yet. Since the drm_plane_state reset helper doesn't take care of
> reading that value when it's called, it means that in most cases the
> initial value will be 0, or the drivers will have to work around it.
> 
> Let's add some code in __drm_atomic_helper_plane_state_reset() to get
> the initial zpos value if the property has been attached in order to fix
> this.
> 
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index ddcf5c2c8e6a..1412cee404ca 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -243,11 +243,22 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
>  void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state,
>  					   struct drm_plane *plane)
>  {
> +	u64 val;
> +
>  	plane_state->plane = plane;
>  	plane_state->rotation = DRM_MODE_ROTATE_0;
>  
>  	plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
>  	plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> +
> +	if (plane->zpos_property) {
> +		if (!drm_object_property_get_default_value(&plane->base,
> +							   plane->zpos_property,
> +							   &val)) {
> +			plane_state->zpos = val;
> +			plane_state->normalized_zpos = val;
> +		}
> +	}
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset);
>  

Re: [PATCH 07/23] drm/object: Add default zpos value at reset

[Laurent Pinchart]

Hi Maxime and Dave,

Thank you for the patch.

On Mon, Feb 07, 2022 at 05:34:59PM +0100, Maxime Ripard wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> The drm_plane_create_zpos_property() function asks for an initial value,
> and will set the associated plane state variable with that value if a
> state is present.
> 
> However, that function is usually called at a time where there's no
> state yet. Since the drm_plane_state reset helper doesn't take care of
> reading that value when it's called, it means that in most cases the
> initial value will be 0, or the drivers will have to work around it.
> 
> Let's add some code in __drm_atomic_helper_plane_state_reset() to get
> the initial zpos value if the property has been attached in order to fix
> this.
> 
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Looks reasonable.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index ddcf5c2c8e6a..1412cee404ca 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -243,11 +243,22 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
>  void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state,
>  					   struct drm_plane *plane)
>  {
> +	u64 val;
> +
>  	plane_state->plane = plane;
>  	plane_state->rotation = DRM_MODE_ROTATE_0;
>  
>  	plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
>  	plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> +
> +	if (plane->zpos_property) {
> +		if (!drm_object_property_get_default_value(&plane->base,
> +							   plane->zpos_property,
> +							   &val)) {
> +			plane_state->zpos = val;
> +			plane_state->normalized_zpos = val;
> +		}
> +	}
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_state_reset);
>  

-- 
Regards,

Laurent Pinchart

[PATCH 08/23] drm/tegra: plane: Remove redundant zpos initialisation

[Maxime Ripard]

The tegra KMS driver will call drm_plane_create_zpos_property() with an
init value of the plane index.

Since the initial value wasn't carried over in the state, the driver had
to set it again in tegra_plane_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: linux-tegra@vger.kernel.org
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/tegra/plane.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index ec0822c86926..a00913d064d3 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -25,7 +25,6 @@ static void tegra_plane_destroy(struct drm_plane *plane)
 
 static void tegra_plane_reset(struct drm_plane *plane)
 {
-	struct tegra_plane *p = to_tegra_plane(plane);
 	struct tegra_plane_state *state;
 	unsigned int i;
 
@@ -38,8 +37,6 @@ static void tegra_plane_reset(struct drm_plane *plane)
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
 	if (state) {
 		__drm_atomic_helper_plane_reset(plane, &state->base);
-		plane->state->zpos = p->index;
-		plane->state->normalized_zpos = p->index;
 
 		for (i = 0; i < 3; i++)
 			state->iova[i] = DMA_MAPPING_ERROR;
-- 
2.34.1

[PATCH 08/23] drm/tegra: plane: Remove redundant zpos initialisation

[Maxime Ripard]

The tegra KMS driver will call drm_plane_create_zpos_property() with an
init value of the plane index.

Since the initial value wasn't carried over in the state, the driver had
to set it again in tegra_plane_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: linux-tegra@vger.kernel.org
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/tegra/plane.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index ec0822c86926..a00913d064d3 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -25,7 +25,6 @@ static void tegra_plane_destroy(struct drm_plane *plane)
 
 static void tegra_plane_reset(struct drm_plane *plane)
 {
-	struct tegra_plane *p = to_tegra_plane(plane);
 	struct tegra_plane_state *state;
 	unsigned int i;
 
@@ -38,8 +37,6 @@ static void tegra_plane_reset(struct drm_plane *plane)
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
 	if (state) {
 		__drm_atomic_helper_plane_reset(plane, &state->base);
-		plane->state->zpos = p->index;
-		plane->state->normalized_zpos = p->index;
 
 		for (i = 0; i < 3; i++)
 			state->iova[i] = DMA_MAPPING_ERROR;
-- 
2.34.1

[PATCH 09/23] drm/komeda: plane: Remove redundant zpos initialisation

[Maxime Ripard]

The komeda KMS driver will call drm_plane_create_zpos_property() with an
init value of the plane index.

Since the initial value wasn't carried over in the state, the driver had
to set it again in komeda_plane_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: Brian Starkey <brian.starkey@arm.com>
Cc: "James (Qian) Wang" <james.qian.wang@arm.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Mihail Atanassov <mihail.atanassov@arm.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
index 1778f6e1ea56..383bb2039bd4 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
@@ -135,7 +135,6 @@ static void komeda_plane_destroy(struct drm_plane *plane)
 static void komeda_plane_reset(struct drm_plane *plane)
 {
 	struct komeda_plane_state *state;
-	struct komeda_plane *kplane = to_kplane(plane);
 
 	if (plane->state)
 		__drm_atomic_helper_plane_destroy_state(plane->state);
@@ -146,7 +145,6 @@ static void komeda_plane_reset(struct drm_plane *plane)
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
 	if (state) {
 		__drm_atomic_helper_plane_reset(plane, &state->base);
-		state->base.zpos = kplane->layer->base.id;
 		state->base.color_encoding = DRM_COLOR_YCBCR_BT601;
 		state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE;
 	}
-- 
2.34.1

[PATCH 10/23] drm/exynos: plane: Remove redundant zpos initialisation

[Maxime Ripard]

The exynos KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane purpose.

Since the initial value wasn't carried over in the state, the driver had
to set it again in exynos_drm_plane_reset(). However, the helpers have
been adjusted to set it properly at reset, so this is not needed
anymore.

Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index df76bdee7dca..3615cf329e32 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -122,7 +122,6 @@ static void exynos_plane_mode_set(struct exynos_drm_plane_state *exynos_state)
 
 static void exynos_drm_plane_reset(struct drm_plane *plane)
 {
-	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
 	struct exynos_drm_plane_state *exynos_state;
 
 	if (plane->state) {
@@ -133,10 +132,8 @@ static void exynos_drm_plane_reset(struct drm_plane *plane)
 	}
 
 	exynos_state = kzalloc(sizeof(*exynos_state), GFP_KERNEL);
-	if (exynos_state) {
+	if (exynos_state)
 		__drm_atomic_helper_plane_reset(plane, &exynos_state->base);
-		plane->state->zpos = exynos_plane->config->zpos;
-	}
 }
 
 static struct drm_plane_state *
-- 
2.34.1

[PATCH 10/23] drm/exynos: plane: Remove redundant zpos initialisation

[Maxime Ripard]

The exynos KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane purpose.

Since the initial value wasn't carried over in the state, the driver had
to set it again in exynos_drm_plane_reset(). However, the helpers have
been adjusted to set it properly at reset, so this is not needed
anymore.

Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index df76bdee7dca..3615cf329e32 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -122,7 +122,6 @@ static void exynos_plane_mode_set(struct exynos_drm_plane_state *exynos_state)
 
 static void exynos_drm_plane_reset(struct drm_plane *plane)
 {
-	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
 	struct exynos_drm_plane_state *exynos_state;
 
 	if (plane->state) {
@@ -133,10 +132,8 @@ static void exynos_drm_plane_reset(struct drm_plane *plane)
 	}
 
 	exynos_state = kzalloc(sizeof(*exynos_state), GFP_KERNEL);
-	if (exynos_state) {
+	if (exynos_state)
 		__drm_atomic_helper_plane_reset(plane, &exynos_state->base);
-		plane->state->zpos = exynos_plane->config->zpos;
-	}
 }
 
 static struct drm_plane_state *
-- 
2.34.1

[PATCH 10/23] drm/exynos: plane: Remove redundant zpos initialisation

[Maxime Ripard]

The exynos KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane purpose.

Since the initial value wasn't carried over in the state, the driver had
to set it again in exynos_drm_plane_reset(). However, the helpers have
been adjusted to set it properly at reset, so this is not needed
anymore.

Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index df76bdee7dca..3615cf329e32 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -122,7 +122,6 @@ static void exynos_plane_mode_set(struct exynos_drm_plane_state *exynos_state)
 
 static void exynos_drm_plane_reset(struct drm_plane *plane)
 {
-	struct exynos_drm_plane *exynos_plane = to_exynos_plane(plane);
 	struct exynos_drm_plane_state *exynos_state;
 
 	if (plane->state) {
@@ -133,10 +132,8 @@ static void exynos_drm_plane_reset(struct drm_plane *plane)
 	}
 
 	exynos_state = kzalloc(sizeof(*exynos_state), GFP_KERNEL);
-	if (exynos_state) {
+	if (exynos_state)
 		__drm_atomic_helper_plane_reset(plane, &exynos_state->base);
-		plane->state->zpos = exynos_plane->config->zpos;
-	}
 }
 
 static struct drm_plane_state *
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 11/23] drm/imx: ipuv3-plane: Remove redundant zpos initialisation

[Maxime Ripard]

The imx KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane purpose.

Since the initial value wasn't carried over in the state, the driver had
to set it again in ipu_plane_state_reset(). However, the helpers have
been adjusted to set it properly at reset, so this is not needed
anymore.

Cc: linux-arm-kernel@lists.infradead.org
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/imx/ipuv3-plane.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 846c1aae69c8..414bdf08d0b0 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -297,7 +297,6 @@ void ipu_plane_disable_deferred(struct drm_plane *plane)
 
 static void ipu_plane_state_reset(struct drm_plane *plane)
 {
-	unsigned int zpos = (plane->type == DRM_PLANE_TYPE_PRIMARY) ? 0 : 1;
 	struct ipu_plane_state *ipu_state;
 
 	if (plane->state) {
@@ -311,8 +310,6 @@ static void ipu_plane_state_reset(struct drm_plane *plane)
 
 	if (ipu_state) {
 		__drm_atomic_helper_plane_reset(plane, &ipu_state->base);
-		ipu_state->base.zpos = zpos;
-		ipu_state->base.normalized_zpos = zpos;
 		ipu_state->base.color_encoding = DRM_COLOR_YCBCR_BT601;
 		ipu_state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE;
 	}
-- 
2.34.1

[PATCH 11/23] drm/imx: ipuv3-plane: Remove redundant zpos initialisation

[Maxime Ripard]

The imx KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane purpose.

Since the initial value wasn't carried over in the state, the driver had
to set it again in ipu_plane_state_reset(). However, the helpers have
been adjusted to set it properly at reset, so this is not needed
anymore.

Cc: linux-arm-kernel@lists.infradead.org
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/imx/ipuv3-plane.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 846c1aae69c8..414bdf08d0b0 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -297,7 +297,6 @@ void ipu_plane_disable_deferred(struct drm_plane *plane)
 
 static void ipu_plane_state_reset(struct drm_plane *plane)
 {
-	unsigned int zpos = (plane->type == DRM_PLANE_TYPE_PRIMARY) ? 0 : 1;
 	struct ipu_plane_state *ipu_state;
 
 	if (plane->state) {
@@ -311,8 +310,6 @@ static void ipu_plane_state_reset(struct drm_plane *plane)
 
 	if (ipu_state) {
 		__drm_atomic_helper_plane_reset(plane, &ipu_state->base);
-		ipu_state->base.zpos = zpos;
-		ipu_state->base.normalized_zpos = zpos;
 		ipu_state->base.color_encoding = DRM_COLOR_YCBCR_BT601;
 		ipu_state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE;
 	}
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 12/23] drm/msm/mdp5: Remove redundant zpos initialisation

[Maxime Ripard]

The mdp KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane purpose.

Since the initial value wasn't carried over in the state, the driver had
to set it again in mdp5_plane_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: freedreno@lists.freedesktop.org
Cc: linux-arm-msm@vger.kernel.org
Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Sean Paul <sean@poorly.run>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index d60982f4bd11..5d8ac84c510b 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -98,13 +98,6 @@ static void mdp5_plane_reset(struct drm_plane *plane)
 
 	kfree(to_mdp5_plane_state(plane->state));
 	mdp5_state = kzalloc(sizeof(*mdp5_state), GFP_KERNEL);
-
-	if (plane->type == DRM_PLANE_TYPE_PRIMARY)
-		mdp5_state->base.zpos = STAGE_BASE;
-	else
-		mdp5_state->base.zpos = STAGE0 + drm_plane_index(plane);
-	mdp5_state->base.normalized_zpos = mdp5_state->base.zpos;
-
 	__drm_atomic_helper_plane_reset(plane, &mdp5_state->base);
 }
 
-- 
2.34.1

[PATCH 12/23] drm/msm/mdp5: Remove redundant zpos initialisation

[Maxime Ripard]

The mdp KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane purpose.

Since the initial value wasn't carried over in the state, the driver had
to set it again in mdp5_plane_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: freedreno@lists.freedesktop.org
Cc: linux-arm-msm@vger.kernel.org
Cc: Abhinav Kumar <quic_abhinavk@quicinc.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Sean Paul <sean@poorly.run>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index d60982f4bd11..5d8ac84c510b 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -98,13 +98,6 @@ static void mdp5_plane_reset(struct drm_plane *plane)
 
 	kfree(to_mdp5_plane_state(plane->state));
 	mdp5_state = kzalloc(sizeof(*mdp5_state), GFP_KERNEL);
-
-	if (plane->type == DRM_PLANE_TYPE_PRIMARY)
-		mdp5_state->base.zpos = STAGE_BASE;
-	else
-		mdp5_state->base.zpos = STAGE0 + drm_plane_index(plane);
-	mdp5_state->base.normalized_zpos = mdp5_state->base.zpos;
-
 	__drm_atomic_helper_plane_reset(plane, &mdp5_state->base);
 }
 
-- 
2.34.1

[PATCH 13/23] drm/nouveau/kms: Remove redundant zpos initialisation

[Maxime Ripard]

The nouveau KMS driver will call drm_plane_create_zpos_property() with
an init value depending on the plane purpose.

Since the initial value wasn't carried over in the state, the driver had
to set it again in nv50_wndw_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: nouveau@lists.freedesktop.org
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Karol Herbst <kherbst@redhat.com>
Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 133c8736426a..0c1a2ea0ed04 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -635,8 +635,6 @@ nv50_wndw_reset(struct drm_plane *plane)
 		plane->funcs->atomic_destroy_state(plane, plane->state);
 
 	__drm_atomic_helper_plane_reset(plane, &asyw->state);
-	plane->state->zpos = nv50_wndw_zpos_default(plane);
-	plane->state->normalized_zpos = nv50_wndw_zpos_default(plane);
 }
 
 static void
-- 
2.34.1

[Nouveau] [PATCH 13/23] drm/nouveau/kms: Remove redundant zpos initialisation

[Maxime Ripard]

The nouveau KMS driver will call drm_plane_create_zpos_property() with
an init value depending on the plane purpose.

Since the initial value wasn't carried over in the state, the driver had
to set it again in nv50_wndw_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: nouveau@lists.freedesktop.org
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Karol Herbst <kherbst@redhat.com>
Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 133c8736426a..0c1a2ea0ed04 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -635,8 +635,6 @@ nv50_wndw_reset(struct drm_plane *plane)
 		plane->funcs->atomic_destroy_state(plane, plane->state);
 
 	__drm_atomic_helper_plane_reset(plane, &asyw->state);
-	plane->state->zpos = nv50_wndw_zpos_default(plane);
-	plane->state->normalized_zpos = nv50_wndw_zpos_default(plane);
 }
 
 static void
-- 
2.34.1

[PATCH 14/23] drm/omap: plane: Fix zpos initial value mismatch

[Maxime Ripard]

While the omap_plane_init() function calls
drm_plane_create_zpos_property() with an initial value of 0,
omap_plane_reset() will force it to another value depending on the plane
type.

Fix the discrepancy by setting the initial zpos value to the same value
in the drm_plane_create_zpos_property() call.

Cc: Tomi Valkeinen <tomba@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/omapdrm/omap_plane.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index b35205c4e979..e67baf9a942c 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -533,6 +533,7 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 	unsigned int num_planes = dispc_get_num_ovls(priv->dispc);
 	struct drm_plane *plane;
 	struct omap_plane *omap_plane;
+	unsigned int zpos;
 	int ret;
 	u32 nformats;
 	const u32 *formats;
@@ -564,7 +565,16 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 	drm_plane_helper_add(plane, &omap_plane_helper_funcs);
 
 	omap_plane_install_properties(plane, &plane->base);
-	drm_plane_create_zpos_property(plane, 0, 0, num_planes - 1);
+
+	/*
+	 * Set the zpos default depending on whether we are a primary or overlay
+	 * plane.
+	 */
+	if (plane->type == DRM_PLANE_TYPE_PRIMARY)
+		zpos = 0;
+	else
+		zpos = omap_plane->id;
+	drm_plane_create_zpos_property(plane, zpos, 0, num_planes - 1);
 	drm_plane_create_alpha_property(plane);
 	drm_plane_create_blend_mode_property(plane, BIT(DRM_MODE_BLEND_PREMULTI) |
 					     BIT(DRM_MODE_BLEND_COVERAGE));
-- 
2.34.1

Re: [PATCH 14/23] drm/omap: plane: Fix zpos initial value mismatch

[Tomi Valkeinen]

On 07/02/2022 18:35, Maxime Ripard wrote:
> While the omap_plane_init() function calls
> drm_plane_create_zpos_property() with an initial value of 0,
> omap_plane_reset() will force it to another value depending on the plane
> type.
> 
> Fix the discrepancy by setting the initial zpos value to the same value
> in the drm_plane_create_zpos_property() call.
> 
> Cc: Tomi Valkeinen <tomba@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>   drivers/gpu/drm/omapdrm/omap_plane.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
> index b35205c4e979..e67baf9a942c 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -533,6 +533,7 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
>   	unsigned int num_planes = dispc_get_num_ovls(priv->dispc);
>   	struct drm_plane *plane;
>   	struct omap_plane *omap_plane;
> +	unsigned int zpos;
>   	int ret;
>   	u32 nformats;
>   	const u32 *formats;
> @@ -564,7 +565,16 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
>   	drm_plane_helper_add(plane, &omap_plane_helper_funcs);
>   
>   	omap_plane_install_properties(plane, &plane->base);
> -	drm_plane_create_zpos_property(plane, 0, 0, num_planes - 1);
> +
> +	/*
> +	 * Set the zpos default depending on whether we are a primary or overlay
> +	 * plane.
> +	 */
> +	if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> +		zpos = 0;
> +	else
> +		zpos = omap_plane->id;
> +	drm_plane_create_zpos_property(plane, zpos, 0, num_planes - 1);
>   	drm_plane_create_alpha_property(plane);
>   	drm_plane_create_blend_mode_property(plane, BIT(DRM_MODE_BLEND_PREMULTI) |
>   					     BIT(DRM_MODE_BLEND_COVERAGE));

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi

[PATCH 15/23] drm/omap: plane: Remove redundant zpos initialisation

[Maxime Ripard]

The omap KMS driver will call drm_plane_create_zpos_property() with an
init value of the plane index and the plane type.

Since the initial value wasn't carried over in the state, the driver had
to set it again in omap_plane_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: Tomi Valkeinen <tomba@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/omapdrm/omap_plane.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index e67baf9a942c..d96bc929072a 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -414,13 +414,6 @@ static void omap_plane_reset(struct drm_plane *plane)
 		return;
 
 	__drm_atomic_helper_plane_reset(plane, &omap_state->base);
-
-	/*
-	 * Set the zpos default depending on whether we are a primary or overlay
-	 * plane.
-	 */
-	plane->state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY
-			   ? 0 : omap_plane->id;
 	plane->state->color_encoding = DRM_COLOR_YCBCR_BT601;
 	plane->state->color_range = DRM_COLOR_YCBCR_FULL_RANGE;
 }
-- 
2.34.1

Re: [PATCH 15/23] drm/omap: plane: Remove redundant zpos initialisation

[Tomi Valkeinen]

On 07/02/2022 18:35, Maxime Ripard wrote:
> The omap KMS driver will call drm_plane_create_zpos_property() with an
> init value of the plane index and the plane type.
> 
> Since the initial value wasn't carried over in the state, the driver had
> to set it again in omap_plane_reset(). However, the helpers have been
> adjusted to set it properly at reset, so this is not needed anymore.
> 
> Cc: Tomi Valkeinen <tomba@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>   drivers/gpu/drm/omapdrm/omap_plane.c | 7 -------
>   1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
> index e67baf9a942c..d96bc929072a 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -414,13 +414,6 @@ static void omap_plane_reset(struct drm_plane *plane)
>   		return;
>   
>   	__drm_atomic_helper_plane_reset(plane, &omap_state->base);
> -
> -	/*
> -	 * Set the zpos default depending on whether we are a primary or overlay
> -	 * plane.
> -	 */
> -	plane->state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY
> -			   ? 0 : omap_plane->id;
>   	plane->state->color_encoding = DRM_COLOR_YCBCR_BT601;
>   	plane->state->color_range = DRM_COLOR_YCBCR_FULL_RANGE;
>   }

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi

[PATCH 16/23] drm/rcar: plane: Remove redundant zpos initialisation

[Maxime Ripard]

The rcar-du KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane type.

Since the initial value wasn't carried over in the state, the driver had
to set it again in rcar_du_plane_reset() and rcar_du_vsp_plane_reset().
However, the helpers have been adjusted to set it properly at reset, so
this is not needed anymore.

Cc: linux-renesas-soc@vger.kernel.org
Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 1 -
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c   | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index 862197be1e01..9dda5e06457d 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -696,7 +696,6 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
 	state->hwindex = -1;
 	state->source = RCAR_DU_PLANE_MEMORY;
 	state->colorkey = RCAR_DU_COLORKEY_NONE;
-	state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
 }
 
 static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index b7fc5b069cbc..719c60034952 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -362,7 +362,6 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane)
 		return;
 
 	__drm_atomic_helper_plane_reset(plane, &state->state);
-	state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
 }
 
 static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {
-- 
2.34.1

[PATCH 16/23] drm/rcar: plane: Remove redundant zpos initialisation

[Maxime Ripard]

The rcar-du KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane type.

Since the initial value wasn't carried over in the state, the driver had
to set it again in rcar_du_plane_reset() and rcar_du_vsp_plane_reset().
However, the helpers have been adjusted to set it properly at reset, so
this is not needed anymore.

Cc: linux-renesas-soc@vger.kernel.org
Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 1 -
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c   | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index 862197be1e01..9dda5e06457d 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -696,7 +696,6 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
 	state->hwindex = -1;
 	state->source = RCAR_DU_PLANE_MEMORY;
 	state->colorkey = RCAR_DU_COLORKEY_NONE;
-	state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
 }
 
 static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index b7fc5b069cbc..719c60034952 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -362,7 +362,6 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane)
 		return;
 
 	__drm_atomic_helper_plane_reset(plane, &state->state);
-	state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
 }
 
 static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {
-- 
2.34.1

Re: [PATCH 16/23] drm/rcar: plane: Remove redundant zpos initialisation

[Laurent Pinchart]

Hello Maxime,

Thank you for the patch.

On Mon, Feb 07, 2022 at 05:35:08PM +0100, Maxime Ripard wrote:
> The rcar-du KMS driver will call drm_plane_create_zpos_property() with an
> init value depending on the plane type.
> 
> Since the initial value wasn't carried over in the state, the driver had
> to set it again in rcar_du_plane_reset() and rcar_du_vsp_plane_reset().
> However, the helpers have been adjusted to set it properly at reset, so
> this is not needed anymore.
> 
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c | 1 -
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c   | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> index 862197be1e01..9dda5e06457d 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> @@ -696,7 +696,6 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
>  	state->hwindex = -1;
>  	state->source = RCAR_DU_PLANE_MEMORY;
>  	state->colorkey = RCAR_DU_COLORKEY_NONE;
> -	state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
>  }
>  
>  static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> index b7fc5b069cbc..719c60034952 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -362,7 +362,6 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane)
>  		return;
>  
>  	__drm_atomic_helper_plane_reset(plane, &state->state);
> -	state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
>  }
>  
>  static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {

-- 
Regards,

Laurent Pinchart

Re: [PATCH 16/23] drm/rcar: plane: Remove redundant zpos initialisation

[Laurent Pinchart]

Hello Maxime,

Thank you for the patch.

On Mon, Feb 07, 2022 at 05:35:08PM +0100, Maxime Ripard wrote:
> The rcar-du KMS driver will call drm_plane_create_zpos_property() with an
> init value depending on the plane type.
> 
> Since the initial value wasn't carried over in the state, the driver had
> to set it again in rcar_du_plane_reset() and rcar_du_vsp_plane_reset().
> However, the helpers have been adjusted to set it properly at reset, so
> this is not needed anymore.
> 
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c | 1 -
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c   | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> index 862197be1e01..9dda5e06457d 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> @@ -696,7 +696,6 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
>  	state->hwindex = -1;
>  	state->source = RCAR_DU_PLANE_MEMORY;
>  	state->colorkey = RCAR_DU_COLORKEY_NONE;
> -	state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
>  }
>  
>  static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> index b7fc5b069cbc..719c60034952 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -362,7 +362,6 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane)
>  		return;
>  
>  	__drm_atomic_helper_plane_reset(plane, &state->state);
> -	state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
>  }
>  
>  static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {

-- 
Regards,

Laurent Pinchart

Re: [PATCH 16/23] drm/rcar: plane: Remove redundant zpos initialisation

[Kieran Bingham]

Quoting Maxime Ripard (2022-02-07 16:35:08)
> The rcar-du KMS driver will call drm_plane_create_zpos_property() with an
> init value depending on the plane type.
> 
> Since the initial value wasn't carried over in the state, the driver had
> to set it again in rcar_du_plane_reset() and rcar_du_vsp_plane_reset().
> However, the helpers have been adjusted to set it properly at reset, so
> this is not needed anymore.

Sounds helpful ;-)

> Cc: linux-renesas-soc@vger.kernel.org
> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c | 1 -
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c   | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> index 862197be1e01..9dda5e06457d 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> @@ -696,7 +696,6 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
>         state->hwindex = -1;
>         state->source = RCAR_DU_PLANE_MEMORY;
>         state->colorkey = RCAR_DU_COLORKEY_NONE;
> -       state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
>  }
>  
>  static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> index b7fc5b069cbc..719c60034952 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -362,7 +362,6 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane)
>                 return;
>  
>         __drm_atomic_helper_plane_reset(plane, &state->state);
> -       state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
>  }
>  
>  static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {
> -- 
> 2.34.1
>

Re: [PATCH 16/23] drm/rcar: plane: Remove redundant zpos initialisation

[Kieran Bingham]

Quoting Maxime Ripard (2022-02-07 16:35:08)
> The rcar-du KMS driver will call drm_plane_create_zpos_property() with an
> init value depending on the plane type.
> 
> Since the initial value wasn't carried over in the state, the driver had
> to set it again in rcar_du_plane_reset() and rcar_du_vsp_plane_reset().
> However, the helpers have been adjusted to set it properly at reset, so
> this is not needed anymore.

Sounds helpful ;-)

> Cc: linux-renesas-soc@vger.kernel.org
> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c | 1 -
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c   | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> index 862197be1e01..9dda5e06457d 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> @@ -696,7 +696,6 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
>         state->hwindex = -1;
>         state->source = RCAR_DU_PLANE_MEMORY;
>         state->colorkey = RCAR_DU_COLORKEY_NONE;
> -       state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
>  }
>  
>  static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> index b7fc5b069cbc..719c60034952 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -362,7 +362,6 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane)
>                 return;
>  
>         __drm_atomic_helper_plane_reset(plane, &state->state);
> -       state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
>  }
>  
>  static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {
> -- 
> 2.34.1
>

[PATCH 17/23] drm/sti: plane: Remove redundant zpos initialisation

[Maxime Ripard]

The sti KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane type.

Since the initial value wasn't carried over in the state, the driver had
to set it again in sti_plane_reset() and rcar_du_vsp_plane_reset().
However, the helpers have been adjusted to set it properly at reset, so
this is not needed anymore.

Cc: Alain Volmat <alain.volmat@foss.st.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/sti/sti_cursor.c | 2 +-
 drivers/gpu/drm/sti/sti_gdp.c    | 2 +-
 drivers/gpu/drm/sti/sti_hqvdp.c  | 2 +-
 drivers/gpu/drm/sti/sti_plane.c  | 6 ------
 drivers/gpu/drm/sti/sti_plane.h  | 1 -
 5 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c
index 1d6051b4f6fe..414c9973aa6d 100644
--- a/drivers/gpu/drm/sti/sti_cursor.c
+++ b/drivers/gpu/drm/sti/sti_cursor.c
@@ -351,7 +351,7 @@ static const struct drm_plane_funcs sti_cursor_plane_helpers_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
 	.destroy = drm_plane_cleanup,
-	.reset = sti_plane_reset,
+	.reset = drm_atomic_helper_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
 	.late_register = sti_cursor_late_register,
diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
index d1a35d97bc45..3db3768a3241 100644
--- a/drivers/gpu/drm/sti/sti_gdp.c
+++ b/drivers/gpu/drm/sti/sti_gdp.c
@@ -905,7 +905,7 @@ static const struct drm_plane_funcs sti_gdp_plane_helpers_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
 	.destroy = drm_plane_cleanup,
-	.reset = sti_plane_reset,
+	.reset = drm_atomic_helper_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
 	.late_register = sti_gdp_late_register,
diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
index 3c61ba8b43e0..2201a50353eb 100644
--- a/drivers/gpu/drm/sti/sti_hqvdp.c
+++ b/drivers/gpu/drm/sti/sti_hqvdp.c
@@ -1283,7 +1283,7 @@ static const struct drm_plane_funcs sti_hqvdp_plane_helpers_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
 	.destroy = drm_plane_cleanup,
-	.reset = sti_plane_reset,
+	.reset = drm_atomic_helper_plane_reset,
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
 	.late_register = sti_hqvdp_late_register,
diff --git a/drivers/gpu/drm/sti/sti_plane.c b/drivers/gpu/drm/sti/sti_plane.c
index 3da4a46df2f2..173409cdb99e 100644
--- a/drivers/gpu/drm/sti/sti_plane.c
+++ b/drivers/gpu/drm/sti/sti_plane.c
@@ -112,12 +112,6 @@ static int sti_plane_get_default_zpos(enum drm_plane_type type)
 	return 0;
 }
 
-void sti_plane_reset(struct drm_plane *plane)
-{
-	drm_atomic_helper_plane_reset(plane);
-	plane->state->zpos = sti_plane_get_default_zpos(plane->type);
-}
-
 static void sti_plane_attach_zorder_property(struct drm_plane *drm_plane,
 					     enum drm_plane_type type)
 {
diff --git a/drivers/gpu/drm/sti/sti_plane.h b/drivers/gpu/drm/sti/sti_plane.h
index 065ffffbfb4a..8e33e629d9b0 100644
--- a/drivers/gpu/drm/sti/sti_plane.h
+++ b/drivers/gpu/drm/sti/sti_plane.h
@@ -81,5 +81,4 @@ void sti_plane_update_fps(struct sti_plane *plane,
 
 void sti_plane_init_property(struct sti_plane *plane,
 			     enum drm_plane_type type);
-void sti_plane_reset(struct drm_plane *plane);
 #endif
-- 
2.34.1

Re: [PATCH 17/23] drm/sti: plane: Remove redundant zpos initialisation

[Alain Volmat]

Hi,

thanks for the patch.

Reviewed-by: Alain Volmat <alain.volmat@foss.st.com>

Alain

On Mon, Feb 07, 2022 at 05:35:09PM +0100, Maxime Ripard wrote:
> The sti KMS driver will call drm_plane_create_zpos_property() with an
> init value depending on the plane type.
> 
> Since the initial value wasn't carried over in the state, the driver had
> to set it again in sti_plane_reset() and rcar_du_vsp_plane_reset().
> However, the helpers have been adjusted to set it properly at reset, so
> this is not needed anymore.
> 
> Cc: Alain Volmat <alain.volmat@foss.st.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/gpu/drm/sti/sti_cursor.c | 2 +-
>  drivers/gpu/drm/sti/sti_gdp.c    | 2 +-
>  drivers/gpu/drm/sti/sti_hqvdp.c  | 2 +-
>  drivers/gpu/drm/sti/sti_plane.c  | 6 ------
>  drivers/gpu/drm/sti/sti_plane.h  | 1 -
>  5 files changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c
> index 1d6051b4f6fe..414c9973aa6d 100644
> --- a/drivers/gpu/drm/sti/sti_cursor.c
> +++ b/drivers/gpu/drm/sti/sti_cursor.c
> @@ -351,7 +351,7 @@ static const struct drm_plane_funcs sti_cursor_plane_helpers_funcs = {
>  	.update_plane = drm_atomic_helper_update_plane,
>  	.disable_plane = drm_atomic_helper_disable_plane,
>  	.destroy = drm_plane_cleanup,
> -	.reset = sti_plane_reset,
> +	.reset = drm_atomic_helper_plane_reset,
>  	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>  	.late_register = sti_cursor_late_register,
> diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
> index d1a35d97bc45..3db3768a3241 100644
> --- a/drivers/gpu/drm/sti/sti_gdp.c
> +++ b/drivers/gpu/drm/sti/sti_gdp.c
> @@ -905,7 +905,7 @@ static const struct drm_plane_funcs sti_gdp_plane_helpers_funcs = {
>  	.update_plane = drm_atomic_helper_update_plane,
>  	.disable_plane = drm_atomic_helper_disable_plane,
>  	.destroy = drm_plane_cleanup,
> -	.reset = sti_plane_reset,
> +	.reset = drm_atomic_helper_plane_reset,
>  	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>  	.late_register = sti_gdp_late_register,
> diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
> index 3c61ba8b43e0..2201a50353eb 100644
> --- a/drivers/gpu/drm/sti/sti_hqvdp.c
> +++ b/drivers/gpu/drm/sti/sti_hqvdp.c
> @@ -1283,7 +1283,7 @@ static const struct drm_plane_funcs sti_hqvdp_plane_helpers_funcs = {
>  	.update_plane = drm_atomic_helper_update_plane,
>  	.disable_plane = drm_atomic_helper_disable_plane,
>  	.destroy = drm_plane_cleanup,
> -	.reset = sti_plane_reset,
> +	.reset = drm_atomic_helper_plane_reset,
>  	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>  	.late_register = sti_hqvdp_late_register,
> diff --git a/drivers/gpu/drm/sti/sti_plane.c b/drivers/gpu/drm/sti/sti_plane.c
> index 3da4a46df2f2..173409cdb99e 100644
> --- a/drivers/gpu/drm/sti/sti_plane.c
> +++ b/drivers/gpu/drm/sti/sti_plane.c
> @@ -112,12 +112,6 @@ static int sti_plane_get_default_zpos(enum drm_plane_type type)
>  	return 0;
>  }
>  
> -void sti_plane_reset(struct drm_plane *plane)
> -{
> -	drm_atomic_helper_plane_reset(plane);
> -	plane->state->zpos = sti_plane_get_default_zpos(plane->type);
> -}
> -
>  static void sti_plane_attach_zorder_property(struct drm_plane *drm_plane,
>  					     enum drm_plane_type type)
>  {
> diff --git a/drivers/gpu/drm/sti/sti_plane.h b/drivers/gpu/drm/sti/sti_plane.h
> index 065ffffbfb4a..8e33e629d9b0 100644
> --- a/drivers/gpu/drm/sti/sti_plane.h
> +++ b/drivers/gpu/drm/sti/sti_plane.h
> @@ -81,5 +81,4 @@ void sti_plane_update_fps(struct sti_plane *plane,
>  
>  void sti_plane_init_property(struct sti_plane *plane,
>  			     enum drm_plane_type type);
> -void sti_plane_reset(struct drm_plane *plane);
>  #endif
> -- 
> 2.34.1
> 

Re: [PATCH 17/23] drm/sti: plane: Remove redundant zpos initialisation

[Philippe CORNU]

On 2/7/22 5:35 PM, Maxime Ripard wrote:
> The sti KMS driver will call drm_plane_create_zpos_property() with an
> init value depending on the plane type.
> 
> Since the initial value wasn't carried over in the state, the driver had
> to set it again in sti_plane_reset() and rcar_du_vsp_plane_reset().

Hi Maxime,
and many thanks for your patches.

Great you added Alain as he is now the drm/sti maintainer (Maintainers 
file should be updated soon)

Minor typo in the commit message as rcar_du_vsp_plane_reset() is not 
part of drm/sti

Reviewed-by: Philippe Cornu <philippe.cornu@foss.st.com>

Philippe :-)


> However, the helpers have been adjusted to set it properly at reset, so
> this is not needed anymore.
> 
> Cc: Alain Volmat <alain.volmat@foss.st.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>   drivers/gpu/drm/sti/sti_cursor.c | 2 +-
>   drivers/gpu/drm/sti/sti_gdp.c    | 2 +-
>   drivers/gpu/drm/sti/sti_hqvdp.c  | 2 +-
>   drivers/gpu/drm/sti/sti_plane.c  | 6 ------
>   drivers/gpu/drm/sti/sti_plane.h  | 1 -
>   5 files changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c
> index 1d6051b4f6fe..414c9973aa6d 100644
> --- a/drivers/gpu/drm/sti/sti_cursor.c
> +++ b/drivers/gpu/drm/sti/sti_cursor.c
> @@ -351,7 +351,7 @@ static const struct drm_plane_funcs sti_cursor_plane_helpers_funcs = {
>   	.update_plane = drm_atomic_helper_update_plane,
>   	.disable_plane = drm_atomic_helper_disable_plane,
>   	.destroy = drm_plane_cleanup,
> -	.reset = sti_plane_reset,
> +	.reset = drm_atomic_helper_plane_reset,
>   	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>   	.late_register = sti_cursor_late_register,
> diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
> index d1a35d97bc45..3db3768a3241 100644
> --- a/drivers/gpu/drm/sti/sti_gdp.c
> +++ b/drivers/gpu/drm/sti/sti_gdp.c
> @@ -905,7 +905,7 @@ static const struct drm_plane_funcs sti_gdp_plane_helpers_funcs = {
>   	.update_plane = drm_atomic_helper_update_plane,
>   	.disable_plane = drm_atomic_helper_disable_plane,
>   	.destroy = drm_plane_cleanup,
> -	.reset = sti_plane_reset,
> +	.reset = drm_atomic_helper_plane_reset,
>   	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>   	.late_register = sti_gdp_late_register,
> diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c
> index 3c61ba8b43e0..2201a50353eb 100644
> --- a/drivers/gpu/drm/sti/sti_hqvdp.c
> +++ b/drivers/gpu/drm/sti/sti_hqvdp.c
> @@ -1283,7 +1283,7 @@ static const struct drm_plane_funcs sti_hqvdp_plane_helpers_funcs = {
>   	.update_plane = drm_atomic_helper_update_plane,
>   	.disable_plane = drm_atomic_helper_disable_plane,
>   	.destroy = drm_plane_cleanup,
> -	.reset = sti_plane_reset,
> +	.reset = drm_atomic_helper_plane_reset,
>   	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>   	.late_register = sti_hqvdp_late_register,
> diff --git a/drivers/gpu/drm/sti/sti_plane.c b/drivers/gpu/drm/sti/sti_plane.c
> index 3da4a46df2f2..173409cdb99e 100644
> --- a/drivers/gpu/drm/sti/sti_plane.c
> +++ b/drivers/gpu/drm/sti/sti_plane.c
> @@ -112,12 +112,6 @@ static int sti_plane_get_default_zpos(enum drm_plane_type type)
>   	return 0;
>   }
>   
> -void sti_plane_reset(struct drm_plane *plane)
> -{
> -	drm_atomic_helper_plane_reset(plane);
> -	plane->state->zpos = sti_plane_get_default_zpos(plane->type);
> -}
> -
>   static void sti_plane_attach_zorder_property(struct drm_plane *drm_plane,
>   					     enum drm_plane_type type)
>   {
> diff --git a/drivers/gpu/drm/sti/sti_plane.h b/drivers/gpu/drm/sti/sti_plane.h
> index 065ffffbfb4a..8e33e629d9b0 100644
> --- a/drivers/gpu/drm/sti/sti_plane.h
> +++ b/drivers/gpu/drm/sti/sti_plane.h
> @@ -81,5 +81,4 @@ void sti_plane_update_fps(struct sti_plane *plane,
>   
>   void sti_plane_init_property(struct sti_plane *plane,
>   			     enum drm_plane_type type);
> -void sti_plane_reset(struct drm_plane *plane);
>   #endif
> 

[PATCH 18/23] drm/sun4i: layer: Remove redundant zpos initialisation

[Maxime Ripard]

The sun4i KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane type.

Since the initial value wasn't carried over in the state, the driver had
to set it again in sun4i_backend_layer_reset().
However, the helpers have been adjusted to set it properly at reset, so
this is not needed anymore.

Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-sunxi@lists.linux.dev
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/sun4i/sun4i_layer.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 929e95f86b5b..6d43080791a0 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -18,7 +18,6 @@
 
 static void sun4i_backend_layer_reset(struct drm_plane *plane)
 {
-	struct sun4i_layer *layer = plane_to_sun4i_layer(plane);
 	struct sun4i_layer_state *state;
 
 	if (plane->state) {
@@ -31,10 +30,8 @@ static void sun4i_backend_layer_reset(struct drm_plane *plane)
 	}
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
-	if (state) {
+	if (state)
 		__drm_atomic_helper_plane_reset(plane, &state->state);
-		plane->state->zpos = layer->id;
-	}
 }
 
 static struct drm_plane_state *
@@ -192,7 +189,8 @@ static const uint64_t sun4i_layer_modifiers[] = {
 
 static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
 						struct sun4i_backend *backend,
-						enum drm_plane_type type)
+						enum drm_plane_type type,
+						unsigned int id)
 {
 	const uint64_t *modifiers = sun4i_layer_modifiers;
 	const uint32_t *formats = sun4i_layer_formats;
@@ -204,6 +202,7 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
 	if (!layer)
 		return ERR_PTR(-ENOMEM);
 
+	layer->id = id;
 	layer->backend = backend;
 
 	if (IS_ERR_OR_NULL(backend->frontend)) {
@@ -226,8 +225,8 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
 			     &sun4i_backend_layer_helper_funcs);
 
 	drm_plane_create_alpha_property(&layer->plane);
-	drm_plane_create_zpos_property(&layer->plane, 0, 0,
-				       SUN4I_BACKEND_NUM_LAYERS - 1);
+	drm_plane_create_zpos_property(&layer->plane, layer->id,
+				       0, SUN4I_BACKEND_NUM_LAYERS - 1);
 
 	return layer;
 }
@@ -249,14 +248,13 @@ struct drm_plane **sun4i_layers_init(struct drm_device *drm,
 		enum drm_plane_type type = i ? DRM_PLANE_TYPE_OVERLAY : DRM_PLANE_TYPE_PRIMARY;
 		struct sun4i_layer *layer;
 
-		layer = sun4i_layer_init_one(drm, backend, type);
+		layer = sun4i_layer_init_one(drm, backend, type, i);
 		if (IS_ERR(layer)) {
 			dev_err(drm->dev, "Couldn't initialize %s plane\n",
 				i ? "overlay" : "primary");
 			return ERR_CAST(layer);
 		}
 
-		layer->id = i;
 		planes[i] = &layer->plane;
 	}
 
-- 
2.34.1

[PATCH 18/23] drm/sun4i: layer: Remove redundant zpos initialisation

[Maxime Ripard]

The sun4i KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane type.

Since the initial value wasn't carried over in the state, the driver had
to set it again in sun4i_backend_layer_reset().
However, the helpers have been adjusted to set it properly at reset, so
this is not needed anymore.

Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-sunxi@lists.linux.dev
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/sun4i/sun4i_layer.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 929e95f86b5b..6d43080791a0 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -18,7 +18,6 @@
 
 static void sun4i_backend_layer_reset(struct drm_plane *plane)
 {
-	struct sun4i_layer *layer = plane_to_sun4i_layer(plane);
 	struct sun4i_layer_state *state;
 
 	if (plane->state) {
@@ -31,10 +30,8 @@ static void sun4i_backend_layer_reset(struct drm_plane *plane)
 	}
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
-	if (state) {
+	if (state)
 		__drm_atomic_helper_plane_reset(plane, &state->state);
-		plane->state->zpos = layer->id;
-	}
 }
 
 static struct drm_plane_state *
@@ -192,7 +189,8 @@ static const uint64_t sun4i_layer_modifiers[] = {
 
 static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
 						struct sun4i_backend *backend,
-						enum drm_plane_type type)
+						enum drm_plane_type type,
+						unsigned int id)
 {
 	const uint64_t *modifiers = sun4i_layer_modifiers;
 	const uint32_t *formats = sun4i_layer_formats;
@@ -204,6 +202,7 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
 	if (!layer)
 		return ERR_PTR(-ENOMEM);
 
+	layer->id = id;
 	layer->backend = backend;
 
 	if (IS_ERR_OR_NULL(backend->frontend)) {
@@ -226,8 +225,8 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
 			     &sun4i_backend_layer_helper_funcs);
 
 	drm_plane_create_alpha_property(&layer->plane);
-	drm_plane_create_zpos_property(&layer->plane, 0, 0,
-				       SUN4I_BACKEND_NUM_LAYERS - 1);
+	drm_plane_create_zpos_property(&layer->plane, layer->id,
+				       0, SUN4I_BACKEND_NUM_LAYERS - 1);
 
 	return layer;
 }
@@ -249,14 +248,13 @@ struct drm_plane **sun4i_layers_init(struct drm_device *drm,
 		enum drm_plane_type type = i ? DRM_PLANE_TYPE_OVERLAY : DRM_PLANE_TYPE_PRIMARY;
 		struct sun4i_layer *layer;
 
-		layer = sun4i_layer_init_one(drm, backend, type);
+		layer = sun4i_layer_init_one(drm, backend, type, i);
 		if (IS_ERR(layer)) {
 			dev_err(drm->dev, "Couldn't initialize %s plane\n",
 				i ? "overlay" : "primary");
 			return ERR_CAST(layer);
 		}
 
-		layer->id = i;
 		planes[i] = &layer->plane;
 	}
 
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 18/23] drm/sun4i: layer: Remove redundant zpos initialisation

[Maxime Ripard]

The sun4i KMS driver will call drm_plane_create_zpos_property() with an
init value depending on the plane type.

Since the initial value wasn't carried over in the state, the driver had
to set it again in sun4i_backend_layer_reset().
However, the helpers have been adjusted to set it properly at reset, so
this is not needed anymore.

Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-sunxi@lists.linux.dev
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/sun4i/sun4i_layer.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 929e95f86b5b..6d43080791a0 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -18,7 +18,6 @@
 
 static void sun4i_backend_layer_reset(struct drm_plane *plane)
 {
-	struct sun4i_layer *layer = plane_to_sun4i_layer(plane);
 	struct sun4i_layer_state *state;
 
 	if (plane->state) {
@@ -31,10 +30,8 @@ static void sun4i_backend_layer_reset(struct drm_plane *plane)
 	}
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
-	if (state) {
+	if (state)
 		__drm_atomic_helper_plane_reset(plane, &state->state);
-		plane->state->zpos = layer->id;
-	}
 }
 
 static struct drm_plane_state *
@@ -192,7 +189,8 @@ static const uint64_t sun4i_layer_modifiers[] = {
 
 static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
 						struct sun4i_backend *backend,
-						enum drm_plane_type type)
+						enum drm_plane_type type,
+						unsigned int id)
 {
 	const uint64_t *modifiers = sun4i_layer_modifiers;
 	const uint32_t *formats = sun4i_layer_formats;
@@ -204,6 +202,7 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
 	if (!layer)
 		return ERR_PTR(-ENOMEM);
 
+	layer->id = id;
 	layer->backend = backend;
 
 	if (IS_ERR_OR_NULL(backend->frontend)) {
@@ -226,8 +225,8 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
 			     &sun4i_backend_layer_helper_funcs);
 
 	drm_plane_create_alpha_property(&layer->plane);
-	drm_plane_create_zpos_property(&layer->plane, 0, 0,
-				       SUN4I_BACKEND_NUM_LAYERS - 1);
+	drm_plane_create_zpos_property(&layer->plane, layer->id,
+				       0, SUN4I_BACKEND_NUM_LAYERS - 1);
 
 	return layer;
 }
@@ -249,14 +248,13 @@ struct drm_plane **sun4i_layers_init(struct drm_device *drm,
 		enum drm_plane_type type = i ? DRM_PLANE_TYPE_OVERLAY : DRM_PLANE_TYPE_PRIMARY;
 		struct sun4i_layer *layer;
 
-		layer = sun4i_layer_init_one(drm, backend, type);
+		layer = sun4i_layer_init_one(drm, backend, type, i);
 		if (IS_ERR(layer)) {
 			dev_err(drm->dev, "Couldn't initialize %s plane\n",
 				i ? "overlay" : "primary");
 			return ERR_CAST(layer);
 		}
 
-		layer->id = i;
 		planes[i] = &layer->plane;
 	}
 
-- 
2.34.1

Re: [PATCH 18/23] drm/sun4i: layer: Remove redundant zpos initialisation

[Jernej Škrabec]

Hi Maxime,

Dne ponedeljek, 07. februar 2022 ob 17:35:10 CET je Maxime Ripard napisal(a):
> The sun4i KMS driver will call drm_plane_create_zpos_property() with an
> init value depending on the plane type.
> 
> Since the initial value wasn't carried over in the state, the driver had
> to set it again in sun4i_backend_layer_reset().
> However, the helpers have been adjusted to set it properly at reset, so
> this is not needed anymore.
> 
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-sunxi@lists.linux.dev
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

Re: [PATCH 18/23] drm/sun4i: layer: Remove redundant zpos initialisation

[Jernej Škrabec]

Hi Maxime,

Dne ponedeljek, 07. februar 2022 ob 17:35:10 CET je Maxime Ripard napisal(a):
> The sun4i KMS driver will call drm_plane_create_zpos_property() with an
> init value depending on the plane type.
> 
> Since the initial value wasn't carried over in the state, the driver had
> to set it again in sun4i_backend_layer_reset().
> However, the helpers have been adjusted to set it properly at reset, so
> this is not needed anymore.
> 
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-sunxi@lists.linux.dev
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 18/23] drm/sun4i: layer: Remove redundant zpos initialisation

[Jernej Škrabec]

Hi Maxime,

Dne ponedeljek, 07. februar 2022 ob 17:35:10 CET je Maxime Ripard napisal(a):
> The sun4i KMS driver will call drm_plane_create_zpos_property() with an
> init value depending on the plane type.
> 
> Since the initial value wasn't carried over in the state, the driver had
> to set it again in sun4i_backend_layer_reset().
> However, the helpers have been adjusted to set it properly at reset, so
> this is not needed anymore.
> 
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-sunxi@lists.linux.dev
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

[PATCH 19/23] drm/object: Add default color encoding and range value at reset

[Maxime Ripard]

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

The drm_plane_create_color_properties() function asks for an initial
value for the color encoding and range, and will set the associated
plane state variable with that value if a state is present.

However, that function is usually called at a time where there's no
state yet. Since the drm_plane_state reset helper doesn't take care of
reading that value when it's called, it means that in most cases the
initial value will be 0 (so DRM_COLOR_YCBCR_BT601 and
DRM_COLOR_YCBCR_LIMITED_RANGE, respectively), or the drivers will have
to work around it.

Let's add some code in __drm_atomic_helper_plane_state_reset() to get
the initial encoding and range value if the property has been attached
in order to fix this.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 1412cee404ca..3b6d3bdbd099 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -251,6 +251,20 @@ void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state,
 	plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
 	plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
 
+	if (plane->color_encoding_property) {
+		if (!drm_object_property_get_default_value(&plane->base,
+							   plane->color_encoding_property,
+							   &val))
+			plane_state->color_encoding = val;
+	}
+
+	if (plane->color_range_property) {
+		if (!drm_object_property_get_default_value(&plane->base,
+							   plane->color_range_property,
+							   &val))
+			plane_state->color_range = val;
+	}
+
 	if (plane->zpos_property) {
 		if (!drm_object_property_get_default_value(&plane->base,
 							   plane->zpos_property,
-- 
2.34.1

Re: [PATCH 19/23] drm/object: Add default color encoding and range value at reset

[Harry Wentland]

On 2022-02-07 11:35, Maxime Ripard wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> The drm_plane_create_color_properties() function asks for an initial
> value for the color encoding and range, and will set the associated
> plane state variable with that value if a state is present.
> 
> However, that function is usually called at a time where there's no
> state yet. Since the drm_plane_state reset helper doesn't take care of
> reading that value when it's called, it means that in most cases the
> initial value will be 0 (so DRM_COLOR_YCBCR_BT601 and
> DRM_COLOR_YCBCR_LIMITED_RANGE, respectively), or the drivers will have
> to work around it.
> 
> Let's add some code in __drm_atomic_helper_plane_state_reset() to get
> the initial encoding and range value if the property has been attached
> in order to fix this.
> 
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 1412cee404ca..3b6d3bdbd099 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -251,6 +251,20 @@ void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state,
>  	plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
>  	plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
>  
> +	if (plane->color_encoding_property) {
> +		if (!drm_object_property_get_default_value(&plane->base,
> +							   plane->color_encoding_property,
> +							   &val))
> +			plane_state->color_encoding = val;
> +	}
> +
> +	if (plane->color_range_property) {
> +		if (!drm_object_property_get_default_value(&plane->base,
> +							   plane->color_range_property,
> +							   &val))
> +			plane_state->color_range = val;
> +	}
> +
>  	if (plane->zpos_property) {
>  		if (!drm_object_property_get_default_value(&plane->base,
>  							   plane->zpos_property,

[PATCH 20/23] drm/komeda: plane: Remove redundant color encoding and range initialisation

[Maxime Ripard]

The komeda KMS driver will call drm_plane_create_color_properties() with
a default encoding and range values of BT601 and Limited Range,
respectively.

Since the initial value wasn't carried over in the state, the driver had
to set it again in komeda_plane_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: Brian Starkey <brian.starkey@arm.com>
Cc: "James (Qian) Wang" <james.qian.wang@arm.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Mihail Atanassov <mihail.atanassov@arm.com>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
index 383bb2039bd4..541949f2d44a 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
@@ -143,11 +143,8 @@ static void komeda_plane_reset(struct drm_plane *plane)
 	plane->state = NULL;
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
-	if (state) {
+	if (state)
 		__drm_atomic_helper_plane_reset(plane, &state->base);
-		state->base.color_encoding = DRM_COLOR_YCBCR_BT601;
-		state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE;
-	}
 }
 
 static struct drm_plane_state *
-- 
2.34.1

[PATCH 21/23] drm/armada: overlay: Remove redundant color encoding and range initialisation

[Maxime Ripard]

The armada KMS driver will call drm_plane_create_color_properties() with
a default encoding and range values of BT601 and Limited Range,
respectively.

Since the initial value wasn't carried over in the state, the driver had
to set it again in armada_overlay_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: Russell King <linux@armlinux.org.uk>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/armada/armada_overlay.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c
index 424250535fed..a25539039efd 100644
--- a/drivers/gpu/drm/armada/armada_overlay.c
+++ b/drivers/gpu/drm/armada/armada_overlay.c
@@ -325,8 +325,6 @@ static void armada_overlay_reset(struct drm_plane *plane)
 		state->contrast = DEFAULT_CONTRAST;
 		state->saturation = DEFAULT_SATURATION;
 		__drm_atomic_helper_plane_reset(plane, &state->base.base);
-		state->base.base.color_encoding = DEFAULT_ENCODING;
-		state->base.base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE;
 	}
 }
 
-- 
2.34.1

[PATCH 22/23] drm/imx: ipuv3-plane: Remove redundant color encoding and range initialisation

[Maxime Ripard]

The imx KMS driver will call drm_plane_create_color_properties() with
a default encoding and range values of BT601 and Limited Range,
respectively.

Since the initial value wasn't carried over in the state, the driver had
to set it again in ipu_plane_state_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: linux-arm-kernel@lists.infradead.org
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/imx/ipuv3-plane.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 414bdf08d0b0..36b32e8806e3 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -308,11 +308,8 @@ static void ipu_plane_state_reset(struct drm_plane *plane)
 
 	ipu_state = kzalloc(sizeof(*ipu_state), GFP_KERNEL);
 
-	if (ipu_state) {
+	if (ipu_state)
 		__drm_atomic_helper_plane_reset(plane, &ipu_state->base);
-		ipu_state->base.color_encoding = DRM_COLOR_YCBCR_BT601;
-		ipu_state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE;
-	}
 }
 
 static struct drm_plane_state *
-- 
2.34.1

[PATCH 22/23] drm/imx: ipuv3-plane: Remove redundant color encoding and range initialisation

[Maxime Ripard]

The imx KMS driver will call drm_plane_create_color_properties() with
a default encoding and range values of BT601 and Limited Range,
respectively.

Since the initial value wasn't carried over in the state, the driver had
to set it again in ipu_plane_state_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: linux-arm-kernel@lists.infradead.org
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/imx/ipuv3-plane.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 414bdf08d0b0..36b32e8806e3 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -308,11 +308,8 @@ static void ipu_plane_state_reset(struct drm_plane *plane)
 
 	ipu_state = kzalloc(sizeof(*ipu_state), GFP_KERNEL);
 
-	if (ipu_state) {
+	if (ipu_state)
 		__drm_atomic_helper_plane_reset(plane, &ipu_state->base);
-		ipu_state->base.color_encoding = DRM_COLOR_YCBCR_BT601;
-		ipu_state->base.color_range = DRM_COLOR_YCBCR_LIMITED_RANGE;
-	}
 }
 
 static struct drm_plane_state *
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 23/23] drm/omap: plane: Remove redundant color encoding and range initialisation

[Maxime Ripard]

The omap KMS driver will call drm_plane_create_color_properties() with
a default encoding and range values of BT601 and Full Range,
respectively.

Since the initial value wasn't carried over in the state, the driver had
to set it again in omap_plane_reset(). However, the helpers have been
adjusted to set it properly at reset, so this is not needed anymore.

Cc: Tomi Valkeinen <tomba@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/omapdrm/omap_plane.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index d96bc929072a..b83d91ec030a 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -403,7 +403,6 @@ void omap_plane_install_properties(struct drm_plane *plane,
 
 static void omap_plane_reset(struct drm_plane *plane)
 {
-	struct omap_plane *omap_plane = to_omap_plane(plane);
 	struct omap_plane_state *omap_state;
 
 	if (plane->state)
@@ -414,8 +413,6 @@ static void omap_plane_reset(struct drm_plane *plane)
 		return;
 
 	__drm_atomic_helper_plane_reset(plane, &omap_state->base);
-	plane->state->color_encoding = DRM_COLOR_YCBCR_BT601;
-	plane->state->color_range = DRM_COLOR_YCBCR_FULL_RANGE;
 }
 
 static struct drm_plane_state *
-- 
2.34.1

Re: [PATCH 23/23] drm/omap: plane: Remove redundant color encoding and range initialisation

[Tomi Valkeinen]

On 07/02/2022 18:35, Maxime Ripard wrote:
> The omap KMS driver will call drm_plane_create_color_properties() with
> a default encoding and range values of BT601 and Full Range,
> respectively.
> 
> Since the initial value wasn't carried over in the state, the driver had
> to set it again in omap_plane_reset(). However, the helpers have been
> adjusted to set it properly at reset, so this is not needed anymore.
> 
> Cc: Tomi Valkeinen <tomba@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>   drivers/gpu/drm/omapdrm/omap_plane.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
> index d96bc929072a..b83d91ec030a 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -403,7 +403,6 @@ void omap_plane_install_properties(struct drm_plane *plane,
>   
>   static void omap_plane_reset(struct drm_plane *plane)
>   {
> -	struct omap_plane *omap_plane = to_omap_plane(plane);
>   	struct omap_plane_state *omap_state;
>   
>   	if (plane->state)
> @@ -414,8 +413,6 @@ static void omap_plane_reset(struct drm_plane *plane)
>   		return;
>   
>   	__drm_atomic_helper_plane_reset(plane, &omap_state->base);
> -	plane->state->color_encoding = DRM_COLOR_YCBCR_BT601;
> -	plane->state->color_range = DRM_COLOR_YCBCR_FULL_RANGE;
>   }
>   
>   static struct drm_plane_state *

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi