diff mbox series

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

Message ID 20190718145053.25808-12-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: YCbCr output fixes and prep work for YCbCr 4:4:4 output | expand

Commit Message

Ville Syrjälä July 18, 2019, 2:50 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@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@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(-)

Comments

Gwan-gyeong Mun Sept. 20, 2019, 12:19 p.m. UTC | #1 
On Thu, 2019-07-18 at 17:50 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@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@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@intel.com>
diff mbox series

Patch

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