Re: [PATCH 07/12] drm/i915: Sprinke a few sanity check WARNS during csc assignment

["Nautiyal, Ankit K" <ankit.k.nautiyal@xxxxxxxxx>]

LGTM.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>

On 3/29/2023 7:19 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Make sure the csc enable bit(s) match the way we're about to
> fill the csc matrices.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
>   drivers/gpu/drm/i915/display/intel_color.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 7e8820583942..2988c91d8ff6 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -373,10 +373,16 @@ static void ilk_assign_csc(struct intel_crtc_state *crtc_state)
>   	bool limited_color_range = ilk_csc_limited_range(crtc_state);
>   
>   	if (crtc_state->hw.ctm) {
> +		drm_WARN_ON(&i915->drm, !crtc_state->csc_enable);
> +
>   		ilk_csc_convert_ctm(crtc_state, &crtc_state->csc, limited_color_range);
>   	} else if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) {
> +		drm_WARN_ON(&i915->drm, !crtc_state->csc_enable);
> +
>   		ilk_csc_copy(i915, &crtc_state->csc, &ilk_csc_matrix_rgb_to_ycbcr);
>   	} else if (limited_color_range) {
> +		drm_WARN_ON(&i915->drm, !crtc_state->csc_enable);
> +
>   		ilk_csc_copy(i915, &crtc_state->csc, &ilk_csc_matrix_limited_range);
>   	} else if (crtc_state->csc_enable) {
>   		/*
> @@ -406,16 +412,26 @@ static void icl_assign_csc(struct intel_crtc_state *crtc_state)
>   	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
>   
>   	if (crtc_state->hw.ctm) {
> +		drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & ICL_CSC_ENABLE) == 0);
> +
>   		ilk_csc_convert_ctm(crtc_state, &crtc_state->csc, false);
>   	} else {
> +		drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & ICL_CSC_ENABLE) != 0);
> +
>   		intel_csc_clear(&crtc_state->csc);
>   	}
>   
>   	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB) {
> +		drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & ICL_OUTPUT_CSC_ENABLE) == 0);
> +
>   		ilk_csc_copy(i915, &crtc_state->output_csc, &ilk_csc_matrix_rgb_to_ycbcr);
>   	} else if (crtc_state->limited_color_range) {
> +		drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & ICL_OUTPUT_CSC_ENABLE) == 0);
> +
>   		ilk_csc_copy(i915, &crtc_state->output_csc, &ilk_csc_matrix_limited_range);
>   	} else {
> +		drm_WARN_ON(&i915->drm, (crtc_state->csc_mode & ICL_OUTPUT_CSC_ENABLE) != 0);
> +
>   		intel_csc_clear(&crtc_state->output_csc);
>   	}
>   }
> @@ -476,9 +492,15 @@ static void chv_load_cgm_csc(struct intel_crtc *crtc,
>   
>   static void chv_assign_csc(struct intel_crtc_state *crtc_state)
>   {
> +	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +
>   	if (crtc_state->hw.ctm) {
> +		drm_WARN_ON(&i915->drm, (crtc_state->cgm_mode & CGM_PIPE_MODE_CSC) == 0);
> +
>   		chv_cgm_csc_convert_ctm(crtc_state, &crtc_state->csc);
>   	} else {
> +		drm_WARN_ON(&i915->drm, (crtc_state->cgm_mode & CGM_PIPE_MODE_CSC) != 0);
> +
>   		intel_csc_clear(&crtc_state->csc);
>   	}
>   }