Re: [PATCH v4 07/11] drm/amd/display: Implement fixed matrix colorop color space mapping

Hi, as per my previous comment:

Tested-by: Robert Mader <[email protected]>

On 22.07.26 15:46, Harry Wentland wrote:
> Add __set_dm_plane_colorop_fixed_matrix() which maps the fixed matrix
> type enum to the appropriate dc_color_space for DC programming:
>
>    YCBCR601_FULL_RGB     -> COLOR_SPACE_YCBCR601
>    YCBCR601_LIMITED_RGB  -> COLOR_SPACE_YCBCR601_LIMITED
>    YCBCR709_FULL_RGB     -> COLOR_SPACE_YCBCR709
>    YCBCR709_LIMITED_RGB  -> COLOR_SPACE_YCBCR709_LIMITED
>    YCBCR2020_FULL_RGB_NC -> COLOR_SPACE_2020_YCBCR_FULL
>    YCBCR2020_LIMITED_RGB_NC -> COLOR_SPACE_2020_YCBCR_LIMITED
>
> When the fixed matrix is bypassed, color_space is set to
> COLOR_SPACE_UNKNOWN.
>
> Update amdgpu_dm_plane_set_colorop_properties() to process the
> fixed matrix colorop first (before DEGAM), matching the new pipeline
> order.
>
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Harry Wentland <[email protected]>
> Reviewed-by: Alex Hung <[email protected]>
> ---
>   .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 68 ++++++++++++++++++-
>   1 file changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> index 357c7c5c85cf..62791077ceef 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> @@ -1545,6 +1545,61 @@ __set_colorop_in_tf_1d_curve(struct dc_plane_state *dc_plane_state,
>   }
>   EXPORT_IF_KUNIT(__set_colorop_in_tf_1d_curve);
>   
> +static int
> +__set_dm_plane_colorop_fixed_matrix(struct drm_plane_state *plane_state,
> +				    struct dc_plane_state *dc_plane_state,
> +				    struct drm_colorop *colorop)
> +{
> +	struct drm_colorop *old_colorop;
> +	struct drm_colorop_state *colorop_state = NULL, *new_colorop_state;
> +	struct drm_atomic_commit *state = plane_state->state;
> +	int i = 0;
> +
> +	old_colorop = colorop;
> +
> +	for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
> +		if (new_colorop_state->colorop == old_colorop) {
> +			colorop_state = new_colorop_state;
> +			break;
> +		}
> +	}
> +
> +	if (!colorop_state)
> +		return -EINVAL;
> +
> +	if (colorop_state->bypass) {
> +		dc_plane_state->color_space = COLOR_SPACE_SRGB;
> +		return 0;
> +	}
> +
> +	switch (colorop_state->fixed_matrix_type) {
> +	case DRM_COLOROP_FM_YCBCR601_FULL_RGB:
> +		dc_plane_state->color_space = COLOR_SPACE_YCBCR601;
> +		break;
> +	case DRM_COLOROP_FM_YCBCR601_LIMITED_RGB:
> +		dc_plane_state->color_space = COLOR_SPACE_YCBCR601_LIMITED;
> +		break;
> +	case DRM_COLOROP_FM_YCBCR709_FULL_RGB:
> +		dc_plane_state->color_space = COLOR_SPACE_YCBCR709;
> +		break;
> +	case DRM_COLOROP_FM_YCBCR709_LIMITED_RGB:
> +		dc_plane_state->color_space = COLOR_SPACE_YCBCR709_LIMITED;
> +		break;
> +	case DRM_COLOROP_FM_YCBCR2020_NC_FULL_RGB:
> +		dc_plane_state->color_space = COLOR_SPACE_2020_YCBCR_FULL;
> +		break;
> +	case DRM_COLOROP_FM_YCBCR2020_NC_LIMITED_RGB:
> +		dc_plane_state->color_space = COLOR_SPACE_2020_YCBCR_LIMITED;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	dc_plane_state->update_bits.full_update = 1;
> +
> +	return 0;
> +}
> +
>   static int
>   __set_dm_plane_colorop_degamma(struct drm_plane_state *plane_state,
>   			       struct dc_plane_state *dc_plane_state,
> @@ -1946,10 +2001,21 @@ amdgpu_dm_plane_set_colorop_properties(struct drm_plane_state *plane_state,
>   	bool has_3dlut = adev->dm.dc->caps.color.dpp.hw_3d_lut || adev->dm.dc->caps.color.mpc.preblend;
>   	int ret;
>   
> -	/* 1D Curve - DEGAM TF */
> +	/* Fixed Matrix (YUV to RGB) */
>   	if (!colorop)
>   		return -EINVAL;
>   
> +	ret = __set_dm_plane_colorop_fixed_matrix(plane_state, dc_plane_state, colorop);
> +	if (ret)
> +		return ret;
> +
> +	/* 1D Curve - DEGAM TF */
> +	colorop = colorop->next;
> +	if (!colorop) {
> +		drm_dbg(dev, "no degamma colorop found\n");
> +		return -EINVAL;
> +	}
> +
>   	ret = __set_dm_plane_colorop_degamma(plane_state, dc_plane_state, colorop);
>   	if (ret)
>   		return ret;

-- 
Robert Mader
Consultant Software Developer

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