Re: [PATCH 62/66] drm/i915: ->disable hook for WRPLLs

Damien Lespiau
2014-05-20T11:30:17Z

On Thu, Apr 24, 2014 at 11:55:38PM +0200, Daniel Vetter wrote:
> Currently still with a redudant WARN_ON in there, the common shared
> dpll code will take care of this in the future.
>
> Also we need to flip the switch for the transitional hack now to make
> sure that we disable the right pll.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

With the small bikeshed below:

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

--
Damien

> ---
> drivers/gpu/drm/i915/intel_ddi.c | 27 ++++++++++++++++-----------
> drivers/gpu/drm/i915/intel_display.c | 8 +++++---
> 2 files changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 73875ee4bb68..e7e1a84fe453 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -260,28 +260,21 @@ 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);
> - uint32_t val;
> +
> + struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(intel_crtc);

Spurious new line.

> switch (intel_crtc->config.ddi_pll_sel) {
> case PORT_CLK_SEL_WRPLL1:
> plls->wrpll1_refcount--;
> if (plls->wrpll1_refcount == 0) {
> - DRM_DEBUG_KMS("Disabling WRPLL 1\n");
> - val = I915_READ(WRPLL_CTL1);
> - WARN_ON(!(val & WRPLL_PLL_ENABLE));
> - I915_WRITE(WRPLL_CTL1, val & ~WRPLL_PLL_ENABLE);
> - POSTING_READ(WRPLL_CTL1);
> + 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) {
> - DRM_DEBUG_KMS("Disabling WRPLL 2\n");
> - val = I915_READ(WRPLL_CTL2);
> - WARN_ON(!(val & WRPLL_PLL_ENABLE));
> - I915_WRITE(WRPLL_CTL2, val & ~WRPLL_PLL_ENABLE);
> - POSTING_READ(WRPLL_CTL2);
> + pll->disable(dev_priv, pll);
> }
> intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
> break;
> @@ -1187,6 +1180,17 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
> }
> }
>
> +static void hsw_ddi_pll_disable(struct drm_i915_private *dev_priv,
> + struct intel_shared_dpll *pll)
> +{
> + 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));
> +}
> +
> static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
> struct intel_shared_dpll *pll,
> struct intel_dpll_hw_state *hw_state)
> @@ -1217,6 +1221,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
> for (i = 0; i < 2; 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;
> dev_priv->shared_dplls[i].get_hw_state =
> hsw_ddi_pll_get_hw_state;
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b5086fd83596..b890c97f1312 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4788,9 +4788,11 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> if (HAS_IPS(dev))
> hsw_compute_ips_config(crtc, pipe_config);
>
> - /* XXX: PCH clock sharing is done in ->mode_set, so make sure the old
> - * clock survives for now. */
> - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
> + /*
> + * XXX: PCH/WRPLL clock sharing is done in ->mode_set, so make sure the
> + * old clock survives for now.
> + */
> + if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev) || HAS_DDI(dev))
> pipe_config->shared_dpll = crtc->config.shared_dpll;
>
> if (pipe_config->has_pch_encoder)
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx