[Intel-gfx] [PATCH 11/12] drm/i915: Set up ILK/SNB csc unit properly for YCbCr output

[Mun, Gwan-gyeong]

On Thu, 2019-07-18 at 17:50 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Prepare the pipe csc for YCbCr output on ilk/snb. The main difference
> to IVB+ is the lack of explicit post offsets, and instead we must
> configure the CSC info RGB->YUV mode (which takes care of offsetting
> Cb/Cr properly) and enable the "black screen offset" bit to add the
> required offset to Y.
> 
> And while at it throw some comments around the bit defines to
> document which platforms have which bits.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 25 +++++++++++++++++---
> --
>  drivers/gpu/drm/i915/i915_reg.h            | 10 ++++-----
>  2 files changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 736c42720daf..a902f7809840 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1213,6 +1213,21 @@ static u32 ilk_gamma_mode(const struct
> intel_crtc_state *crtc_state)
>  		return GAMMA_MODE_MODE_10BIT;
>  }
>  
> +static u32 ilk_csc_mode(const struct intel_crtc_state *crtc_state)
> +{
> +	/*
> +	 * CSC comes after the LUT in RGB->YCbCr mode.
> +	 * RGB->YCbCr needs the limited range offsets added to
> +	 * the output. RGB limited range output is handled by
> +	 * the hw automagically elsewhere.
> +	 */
> +	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB)
> +		return CSC_BLACK_SCREEN_OFFSET;
> +
> +	return CSC_MODE_YUV_TO_RGB |
> +		CSC_POSITION_BEFORE_GAMMA;
> +}
> +
>  static int ilk_color_check(struct intel_crtc_state *crtc_state)
>  {
>  	int ret;
> @@ -1226,15 +1241,15 @@ static int ilk_color_check(struct
> intel_crtc_state *crtc_state)
>  		!crtc_state->c8_planes;
>  
>  	/*
> -	 * We don't expose the ctm on ilk/snb currently,
> -	 * nor do we enable YCbCr output. Also RGB limited
> -	 * range output is handled by the hw automagically.
> +	 * We don't expose the ctm on ilk/snb currently, also RGB
> +	 * limited range output is handled by the hw automagically.
>  	 */
> -	crtc_state->csc_enable = false;
> +	crtc_state->csc_enable =
> +		crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB;
>  
>  	crtc_state->gamma_mode = ilk_gamma_mode(crtc_state);
>  
> -	crtc_state->csc_mode = 0;
> +	crtc_state->csc_mode = ilk_csc_mode(crtc_state);
>  
>  	ret = intel_color_add_affected_planes(crtc_state);
>  	if (ret)
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 58471312b8b2..33d535ae0944 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -10106,11 +10106,11 @@ enum skl_power_gate {
>  #define _PIPE_A_CSC_COEFF_BV	0x49024
>  
>  #define _PIPE_A_CSC_MODE	0x49028
> -#define  ICL_CSC_ENABLE			(1 << 31)
> -#define  ICL_OUTPUT_CSC_ENABLE		(1 << 30)
> -#define  CSC_BLACK_SCREEN_OFFSET	(1 << 2)
> -#define  CSC_POSITION_BEFORE_GAMMA	(1 << 1)
> -#define  CSC_MODE_YUV_TO_RGB		(1 << 0)
> +#define  ICL_CSC_ENABLE			(1 << 31) /* icl+ */
> +#define  ICL_OUTPUT_CSC_ENABLE		(1 << 30) /* icl+ */
> +#define  CSC_BLACK_SCREEN_OFFSET	(1 << 2) /* ilk/snb */
> +#define  CSC_POSITION_BEFORE_GAMMA	(1 << 1) /* pre-glk */
> +#define  CSC_MODE_YUV_TO_RGB		(1 << 0) /* ilk/snb */
>  
>  #define _PIPE_A_CSC_PREOFF_HI	0x49030
>  #define _PIPE_A_CSC_PREOFF_ME	0x49034

The changes look good to me.
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun at intel.com>