From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Austin Hu <austin.hu@intel.com>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
robin.clark@oss.qualcomm.com
Subject: Re: [PATCH] drm/atomic: clear the state of plane which is unused.
Date: Tue, 17 Mar 2026 09:00:24 +0200 [thread overview]
Message-ID: <abj79EU0bMEkcSD3@intel.com> (raw)
In-Reply-To: <20260316225507.392191-1-austin.hu@intel.com>
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
next prev parent reply other threads:[~2026-03-17 7:00 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-16 22:55 [PATCH] drm/atomic: clear the state of plane which is unused Austin Hu
2026-03-17 7:00 ` Ville Syrjälä [this message]
2026-03-17 20:07 ` Austin Hu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=abj79EU0bMEkcSD3@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=austin.hu@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robin.clark@oss.qualcomm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.