diff mbox series

[08/12] drm/i915: Simplify intel_get_crtc_ycbcr_config()

Message ID 20190718145053.25808-9-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 Syrjala July 18, 2019, 2:50 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Make intel_get_crtc_ycbcr_config() simpler and rename it
to bdw_get_pipemisc_output_format() to better reflect what
it does.

Also toss in some comments to document that the 4:2:0 PIPECONF
bits are glk+ only. They are mbz on earlier platforms so reading
them unconditionally is safe however.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 71 +++++++++-----------
 drivers/gpu/drm/i915/i915_reg.h              |  4 +-
 2 files changed, 34 insertions(+), 41 deletions(-)

Comments

Gwan-gyeong Mun Sept. 18, 2019, 7:02 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>
> 
> Make intel_get_crtc_ycbcr_config() simpler and rename it
> to bdw_get_pipemisc_output_format() to better reflect what
> it does.
> 
> Also toss in some comments to document that the 4:2:0 PIPECONF
> bits are glk+ only. They are mbz on earlier platforms so reading
> them unconditionally is safe however.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 71 +++++++++---------
> --
>  drivers/gpu/drm/i915/i915_reg.h              |  4 +-
>  2 files changed, 34 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index ffdc350dc24a..1dd1aa29a649 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8713,47 +8713,24 @@ static void chv_crtc_clock_get(struct
> intel_crtc *crtc,
>  	pipe_config->port_clock = chv_calc_dpll_params(refclk, &clock);
>  }
>  
> -static void intel_get_crtc_ycbcr_config(struct intel_crtc *crtc,
> -					struct intel_crtc_state
> *pipe_config)
> +static enum intel_output_format
> +bdw_get_pipemisc_output_format(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	enum intel_output_format output = INTEL_OUTPUT_FORMAT_RGB;
> -
> -	pipe_config->lspcon_downsampling = false;
> -
> -	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
> -		u32 tmp = I915_READ(PIPEMISC(crtc->pipe));
> -
> -		if (tmp & PIPEMISC_OUTPUT_COLORSPACE_YUV) {
> -			bool ycbcr420_enabled = tmp &
> PIPEMISC_YUV420_ENABLE;
> -			bool blend = tmp &
> PIPEMISC_YUV420_MODE_FULL_BLEND;
> -
> -			if (ycbcr420_enabled) {
> -				/* We support 4:2:0 in full blend mode
> only */
> -				if (!blend)
> -					output =
> INTEL_OUTPUT_FORMAT_INVALID;
> -				else if (!(IS_GEMINILAKE(dev_priv) ||
> -					   INTEL_GEN(dev_priv) >= 10))
> -					output =
> INTEL_OUTPUT_FORMAT_INVALID;
> -				else
> -					output =
> INTEL_OUTPUT_FORMAT_YCBCR420;
> -			} else {
> -				/*
> -				 * Currently there is no interface
> defined to
> -				 * check user preference between
> RGB/YCBCR444
> -				 * or YCBCR420. So the only possible
> case for
> -				 * YCBCR444 usage is driving YCBCR420
> output
> -				 * with LSPCON, when pipe is configured
> for
> -				 * YCBCR444 output and LSPCON takes
> care of
> -				 * downsampling it.
> -				 */
> -				pipe_config->lspcon_downsampling =
> true;
> -				output = INTEL_OUTPUT_FORMAT_YCBCR444;
> -			}
> -		}
> -	}
> +	u32 tmp;
> +
> +	tmp = I915_READ(PIPEMISC(crtc->pipe));
>  
> -	pipe_config->output_format = output;
> +	if (tmp & PIPEMISC_YUV420_ENABLE) {
> +		/* We support 4:2:0 in full blend mode only */
> +		WARN_ON((tmp & PIPEMISC_YUV420_MODE_FULL_BLEND) == 0);
> +
> +		return INTEL_OUTPUT_FORMAT_YCBCR420;
> +	} else if (tmp & PIPEMISC_OUTPUT_COLORSPACE_YUV) {
> +		return INTEL_OUTPUT_FORMAT_YCBCR444;
> +	} else {
> +		return INTEL_OUTPUT_FORMAT_RGB;
> +	}
>  }
>  
>  static void i9xx_get_pipe_color_config(struct intel_crtc_state
> *crtc_state)
> @@ -10445,7 +10422,23 @@ static bool haswell_get_pipe_config(struct
> intel_crtc *crtc,
>  	}
>  
>  	intel_get_pipe_src_size(crtc, pipe_config);
> -	intel_get_crtc_ycbcr_config(crtc, pipe_config);
> +
> +	if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) {
> +		pipe_config->output_format =
> +			bdw_get_pipemisc_output_format(crtc);
> +
> +		/*
> +		 * Currently there is no interface defined to
> +		 * check user preference between RGB/YCBCR444
> +		 * or YCBCR420. So the only possible case for
> +		 * YCBCR444 usage is driving YCBCR420 output
> +		 * with LSPCON, when pipe is configured for
> +		 * YCBCR444 output and LSPCON takes care of
> +		 * downsampling it.
> +		 */
> +		pipe_config->lspcon_downsampling =
> +			pipe_config->output_format ==
> INTEL_OUTPUT_FORMAT_YCBCR444;
> +	}
>  
>  	pipe_config->gamma_mode = I915_READ(GAMMA_MODE(crtc->pipe));
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 91bf714897e5..66f7f417231f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5803,8 +5803,8 @@ enum {
>  
>  #define _PIPE_MISC_A			0x70030
>  #define _PIPE_MISC_B			0x71030
> -#define   PIPEMISC_YUV420_ENABLE	(1 << 27)
> -#define   PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26)
> +#define   PIPEMISC_YUV420_ENABLE	(1 << 27) /* glk+ */
> +#define   PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26) /* glk+ */
>  #define   PIPEMISC_HDR_MODE_PRECISION	(1 << 23) /* icl+ */
>  #define   PIPEMISC_OUTPUT_COLORSPACE_YUV  (1 << 11)
>  #define   PIPEMISC_DITHER_BPC_MASK	(7 << 5)
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_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index ffdc350dc24a..1dd1aa29a649 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8713,47 +8713,24 @@  static void chv_crtc_clock_get(struct intel_crtc *crtc,
 	pipe_config->port_clock = chv_calc_dpll_params(refclk, &clock);
 }
 
-static void intel_get_crtc_ycbcr_config(struct intel_crtc *crtc,
-					struct intel_crtc_state *pipe_config)
+static enum intel_output_format
+bdw_get_pipemisc_output_format(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	enum intel_output_format output = INTEL_OUTPUT_FORMAT_RGB;
-
-	pipe_config->lspcon_downsampling = false;
-
-	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
-		u32 tmp = I915_READ(PIPEMISC(crtc->pipe));
-
-		if (tmp & PIPEMISC_OUTPUT_COLORSPACE_YUV) {
-			bool ycbcr420_enabled = tmp & PIPEMISC_YUV420_ENABLE;
-			bool blend = tmp & PIPEMISC_YUV420_MODE_FULL_BLEND;
-
-			if (ycbcr420_enabled) {
-				/* We support 4:2:0 in full blend mode only */
-				if (!blend)
-					output = INTEL_OUTPUT_FORMAT_INVALID;
-				else if (!(IS_GEMINILAKE(dev_priv) ||
-					   INTEL_GEN(dev_priv) >= 10))
-					output = INTEL_OUTPUT_FORMAT_INVALID;
-				else
-					output = INTEL_OUTPUT_FORMAT_YCBCR420;
-			} else {
-				/*
-				 * Currently there is no interface defined to
-				 * check user preference between RGB/YCBCR444
-				 * or YCBCR420. So the only possible case for
-				 * YCBCR444 usage is driving YCBCR420 output
-				 * with LSPCON, when pipe is configured for
-				 * YCBCR444 output and LSPCON takes care of
-				 * downsampling it.
-				 */
-				pipe_config->lspcon_downsampling = true;
-				output = INTEL_OUTPUT_FORMAT_YCBCR444;
-			}
-		}
-	}
+	u32 tmp;
+
+	tmp = I915_READ(PIPEMISC(crtc->pipe));
 
-	pipe_config->output_format = output;
+	if (tmp & PIPEMISC_YUV420_ENABLE) {
+		/* We support 4:2:0 in full blend mode only */
+		WARN_ON((tmp & PIPEMISC_YUV420_MODE_FULL_BLEND) == 0);
+
+		return INTEL_OUTPUT_FORMAT_YCBCR420;
+	} else if (tmp & PIPEMISC_OUTPUT_COLORSPACE_YUV) {
+		return INTEL_OUTPUT_FORMAT_YCBCR444;
+	} else {
+		return INTEL_OUTPUT_FORMAT_RGB;
+	}
 }
 
 static void i9xx_get_pipe_color_config(struct intel_crtc_state *crtc_state)
