Re: [PATCH] drm/i915/icl: Update csc and gamma enable checks

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 03 Oct 2024, Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com> wrote:
> With ICL, we have a way to check if gamma and csc are enabled on
> a pipe using bits in GAMMA_MODE and CSC_MODE. So, use them as well
> along with the existing BOTTOM_COLOR checks.

Why?

That's the most important question any commit message must answer.

The state checker is supposed to check that the hardware state matches
the software state exactly. AFAICT this change makes the write and read
asymmetric.

.gamma_mode and .csc_mode are already checked verbatim. What benefit
does this change bring?

BR,
Jani.


>
> BSpec: 7463, 7466
> Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 24 ++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 50f41aeb3c28..1bf36898dc7e 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1076,6 +1076,26 @@ static void skl_get_config(struct intel_crtc_state *crtc_state)
>  		crtc_state->csc_enable = true;
>  }
>  
> +static void icl_get_config(struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	u32 bottom_color;
> +
> +	crtc_state->gamma_mode = hsw_read_gamma_mode(crtc);
> +	crtc_state->csc_mode = ilk_read_csc_mode(crtc);
> +
> +	bottom_color = intel_de_read(i915, SKL_BOTTOM_COLOR(crtc->pipe));
> +
> +	if ((bottom_color & SKL_BOTTOM_COLOR_GAMMA_ENABLE) ||
> +	    (crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE))
> +		crtc_state->gamma_enable = true;
> +
> +	if ((bottom_color & SKL_BOTTOM_COLOR_CSC_ENABLE) ||
> +	    (crtc_state->csc_mode & ICL_CSC_ENABLE))
> +		crtc_state->csc_enable = true;
> +}
> +
>  static void skl_color_commit_arm(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> @@ -3811,7 +3831,7 @@ static const struct intel_color_funcs tgl_color_funcs = {
>  	.read_luts = icl_read_luts,
>  	.lut_equal = icl_lut_equal,
>  	.read_csc = icl_read_csc,
> -	.get_config = skl_get_config,
> +	.get_config = icl_get_config,
>  };
>  
>  static const struct intel_color_funcs icl_color_funcs = {
> @@ -3823,7 +3843,7 @@ static const struct intel_color_funcs icl_color_funcs = {
>  	.read_luts = icl_read_luts,
>  	.lut_equal = icl_lut_equal,
>  	.read_csc = icl_read_csc,
> -	.get_config = skl_get_config,
> +	.get_config = icl_get_config,
>  };
>  
>  static const struct intel_color_funcs glk_color_funcs = {

-- 
Jani Nikula, Intel