diff mbox

[1/8] drm/i915: extract ironlake_set_pipeconf form ironlake_crtc_mode_set

Message ID 1347455196-5167-2-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni Sept. 12, 2012, 1:06 p.m. UTC
From: Paulo Zanoni <paulo.r.zanoni@intel.com>

Because ironlake_crtc_mode_set is a giant function that used to have
404 lines. Let's try to make it less complex/confusing.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   88 ++++++++++++++++++++--------------
 1 file changed, 52 insertions(+), 36 deletions(-)

Comments

Daniel Vetter Sept. 12, 2012, 2:03 p.m. UTC | #1
On Wed, Sep 12, 2012 at 10:06:29AM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Because ironlake_crtc_mode_set is a giant function that used to have
> 404 lines. Let's try to make it less complex/confusing.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   88 ++++++++++++++++++++--------------
>  1 file changed, 52 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e061acd..244bce6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4646,6 +4646,50 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
>  	return 120000;
>  }
>  
> +static void ironlake_set_pipeconf(struct drm_crtc *crtc,
> +				  struct drm_display_mode *adjusted_mode,
> +				  bool dither)
> +{
> +	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	int pipe = intel_crtc->pipe;
> +	uint32_t val;
> +
> +	val = I915_READ(PIPECONF(pipe));
> +
> +	val &= ~PIPE_BPC_MASK;
> +	switch (intel_crtc->bpp) {
> +	case 18:
> +		val |= PIPE_6BPC;
> +		break;
> +	case 24:
> +		val |= PIPE_8BPC;
> +		break;
> +	case 30:
> +		val |= PIPE_10BPC;
> +		break;
> +	case 36:
> +		val |= PIPE_12BPC;
> +		break;
> +	default:
> +		val |= PIPE_8BPC;
> +		break;
> +	}
> +
> +	val &= ~(PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
> +	if (dither)
> +		val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
> +
> +	val &= ~PIPECONF_INTERLACE_MASK;
> +	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
> +		val |= PIPECONF_INTERLACED_ILK;
> +	else
> +		val |= PIPECONF_PROGRESSIVE;
> +
> +	I915_WRITE(PIPECONF(pipe), val);
> +	POSTING_READ(PIPECONF(pipe));
> +}
> +
>  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  				  struct drm_display_mode *mode,
>  				  struct drm_display_mode *adjusted_mode,
> @@ -4659,7 +4703,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	int plane = intel_crtc->plane;
>  	int refclk, num_connectors = 0;
>  	intel_clock_t clock, reduced_clock;
> -	u32 dpll, fp = 0, fp2 = 0, dspcntr, pipeconf;
> +	u32 dpll, fp = 0, fp2 = 0, dspcntr;
>  	bool ok, has_reduced_clock = false, is_sdvo = false;
>  	bool is_crt = false, is_lvds = false, is_tv = false, is_dp = false;
>  	struct intel_encoder *encoder, *edp_encoder = NULL;
> @@ -4768,32 +4812,17 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  		target_clock = adjusted_mode->clock;
>  
>  	/* determine panel color depth */
> -	temp = I915_READ(PIPECONF(pipe));
> -	temp &= ~PIPE_BPC_MASK;
>  	dither = intel_choose_pipe_bpp_dither(crtc, fb, &pipe_bpp, mode);
> -	switch (pipe_bpp) {
> -	case 18:
> -		temp |= PIPE_6BPC;
> -		break;
> -	case 24:
> -		temp |= PIPE_8BPC;
> -		break;
> -	case 30:
> -		temp |= PIPE_10BPC;
> -		break;
> -	case 36:
> -		temp |= PIPE_12BPC;
> -		break;
> -	default:
> +	if (is_lvds && dev_priv->lvds_dither)
> +		dither = true;
> +
> +	if (pipe_bpp != 18 && pipe_bpp != 24 && pipe_bpp != 30 &&
> +	    pipe_bpp != 36) {
>  		WARN(1, "intel_choose_pipe_bpp returned invalid value %d\n",
> -			pipe_bpp);
> -		temp |= PIPE_8BPC;
> +		     pipe_bpp);
>  		pipe_bpp = 24;
> -		break;

choose_pipe_bpp_dither already has this fallback code and we duplicate
things here, again. Can you follow up with a 2nd patch to rip this out and
add a BUG(); to the new ilk_set_pipeconf function for the default: bpp
case and directly store the bpp in intel_crtc->bpp?

Patch looks good, applied.
-Daniel
>  	}
> -
>  	intel_crtc->bpp = pipe_bpp;
> -	I915_WRITE(PIPECONF(pipe), temp);
>  
>  	if (!lane) {
>  		/*
> @@ -4877,9 +4906,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	else
>  		dpll |= PLL_REF_INPUT_DREFCLK;
>  
> -	/* setup pipeconf */
> -	pipeconf = I915_READ(PIPECONF(pipe));
> -
>  	/* Set up the display plane register */
>  	dspcntr = DISPPLANE_GAMMA_ENABLE;
>  
> @@ -4942,12 +4968,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  		I915_WRITE(PCH_LVDS, temp);
>  	}
>  
> -	pipeconf &= ~PIPECONF_DITHER_EN;
> -	pipeconf &= ~PIPECONF_DITHER_TYPE_MASK;
> -	if ((is_lvds && dev_priv->lvds_dither) || dither) {
> -		pipeconf |= PIPECONF_DITHER_EN;
> -		pipeconf |= PIPECONF_DITHER_TYPE_SP;
> -	}
>  	if (is_dp && !is_cpu_edp) {
>  		intel_dp_set_m_n(crtc, mode, adjusted_mode);
>  	} else {
> @@ -4983,9 +5003,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  		}
>  	}
>  
> -	pipeconf &= ~PIPECONF_INTERLACE_MASK;
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
> -		pipeconf |= PIPECONF_INTERLACED_ILK;
>  		/* the chip adds 2 halflines automatically */
>  		adjusted_mode->crtc_vtotal -= 1;
>  		adjusted_mode->crtc_vblank_end -= 1;
> @@ -4993,7 +5011,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  			   adjusted_mode->crtc_hsync_start
>  			   - adjusted_mode->crtc_htotal/2);
>  	} else {
> -		pipeconf |= PIPECONF_PROGRESSIVE;
>  		I915_WRITE(VSYNCSHIFT(pipe), 0);
>  	}
>  
> @@ -5031,8 +5048,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	if (is_cpu_edp)
>  		ironlake_set_pll_edp(crtc, adjusted_mode->clock);
>  
> -	I915_WRITE(PIPECONF(pipe), pipeconf);
> -	POSTING_READ(PIPECONF(pipe));
> +	ironlake_set_pipeconf(crtc, adjusted_mode, dither);
>  
>  	intel_wait_for_vblank(dev, pipe);
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e061acd..244bce6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4646,6 +4646,50 @@  static int ironlake_get_refclk(struct drm_crtc *crtc)
 	return 120000;
 }
 
