Re: [Intel-gfx] [PATCH 4/5] drm/i915/tgl: Fix Combo PHY DPLL fractional divider for 38.4MHz ref clock

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Imre Deak <imre.deak@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 4/5] drm/i915/tgl: Fix Combo PHY DPLL fractional divider for 38.4MHz ref clock
Date: Thu, 1 Oct 2020 19:45:58 +0300	[thread overview]
Message-ID: <20201001164558.GW6112@intel.com> (raw)
In-Reply-To: <20200929002929.783620-5-imre.deak@intel.com>

On Tue, Sep 29, 2020 at 03:29:28AM +0300, Imre Deak wrote:
> Apply Display WA #22010492432 for combo PHY PLLs too. This should fix a
> problem where the PLL output frequency is slightly off with the current
> PLL fractional divider value.
> 
> I haven't seen an actual case where this causes a problem, but let's
> follow the spec. It's also needed on some EHL platforms, but for that we
> also need a way to distinguish the affected EHL SKUs, so I leave that
> for a follow-up.
> 
> v2:
> - Apply the WA at one place when calculating the PLL dividers from the
>   frequency and the frequency from the dividers for all the combo PLL
>   use cases (DP, HDMI, TBT). (Ville)
> 
> 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/display/intel_dpll_mgr.c | 34 +++++++++++--------
>  1 file changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index ded2b2dfe319..e7b058340a1a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -2694,6 +2694,16 @@ static void cnl_wrpll_decode_divs(const struct skl_wrpll_params *wrpll_params,
>  	}
>  }
>  
> +/*
> + * Display WA #22010492432: tgl
> + * Program half of the nominal DCO divider fraction value.
> + */
> +static bool
> +tgl_combo_pll_div_frac_wa_needed(struct drm_i915_private *i915)
> +{
> +	return IS_TIGERLAKE(i915) && i915->dpll.ref_clks.nssc == 38400;
> +}
> +
>  static int __cnl_ddi_wrpll_get_freq(struct drm_i915_private *dev_priv,
>  				    const struct intel_shared_dpll *pll,
>  				    int ref_clock)
> @@ -2719,6 +2729,9 @@ static int __cnl_ddi_wrpll_get_freq(struct drm_i915_private *dev_priv,
>  	dco_fraction = (pll_state->cfgcr0 & DPLL_CFGCR0_DCO_FRACTION_MASK) >>
>  		       DPLL_CFGCR0_DCO_FRACTION_SHIFT;
>  
> +	if (tgl_combo_pll_div_frac_wa_needed(dev_priv))
> +		dco_fraction *= 2;
> +
>  	return skl_wrpll_calc_freq(ref_clock, dco_integer, dco_fraction, pdiv, qdiv, kdiv);
>  }
>  
> @@ -2992,16 +3005,6 @@ static const struct skl_wrpll_params tgl_tbt_pll_24MHz_values = {
>  	/* the following params are unused */
>  };
>  
> -/*
> - * Display WA #22010492432: tgl
> - * Divide the nominal .dco_fraction value by 2.
> - */
> -static const struct skl_wrpll_params tgl_tbt_pll_38_4MHz_values = {
> -	.dco_integer = 0x54, .dco_fraction = 0x1800,
> -	/* the following params are unused */
> -	.pdiv = 0, .kdiv = 0, .qdiv_mode = 0, .qdiv_ratio = 0,
> -};
> -
>  static int icl_wrpll_ref_clock(struct drm_i915_private *i915);
>  
>  static bool icl_dp_combo_pll_clock_match(struct drm_i915_private *i915, int clock,
> @@ -3059,14 +3062,12 @@ static bool icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,
>  			MISSING_CASE(dev_priv->dpll.ref_clks.nssc);
>  			fallthrough;
>  		case 19200:
> +		case 38400:
>  			*pll_params = tgl_tbt_pll_19_2MHz_values;
>  			break;
>  		case 24000:
>  			*pll_params = tgl_tbt_pll_24MHz_values;
>  			break;
> -		case 38400:
> -			*pll_params = tgl_tbt_pll_38_4MHz_values;
> -			break;
>  		}
>  	} else {
>  		switch (dev_priv->dpll.ref_clks.nssc) {
> @@ -3133,9 +3134,14 @@ static void icl_calc_dpll_state(struct drm_i915_private *i915,
>  				const struct skl_wrpll_params *pll_params,
>  				struct intel_dpll_hw_state *pll_state)
>  {
> +	u32 dco_fraction = pll_params->dco_fraction;
> +
>  	memset(pll_state, 0, sizeof(*pll_state));
>  
> -	pll_state->cfgcr0 = DPLL_CFGCR0_DCO_FRACTION(pll_params->dco_fraction) |
> +	if (tgl_combo_pll_div_frac_wa_needed(i915))
> +		dco_fraction = DIV_ROUND_CLOSEST(dco_fraction, 2);
> +
> +	pll_state->cfgcr0 = DPLL_CFGCR0_DCO_FRACTION(dco_fraction) |
>  			    pll_params->dco_integer;
>  
>  	pll_state->cfgcr1 = DPLL_CFGCR1_QDIV_RATIO(pll_params->qdiv_ratio) |
> -- 
> 2.25.1

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