@@ -10445,7 +10422,23 @@  static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	}
 
 	intel_get_pipe_src_size(crtc, pipe_config);
-	intel_get_crtc_ycbcr_config(crtc, pipe_config);
+
+	if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv)) {
+		pipe_config->output_format =
+			bdw_get_pipemisc_output_format(crtc);
+
+		/*
+		 * Currently there is no interface defined to
+		 * check user preference between RGB/YCBCR444
+		 * or YCBCR420. So the only possible case for
+		 * YCBCR444 usage is driving YCBCR420 output
+		 * with LSPCON, when pipe is configured for
+		 * YCBCR444 output and LSPCON takes care of
+		 * downsampling it.
+		 */
+		pipe_config->lspcon_downsampling =
+			pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR444;
+	}
 
 	pipe_config->gamma_mode = I915_READ(GAMMA_MODE(crtc->pipe));
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 91bf714897e5..66f7f417231f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5803,8 +5803,8 @@  enum {
 
 #define _PIPE_MISC_A			0x70030
 #define _PIPE_MISC_B			0x71030
-#define   PIPEMISC_YUV420_ENABLE	(1 << 27)
-#define   PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26)
+#define   PIPEMISC_YUV420_ENABLE	(1 << 27) /* glk+ */
+#define   PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26) /* glk+ */
 #define   PIPEMISC_HDR_MODE_PRECISION	(1 << 23) /* icl+ */
 #define   PIPEMISC_OUTPUT_COLORSPACE_YUV  (1 << 11)
 #define   PIPEMISC_DITHER_BPC_MASK	(7 << 5)