[Intel-gfx] [PATCH v2 2/2] drm/i915/bxt: Sanitize CDCLK to fix breakage during S4 resume

[Ville Syrjälä]

On Tue, May 24, 2016 at 03:38:33PM +0300, Imre Deak wrote:
> I noticed that during S4 resume BIOS incorrectly sets bits 18, 19 which
> are reserved/MBZ and sets the decimal frequency fields to all 0xff in
> the CDCLK register. The result is a hard lockup as display register
> accesses are attempted later. Work around this by sanitizing the CDCLK
> PLL/dividers the same way it's done on SKL.
> 
> While this is clearly a BIOS bug which should be fixed separately, it
> doesn't hurt to check/sanitize this regardless.
> 
> v2:
> - Use the same condition for VCO and CDCLK in broxton_init_cdclk as is
>   used in skl_init_cdclk for the same purpose.
> 
> CC: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 47b2466..19f947f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5412,11 +5412,58 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int cdclk)
>  	intel_update_cdclk(dev_priv->dev);
>  }
>  
> +static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
> +{
> +	u32 cdctl, expected;
> +
> +	intel_update_cdclk(dev_priv->dev);
> +
> +	if (dev_priv->cdclk_pll.vco == 0 ||
> +	    dev_priv->cdclk_freq == dev_priv->cdclk_pll.ref)
> +		goto sanitize;
> +
> +	/* DPLL okay; verify the cdclock
> +	 *
> +	 * Some BIOS versions leave an incorrect decimal frequency value and
> +	 * set reserved MBZ bits in CDCLK_CTL at least during exiting from S4,
> +	 * so sanitize this register.
> +	 */
> +	cdctl = I915_READ(CDCLK_CTL);
> +	/*
> +	 * Let's ignore the pipe field, since BIOS could have configured the
> +	 * dividers both synching to an active pipe, or asynchronously
> +	 * (PIPE_NONE).
> +	 */
> +	cdctl &= ~BXT_CDCLK_CD2X_PIPE_NONE;
> +
> +	expected = (cdctl & BXT_CDCLK_CD2X_DIV_SEL_MASK) |
> +		   skl_cdclk_decimal(dev_priv->cdclk_freq);
> +	/*
> +	 * Disable SSA Precharge when CD clock frequency < 500 MHz,
> +	 * enable otherwise.
> +	 */
> +	if (dev_priv->cdclk_freq >= 500000)
> +		expected |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
> +
> +	if (cdctl == expected)
> +		/* All well; nothing to sanitize */
> +		return;
> +
> +sanitize:
> +	DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
> +
> +	/* force cdclk programming */
> +	dev_priv->cdclk_freq = 0;
> +
> +	/* force full PLL disable + enable */
> +	dev_priv->cdclk_pll.vco = -1;
> +}
> +
>  void broxton_init_cdclk(struct drm_i915_private *dev_priv)
>  {
> -	intel_update_cdclk(dev_priv->dev);
> +	bxt_sanitize_cdclk(dev_priv);
>  
> -	if (dev_priv->cdclk_pll.vco != 0)
> +	if (dev_priv->cdclk_freq != 0 && dev_priv->cdclk_pll.vco != 0)
>  		return;
>  
>  	/*
> -- 
> 2.5.0

-- 
Ville Syrjälä
Intel OTC