Re: [PATCH v9 4/4] drm/amd/display: use plane color_mgmt_changed to track colorop changes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The logic in `should_reset_plane` fails to trigger a reset for all planes on a CRTC when another plane's color management changes via the new DRM Color Pipeline API, leading to plane z-order corruption.
--

commit 52aefc9e9f5291e7e9724eb87eff29b790339396
Author: Melissa Wen <mwen@igalia.com>

drm/amd/display: use plane color_mgmt_changed to track colorop changes

Ensure the driver tracks changes in any colorop property of a plane
color pipeline by using the same mechanism of CRTC color management and
update plane color blocks when any colorop property changes.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 8f80420e9f0b8..56ba9bf40ed9f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
[ ... ]
> @@ -12208,6 +12208,10 @@ static bool should_reset_plane(struct drm_atomic_state *state,
>  	if (new_crtc_state->color_mgmt_changed)
>  		return true;
>  
> +	/* Plane color pipeline or its colorop changes. */
> +	if (new_plane_state->color_mgmt_changed)
> +		return true;
> +
>  	/*
>  	 * On zpos change, planes need to be reordered by removing and re-adding

[Severity: High]
Does this code need to check color_mgmt_changed for other planes on the
CRTC as well?

Looking further down in should_reset_plane(), there is a loop over
other planes that checks legacy AMD color properties, but it misses checking
new_other_state->color_mgmt_changed:

	for_each_oldnew_plane_in_state(state, other, old_other_state, new_other_state, i) {
		...
		/* HDR/Transfer Function changes. */
		if (dm_old_other_state->degamma_tf != dm_new_other_state->degamma_tf ||
		...

If Plane A has color_mgmt_changed == true, should_reset_plane() correctly
returns true and Plane A gets reset.

However, when should_reset_plane() is evaluated for Plane B on the same CRTC,
it iterates over the other planes (including Plane A). Since the loop does
not check Plane A's color_mgmt_changed state, it could return false for Plane B.

Could this cause only some planes on the CRTC to be removed and re-added,
resulting in a scrambled hardware z-order or pipeline acquisition errors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609110420.1298352-1-mwen@igalia.com?part=4