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

[Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>]

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>

-- 
regards,
Sivakumar

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx