[PATCH] drm/atomic: clear the state of plane which is unused.

[PATCH] drm/atomic: clear the state of plane which is unused.

[Austin Hu]

Some libdrm user mode apps (including compositor) disable unused
plane with its previously binded CRTC or frame buffer object, but
may not reset its properties like src/dst coordinates, so that some
values of the unused plane state would be sticky. And even with
some Display Engine IP like Intel, a plane disabled from user mode
in current frame may be linked with another plane (e.g. to DMA Y or
UV semi-planar buffer) in KMD driver, so that its sticky UAPI
settings mismatch with the ones programmed to registers.

So clear the unused plane state during each atomic commit, to make
sure each plane state is up to date. This change might be
unnecessary if user mode already clears plane property, but keep it
just in case.

Signed-off-by: Austin Hu <austin.hu@intel.com>
---
 drivers/gpu/drm/drm_atomic.c | 62 ++++++++++++++++++++++++++++++++++--
 1 file changed, 59 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index dd9f27cfe991..0ce57e540a86 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -690,6 +690,59 @@ plane_switching_crtc(const struct drm_plane_state *old_plane_state,
 	return true;
 }
 
+/**
+ * drm_atomic_plane_state_reset - reset plane state
+ * @plane: plane to reset
+ * @plane_state: plane state to reset
+ *
+ * This function resets the plane state to its default values, after
+ * the plane is disabled or when the plane state is no longer valid.
+ */
+static int drm_atomic_plane_state_reset(struct drm_plane *plane,
+				      struct drm_plane_state *plane_state)
+{
+	int ret;
+
+	ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
+	if (ret != 0)
+		return ret;
+
+	drm_atomic_set_fb_for_plane(plane_state, NULL);
+
+	plane_state->crtc_x = 0;
+	plane_state->crtc_y = 0;
+	plane_state->crtc_w = 0;
+	plane_state->crtc_h = 0;
+	plane_state->src_x = 0;
+	plane_state->src_y = 0;
+	plane_state->src_w = 0;
+	plane_state->src_h = 0;
+
+	plane_state->alpha = 0;
+	plane_state->pixel_blend_mode = 0;
+	plane_state->rotation = 0;
+	plane_state->zpos = 0;
+	plane_state->color_encoding = 0;
+	plane_state->color_range = 0;
+
+	drm_atomic_set_colorop_for_plane(plane_state, NULL);
+
+	bool replaced = false;
+
+	ret = drm_property_replace_blob_from_id(
+		plane->dev, &plane_state->fb_damage_clips, 0, -1, -1,
+		sizeof(struct drm_mode_rect), &replaced);
+	if (ret)
+		return ret;
+
+	plane_state->scaling_filter = 0;
+	plane_state->hotspot_x = 0;
+	plane_state->hotspot_y = 0;
+	plane_state->visible = false;
+
+	return 0;
+}
+
 /**
  * drm_atomic_plane_check - check plane state
  * @old_plane_state: old plane state to check
@@ -701,7 +754,7 @@ plane_switching_crtc(const struct drm_plane_state *old_plane_state,
  * Zero on success, error code on failure
  */
 static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state,
-				  const struct drm_plane_state *new_plane_state)
+				  struct drm_plane_state *new_plane_state)
 {
 	struct drm_plane *plane = new_plane_state->plane;
 	struct drm_crtc *crtc = new_plane_state->crtc;
@@ -721,9 +774,12 @@ static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state,
 		return -EINVAL;
 	}
 
-	/* if disabled, we don't care about the rest of the state: */
+	/*
+	 * if disabled, we don't care about the rest of the state:
+	 * but clear the plane state.
+	 */
 	if (!crtc)
