[Intel-gfx] [PATCH V2] drm/i915/ehl: Implement W/A 22010492432

[Surendrakumar Upadhyay, TejaskumarX]

> -----Original Message-----
> From: Imre Deak <imre.deak at intel.com>
> Sent: 03 November 2020 21:13
> To: Surendrakumar Upadhyay, TejaskumarX
> <tejaskumarx.surendrakumar.upadhyay at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; Pandey, Hariom
> <hariom.pandey at intel.com>
> Subject: Re: [PATCH V2] drm/i915/ehl: Implement W/A 22010492432
> 
> On Tue, Nov 03, 2020 at 07:16:51PM +0530, Tejas Upadhyay wrote:
> > As per W/A implemented for TGL to program half of the nominal DCO
> > divider fraction value which is also applicable on EHL.
> >
> > Changes since V1:
> > 	- ehl_ used as to keep earliest platform prefix
> > 	- WA required B0 stepping onwards
> >
> > Cc: Deak Imre <imre.deak at intel.com>
> > Signed-off-by: Tejas Upadhyay
> > <tejaskumarx.surendrakumar.upadhyay at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 13 ++++++++-----
> >  drivers/gpu/drm/i915/i915_drv.h               |  1 +
> >  2 files changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index eaef7a2d041f..cb6ebf627c04 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -2636,13 +2636,16 @@ static bool cnl_ddi_hdmi_pll_dividers(struct
> > intel_crtc_state *crtc_state)  }
> >
> >  /*
> > - * Display WA #22010492432: tgl
> > + * Display WA #22010492432: ehl, tgl
> >   * Program half of the nominal DCO divider fraction value.
> >   */
> >  static bool
> > -tgl_combo_pll_div_frac_wa_needed(struct drm_i915_private *i915)
> > +ehl_combo_pll_div_frac_wa_needed(struct drm_i915_private *i915)
> >  {
> > -	return IS_TIGERLAKE(i915) && i915->dpll.ref_clks.nssc == 38400;
> > +	return ((IS_PLATFORM(i915, INTEL_ELKHARTLAKE) &&
> > +		IS_JSL_EHL_REVID(i915, EHL_REVID_B0, EHL_REVID_B0)) ||
> 
> Imo, better to add a definition for IS_ELKHARTLAKE() and IS_EHL_REVID().
> 
It has been already discussed in previous EHL/JSL PCI id split patch (between Matt Roper/Ville/Me) that we will not keep IS_ELKHARTLAKE() and IS_EHL_REVID() instead we will replace with IS_PLATFORM(i915, INTEL_ELKHARTLAKE) and IS_JSL_EHL_REVID .

> It also applies after B0, so it'd be
> 		IS_EHL_REVID(EHL_REVID_B0, REVID_FOREVER);
B0 is latest revision. So current logic should be fine.
> 
> > +		IS_TIGERLAKE(i915)) &&
> > +		i915->dpll.ref_clks.nssc == 38400;
> >  }
> >
> >  static int __cnl_ddi_wrpll_get_freq(struct drm_i915_private
> > *dev_priv, @@ -2696,7 +2699,7 @@ 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))
> > +	if (ehl_combo_pll_div_frac_wa_needed(dev_priv))
> >  		dco_fraction *= 2;
> >
> >  	dco_freq += (dco_fraction * ref_clock) / 0x8000; @@ -3086,7 +3089,7
> > @@ static void icl_calc_dpll_state(struct drm_i915_private *i915,
> >
> >  	memset(pll_state, 0, sizeof(*pll_state));
> >
> > -	if (tgl_combo_pll_div_frac_wa_needed(i915))
> > +	if (ehl_combo_pll_div_frac_wa_needed(i915))
> >  		dco_fraction = DIV_ROUND_CLOSEST(dco_fraction, 2);
> >
> >  	pll_state->cfgcr0 = DPLL_CFGCR0_DCO_FRACTION(dco_fraction) | diff
> > --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index d548e10e1600..8bf59b57efc9
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1560,6 +1560,7 @@ extern const struct i915_rev_steppings
> kbl_revids[];
> >  	(IS_ICELAKE(p) && IS_REVID(p, since, until))
> >
> >  #define EHL_REVID_A0            0x0
> > +#define EHL_REVID_B0            0x2
> 
> Where are the steppings specified for EHL? At least on the BSpec/29153 page
> I see EHL/B0 being 1.
> 
> >
> >  #define IS_JSL_EHL_REVID(p, since, until) \
> >  	(IS_JSL_EHL(p) && IS_REVID(p, since, until))
> > --
> > 2.28.0
> >