[Intel-gfx] [PATCH 64/66] drm/i915: Switch to common shared dpll framework for WRPLLs

Damien Lespiau damien.lespiau at intel.com
Tue May 20 13:38:28 CEST 2014


On Thu, Apr 24, 2014 at 11:55:40PM +0200, Daniel Vetter wrote:
> Mostly this patch is one big excersize in deleting code and asserts
> which are no longer needed. Note that we still abuse the shared dpll
> framework a bit since we call the enable/disable functions from the
> crtc mode_set and off hooks. But changing the actual hardware sequence
> will be done in the next step.
> 
> Note that besides the massive amount of changes in this patch the
> places and order in which the low-level WRPLL code is called is
> absolutely unchanged.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>

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

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |   6 --
>  drivers/gpu/drm/i915/i915_reg.h      |   1 +
>  drivers/gpu/drm/i915/intel_ddi.c     | 142 ++++-------------------------------
>  drivers/gpu/drm/i915/intel_display.c |  14 ++--
>  drivers/gpu/drm/i915/intel_drv.h     |   9 ++-
>  5 files changed, 27 insertions(+), 145 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bebc507f776b..73371161777b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -225,11 +225,6 @@ void intel_link_compute_m_n(int bpp, int nlanes,
>  			    int pixel_clock, int link_clock,
>  			    struct intel_link_m_n *m_n);
>  
> -struct intel_ddi_plls {
> -	int wrpll1_refcount;
> -	int wrpll2_refcount;
> -};
> -
>  /* Interface history:
>   *
>   * 1.1: Original.
> @@ -1399,7 +1394,6 @@ struct drm_i915_private {
>  
>  	int num_shared_dpll;
>  	struct intel_shared_dpll shared_dplls[I915_NUM_PLLS];
> -	struct intel_ddi_plls ddi_plls;
>  	int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
>  
>  	/* Reclocking support */
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 99051e6348b8..fcb1ca6eadb5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5529,6 +5529,7 @@ enum punit_power_well {
>  #define  PORT_CLK_SEL_LCPLL_1350	(1<<29)
>  #define  PORT_CLK_SEL_LCPLL_810		(2<<29)
>  #define  PORT_CLK_SEL_SPLL		(3<<29)
> +#define  PORT_CLK_SEL_WRPLL(pll)	(((pll)+4)<<29)
>  #define  PORT_CLK_SEL_WRPLL1		(4<<29)
>  #define  PORT_CLK_SEL_WRPLL2		(5<<29)
>  #define  PORT_CLK_SEL_NONE		(7<<29)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 21e451ea0c69..7386a1212e71 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -257,31 +257,12 @@ intel_ddi_get_crtc_encoder(struct drm_crtc *crtc)
>  
>  void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
>  {
> -	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> -	struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  
> -	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(intel_crtc);
> -
> -	switch (intel_crtc->config.ddi_pll_sel) {
> -	case PORT_CLK_SEL_WRPLL1:
> -		plls->wrpll1_refcount--;
> -		if (plls->wrpll1_refcount == 0) {
> -			pll->disable(dev_priv, pll);
> -		}
> -		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
> -		break;
> -	case PORT_CLK_SEL_WRPLL2:
> -		plls->wrpll2_refcount--;
> -		if (plls->wrpll2_refcount == 0) {
> -			pll->disable(dev_priv, pll);
> -		}
> -		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
> -		break;
> -	}
> +	if (intel_crtc_to_shared_dpll(intel_crtc))
> +		intel_disable_shared_dpll(intel_crtc);
>  
> -	WARN(plls->wrpll1_refcount < 0, "Invalid WRPLL1 refcount\n");
> -	WARN(plls->wrpll2_refcount < 0, "Invalid WRPLL2 refcount\n");
> +	intel_put_shared_dpll(intel_crtc);
>  }
>  
>  #define LC_FREQ 2700
> @@ -601,17 +582,14 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
>  {
>  	struct drm_crtc *crtc = &intel_crtc->base;
>  	struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
> -	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> -	struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
>  	int type = intel_encoder->type;
> -	enum pipe pipe = intel_crtc->pipe;
>  	int clock = intel_crtc->config.port_clock;
>  
>  	intel_ddi_put_crtc_pll(crtc);
>  
>  	if (type == INTEL_OUTPUT_HDMI) {
>  		struct intel_shared_dpll *pll;
> -		uint32_t reg, val;
> +		uint32_t val;
>  		unsigned p, n2, r2;
>  
>  		intel_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
> @@ -620,79 +598,21 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
>  		      WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
>  		      WRPLL_DIVIDER_POST(p);
>  
> -		if (val == I915_READ(WRPLL_CTL1)) {
> -			DRM_DEBUG_KMS("Reusing WRPLL 1 on pipe %c\n",
> -				      pipe_name(pipe));
> -			reg = WRPLL_CTL1;
> -		} else if (val == I915_READ(WRPLL_CTL2)) {
> -			DRM_DEBUG_KMS("Reusing WRPLL 2 on pipe %c\n",
> -				      pipe_name(pipe));
> -			reg = WRPLL_CTL2;
> -		} else if (plls->wrpll1_refcount == 0) {
> -			DRM_DEBUG_KMS("Using WRPLL 1 on pipe %c\n",
> -				      pipe_name(pipe));
> -			reg = WRPLL_CTL1;
> -		} else if (plls->wrpll2_refcount == 0) {
> -			DRM_DEBUG_KMS("Using WRPLL 2 on pipe %c\n",
> -				      pipe_name(pipe));
> -			reg = WRPLL_CTL2;
> -		} else {
> -			DRM_ERROR("No WRPLLs available!\n");
> -			return false;
> -		}
> +		intel_crtc->config.dpll_hw_state.wrpll = val;
>  
> -		DRM_DEBUG_KMS("WRPLL: %dKHz refresh rate with p=%d, n2=%d r2=%d\n",
> -			      clock, p, n2, r2);
> -
> -		if (reg == WRPLL_CTL1) {
> -			plls->wrpll1_refcount++;
> -			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL1;
> -			intel_crtc->config.shared_dpll = DPLL_ID_WRPLL1;
> -		} else {
> -			plls->wrpll2_refcount++;
> -			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
> -			intel_crtc->config.shared_dpll = DPLL_ID_WRPLL2;
> +		pll = intel_get_shared_dpll(intel_crtc);
> +		if (pll == NULL) {
> +			DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
> +					 pipe_name(intel_crtc->pipe));
> +			return false;
>  		}
>  
> -		intel_crtc->config.dpll_hw_state.wrpll = val;
> -
> -		pll = &dev_priv->shared_dplls[intel_crtc->config.shared_dpll];
> -		pll->hw_state.wrpll = val;
> +		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
>  	}
>  
>  	return true;
>  }
>  
> -/*
> - * To be called after intel_ddi_pll_select(). That one selects the PLL to be
> - * used, this one actually enables the PLL.
> - */
> -void intel_ddi_pll_enable(struct intel_crtc *crtc)
> -{
> -	struct drm_device *dev = crtc->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
> -	int refcount;
> -	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
> -
> -	switch (crtc->config.ddi_pll_sel) {
> -	case PORT_CLK_SEL_WRPLL1:
> -	case PORT_CLK_SEL_WRPLL2:
> -		if (crtc->config.ddi_pll_sel == PORT_CLK_SEL_WRPLL1) {
> -			refcount = plls->wrpll1_refcount;
> -		} else {
> -			refcount = plls->wrpll2_refcount;
> -		}
> -		break;
> -	default:
> -		return;
> -	}
> -
> -	if (refcount == 1) {
> -		pll->enable(dev_priv, pll);
> -	}
> -}
> -
>  void intel_ddi_set_pipe_settings(struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> @@ -929,35 +849,6 @@ static bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
>  	return intel_ddi_get_port_state(encoder, pipe, port);
>  }
>  
> -void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	enum pipe pipe;
> -	struct intel_crtc *intel_crtc;
> -
> -	dev_priv->ddi_plls.wrpll1_refcount = 0;
> -	dev_priv->ddi_plls.wrpll2_refcount = 0;
> -
> -	for_each_pipe(pipe) {
> -		intel_crtc =
> -			to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> -
> -		if (!intel_crtc->active) {
> -			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
> -			continue;
> -		}
> -
> -		switch (intel_crtc->config.ddi_pll_sel) {
> -		case PORT_CLK_SEL_WRPLL1:
> -			dev_priv->ddi_plls.wrpll1_refcount++;
> -			break;
> -		case PORT_CLK_SEL_WRPLL2:
> -			dev_priv->ddi_plls.wrpll2_refcount++;
> -			break;
> -		}
> -	}
> -}
> -
>  void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
>  {
>  	struct drm_crtc *crtc = &intel_crtc->base;
> @@ -1157,10 +1048,6 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
>  static void hsw_ddi_pll_enable(struct drm_i915_private *dev_priv,
>  			       struct intel_shared_dpll *pll)
>  {
> -	uint32_t cur_val;
> -
> -	cur_val = I915_READ(WRPLL_CTL(pll->id));
> -	WARN(cur_val & WRPLL_PLL_ENABLE, "%s already enabled\n", pll->name);
>  	I915_WRITE(WRPLL_CTL(pll->id), pll->hw_state.wrpll);
>  	POSTING_READ(WRPLL_CTL(pll->id));
>  	udelay(20);
> @@ -1172,7 +1059,6 @@ static void hsw_ddi_pll_disable(struct drm_i915_private *dev_priv,
>  	uint32_t val;
>  
>  	val = I915_READ(WRPLL_CTL(pll->id));
> -	WARN_ON(!(val & WRPLL_PLL_ENABLE));
>  	I915_WRITE(WRPLL_CTL(pll->id), val & ~WRPLL_PLL_ENABLE);
>  	POSTING_READ(WRPLL_CTL(pll->id));
>  }
> @@ -1200,11 +1086,9 @@ void intel_ddi_pll_init(struct drm_device *dev)
>  	uint32_t val = I915_READ(LCPLL_CTL);
>  	int i;
>  
> -	/* Dummy setup until everything is moved over to avoid upsetting the hw
> -	 * state cross checker. */
> -	dev_priv->num_shared_dpll = 0;
> +	dev_priv->num_shared_dpll = 2;
>  
> -	for (i = 0; i < 2; i++) {
> +	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
>  		dev_priv->shared_dplls[i].id = i;
>  		dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
>  		dev_priv->shared_dplls[i].disable = hsw_ddi_pll_disable;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b890c97f1312..2fd77eba57f3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1599,7 +1599,7 @@ static void intel_enable_shared_dpll(struct intel_crtc *crtc)
>  	pll->on = true;
>  }
>  
> -static void intel_disable_shared_dpll(struct intel_crtc *crtc)
> +void intel_disable_shared_dpll(struct intel_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3232,7 +3232,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
>  	ironlake_enable_pch_transcoder(dev_priv, pipe);
>  }
>  
> -static void intel_put_shared_dpll(struct intel_crtc *crtc)
> +void intel_put_shared_dpll(struct intel_crtc *crtc)
>  {
>  	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
>  
> @@ -3252,7 +3252,7 @@ static void intel_put_shared_dpll(struct intel_crtc *crtc)
>  	crtc->config.shared_dpll = DPLL_ID_PRIVATE;
>  }
>  
> -static struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
> +struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
>  	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
> @@ -7001,7 +7001,9 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	if (!intel_ddi_pll_select(intel_crtc))
>  		return -EINVAL;
> -	intel_ddi_pll_enable(intel_crtc);
> +
> +	if (intel_crtc_to_shared_dpll(intel_crtc))
> +		intel_enable_shared_dpll(intel_crtc);
>  
>  	intel_crtc->lowfreq_avail = false;
>  
> @@ -11531,10 +11533,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  			      crtc->active ? "enabled" : "disabled");
>  	}
>  
> -	/* FIXME: Smash this into the new shared dpll infrastructure. */
> -	if (HAS_DDI(dev))
> -		intel_ddi_setup_hw_pll_state(dev);
> -
>  	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
>  		struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e1d079fe47ea..acd32e8e5e13 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -682,9 +682,7 @@ void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
>  				       enum transcoder cpu_transcoder);
>  void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc);
>  void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc);
> -void intel_ddi_setup_hw_pll_state(struct drm_device *dev);
>  bool intel_ddi_pll_select(struct intel_crtc *crtc);
> -void intel_ddi_pll_enable(struct intel_crtc *crtc);
>  void intel_ddi_put_crtc_pll(struct drm_crtc *crtc);
>  void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
>  void intel_ddi_prepare_link_retrain(struct drm_encoder *encoder);
> @@ -740,12 +738,19 @@ __intel_framebuffer_create(struct drm_device *dev,
>  void intel_prepare_page_flip(struct drm_device *dev, int plane);
>  void intel_finish_page_flip(struct drm_device *dev, int pipe);
>  void intel_finish_page_flip_plane(struct drm_device *dev, int plane);
> +
> +/* shared dpll functions */
>  struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
>  void assert_shared_dpll(struct drm_i915_private *dev_priv,
>  			struct intel_shared_dpll *pll,
>  			bool state);
>  #define assert_shared_dpll_enabled(d, p) assert_shared_dpll(d, p, true)
>  #define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false)
> +void intel_disable_shared_dpll(struct intel_crtc *crtc);
> +struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc);
> +void intel_put_shared_dpll(struct intel_crtc *crtc);
> +
> +/* modesetting asserts */
>  void assert_pll(struct drm_i915_private *dev_priv,
>  		enum pipe pipe, bool state);
>  #define assert_pll_enabled(d, p) assert_pll(d, p, true)
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



More information about the Intel-gfx mailing list