[Intel-gfx] [PATCH 2/4] drm/i915: Fix CD2X pipe select masking during cdclk sanitation

[Matt Roper]

On Wed, Sep 11, 2019 at 04:31:27PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> We're forgetting to mask off all three pipe select bits from the
> CDCLK_CTL value on icl+ which may lead to the extra bit being
> left in. That will cause us to consider the current hardware
> cdclk state as invalid, and we proceed to sanitize it even
> though the hardware may have active pipes and whatnot.
> 
> Fix up the mask so we get rid of all three pipe select bits
> and thus hopefully no longer sanitize cdclk when it's already
> correctly programmed.
> 
> Cc: Matt Roper <matthew.d.roper at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111641
> Fixes: 0c1279b58fc7 ("drm/i915: Consolidate {bxt,cnl,icl}_init_cdclk")
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>

I'm confused why pre-merge CI flagged the results as a success if TGL
was hitting this?


Matt

> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 42 ++++++++++++----------
>  1 file changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 6b75d2a91cd9..f59a6f775177 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1464,6 +1464,26 @@ static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
>  	dev_priv->cdclk.hw.vco = vco;
>  }
>  
> +static u32 bxt_cdclk_cd2x_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
> +{
> +	if (INTEL_GEN(dev_priv) >= 12) {
> +		if (pipe == INVALID_PIPE)
> +			return TGL_CDCLK_CD2X_PIPE_NONE;
> +		else
> +			return TGL_CDCLK_CD2X_PIPE(pipe);
> +	} else if (INTEL_GEN(dev_priv) >= 11) {
> +		if (pipe == INVALID_PIPE)
> +			return ICL_CDCLK_CD2X_PIPE_NONE;
> +		else
> +			return ICL_CDCLK_CD2X_PIPE(pipe);
> +	} else {
> +		if (pipe == INVALID_PIPE)
> +			return BXT_CDCLK_CD2X_PIPE_NONE;
> +		else
> +			return BXT_CDCLK_CD2X_PIPE(pipe);
> +	}
> +}
> +
>  static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>  			  const struct intel_cdclk_state *cdclk_state,
>  			  enum pipe pipe)
> @@ -1534,24 +1554,8 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>  			bxt_de_pll_enable(dev_priv, vco);
>  	}
>  
> -	val = divider | skl_cdclk_decimal(cdclk);
> -
> -	if (INTEL_GEN(dev_priv) >= 12) {
> -		if (pipe == INVALID_PIPE)
> -			val |= TGL_CDCLK_CD2X_PIPE_NONE;
> -		else
> -			val |= TGL_CDCLK_CD2X_PIPE(pipe);
> -	} else if (INTEL_GEN(dev_priv) >= 11) {
> -		if (pipe == INVALID_PIPE)
> -			val |= ICL_CDCLK_CD2X_PIPE_NONE;
> -		else
> -			val |= ICL_CDCLK_CD2X_PIPE(pipe);
> -	} else {
> -		if (pipe == INVALID_PIPE)
> -			val |= BXT_CDCLK_CD2X_PIPE_NONE;
> -		else
> -			val |= BXT_CDCLK_CD2X_PIPE(pipe);
> -	}
> +	val = divider | skl_cdclk_decimal(cdclk) |
> +		bxt_cdclk_cd2x_pipe(dev_priv, pipe);
>  
>  	/*
>  	 * Disable SSA Precharge when CD clock frequency < 500 MHz,
> @@ -1620,7 +1624,7 @@ static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
>  	 * dividers both synching to an active pipe, or asynchronously
>  	 * (PIPE_NONE).
>  	 */
> -	cdctl &= ~BXT_CDCLK_CD2X_PIPE_NONE;
> +	cdctl &= ~bxt_cdclk_cd2x_pipe(dev_priv, INVALID_PIPE);
>  
>  	/* Make sure this is a legal cdclk value for the platform */
>  	cdclk = bxt_calc_cdclk(dev_priv, dev_priv->cdclk.hw.cdclk);
> -- 
> 2.21.0
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795