-		return 0;
+		return drm_atomic_plane_state_reset(plane, new_plane_state);
 
 	/* Check whether this plane is usable on this CRTC */
 	if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) {
-- 
2.34.1

Re: [PATCH] drm/atomic: clear the state of plane which is unused.

[Ville Syrjälä]

On Mon, Mar 16, 2026 at 03:55:07PM -0700, Austin Hu wrote:
> Some libdrm user mode apps (including compositor) disable unused
> plane with its previously binded CRTC or frame buffer object, but
> may not reset its properties like src/dst coordinates, so that some
> values of the unused plane state would be sticky. And even with
> some Display Engine IP like Intel, a plane disabled from user mode
> in current frame may be linked with another plane (e.g. to DMA Y or
> UV semi-planar buffer) in KMD driver, so that its sticky UAPI
> settings mismatch with the ones programmed to registers.
> 
> So clear the unused plane state during each atomic commit, to make
> sure each plane state is up to date. This change might be
> unnecessary if user mode already clears plane property, but keep it
> just in case.

The kernel isn't allowed to magically change the value of
properties set by userspace, apart from a few special cases.

No idea what you're actually trying to fix here, but sounds
like your userspace might be broken and needs to be fixed.

> 
> Signed-off-by: Austin Hu <austin.hu@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic.c | 62 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 59 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index dd9f27cfe991..0ce57e540a86 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -690,6 +690,59 @@ plane_switching_crtc(const struct drm_plane_state *old_plane_state,
>  	return true;
>  }
>  
> +/**
> + * drm_atomic_plane_state_reset - reset plane state
> + * @plane: plane to reset
> + * @plane_state: plane state to reset
> + *
> + * This function resets the plane state to its default values, after
> + * the plane is disabled or when the plane state is no longer valid.
> + */
> +static int drm_atomic_plane_state_reset(struct drm_plane *plane,
> +				      struct drm_plane_state *plane_state)
> +{
> +	int ret;
> +
> +	ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
> +	if (ret != 0)
> +		return ret;
> +
> +	drm_atomic_set_fb_for_plane(plane_state, NULL);
> +
> +	plane_state->crtc_x = 0;
> +	plane_state->crtc_y = 0;
> +	plane_state->crtc_w = 0;
> +	plane_state->crtc_h = 0;
> +	plane_state->src_x = 0;
> +	plane_state->src_y = 0;
> +	plane_state->src_w = 0;
> +	plane_state->src_h = 0;
> +
> +	plane_state->alpha = 0;
> +	plane_state->pixel_blend_mode = 0;
> +	plane_state->rotation = 0;
> +	plane_state->zpos = 0;
> +	plane_state->color_encoding = 0;
> +	plane_state->color_range = 0;
> +
> +	drm_atomic_set_colorop_for_plane(plane_state, NULL);
> +
> +	bool replaced = false;
> +
> +	ret = drm_property_replace_blob_from_id(
> +		plane->dev, &plane_state->fb_damage_clips, 0, -1, -1,
> +		sizeof(struct drm_mode_rect), &replaced);
> +	if (ret)
> +		return ret;
> +
> +	plane_state->scaling_filter = 0;
> +	plane_state->hotspot_x = 0;
> +	plane_state->hotspot_y = 0;
> +	plane_state->visible = false;
> +
> +	return 0;
> +}
> +
>  /**
>   * drm_atomic_plane_check - check plane state
>   * @old_plane_state: old plane state to check
> @@ -701,7 +754,7 @@ plane_switching_crtc(const struct drm_plane_state *old_plane_state,
>   * Zero on success, error code on failure
>   */
>  static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state,
> -				  const struct drm_plane_state *new_plane_state)
> +				  struct drm_plane_state *new_plane_state)
>  {
>  	struct drm_plane *plane = new_plane_state->plane;
>  	struct drm_crtc *crtc = new_plane_state->crtc;
> @@ -721,9 +774,12 @@ static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state,
>  		return -EINVAL;
>  	}
>  
> -	/* if disabled, we don't care about the rest of the state: */
> +	/*
> +	 * if disabled, we don't care about the rest of the state:
> +	 * but clear the plane state.
> +	 */
>  	if (!crtc)
> -		return 0;
> +		return drm_atomic_plane_state_reset(plane, new_plane_state);
>  
>  	/* Check whether this plane is usable on this CRTC */
>  	if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) {
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

Re: [PATCH] drm/atomic: clear the state of plane which is unused.

[Austin Hu]

Hi Ville,

Thanks for your comments!

> The kernel isn't allowed to magically change the value of
> properties set by userspace, apart from a few special cases.
>
> No idea what you're actually trying to fix here, but sounds
> like your userspace might be broken and needs to be fixed.

I agree that userspace should reset unused plane properties.
Otherwise, the drm_plane_state UAPI "sticky" values may not
match what is actually programmed to hardware, which can be
confusing.

For example, i915_display_info debugfs shows:

    [PLANE:123:plane 4A]: type=OVL
        uapi: [FB:0] n/a,0x0,0x0,, visible=Y plane,
              src=125.000000x48.000000+0.000000+0.000000,
              dst=125x48+897+0, rotation=0 (0x00000001)
        planar: Linked to [PLANE:33:plane 1A] as a Y plane
        hw: [FB:567] NV12 little-endian (0x3231564e),
            0x100000000000010,2048x1088, visible=no,
            src=1920.000000x1080.000000+0.000000+0.000000,
            dst=1920x1080+0+0, rotation=0 (0x00000001)

That said, I was wondering if DRM could also reset unused
plane state defensively, in case userspace does not do so.
For example:

https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/-/blob/main/drm/DrmPlane.cpp#L464

Maybe this is not the right way, but my intention was to avoid
such mismatches.

Thanks,
Austin