Re: [PATCH v4 10/11] drm/amd/display: Set color_space to plane_infos

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Changes to DRM plane color pipeline properties (drm_colorop) do not trigger a DC plane state reset, causing the hardware to use stale color space and colorop properties.
--

--- Patch [10]: [PATCH v4 10/11] drm/amd/display: Set color_space to plane_infos ---
commit d69c24f2afbe9ab20f934e3e3173c2c3d2c7b892
Author: Harry Wentland <harry.wentland@amd.com>

drm/amd/display: Set color_space to plane_infos

This commit assigns the color space from the dc_plane directly to the
plane_infos array in the update bundle during plane commits. This replaces
the legacy behavior that handled COLOR_ENCODING and COLOR_RANGE inside
fill_plane_color_attributes.

> 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 4f07fe5688262..779f98e8d2d7d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4288,6 +4288,8 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_commit *state,
>  			&bundle->flip_addrs[planes_count].address,
>  			afb->tmz_surface);
>  
> +		bundle->plane_infos[planes_count].color_space = dc_plane->color_space;
> +
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Could this assignment use a stale color_space value?

When userspace dynamically updates a property on an active drm_colorop
pipeline, DRM core sets new_plane_state->color_mgmt_changed = true.

However, should_reset_plane() incorrectly omits checking
new_other_state->color_mgmt_changed or iterating over drm_colorop_state
property changes:

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:should_reset_plane() {
	...
	/* Colorspace changes. */
	if (old_other_state->color_range != new_other_state->color_range ||
	    old_other_state->color_encoding != new_other_state->color_encoding)
		return true;
	...
}

Because should_reset_plane() returns false when only colorop pipeline
properties are modified, dm_update_plane_state() skips recreating the
dc_plane_state and bypasses amdgpu_dm_update_plane_color_mgmt().

As a result, stale color space parameters are passed to DC here. Should
should_reset_plane() be updated to properly check for color management changes
so the hardware color pipeline does not retain stale configuration values?

>  		drm_dbg_state(state->dev, "plane: id=%d dcc_en=%d\n",
>  				 new_plane_state->plane->index,
>  				 bundle->plane_infos[planes_count].dcc.enable);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722134607.136293-1-harry.wentland@amd.com?part=10