Re: [PATCH v4 09/11] drm/amd/display: Check actual state during commit_tail

Robert Mader <[email protected]> Tue, 28 Jul 2026 18:02:57 +0200
Newsgroups org.freedesktop.lists.amd-gfx,org.freedesktop.lists.dri-devel
Message-ID <[email protected]>
Tested-by: Robert Mader <[email protected]>

On 22.07.26 15:46, Harry Wentland wrote:
> Previously we checked the plane_state->state's plane_color_pipeline
> client config when determining whether fill_plane_color_attributes
> should run, i.e., whether we had a plane color pipeline and should
> use that code for handling fixed matrix properties, or whether we
> should look at legacy plane encoding and range.
>
> The problem is that we also call this during commit_tail, during
> which plane_state doesn't have the state backpointer. This meant
> that during commit_tail we'd look at the legacy plane COLOR_RANGE
> and COLOR_ENCODING values and overwrite what we pulled from the
> fixed matrix colorop in atomic_check previously.
>
> Instead pass the drm_atomic_state in explicitly and check that.
>
> Assisted-by:Claude:claude-sonnet-4.5
> Signed-off-by: Harry Wentland <[email protected]>
> Reviewed-by: Alex Hung <[email protected]>
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
>
> 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 67b825cbb88f..b7d815997753 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2984,7 +2984,8 @@ STATIC_IFN_KUNIT bool modereset_required(struct drm_crtc_state *crtc_state)
>   EXPORT_IF_KUNIT(modereset_required);
>   
>   STATIC_IFN_KUNIT int
> -fill_plane_color_attributes(const struct drm_plane_state *plane_state,
> +fill_plane_color_attributes(struct drm_atomic_commit *state,
> +			    const struct drm_plane_state *plane_state,
>   			    const enum surface_pixel_format format,
>   			    enum dc_color_space *color_space)
>   {
> @@ -2993,7 +2994,7 @@ fill_plane_color_attributes(const struct drm_plane_state *plane_state,
>   	*color_space = COLOR_SPACE_SRGB;
>   
>   	/* Ignore properties when DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is set */
> -	if (plane_state->state && plane_state->state->plane_color_pipeline)
> +	if (state && state->plane_color_pipeline)
>   		return 0;
>   
>   	/* DRM color properties only affect non-RGB formats. */
> @@ -3034,6 +3035,7 @@ EXPORT_IF_KUNIT(fill_plane_color_attributes);
>   
>   static int
>   fill_dc_plane_info_and_addr(struct amdgpu_device *adev,
> +			    struct drm_atomic_commit *state,
>   			    const struct drm_plane_state *plane_state,
>   			    const u64 tiling_flags,
>   			    struct dc_plane_info *plane_info,
> @@ -3127,7 +3129,7 @@ fill_dc_plane_info_and_addr(struct amdgpu_device *adev,
>   
>   	plane_info->layer_index = plane_state->normalized_zpos;
>   
> -	ret = fill_plane_color_attributes(plane_state, plane_info->format,
> +	ret = fill_plane_color_attributes(state, plane_state, plane_info->format,
>   					  &plane_info->color_space);
>   	if (ret)
>   		return ret;
> @@ -3168,7 +3170,7 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev,
>   	dc_plane_state->clip_rect = scaling_info.clip_rect;
>   	dc_plane_state->scaling_quality = scaling_info.scaling_quality;
>   
> -	ret = fill_dc_plane_info_and_addr(adev, plane_state,
> +	ret = fill_dc_plane_info_and_addr(adev, plane_state->state, plane_state,
>   					  afb->tiling_flags,
>   					  &plane_info,
>   					  &dc_plane_state->address,
> @@ -4226,7 +4228,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_commit *state,
>   		}
>   
>   		fill_dc_plane_info_and_addr(
> -			dm->adev, new_plane_state,
> +			dm->adev, state, new_plane_state,
>   			afb->tiling_flags,
>   			&bundle->plane_infos[planes_count],
>   			&bundle->flip_addrs[planes_count].address,

-- 
Robert Mader
Consultant Software Developer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718