On Wed, 2020-02-26 at 16:02 -0800, Lucas De Marchi wrote:
> On Tue, Feb 25, 2020 at 5:45 PM José Roberto de Souza
> <jose.souza@xxxxxxxxx> wrote:
> > Spliting GT and display revisions id to correctly apply workarounds
> > because we have some issues that were fixed in display B0 but no
> > hardware was made with B0 stepping, so to keep consistent with
> > BSpec
> > splitting it from GT and adding this adittional handling.
> >
> > BSpec: 52890
> > BSpec: 44455
> > Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> > Cc: James Ausmus <james.ausmus@xxxxxxxxx>
> > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > ---
> > .../drm/i915/display/intel_display_power.c | 2 +-
> > drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
> > drivers/gpu/drm/i915/gt/intel_workarounds.c | 10 +++---
> > drivers/gpu/drm/i915/i915_drv.h | 36
> > +++++++++++++++++--
> > drivers/gpu/drm/i915/intel_pm.c | 2 +-
> > 5 files changed, 42 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> > b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 6e25a1317161..82af963106ab 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -5010,7 +5010,7 @@ static void tgl_bw_buddy_init(struct
> > drm_i915_private *dev_priv)
> > const struct buddy_page_mask *table;
> > int i;
> >
> > - if (IS_TGL_REVID(dev_priv, TGL_REVID_A0, TGL_REVID_A0))
> > + if (IS_TGL_DISP_REVID(dev_priv, TGL_DISP_REVID_A0,
> > TGL_DISP_REVID_A0))
> > /* Wa_1409767108: tgl */
> > table = wa_1409767108_buddy_page_masks;
> > else
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index 39b0125b7143..852981d533a8 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -4124,7 +4124,7 @@ static int gen12_emit_flush_render(struct
> > i915_request *request,
> > /*
> > * Wa_1604544889:tgl
> > */
> > - if (IS_TGL_REVID(request->i915, TGL_REVID_A0,
> > TGL_REVID_A0)) {
> > + if (IS_TGL_GT_REVID(request->i915, TGL_GT_REVID_A0,
> > TGL_GT_REVID_A0)) {
> > flags = 0;
> > flags |= PIPE_CONTROL_CS_STALL;
> > flags |= PIPE_CONTROL_HDC_PIPELINE_FLUSH;
> > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > index 887e0dc701f7..bc6114b6dc8f 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > @@ -596,8 +596,8 @@ static void tgl_ctx_workarounds_init(struct
> > intel_engine_cs *engine,
> > * the read of FF_MODE2.
> > */
> > wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
> > - IS_TGL_REVID(engine->i915, TGL_REVID_A0,
> > TGL_REVID_A0) ? 0 :
> > - FF_MODE2_TDS_TIMER_MASK);
> > + IS_TGL_GT_REVID(engine->i915, TGL_GT_REVID_A0,
> > + TGL_GT_REVID_A0) ? 0 :
> > FF_MODE2_TDS_TIMER_MASK);
> > }
> >
> > static void
> > @@ -931,13 +931,13 @@ static void
> > tgl_gt_workarounds_init(struct drm_i915_private *i915, struct
> > i915_wa_list *wal)
> > {
> > /* Wa_1409420604:tgl */
> > - if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> > + if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0,
> > TGL_GT_REVID_A0))
> > wa_write_or(wal,
> > SUBSLICE_UNIT_LEVEL_CLKGATE2,
> > CPSSUNIT_CLKGATE_DIS);
> >
> > /* Wa_1409180338:tgl */
> > - if (IS_TGL_REVID(i915, TGL_REVID_A0, TGL_REVID_A0))
> > + if (IS_TGL_GT_REVID(i915, TGL_GT_REVID_A0,
> > TGL_GT_REVID_A0))
> > wa_write_or(wal,
> > SLICE_UNIT_LEVEL_CLKGATE,
> > L3_CLKGATE_DIS | L3_CR2X_CLKGATE_DIS);
> > @@ -1329,7 +1329,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_GT_REVID(i915, TGL_GT_REVID_A0,
> > TGL_GT_REVID_A0)) {
> > /* 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 ea13fc0b409b..4fa01f8d3f33 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1574,11 +1574,43 @@ IS_SUBPLATFORM(const struct
> > drm_i915_private *i915,
> > #define IS_ICL_REVID(p, since, until) \
> > (IS_ICELAKE(p) && IS_REVID(p, since, until))
> >
> > -#define TGL_REVID_A0 0x0
> > +#define TGL_GT_REVID_A0 0x0
> >
> > -#define IS_TGL_REVID(p, since, until) \
> > +#define IS_TGL_GT_REVID(p, since, until) \
> > (IS_TIGERLAKE(p) && IS_REVID(p, since, until))
> >
> > +/*
> > + * revid=0x0 = DISP_REVID_A0
> > + * revid=0x1 = DISP_REVID_C0
> > + * revid=0x2 = DISP_REVID_D0
> > + *
> > + * So ids bellow will not match PCI revid and the function bellow
> > is used.
> > + */
> > +#define TGL_DISP_REVID_A0 0x0
> > +#define TGL_DISP_REVID_B0 0x1
> > +#define TGL_DISP_REVID_C0 0x2
> > +#define TGL_DISP_REVID_D0 0x3
> > +
> > +static inline bool
> > +_tgl_disp_revid(struct drm_i915_private *p, u8 since, u8 until)
> > +{
> > + const u8 gt2_disp_revid[] = {
> > + TGL_DISP_REVID_A0,
> > + TGL_DISP_REVID_C0,
> > + TGL_DISP_REVID_D0
>
> So we are hardcoding the mapping in the code using the same
> information. Why do we even need any of this?
> Just define it like this:
>
> #define TGL_DISP_REVID_A0 0x0
> #define TGL_DISP_REVID_C0 0x1
> #define TGL_DISP_REVID_D0 0x2
What to do with the workarounds fixed or introduced in display B0?
>
> Then in the workarounds we continue to use IS_TGL_REVID(). This
> means
> we document it is a "display wa",
> but since we don't have a INTEL_DISPLAY_REVID(), IMO it doesn't make
> sense to have all this function to do
> the mapping.... just make it a compile-time map.
>
> Lucas De Marchi
>
> > + };
> > + u8 disp_revid;
> > +
> > + if (INTEL_REVID(p) >= ARRAY_SIZE(gt2_disp_revid))
> > + disp_revid = TGL_DISP_REVID_D0;
> > + else
> > + disp_revid = gt2_disp_revid[INTEL_REVID(p)];
> > +
> > + return IS_TIGERLAKE(p) && disp_revid >= since && disp_revid
> > <= until;
> > +}
> > +
> > +#define IS_TGL_DISP_REVID(p, since, until) _tgl_disp_revid(p,
> > since, until)
> > +
> > #define IS_LP(dev_priv) (INTEL_INFO(dev_priv)->is_lp)
> > #define IS_GEN9_LP(dev_priv) (IS_GEN(dev_priv, 9) &&
> > IS_LP(dev_priv))
> > #define IS_GEN9_BC(dev_priv) (IS_GEN(dev_priv, 9) &&
> > !IS_LP(dev_priv))
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 22aa205793e5..49484d5f5f84 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -6852,7 +6852,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_DISP_REVID_A0,
> > TGL_DISP_REVID_A0))
> > I915_WRITE(GEN9_CLKGATE_DIS_3,
> > I915_READ(GEN9_CLKGATE_DIS_3) |
> > TGL_VRH_GATING_DIS);
> > }
> > --
> > 2.25.1
> >
> > _______________________________________________
> > 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