+static void ironlake_set_pipeconf(struct drm_crtc *crtc,
+				  struct drm_display_mode *adjusted_mode,
+				  bool dither)
+{
+	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int pipe = intel_crtc->pipe;
+	uint32_t val;
+
+	val = I915_READ(PIPECONF(pipe));
+
+	val &= ~PIPE_BPC_MASK;
+	switch (intel_crtc->bpp) {
+	case 18:
+		val |= PIPE_6BPC;
+		break;
+	case 24:
+		val |= PIPE_8BPC;
+		break;
+	case 30:
+		val |= PIPE_10BPC;
+		break;
+	case 36:
+		val |= PIPE_12BPC;
+		break;
+	default:
+		val |= PIPE_8BPC;
+		break;
+	}
+
+	val &= ~(PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
+	if (dither)
+		val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
+
+	val &= ~PIPECONF_INTERLACE_MASK;
+	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
+		val |= PIPECONF_INTERLACED_ILK;
+	else
+		val |= PIPECONF_PROGRESSIVE;
+
+	I915_WRITE(PIPECONF(pipe), val);
+	POSTING_READ(PIPECONF(pipe));
+}
+
 static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 				  struct drm_display_mode *mode,
 				  struct drm_display_mode *adjusted_mode,
@@ -4659,7 +4703,7 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	int plane = intel_crtc->plane;
 	int refclk, num_connectors = 0;
 	intel_clock_t clock, reduced_clock;
-	u32 dpll, fp = 0, fp2 = 0, dspcntr, pipeconf;
+	u32 dpll, fp = 0, fp2 = 0, dspcntr;
 	bool ok, has_reduced_clock = false, is_sdvo = false;
 	bool is_crt = false, is_lvds = false, is_tv = false, is_dp = false;
 	struct intel_encoder *encoder, *edp_encoder = NULL;
@@ -4768,32 +4812,17 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		target_clock = adjusted_mode->clock;
 
 	/* determine panel color depth */
-	temp = I915_READ(PIPECONF(pipe));
-	temp &= ~PIPE_BPC_MASK;
 	dither = intel_choose_pipe_bpp_dither(crtc, fb, &pipe_bpp, mode);
-	switch (pipe_bpp) {
-	case 18:
-		temp |= PIPE_6BPC;
-		break;
-	case 24:
-		temp |= PIPE_8BPC;
-		break;
-	case 30:
-		temp |= PIPE_10BPC;
-		break;
-	case 36:
-		temp |= PIPE_12BPC;
-		break;
-	default:
+	if (is_lvds && dev_priv->lvds_dither)
+		dither = true;
+
+	if (pipe_bpp != 18 && pipe_bpp != 24 && pipe_bpp != 30 &&
+	    pipe_bpp != 36) {
 		WARN(1, "intel_choose_pipe_bpp returned invalid value %d\n",
-			pipe_bpp);
-		temp |= PIPE_8BPC;
+		     pipe_bpp);
 		pipe_bpp = 24;
-		break;
 	}
-
 	intel_crtc->bpp = pipe_bpp;
-	I915_WRITE(PIPECONF(pipe), temp);
 
 	if (!lane) {
 		/*
@@ -4877,9 +4906,6 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	else
 		dpll |= PLL_REF_INPUT_DREFCLK;
 
-	/* setup pipeconf */
-	pipeconf = I915_READ(PIPECONF(pipe));
-
 	/* Set up the display plane register */
 	dspcntr = DISPPLANE_GAMMA_ENABLE;
 
@@ -4942,12 +4968,6 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		I915_WRITE(PCH_LVDS, temp);
 	}
 
-	pipeconf &= ~PIPECONF_DITHER_EN;
-	pipeconf &= ~PIPECONF_DITHER_TYPE_MASK;
-	if ((is_lvds && dev_priv->lvds_dither) || dither) {
-		pipeconf |= PIPECONF_DITHER_EN;
-		pipeconf |= PIPECONF_DITHER_TYPE_SP;
-	}
 	if (is_dp && !is_cpu_edp) {
 		intel_dp_set_m_n(crtc, mode, adjusted_mode);
 	} else {
@@ -4983,9 +5003,7 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		}
 	}
 
-	pipeconf &= ~PIPECONF_INTERLACE_MASK;
 	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
-		pipeconf |= PIPECONF_INTERLACED_ILK;
 		/* the chip adds 2 halflines automatically */
 		adjusted_mode->crtc_vtotal -= 1;
 		adjusted_mode->crtc_vblank_end -= 1;
@@ -4993,7 +5011,6 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 			   adjusted_mode->crtc_hsync_start
 			   - adjusted_mode->crtc_htotal/2);
 	} else {
-		pipeconf |= PIPECONF_PROGRESSIVE;
 		I915_WRITE(VSYNCSHIFT(pipe), 0);
 	}
 
@@ -5031,8 +5048,7 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	if (is_cpu_edp)
 		ironlake_set_pll_edp(crtc, adjusted_mode->clock);
 
-	I915_WRITE(PIPECONF(pipe), pipeconf);
-	POSTING_READ(PIPECONF(pipe));
+	ironlake_set_pipeconf(crtc, adjusted_mode, dither);
 
 	intel_wait_for_vblank(dev, pipe);