Re: [PATCH v2 1/2] drm/i915/gen9: Assume CDCLK PLL is off if it's not locked

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

On Tue, May 24, 2016 at 03:38:32PM +0300, Imre Deak wrote:
> If the CDCLK PLL isn't locked or incorrectly configured we can just
> assume that it's off resulting in fully re-initializing both CDCLK PLL
> and CDCLK dividers. This way the CDCLK PLL sanitization added in the
> following patch can be done on BXT the same way as it's done on SKL.
> 
> v2: (Ville)
> - Remove the remaining PLL specific checks from skl_sanitize_cdclk() and
>   depend instead on the corresponding check in skl_dpll0_update().
> - Use vco == 0 instead of the corresponding boolean check in
>   skl_sanitize_cdclk().
> 
> CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++---------------------
>  1 file changed, 16 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c1e666b..47b2466 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5461,21 +5461,22 @@ skl_dpll0_update(struct drm_i915_private *dev_priv)
>  	u32 val;
>  
>  	dev_priv->cdclk_pll.ref = 24000;
> +	dev_priv->cdclk_pll.vco = 0;
>  
>  	val = I915_READ(LCPLL1_CTL);
> -	if ((val & LCPLL_PLL_ENABLE) == 0) {
> -		dev_priv->cdclk_pll.vco = 0;
> +	if ((val & LCPLL_PLL_ENABLE) == 0)
>  		return;
> -	}
>  
> -	WARN_ON((val & LCPLL_PLL_LOCK) == 0);
> +	if (WARN_ON((val & LCPLL_PLL_LOCK) == 0))
> +		return;
>  
>  	val = I915_READ(DPLL_CTRL1);
>  
> -	WARN_ON((val & (DPLL_CTRL1_HDMI_MODE(SKL_DPLL0) |
> -			DPLL_CTRL1_SSC(SKL_DPLL0) |
> -			DPLL_CTRL1_OVERRIDE(SKL_DPLL0))) !=
> -		DPLL_CTRL1_OVERRIDE(SKL_DPLL0));
> +	if (WARN_ON((val & (DPLL_CTRL1_HDMI_MODE(SKL_DPLL0) |
> +			    DPLL_CTRL1_SSC(SKL_DPLL0) |
> +			    DPLL_CTRL1_OVERRIDE(SKL_DPLL0))) !=
> +		    DPLL_CTRL1_OVERRIDE(SKL_DPLL0)))
> +		return;
>  
>  	switch (val & DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0)) {
>  	case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, SKL_DPLL0):
> @@ -5490,7 +5491,6 @@ skl_dpll0_update(struct drm_i915_private *dev_priv)
>  		break;
>  	default:
>  		MISSING_CASE(val & DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0));
> -		dev_priv->cdclk_pll.vco = 0;
>  		break;
>  	}
>  }
> @@ -5690,19 +5690,12 @@ static void skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>  	if ((I915_READ(SWF_ILK(0x18)) & 0x00FFFFFF) == 0)
>  		goto sanitize;
>  
> +	intel_update_cdclk(dev_priv->dev);
>  	/* Is PLL enabled and locked ? */
> -	if ((I915_READ(LCPLL1_CTL) & (LCPLL_PLL_ENABLE | LCPLL_PLL_LOCK)) !=
> -	    (LCPLL_PLL_ENABLE | LCPLL_PLL_LOCK))
> +	if (dev_priv->cdclk_pll.vco == 0 ||
> +	    dev_priv->cdclk_freq == dev_priv->cdclk_pll.ref)
>  		goto sanitize;
>  
> -	if ((I915_READ(DPLL_CTRL1) & (DPLL_CTRL1_HDMI_MODE(SKL_DPLL0) |
> -				      DPLL_CTRL1_SSC(SKL_DPLL0) |
> -				      DPLL_CTRL1_OVERRIDE(SKL_DPLL0))) !=
> -	    DPLL_CTRL1_OVERRIDE(SKL_DPLL0))
> -		goto sanitize;
> -
> -	intel_update_cdclk(dev_priv->dev);
> -
>  	/* DPLL okay; verify the cdclock
>  	 *
>  	 * Noticed in some instances that the freq selection is correct but
> @@ -6608,14 +6601,14 @@ static void bxt_de_pll_update(struct drm_i915_private *dev_priv)
>  	u32 val;
>  
>  	dev_priv->cdclk_pll.ref = 19200;
> +	dev_priv->cdclk_pll.vco = 0;
>  
>  	val = I915_READ(BXT_DE_PLL_ENABLE);
> -	if ((val & BXT_DE_PLL_PLL_ENABLE) == 0) {
> -		dev_priv->cdclk_pll.vco = 0;
> +	if ((val & BXT_DE_PLL_PLL_ENABLE) == 0)
>  		return;
> -	}
>  
> -	WARN_ON((val & BXT_DE_PLL_LOCK) == 0);
> +	if (WARN_ON((val & BXT_DE_PLL_LOCK) == 0))
> +		return;
>  
>  	val = I915_READ(BXT_DE_PLL_CTL);
>  	dev_priv->cdclk_pll.vco = (val & BXT_DE_PLL_RATIO_MASK) *
> -- 
> 2.5.0

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