With the stepping fix mentioned below,
Reviewed-by: Anusha Srivatsa <anusha.srivtsa@xxxxxxxxx>
> -----Original Message-----
> From: Souza, Jose <jose.souza@xxxxxxxxx>
> Sent: Thursday, August 27, 2020 3:57 PM
> To: Srivatsa, Anusha <anusha.srivatsa@xxxxxxxxx>; intel-
> gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Bai, Guangyao <guangyao.bai@xxxxxxxxx>; Lee, Penne Y
> <penne.y.lee@xxxxxxxxx>
> Subject: Re: [PATCH] drm/i915/tgl: Fix stepping WA matching
>
> On Thu, 2020-08-27 at 13:48 -0700, Srivatsa, Anusha wrote:
> > > -----Original Message-----
> > > From: Intel-gfx <
> > > intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx
> > > > On Behalf Of Souza,
> > > Jose
> > > Sent: Tuesday, August 25, 2020 12:49 PM
> > > To:
> > > intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > >
> > > Cc: Bai, Guangyao <
> > > guangyao.bai@xxxxxxxxx
> > > >; Lee, Penne Y
> > > <
> > > penne.y.lee@xxxxxxxxx
> > > >
> > > Subject: Re: [PATCH] drm/i915/tgl: Fix stepping WA
> > > matching
> > >
> > > On Wed, 2020-08-19 at 13:33 -0700, José Roberto de Souza wrote:
> > > > TGL made stepping a litte mess, workarounds refer to the stepping
> > > > of the IP(GT or Display) not of the GPU stepping so it would
> > > > already require the same solution as used in commit 96c5a15f9f39
> > > > ("drm/i915/kbl: Fix revision ID checks").
> > > > But to make things even more messy it have a different IP stepping
> > > > mapping between SKUs and the same stepping revision of GT do not
> > > > match the same HW between TGL U/Y and regular TGL.
> > > >
> > > > So it was required to have 2 different macros to check GT WAs
> > > > while for Display we are able to use just one macro that uses the
> > > > right revids table.
> > > >
> > > > All TGL workarounds checked and updated accordingly.
> > > >
> > > > BSpec: 52890
> > > > BSpec: 55378
> > > > BSpec: 44455
> > > > Cc: Penne Lee <
> > > > penne.y.lee@xxxxxxxxx
> > > >
> > > >
> > > > Cc: Guangyao Bai <
> > > > guangyao.bai@xxxxxxxxx
> > > >
> > > >
> > > > Cc: Matt Roper <
> > > > matthew.d.roper@xxxxxxxxx
> > > >
> > > >
> > > > Signed-off-by: José Roberto de Souza < jose.souza@xxxxxxxxx
> > > >
> > > >
> > > > ---
> > > > .../drm/i915/display/intel_display_power.c | 2 +-
> > > > drivers/gpu/drm/i915/display/intel_psr.c | 4 +-
> > > > drivers/gpu/drm/i915/display/intel_sprite.c | 2 +-
> > > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 24 ++++++++++--
> > > > drivers/gpu/drm/i915/i915_drv.h | 39 ++++++++++++++++---
> > > > drivers/gpu/drm/i915/intel_pm.c | 2 +-
> > > > 6 files changed, 59 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > index 7946c6af4b1e..7277e58b01f1 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > @@ -5263,7 +5263,7 @@ static void tgl_bw_buddy_init(struct
> > >
> > > drm_i915_private *dev_priv)
> > > > unsigned long abox_mask = INTEL_INFO(dev_priv)->abox_mask;
> > > > int config, i;
> > > >
> > > > - if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_B0))
> > > > + if (IS_TGL_DISP_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_B0))
> > > > /* Wa_1409767108: tgl */
> > > > table = wa_1409767108_buddy_page_masks;
> > > > else
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index 2b004ee9619c..8a9d0bdde1bf 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -555,7 +555,7 @@ static void hsw_activate_psr2(struct intel_dp
> > > > *intel_dp)
> > > >
> > > > if (dev_priv->psr.psr2_sel_fetch_enabled) {
> > > > /* WA 1408330847 */
> > > > - if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0) ||
> > > > + if (IS_TGL_DISP_REVID(dev_priv, TGL_REVID_A0,
> > >
> > > TGL_REVID_A0) ||
> > > > IS_RKL_REVID(dev_priv, RKL_REVID_A0, RKL_REVID_A0))
> > > > intel_de_rmw(dev_priv, CHICKEN_PAR1_1,
> > > > DIS_RAM_BYPASS_PSR2_MAN_TRACK,
> > >
> > > @@ -1109,7 +1109,7 @@ static
> > > > void intel_psr_disable_locked(struct intel_dp *intel_dp)
> > > >
> > > > /* WA 1408330847 */
> > > > if (dev_priv->psr.psr2_sel_fetch_enabled &&
> > > > - (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0) ||
> > > > + (IS_TGL_DISP_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0) ||
> > > > IS_RKL_REVID(dev_priv, RKL_REVID_A0, RKL_REVID_A0)))
> > > > intel_de_rmw(dev_priv, CHICKEN_PAR1_1,
> > > > DIS_RAM_BYPASS_PSR2_MAN_TRACK, 0); diff --git
> > > > a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > index c26ca029fc0a..1797a06cfd60 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> > > > @@ -2845,7 +2845,7 @@ static bool
> > >
> > > gen12_plane_supports_mc_ccs(struct
> > > > drm_i915_private *dev_priv, {
> > > > /* Wa_14010477008:tgl[a0..c0],rkl[all] */
> > > > if (IS_ROCKETLAKE(dev_priv) ||
> > > > - IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_C0))
> > > > + IS_TGL_DISP_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_C0))
> > > > return false;
> > > >
> > > > return plane_id < PLANE_SPRITE4;
> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > > b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > > index be5a4685c991..860d6ae1d866 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > > @@ -70,6 +70,19 @@ const struct i915_rev_steppings kbl_revids[] = {
> > > > [7] = { .gt_stepping = KBL_REVID_G0, .disp_stepping = KBL_REVID_C0
> > > > }, };
> > > >
> > > > +const struct i915_rev_steppings tgl_uy_revids[] = {
> > > > + [0] = { .gt_stepping = TGL_REVID_A0, .disp_stepping = TGL_REVID_A0
> > >
> > > },
> > > > + [1] = { .gt_stepping = TGL_REVID_B0, .disp_stepping = TGL_REVID_C0
> > >
> > > },
> > > > + [2] = { .gt_stepping = TGL_REVID_B1, .disp_stepping = TGL_REVID_C0
> > >
> > > },
> > > > + [3] = { .gt_stepping = TGL_REVID_C0, .disp_stepping = TGL_REVID_D0
> > > > +}, };
> > > > +
> > > > +/* Same GT stepping between tgl_uy_revids and tgl_revids don't mean
> > > > +the same HW */ const struct i915_rev_steppings tgl_revids[] = {
> > > > + [0] = { .gt_stepping = TGL_REVID_A0, .disp_stepping = TGL_REVID_A0
> >
> > Shouldn’t this have display stepping as B0? Or am I missing something?
>
> It should, thanks for catching this.
>
> Any other issues? Can I add your rv-b with the above fixed?
>
> >
> > Anusha
> >
> > > },
> > > > + [1] = { .gt_stepping = TGL_REVID_B0, .disp_stepping = TGL_REVID_D0
> > > > +}, };
> > > > +
> > > > static void wa_init_start(struct i915_wa_list *wal, const char *name,
> > > > const char *engine_name) {
> > > > wal->name = name;
> > > > @@ -1219,13 +1232,13 @@ tgl_gt_workarounds_init(struct
> > >
> > > drm_i915_private *i915, struct i915_wa_list *wal)
> > > > gen12_gt_workarounds_init(i915, wal);
> > > >
> > > > /* Wa_1409420604:tgl */
> > > > - if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> > > > + if (IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> > > > wa_write_or(wal,
> > > > SUBSLICE_UNIT_LEVEL_CLKGATE2,
> > > > CPSSUNIT_CLKGATE_DIS);
> > > >
> > > > /* Wa_1607087056:tgl also know as BUG:1409180338 */
> > > > - if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> > > > + if (IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> > > > wa_write_or(wal,
> > > > SLICE_UNIT_LEVEL_CLKGATE,
> > > > L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS); @@ -
> > >
> > > 1660,7 +1673,7 @@
> > > > rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list
> > > > *wal) {
> > > > struct drm_i915_private *i915 = engine->i915;
> > > >
> > > > - if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
> > > > + if (IS_TGL_UY_GT_REVID(i915, TGL_REVID_A0, TGL_REVID_A0)) {
> > > > /*
> > > > * Wa_1607138336:tgl
> > > > * Wa_1607063988:tgl
> > > > @@ -1697,6 +1710,9 @@ rcs_engine_wa_init(struct intel_engine_cs
> > >
> > > *engine, struct i915_wa_list *wal)
> > > > wa_masked_en(wal, GEN9_ROW_CHICKEN4,
> > >
> > > GEN12_DISABLE_TDL_PUSH);
> > > > /*
> > > > + * TODO: still pending the decision if this WA is permanent for
> > > > + * plain TGL SKU(not TGL-U/Y).
> > >
> > > Wa_14010919138(mentioned one line bellow 'Wa_22010931296:tgl
> B0+') is
> > > now permanent so this TODO can be removed.
> > >
> > > > + *
> > > > * Wa_1407928979:tgl A*
> > > > * Wa_18011464164:tgl B0+
> > > > * Wa_22010931296:tgl B0+
> > > > @@ -1718,7 +1734,7 @@ rcs_engine_wa_init(struct intel_engine_cs
> > >
> > > *engine, struct i915_wa_list *wal)
> > > > GEN8_RC_SEMA_IDLE_MSG_DISABLE);
> > > > }
> > > >
> > > > - if (IS_TIGERLAKE(i915)) {
> > > > + if (IS_TGL_U(i915) || IS_TGL_Y(i915)) {
> > > > /* Wa_1606700617:tgl */
> > > > wa_masked_en(wal,
> > > > GEN9_CS_DEBUG_MODE1,
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > > b/drivers/gpu/drm/i915/i915_drv.h index
> 8e25dd15a2ec..499eb0a4ad3a
> > > > 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -1572,12 +1572,41 @@ extern const struct i915_rev_steppings
> > > > kbl_revids[]; #define IS_EHL_REVID(p, since, until) \
> > > > (IS_ELKHARTLAKE(p) && IS_REVID(p, since, until))
> > > >
> > > > -#define TGL_REVID_A0 0x0
> > > > -#define TGL_REVID_B0 0x1
> > > > -#define TGL_REVID_C0 0x2
> > > > +enum {
> > > > + TGL_REVID_A0,
> > > > + TGL_REVID_B0,
> > > > + TGL_REVID_B1,
> > > > + TGL_REVID_C0,
> > > > + TGL_REVID_D0,
> > > > +};
> > > > +
> > > > +extern const struct i915_rev_steppings tgl_uy_revids[]; extern const
> > > > +struct i915_rev_steppings tgl_revids[];
> > > > +
> > > > +static inline const struct i915_rev_steppings * tgl_revids_get(struct
> > > > +drm_i915_private *dev_priv) {
> > > > + if (IS_TGL_U(dev_priv) || IS_TGL_Y(dev_priv))
> > > > + return tgl_uy_revids;
> > > > + else
> > > > + return tgl_revids;
> > > > +}
> > > >
> > > > -#define IS_TGL_REVID(p, since, until) \
> > > > - (IS_TIGERLAKE(p) && IS_REVID(p, since, until))
> > > > +#define IS_TGL_DISP_REVID(p, since, until) \
> > > > + (IS_TIGERLAKE(p) && \
> > > > + tgl_revids_get(p)->disp_stepping >= (since) && \
> > > > + tgl_revids_get(p)->disp_stepping <= (until))
> > > > +
> > > > +#define IS_TGL_UY_GT_REVID(p, since, until) \
> > > > + ((IS_TGL_U(p) || IS_TGL_Y(p)) && \
> > > > + tgl_uy_revids->gt_stepping >= (since) && \
> > > > + tgl_uy_revids->gt_stepping <= (until))
> > > > +
> > > > +#define IS_TGL_GT_REVID(p, since, until) \
> > > > + (IS_TIGERLAKE(p) && \
> > > > + !(IS_TGL_U(p) || IS_TGL_Y(p)) && \
> > > > + tgl_revids->gt_stepping >= (since) && \
> > > > + tgl_revids->gt_stepping <= (until))
> > > >
> > > > #define RKL_REVID_A0 0x0
> > > > #define RKL_REVID_B0 0x1
> > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > > b/drivers/gpu/drm/i915/intel_pm.c index
> b4bd19266b8c..34e0d22d456b
> > > > 100644
> > > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > > @@ -7136,7 +7136,7 @@ static void tgl_init_clock_gating(struct
> > >
> > > drm_i915_private *dev_priv)
> > > > I915_READ(POWERGATE_ENABLE) | vd_pg_enable);
> > > >
> > > > /* Wa_1409825376:tgl (pre-prod)*/
> > > > - if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> > > > + if (IS_TGL_DISP_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_B1))
> > > > I915_WRITE(GEN9_CLKGATE_DIS_3,
> > >
> > > I915_READ(GEN9_CLKGATE_DIS_3) |
> > > > TGL_VRH_GATING_DIS);
> > > >
> > > >
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > >
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
