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

Ville Syrjälä
2020-10-01T16:46:06Z

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