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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx