diff mbox

[1/9] drm/i915: Keep GMCH DPLL VGA mode always disabled

Message ID 1435580756-20154-2-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä June 29, 2015, 12:25 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We disable the DPLL VGA mode when enabling the DPLL, but we enaable it
again when disabling the DPLL. Having VGA mode enabled even in unused
DPLLs can cause problems for CHV, so it seems wiser to always keep it
disabled. And let's just do that on all GMCH platforms to keep things
as similar as possible between them.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c    | 8 +++++---
 drivers/gpu/drm/i915/intel_dsi.c        | 2 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++----
 3 files changed, 10 insertions(+), 8 deletions(-)

Comments

Sivakumar Thulasimani June 29, 2015, 2:16 p.m. UTC | #1 
On 6/29/2015 5:55 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We disable the DPLL VGA mode when enabling the DPLL, but we enaable it
> again when disabling the DPLL. Having VGA mode enabled even in unused
> DPLLs can cause problems for CHV, so it seems wiser to always keep it
> disabled. And let's just do that on all GMCH platforms to keep things
> as similar as possible between them.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c    | 8 +++++---
>   drivers/gpu/drm/i915/intel_dsi.c        | 2 +-
>   drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++----
>   3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d624f58..dd3b649 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1774,13 +1774,13 @@ static void i9xx_disable_pll(struct intel_crtc *crtc)
>   	/* Make sure the pipe isn't still relying on us */
>   	assert_pipe_disabled(dev_priv, pipe);
>   
> -	I915_WRITE(DPLL(pipe), 0);
> +	I915_WRITE(DPLL(pipe), DPLL_VGA_MODE_DIS);
>   	POSTING_READ(DPLL(pipe));
>   }
>   
>   static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
>   {
> -	u32 val = 0;
> +	u32 val;
>   
>   	/* Make sure the pipe isn't still relying on us */
>   	assert_pipe_disabled(dev_priv, pipe);
> @@ -1789,6 +1789,7 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
>   	 * Leave integrated clock source and reference clock enabled for pipe B.
>   	 * The latter is needed for VGA hotplug / manual detection.
>   	 */
> +	val = DPLL_VGA_MODE_DIS;
>   	if (pipe == PIPE_B)
>   		val = DPLL_INTEGRATED_CRI_CLK_VLV | DPLL_REFA_CLK_ENABLE_VLV;
>   	I915_WRITE(DPLL(pipe), val);
> @@ -1805,7 +1806,8 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
>   	assert_pipe_disabled(dev_priv, pipe);
>   
>   	/* Set PLL en = 0 */
> -	val = DPLL_SSC_REF_CLOCK_CHV | DPLL_REFA_CLK_ENABLE_VLV;
> +	val = DPLL_SSC_REF_CLOCK_CHV |
> +		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
>   	if (pipe != PIPE_A)
>   		val |= DPLL_INTEGRATED_CRI_CLK_VLV;
>   	I915_WRITE(DPLL(pipe), val);
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 3f29385..e087360 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -427,7 +427,7 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>   
>   	/* update the hw state for DPLL */
>   	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
> -		DPLL_REFA_CLK_ENABLE_VLV;
> +		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
>   
>   	tmp = I915_READ(DSPCLK_GATE_D);
>   	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 1a45385..f0e6f49 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -882,7 +882,7 @@ static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
>   	 * display and the reference clock for VGA
>   	 * hotplug / manual detection.
>   	 */
> -	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> +	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
>   		   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
>   	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
>   
> @@ -933,13 +933,13 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
>   	 */
>   	if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
>   		phy = DPIO_PHY0;
> -		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> +		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
>   			   DPLL_REFA_CLK_ENABLE_VLV);
> -		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> +		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
>   			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
>   	} else {
>   		phy = DPIO_PHY1;
> -		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) |
> +		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) | DPLL_VGA_MODE_DIS |
>   			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
>   	}
>   	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
since we keep DPLL_VGA_MODE_DIS always set, even during disable is this 
needed explicitly again here ?  other than this i am fine with this.
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Ville Syrjälä June 29, 2015, 2:31 p.m. UTC | #2 
On Mon, Jun 29, 2015 at 07:46:18PM +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 6/29/2015 5:55 PM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We disable the DPLL VGA mode when enabling the DPLL, but we enaable it
> > again when disabling the DPLL. Having VGA mode enabled even in unused
> > DPLLs can cause problems for CHV, so it seems wiser to always keep it
> > disabled. And let's just do that on all GMCH platforms to keep things
> > as similar as possible between them.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_display.c    | 8 +++++---
> >   drivers/gpu/drm/i915/intel_dsi.c        | 2 +-
> >   drivers/gpu/drm/i915/intel_runtime_pm.c | 8 ++++----
> >   3 files changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d624f58..dd3b649 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -1774,13 +1774,13 @@ static void i9xx_disable_pll(struct intel_crtc *crtc)
> >   	/* Make sure the pipe isn't still relying on us */
> >   	assert_pipe_disabled(dev_priv, pipe);
> >   
> > -	I915_WRITE(DPLL(pipe), 0);
> > +	I915_WRITE(DPLL(pipe), DPLL_VGA_MODE_DIS);
> >   	POSTING_READ(DPLL(pipe));
> >   }
> >   
> >   static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> >   {
> > -	u32 val = 0;
> > +	u32 val;
> >   
> >   	/* Make sure the pipe isn't still relying on us */
> >   	assert_pipe_disabled(dev_priv, pipe);
> > @@ -1789,6 +1789,7 @@ static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> >   	 * Leave integrated clock source and reference clock enabled for pipe B.
> >   	 * The latter is needed for VGA hotplug / manual detection.
> >   	 */
> > +	val = DPLL_VGA_MODE_DIS;
> >   	if (pipe == PIPE_B)
> >   		val = DPLL_INTEGRATED_CRI_CLK_VLV | DPLL_REFA_CLK_ENABLE_VLV;
> >   	I915_WRITE(DPLL(pipe), val);
> > @@ -1805,7 +1806,8 @@ static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
> >   	assert_pipe_disabled(dev_priv, pipe);
> >   
> >   	/* Set PLL en = 0 */
> > -	val = DPLL_SSC_REF_CLOCK_CHV | DPLL_REFA_CLK_ENABLE_VLV;
> > +	val = DPLL_SSC_REF_CLOCK_CHV |
> > +		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> >   	if (pipe != PIPE_A)
> >   		val |= DPLL_INTEGRATED_CRI_CLK_VLV;
> >   	I915_WRITE(DPLL(pipe), val);
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 3f29385..e087360 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -427,7 +427,7 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> >   
> >   	/* update the hw state for DPLL */
> >   	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
> > -		DPLL_REFA_CLK_ENABLE_VLV;
> > +		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
> >   
> >   	tmp = I915_READ(DSPCLK_GATE_D);
> >   	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 1a45385..f0e6f49 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -882,7 +882,7 @@ static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
> >   	 * display and the reference clock for VGA
> >   	 * hotplug / manual detection.
> >   	 */
> > -	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> > +	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> >   		   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> >   	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
> >   
> > @@ -933,13 +933,13 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
> >   	 */
> >   	if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
> >   		phy = DPIO_PHY0;
> > -		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> > +		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> >   			   DPLL_REFA_CLK_ENABLE_VLV);
> > -		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
> > +		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
> >   			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> >   	} else {
> >   		phy = DPIO_PHY1;
> > -		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) |
> > +		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) | DPLL_VGA_MODE_DIS |
> >   			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
> >   	}
> >   	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
> since we keep DPLL_VGA_MODE_DIS always set, even during disable is this 
> needed explicitly again here ?  other than this i am fine with this.
> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>

