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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Plane color management changes (via `color_mgmt_changed`) force a reset for the modified plane but fail to trigger a reset of all other planes on the same CRTC, violating the AMD DC descending z-order requirement.
--

--- Patch [1]: [PATCH] drm/amd/display: use plane color_mgmt_changed to track colorop changes ---
commit 3d974c5531b17eb9dc9b94714b129841cee1a82c
Author: Melissa Wen <[email protected]>

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

This commit ensures the driver tracks changes in any colorop property
of a plane color pipeline. It updates plane color blocks when any
colorop property changes, which fixes missing updates observed in
night mode settings via Gamescope.

> 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 e421c3c81778d..e10937217ad41 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
[ ... ]
> @@ -5675,6 +5675,10 @@ static bool should_reset_plane(struct drm_atomic_commit *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;
> +

[Severity: High]
Does this change require a corresponding update to the cross-plane dependency
loop further down in should_reset_plane()?

If one plane resets due to new_plane_state->color_mgmt_changed, the AMD DC
architecture requires all other planes on the CRTC to be recreated to preserve
descending z-order.

Currently, the loop over for_each_oldnew_plane_in_state() checks legacy
properties like dm_new_other_state->degamma_tf, but doesn't check
new_other_state->color_mgmt_changed.

Will this omission cause a plane with color_mgmt_changed to be removed and
re-added while other planes remain, corrupting the hardware z-order?

>  	/*
>  	 * On zpos change, planes need to be reordered by removing and re-adding
>  	 * them one by one to the dc state, in order of descending zpos.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1