On 2026-03-16 16:52, Melissa Wen wrote:
> For suspend/resume to work correctly, do for colorop state the same we
> do for plane/crtc/connector states: duplicate the state of colorops in a
> color pipeline if it's in use by a given plane when suspending and
> restore cached colorop states when resuming.
>
> Fixes: 2afc3184f3b3 ("drm/plane: Add COLOR PIPELINE property")
> Signed-off-by: Melissa Wen <[email protected]>
Of course. Thanks for fixing this.
Reviewed-by: Harry Wentland <[email protected]>
> ---
>
> Hi,
>
> I've been working on making gamescope use KMS plane color API, instead
> of AMD driver-specific color properties [1] and I found this issue
> during Steam Deck suspend/resume process.
>
Great to hear.
Harry
> Initially I thought I should also set plane color_mgmt_changed to make
> AMD driver reprogram the color state, but looks like it's not needed
> (still testing). Therefore, I think the change here is enough to fix it.
>
> It applies on top of drm-misc-next and is inspired by commit
> 6955d6bca053 ("drm/atomic: Add affected colorops with affected planes").
>
> [1] https://github.com/ValveSoftware/gamescope/pull/2113
>
> drivers/gpu/drm/drm_atomic_helper.c | 12 ++++++++++++
> include/drm/drm_atomic.h | 3 ++-
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 26953ed6b53e..683a0e207f71 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3751,6 +3751,13 @@ drm_atomic_helper_duplicate_state(struct drm_device
> *dev,
> err = PTR_ERR(plane_state);
> goto free;
> }
> +
> + if (plane_state->color_pipeline) {
> + err = drm_atomic_add_affected_colorops(state, plane);
> + if (err)
> + goto free;
> + }
> +
> }
>
> drm_connector_list_iter_begin(dev, &conn_iter);
> @@ -3856,6 +3863,8 @@ int drm_atomic_helper_commit_duplicated_state(struct
> drm_atomic_state *state,
> int i, ret;
> struct drm_plane *plane;
> struct drm_plane_state *new_plane_state;
> + struct drm_colorop *colorop;
> + struct drm_colorop_state *new_colorop_state;
> struct drm_connector *connector;
> struct drm_connector_state *new_conn_state;
> struct drm_crtc *crtc;
> @@ -3866,6 +3875,9 @@ int drm_atomic_helper_commit_duplicated_state(struct
> drm_atomic_state *state,
> for_each_new_plane_in_state(state, plane, new_plane_state, i)
> state->planes[i].old_state = plane->state;
>
> + for_each_new_colorop_in_state(state, colorop, new_colorop_state, i)
> + state->colorops[i].old_state = colorop->state;
> +
> for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
> state->crtcs[i].old_state = crtc->state;
>
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 0b1b32bcd2bd..96fd32a3e92c 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -1102,7 +1102,8 @@ void drm_state_dump(struct drm_device *dev, struct
> drm_printer *p);
> for_each_if ((__state)->colorops[__i].ptr && \
> ((colorop) = (__state)->colorops[__i].ptr, \
> (void)(colorop) /* Only to avoid
> unused-but-set-variable warning */, \
> - (new_colorop_state) =
> (__state)->colorops[__i].new_state, 1))
> + (new_colorop_state) =
> (__state)->colorops[__i].new_state,\
> + (void)(new_colorop_state) /* Only to avoid
> unused-but-set-variable warning */, 1))
>
> /**
> * for_each_oldnew_plane_in_state - iterate over all planes in an atomic
> update
