[PATCH v1] drm/atomic: Clear plane pipeline for legacy clients

[PATCH v1] drm/atomic: Clear plane pipeline for legacy clients

[Robert Mader]

Clients that do not enable DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE and try to
apply a new plane state currently have no way to unset/reset an existing
non-bypass color pipeline, resulting in unexpected behavior.

As we can be sure that such clients do not expect plane pipelines to be
set, let's reset the later for them, ensuring clean bypass-only pipeline
states.

This is a less comprehensive but more backward compatible approach compared
to a DRM_MODE_ATOMIC_RESET flag (link) that is currently being discussed.
Considering the simplicity and clearly defined behavior of this case,
this change could serve as an additional stop-gap solution to smoothen the
transition to the new APIs, resulting in less breakage on systems with
existing clients.

Link: https://lore.kernel.org/all/CAFZQkGzw2MZGivy=oyYgZE3_dmxu-z-NbDgzh9uyqSsj=MG=2w@mail.gmail.com/

Signed-off-by: Robert Mader <robert.mader@collabora.com>

---

If an approach like this was previously rejected or clearly goes against
API rules (or is unacceptable for other reason), please excuse me (and
just let me know).

The main motivation for the patch came up during testing the Weston
implementation (link) for the fixed-matrix color-ops. The test there checks
both the color pipeline and legacy property ways of offloading YCbCr
buffers and the current reset-less situation results in the legacy one
failing once the color pipeline test ran before.

Link: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/2133
---
 drivers/gpu/drm/drm_atomic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 735ab7badc2e..581fde90dbd8 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -656,6 +656,9 @@ drm_atomic_get_plane_state(struct drm_atomic_commit *state,
 	state->planes[index].new_state = plane_state;
 	plane_state->state = state;
 
+	if (!state->plane_color_pipeline)
+		plane_state->color_pipeline = NULL;
+
 	drm_dbg_atomic(plane->dev, "Added [PLANE:%d:%s] %p state to %p\n",
 		       plane->base.id, plane->name, plane_state, state);
 
-- 
2.55.0

Re: [PATCH v1] drm/atomic: Clear plane pipeline for legacy clients

[Michel Dänzer]

On 7/9/26 13:58, Robert Mader wrote:
> Clients that do not enable DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE and try to
> apply a new plane state currently have no way to unset/reset an existing
> non-bypass color pipeline, resulting in unexpected behavior.
> 
> As we can be sure that such clients do not expect plane pipelines to be
> set, let's reset the later for them, ensuring clean bypass-only pipeline
> states.
> 
> This is a less comprehensive but more backward compatible approach compared
> to a DRM_MODE_ATOMIC_RESET flag (link) that is currently being discussed.
> Considering the simplicity and clearly defined behavior of this case,
> this change could serve as an additional stop-gap solution to smoothen the
> transition to the new APIs, resulting in less breakage on systems with
> existing clients.
> 
> Link: https://lore.kernel.org/all/CAFZQkGzw2MZGivy=oyYgZE3_dmxu-z-NbDgzh9uyqSsj=MG=2w@mail.gmail.com/
> 
> Signed-off-by: Robert Mader <robert.mader@collabora.com>
> 
> ---
> 
> If an approach like this was previously rejected or clearly goes against
> API rules (or is unacceptable for other reason), please excuse me (and
> just let me know).
> 
> The main motivation for the patch came up during testing the Weston
> implementation (link) for the fixed-matrix color-ops. The test there checks
> both the color pipeline and legacy property ways of offloading YCbCr
> buffers and the current reset-less situation results in the legacy one
> failing once the color pipeline test ran before.
> 
> Link: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/2133
> ---
>  drivers/gpu/drm/drm_atomic.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 735ab7badc2e..581fde90dbd8 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -656,6 +656,9 @@ drm_atomic_get_plane_state(struct drm_atomic_commit *state,
>  	state->planes[index].new_state = plane_state;
>  	plane_state->state = state;
>  
> +	if (!state->plane_color_pipeline)
> +		plane_state->color_pipeline = NULL;
> +
>  	drm_dbg_atomic(plane->dev, "Added [PLANE:%d:%s] %p state to %p\n",
>  		       plane->base.id, plane->name, plane_state, state);
>  

Is drm_atomic_get_plane_state always called for all planes which might have a color pipeline enabled?

If yes, this looks reasonable.


-- 
Earthling Michel Dänzer       \        GNOME / Xwayland / Mesa developer
https://redhat.com             \               Libre software enthusiast