[PATCH 21/31] drm/i915: consolidate pch pll enable sequence

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 05, 2013 at 01:34:23PM +0200, Daniel Vetter wrote:
> It's been splattered over 3 different places all doing random things.
> Now we have (mostly) the same sequence as i8xx/i9xx, but all called
> from the crtc_enable hook (through the pll->enable function):
> - write new dividers
> - enable vco and wait for stable clocks
> - write again for the pixel mutliplier
> 
> I've left the seemingly random 200 usec delay in there, just in case.
> 
> Also move the encoder->pre_pll_enable hook into the crtc_enable
> function, at the same spot we currently have a hack to enable the lvds
> port. Since that hack is now redundant, kill it.
> 
> While doing this patch I've learned the hard way that we can only fire
> up the LVDS port if both the pch dpll _and_ the fdi rc pll are not yet
> enabled. Otherwise things go haywire, at least on cpt.
> 
> v2: It is paramount to write the FPx divisors before we enable the
> the vco by writing to the DPLL registers, for otherwise the divisors
> won't get updated. This is in line with the i8xx/i9xx dpll.
> 
> v3: To keep the nice abstraction add a ->mode_set callback to set the
> divisors. Also streamline the enabling/disabling code a bit by
> removing some cargo-cult duplication and clearing registers where
> possible in the ->disable hook.
> 
> v4: Remove now unused local variable.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>

For some reason my brain refuses to work on this patch. I say, let's
test it in the wild instead.

