[PATCH] drm/drm_atomic: duplicate colorop states if plane color pipeline in use

[PATCH] drm/drm_atomic: duplicate colorop states if plane color pipeline in use

[Melissa Wen]

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 <mwen@igalia.com>
---

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.

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
-- 
2.51.0

Re: [PATCH] drm/drm_atomic: duplicate colorop states if plane color pipeline in use

[Harry Wentland]

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 <mwen@igalia.com>

Of course. Thanks for fixing this.

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

> ---
> 
> 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

Re: [PATCH] drm/drm_atomic: duplicate colorop states if plane color pipeline in use

[Alex Hung]

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 3/16/26 14: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 <mwen@igalia.com>
> ---
> 
> 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.
> 
> 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

Re: [PATCH] drm/drm_atomic: duplicate colorop states if plane color pipeline in use

[Borah, Chaitanya Kumar]

On 3/17/2026 2:22 AM, 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 <mwen@igalia.com>
> ---
> 
> 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.
> 
> 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;
> +

Nit: Just to maintain the object hierarchy this could be moved above planes.

>   	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))
>

Perhaps this should move to a separate patch.

Otherwise, the changes LGTM.

Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>


>   /**
>    * for_each_oldnew_plane_in_state - iterate over all planes in an atomic update

Re: [PATCH] drm/drm_atomic: duplicate colorop states if plane color pipeline in use

[Melissa Wen]

On 18/03/2026 06:21, Borah, Chaitanya Kumar wrote:
>
>
> On 3/17/2026 2:22 AM, 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 <mwen@igalia.com>
>> ---
>>
>> 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.
>>
>> 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;
>> +
>
> Nit: Just to maintain the object hierarchy this could be moved above 
> planes.

Ack

>
>>       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))
>>
>
> Perhaps this should move to a separate patch.

As the unused-variable warning didn't appear without the 
drm_atomic_helper.c changes made here, I think keeping them together is 
the best for context.
I mean, the problem doesn't exist before.

Thanks for reviewing,

Melissa

>
> Otherwise, the changes LGTM.
>
> Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>
>
>>   /**
>>    * for_each_oldnew_plane_in_state - iterate over all planes in an 
>> atomic update
>