Yes we still need this because during driver init the BIOS may have
left it enabled, and the register loses state when the disp2d/pipe-a
power well goes down and the poweron default has VGA mode enabled.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d624f58..dd3b649 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1774,13 +1774,13 @@  static void i9xx_disable_pll(struct intel_crtc *crtc)
 	/* Make sure the pipe isn't still relying on us */
 	assert_pipe_disabled(dev_priv, pipe);
 
-	I915_WRITE(DPLL(pipe), 0);
+	I915_WRITE(DPLL(pipe), DPLL_VGA_MODE_DIS);
 	POSTING_READ(DPLL(pipe));
 }
 
 static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
-	u32 val = 0;
+	u32 val;
 
 	/* Make sure the pipe isn't still relying on us */
 	assert_pipe_disabled(dev_priv, pipe);
@@ -1789,6 +1789,7 @@  static void vlv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 	 * Leave integrated clock source and reference clock enabled for pipe B.
 	 * The latter is needed for VGA hotplug / manual detection.
 	 */
+	val = DPLL_VGA_MODE_DIS;
 	if (pipe == PIPE_B)
 		val = DPLL_INTEGRATED_CRI_CLK_VLV | DPLL_REFA_CLK_ENABLE_VLV;
 	I915_WRITE(DPLL(pipe), val);
@@ -1805,7 +1806,8 @@  static void chv_disable_pll(struct drm_i915_private *dev_priv, enum pipe pipe)
 	assert_pipe_disabled(dev_priv, pipe);
 
 	/* Set PLL en = 0 */
-	val = DPLL_SSC_REF_CLOCK_CHV | DPLL_REFA_CLK_ENABLE_VLV;
+	val = DPLL_SSC_REF_CLOCK_CHV |
+		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
 	if (pipe != PIPE_A)
 		val |= DPLL_INTEGRATED_CRI_CLK_VLV;
 	I915_WRITE(DPLL(pipe), val);
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 3f29385..e087360 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -427,7 +427,7 @@  static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 
 	/* update the hw state for DPLL */
 	intel_crtc->config->dpll_hw_state.dpll = DPLL_INTEGRATED_CLOCK_VLV |
-		DPLL_REFA_CLK_ENABLE_VLV;
+		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS;
 
 	tmp = I915_READ(DSPCLK_GATE_D);
 	tmp |= DPOUNIT_CLOCK_GATE_DISABLE;
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 1a45385..f0e6f49 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -882,7 +882,7 @@  static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
 	 * display and the reference clock for VGA
 	 * hotplug / manual detection.
 	 */
-	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
+	I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
 		   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
 	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */
 
@@ -933,13 +933,13 @@  static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
 	 */
 	if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
 		phy = DPIO_PHY0;
-		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
+		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
 			   DPLL_REFA_CLK_ENABLE_VLV);
-		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
+		I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) | DPLL_VGA_MODE_DIS |
 			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
 	} else {
 		phy = DPIO_PHY1;
-		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) |
+		I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) | DPLL_VGA_MODE_DIS |
 			   DPLL_REFA_CLK_ENABLE_VLV | DPLL_INTEGRATED_CRI_CLK_VLV);
 	}
 	udelay(1); /* >10ns for cmnreset, >0ns for sidereset */