Acked-by: Damien Lespiau <damien.lespiau at intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 +
>  drivers/gpu/drm/i915/intel_display.c | 75 +++++++++++++-----------------------
>  2 files changed, 29 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4dc94ed..9fc1ea4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -157,6 +157,8 @@ struct intel_shared_dpll {
>  	/* should match the index in the dev_priv->shared_dplls array */
>  	enum intel_dpll_id id;
>  	struct intel_dpll_hw_state hw_state;
> +	void (*mode_set)(struct drm_i915_private *dev_priv,
> +			 struct intel_shared_dpll *pll);
>  	void (*enable)(struct drm_i915_private *dev_priv,
>  		       struct intel_shared_dpll *pll);
>  	void (*disable)(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fc1b5f7..334f86a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3064,13 +3064,7 @@ found:
>  		WARN_ON(pll->on);
>  		assert_shared_dpll_disabled(dev_priv, pll);
>  
> -		/* Wait for the clocks to stabilize before rewriting the regs */
> -		I915_WRITE(PCH_DPLL(pll->id), dpll & ~DPLL_VCO_ENABLE);
> -		POSTING_READ(PCH_DPLL(pll->id));
> -		udelay(150);
> -
> -		I915_WRITE(PCH_FP0(pll->id), fp);
> -		I915_WRITE(PCH_DPLL(pll->id), dpll & ~DPLL_VCO_ENABLE);
> +		pll->mode_set(dev_priv, pll);
>  	}
>  	pll->refcount++;
>  
> @@ -3120,7 +3114,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>  	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
>  	int plane = intel_crtc->plane;
> -	u32 temp;
>  
>  	WARN_ON(!crtc->enabled);
>  
> @@ -3134,12 +3127,9 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>  
>  	intel_update_watermarks(dev);
>  
> -	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> -		temp = I915_READ(PCH_LVDS);
> -		if ((temp & LVDS_PORT_EN) == 0)
> -			I915_WRITE(PCH_LVDS, temp | LVDS_PORT_EN);
> -	}
> -
> +	for_each_encoder_on_crtc(dev, crtc, encoder)
> +		if (encoder->pre_pll_enable)
> +			encoder->pre_pll_enable(encoder);
>  
>  	if (intel_crtc->config.has_pch_encoder) {
>  		/* Note: FDI PLL enabling _must_ be done before we enable the
> @@ -5682,10 +5672,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	if (intel_crtc->config.has_dp_encoder)
>  		intel_dp_set_m_n(intel_crtc);
>  
> -	for_each_encoder_on_crtc(dev, crtc, encoder)
> -		if (encoder->pre_pll_enable)
> -			encoder->pre_pll_enable(encoder);
> -
>  	if (is_lvds && has_reduced_clock && i915_powersave)
>  		intel_crtc->lowfreq_avail = true;
>  	else
> @@ -5694,23 +5680,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	if (intel_crtc->config.has_pch_encoder) {
>  		pll = intel_crtc_to_shared_dpll(intel_crtc);
>  
> -		I915_WRITE(PCH_DPLL(pll->id), dpll);
> -
> -		/* Wait for the clocks to stabilize. */
> -		POSTING_READ(PCH_DPLL(pll->id));
> -		udelay(150);
> -
> -		/* The pixel multiplier can only be updated once the
> -		 * DPLL is enabled and the clocks are stable.
> -		 *
> -		 * So write it again.
> -		 */
> -		I915_WRITE(PCH_DPLL(pll->id), dpll);
> -
> -		if (has_reduced_clock)
> -			I915_WRITE(PCH_FP1(pll->id), fp2);
> -		else
> -			I915_WRITE(PCH_FP1(pll->id), fp);
>  	}
>  
>  	intel_set_pipe_timings(intel_crtc);
> @@ -8735,19 +8704,32 @@ static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv,
>  	return val & DPLL_VCO_ENABLE;
>  }
>  
> +static void ibx_pch_dpll_mode_set(struct drm_i915_private *dev_priv,
> +				  struct intel_shared_dpll *pll)
> +{
> +	I915_WRITE(PCH_FP0(pll->id), pll->hw_state.fp0);
> +	I915_WRITE(PCH_FP1(pll->id), pll->hw_state.fp1);
> +}
> +
>  static void ibx_pch_dpll_enable(struct drm_i915_private *dev_priv,
>  				struct intel_shared_dpll *pll)
>  {
> -	uint32_t reg, val;
> -
>  	/* PCH refclock must be enabled first */
>  	assert_pch_refclk_enabled(dev_priv);
>  
> -	reg = PCH_DPLL(pll->id);
> -	val = I915_READ(reg);
> -	val |= DPLL_VCO_ENABLE;
> -	I915_WRITE(reg, val);
> -	POSTING_READ(reg);
> +	I915_WRITE(PCH_DPLL(pll->id), pll->hw_state.dpll);
> +
> +	/* Wait for the clocks to stabilize. */
> +	POSTING_READ(PCH_DPLL(pll->id));
> +	udelay(150);
> +
> +	/* The pixel multiplier can only be updated once the
> +	 * DPLL is enabled and the clocks are stable.
> +	 *
> +	 * So write it again.
> +	 */
> +	I915_WRITE(PCH_DPLL(pll->id), pll->hw_state.dpll);
> +	POSTING_READ(PCH_DPLL(pll->id));
>  	udelay(200);
>  }
>  
> @@ -8756,7 +8738,6 @@ static void ibx_pch_dpll_disable(struct drm_i915_private *dev_priv,
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  	struct intel_crtc *crtc;
> -	uint32_t reg, val;
>  
>  	/* Make sure no transcoder isn't still depending on us. */
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
> @@ -8764,11 +8745,8 @@ static void ibx_pch_dpll_disable(struct drm_i915_private *dev_priv,
>  			assert_pch_transcoder_disabled(dev_priv, crtc->pipe);
>  	}
>  
> -	reg = PCH_DPLL(pll->id);
> -	val = I915_READ(reg);
> -	val &= ~DPLL_VCO_ENABLE;
> -	I915_WRITE(reg, val);
> -	POSTING_READ(reg);
> +	I915_WRITE(PCH_DPLL(pll->id), 0);
> +	POSTING_READ(PCH_DPLL(pll->id));
>  	udelay(200);
>  }
>  
> @@ -8787,6 +8765,7 @@ static void ibx_pch_dpll_init(struct drm_device *dev)
>  	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
>  		dev_priv->shared_dplls[i].id = i;
>  		dev_priv->shared_dplls[i].name = ibx_pch_dpll_names[i];
> +		dev_priv->shared_dplls[i].mode_set = ibx_pch_dpll_mode_set;
>  		dev_priv->shared_dplls[i].enable = ibx_pch_dpll_enable;
>  		dev_priv->shared_dplls[i].disable = ibx_pch_dpll_disable;
>  		dev_priv->shared_dplls[i].get_hw_state =
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux