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@xxxxxxxxxxxxxx/
> 
> Signed-off-by: Robert Mader <robert.mader@xxxxxxxxxxxxx>
> 
> ---
> 
> 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