Re: [PATCH] drm/i915: Account for CD2X divider in ICL+ vco calculation

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

On Thu, Sep 05, 2019 at 02:38:24PM -0700, Matt Roper wrote:
> When calculating the PLL ratio we were still assuming that the CD2X
> divider is always /1.  For the new frequencies that use a /2 divider
> that needs to be accounted for, otherwise our VCO result will be twice
> as large as it should be.
> 
> Fixes: 3d1da92baffe ("drm/i915: Add 324mhz and 326.4mhz cdclks for gen11+")
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 1329d3e60e26..55801aeefd1c 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1794,6 +1794,7 @@ static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
>  
>  static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
>  {
> +	int cd2xdiv;
>  	int ratio;
>  
>  	if (cdclk == dev_priv->cdclk.hw.bypass)
> @@ -1805,27 +1806,37 @@ static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
>  		/* fall through */
>  	case 172800:
>  	case 307200:
> -	case 326400:
>  	case 556800:
>  	case 652800:
> +		cd2xdiv = 1;
>  		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
>  			dev_priv->cdclk.hw.ref != 38400);
>  		break;
>  	case 180000:
>  	case 312000:
> -	case 324000:
>  	case 552000:
>  	case 648000:
> +		cd2xdiv = 1;
>  		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
>  		break;
>  	case 192000:
> +		cd2xdiv = 1;
>  		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
>  			dev_priv->cdclk.hw.ref != 38400 &&
>  			dev_priv->cdclk.hw.ref != 24000);
>  		break;
> +	case 326400:
> +		cd2xdiv = 2;
> +		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> +			dev_priv->cdclk.hw.ref != 38400);
> +		break;
> +	case 324000:
> +		cd2xdiv = 2;
> +		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> +		break;
>  	}
>  
> -	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> +	ratio = cdclk / (dev_priv->cdclk.hw.ref / cd2xdiv / 2);
>  
>  	return dev_priv->cdclk.hw.ref * ratio;

Isn't that just cdclk*cd2xdiv*2 ?

I do kinda like the idea of not hardcoding the vco ratio here.
We should probably do the same for the older platforms as well...

>  }
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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