diff mbox

drm/i915: clean up vlv ->pre_pll_enable and pll enable sequence

Message ID 1370506973-11989-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter June 6, 2013, 8:22 a.m. UTC 
No need to call the ->pre_pll_enable hook twice if we don't enable the
dpll too early. This should make Jani a bit less grumpy.

v2: Rebase on top of the newly-colored BUG_ONs.

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 45 +++++++++++++++---------------------
 1 file changed, 18 insertions(+), 27 deletions(-)

Comments

Imre Deak July 11, 2013, 2:11 p.m. UTC | #1 
On Thu, 2013-06-06 at 10:22 +0200, Daniel Vetter wrote:
> No need to call the ->pre_pll_enable hook twice if we don't enable the
> dpll too early. This should make Jani a bit less grumpy.
> 
> v2: Rebase on top of the newly-colored BUG_ONs.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 45 +++++++++++++++---------------------
>  1 file changed, 18 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5e43b9a..6e4d666 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1268,32 +1268,38 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
>  	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID);
>  }
>  
> -static void vlv_enable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> +static void vlv_enable_pll(struct intel_crtc *crtc)
>  {
> -	int reg;
> -	u32 val;
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int reg = DPLL(crtc->pipe);
> +	u32 dpll = crtc->config.dpll_hw_state.dpll;
>  
> -	assert_pipe_disabled(dev_priv, pipe);
> +	assert_pipe_disabled(dev_priv, crtc->pipe);
>  
>  	/* No really, not for ILK+ */
>  	BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
>  
>  	/* PLL is protected by panel, make sure we can write it */
>  	if (IS_MOBILE(dev_priv->dev) && !IS_I830(dev_priv->dev))
> -		assert_panel_unlocked(dev_priv, pipe);
> +		assert_panel_unlocked(dev_priv, crtc->pipe);
> +
> +	I915_WRITE(reg, dpll);
> +	POSTING_READ(reg);
> +	udelay(150);
> +
> +	if (wait_for(((I915_READ(reg) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
> +		DRM_ERROR("DPLL %d failed to lock\n", crtc->pipe);
>  
> -	reg = DPLL(pipe);
> -	val = I915_READ(reg);
> -	val |= DPLL_VCO_ENABLE;
>  
>  	/* We do this three times for luck */
> -	I915_WRITE(reg, val);
> +	I915_WRITE(reg, dpll);
>  	POSTING_READ(reg);
>  	udelay(150); /* wait for warmup */
> -	I915_WRITE(reg, val);
> +	I915_WRITE(reg, dpll);
>  	POSTING_READ(reg);
>  	udelay(150); /* wait for warmup */
> -	I915_WRITE(reg, val);
> +	I915_WRITE(reg, dpll);
>  	POSTING_READ(reg);
>  	udelay(150); /* wait for warmup */
>  }
> @@ -3561,7 +3567,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>  		if (encoder->pre_pll_enable)
>  			encoder->pre_pll_enable(encoder);
>  
> -	vlv_enable_pll(dev_priv, pipe);
> +	vlv_enable_pll(intel_crtc);
>  
>  	for_each_encoder_on_crtc(dev, crtc, encoder)
>  		if (encoder->pre_enable)
> @@ -4315,7 +4321,6 @@ static void vlv_update_pll(struct intel_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_encoder *encoder;
>  	int pipe = crtc->pipe;
>  	u32 dpll, mdiv;
>  	u32 bestn, bestm1, bestm2, bestp1, bestp2;
> @@ -4403,10 +4408,6 @@ static void vlv_update_pll(struct intel_crtc *crtc)
>  
>  	vlv_dpio_write(dev_priv, DPIO_PLL_CML(pipe), 0x87871000);
>  
> -	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
> -		if (encoder->pre_pll_enable)
> -			encoder->pre_pll_enable(encoder);
> -
>  	/* Enable DPIO clock input */
>  	dpll = DPLL_EXT_BUFFER_ENABLE_VLV | DPLL_REFA_CLK_ENABLE_VLV |
>  		DPLL_VGA_MODE_DIS | DPLL_INTEGRATED_CLOCK_VLV;
> @@ -4416,20 +4417,10 @@ static void vlv_update_pll(struct intel_crtc *crtc)
>  	dpll |= DPLL_VCO_ENABLE;
>  	crtc->config.dpll_hw_state.dpll = dpll;
>  
> -	I915_WRITE(DPLL(pipe), dpll);
> -	POSTING_READ(DPLL(pipe));
> -	udelay(150);
> -
> -	if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
> -		DRM_ERROR("DPLL %d failed to lock\n", pipe);
> -
>  	dpll_md = (crtc->config.pixel_multiplier - 1)
>  		<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
>  	crtc->config.dpll_hw_state.dpll_md = dpll_md;
>  
> -	I915_WRITE(DPLL_MD(pipe), dpll_md);
> -	POSTING_READ(DPLL_MD(pipe));

This piece was not added to vlv_enable_pll. Other than this patches
29-31 look ok, so on those:

Reviewed-by: Imre Deak <imre.deak@intel.com>
Daniel Vetter July 12, 2013, 4:27 p.m. UTC | #2 
On Thu, Jul 11, 2013 at 05:11:41PM +0300, Imre Deak wrote:
> This piece was not added to vlv_enable_pll. Other than this patches
> 29-31 look ok, so on those:
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Ok, I've pushed the updated patch plus the other two, thanks for the
review.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5e43b9a..6e4d666 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1268,32 +1268,38 @@  static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv,
 	assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID);
 }
 
-static void vlv_enable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
+static void vlv_enable_pll(struct intel_crtc *crtc)
 {
-	int reg;
-	u32 val;
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int reg = DPLL(crtc->pipe);
+	u32 dpll = crtc->config.dpll_hw_state.dpll;
 
-	assert_pipe_disabled(dev_priv, pipe);
+	assert_pipe_disabled(dev_priv, crtc->pipe);
 
 	/* No really, not for ILK+ */
 	BUG_ON(!IS_VALLEYVIEW(dev_priv->dev));
 
 	/* PLL is protected by panel, make sure we can write it */
 	if (IS_MOBILE(dev_priv->dev) && !IS_I830(dev_priv->dev))
-		assert_panel_unlocked(dev_priv, pipe);
+		assert_panel_unlocked(dev_priv, crtc->pipe);
+
+	I915_WRITE(reg, dpll);
+	POSTING_READ(reg);
+	udelay(150);
+
+	if (wait_for(((I915_READ(reg) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
+		DRM_ERROR("DPLL %d failed to lock\n", crtc->pipe);
 
-	reg = DPLL(pipe);
-	val = I915_READ(reg);
-	val |= DPLL_VCO_ENABLE;
 
 	/* We do this three times for luck */
-	I915_WRITE(reg, val);
+	I915_WRITE(reg, dpll);
 	POSTING_READ(reg);
 	udelay(150); /* wait for warmup */
-	I915_WRITE(reg, val);
+	I915_WRITE(reg, dpll);
 	POSTING_READ(reg);
 	udelay(150); /* wait for warmup */
-	I915_WRITE(reg, val);
+	I915_WRITE(reg, dpll);
 	POSTING_READ(reg);
 	udelay(150); /* wait for warmup */
 }
@@ -3561,7 +3567,7 @@  static void valleyview_crtc_enable(struct drm_crtc *crtc)
 		if (encoder->pre_pll_enable)
 			encoder->pre_pll_enable(encoder);
 
-	vlv_enable_pll(dev_priv, pipe);
+	vlv_enable_pll(intel_crtc);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		if (encoder->pre_enable)
@@ -4315,7 +4321,6 @@  static void vlv_update_pll(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_encoder *encoder;
 	int pipe = crtc->pipe;
 	u32 dpll, mdiv;
 	u32 bestn, bestm1, bestm2, bestp1, bestp2;
@@ -4403,10 +4408,6 @@  static void vlv_update_pll(struct intel_crtc *crtc)
 
 	vlv_dpio_write(dev_priv, DPIO_PLL_CML(pipe), 0x87871000);
 
-	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
-		if (encoder->pre_pll_enable)
-			encoder->pre_pll_enable(encoder);
-
 	/* Enable DPIO clock input */
 	dpll = DPLL_EXT_BUFFER_ENABLE_VLV | DPLL_REFA_CLK_ENABLE_VLV |
 		DPLL_VGA_MODE_DIS | DPLL_INTEGRATED_CLOCK_VLV;
@@ -4416,20 +4417,10 @@  static void vlv_update_pll(struct intel_crtc *crtc)
 	dpll |= DPLL_VCO_ENABLE;
 	crtc->config.dpll_hw_state.dpll = dpll;
 
-	I915_WRITE(DPLL(pipe), dpll);
-	POSTING_READ(DPLL(pipe));
-	udelay(150);
-
-	if (wait_for(((I915_READ(DPLL(pipe)) & DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1))
-		DRM_ERROR("DPLL %d failed to lock\n", pipe);
-
 	dpll_md = (crtc->config.pixel_multiplier - 1)
 		<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
 	crtc->config.dpll_hw_state.dpll_md = dpll_md;
 
-	I915_WRITE(DPLL_MD(pipe), dpll_md);
-	POSTING_READ(DPLL_MD(pipe));
-
 	if (crtc->config.has_dp_encoder)
 		intel_dp_set_m_n(crtc);