Message ID | 20191121101226.10779-2-ramalingam.c@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Wa_1604555607 implementation and verification skip | expand |
Quoting Ramalingam C (2019-11-21 10:12:26) > From: Michel Thierry <michel.thierry@intel.com> > > Implement Wa_1604555607 (set the DS pairing timer to 128 cycles). > FF_MODE2 is part of the register state context, that's why it is > implemented here. > > At TGL A0 stepping, FF_MODE2 register read back is broken, hence > disabling the WA verification. > > v2: Rebased on top of the WA refactoring (Oscar) > v3: Correctly add to ctx_workarounds_init (Michel) > v4: > uncore read is used [Tvrtko] > Macros as used for MASK definition [Chris] > v5: > Skip the Wa_1604555607 verification [Ram] > i915 ptr retrieved from engine. [Tvrtko] > > BSpec: 19363 > HSDES: 1604555607 > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > Signed-off-by: Ramalingam C <ramlingam.c@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 25 ++++++++++++++++++--- > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > 2 files changed, 26 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index 399acae2f33f..7cfd2442b736 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -147,19 +147,26 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) > } > > static void > -wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, > - u32 val) > +__wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, > + u32 val, u32 read_mask) > { > struct i915_wa wa = { > .reg = reg, > .mask = mask, > .val = val, > - .read = mask, > + .read = read_mask, > }; > > _wa_add(wal, &wa); You might as well call it wa_add() since it takes all the arguments to _wa_add() and wraps them up into struct that we then copy. > } > > +static void > +wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, > + u32 val) > +{ > + __wa_write_masked_or(wal, reg, mask, val, mask); > +} > + > static void > wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val) > { > @@ -568,9 +575,21 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine, > static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine, > struct i915_wa_list *wal) > { > + u32 val; > + > /* Wa_1409142259:tgl */ > WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3, > GEN12_DISABLE_CPS_AWARE_COLOR_PIPE); > + > + /* Wa_1604555607:tgl */ > + val = intel_uncore_read(engine->uncore, FF_MODE2); > + val &= ~FF_MODE2_TDS_TIMER_MASK; > + val |= FF_MODE2_TDS_TIMER_128; > + if (IS_TGL_REVID(engine->i915, 0, TGL_REVID_A0)) > + __wa_write_masked_or(wal, FF_MODE2, > + FF_MODE2_TDS_TIMER_MASK, val, 0); > + else > + wa_write_masked_or(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val); I still have this plan to do this as MI_MATH ops... But it's not a blocker. -Chris
On 21/11/2019 10:12, Ramalingam C wrote: > From: Michel Thierry <michel.thierry@intel.com> > > Implement Wa_1604555607 (set the DS pairing timer to 128 cycles). > FF_MODE2 is part of the register state context, that's why it is > implemented here. > > At TGL A0 stepping, FF_MODE2 register read back is broken, hence > disabling the WA verification. > > v2: Rebased on top of the WA refactoring (Oscar) > v3: Correctly add to ctx_workarounds_init (Michel) > v4: > uncore read is used [Tvrtko] > Macros as used for MASK definition [Chris] > v5: > Skip the Wa_1604555607 verification [Ram] > i915 ptr retrieved from engine. [Tvrtko] > > BSpec: 19363 > HSDES: 1604555607 > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > Signed-off-by: Ramalingam C <ramlingam.c@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 25 ++++++++++++++++++--- > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > 2 files changed, 26 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index 399acae2f33f..7cfd2442b736 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -147,19 +147,26 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) > } > > static void > -wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, > - u32 val) > +__wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, > + u32 val, u32 read_mask) > { > struct i915_wa wa = { > .reg = reg, > .mask = mask, > .val = val, > - .read = mask, > + .read = read_mask, > }; > > _wa_add(wal, &wa); > } > > +static void > +wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, > + u32 val) > +{ > + __wa_write_masked_or(wal, reg, mask, val, mask); > +} > + > static void > wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val) > { > @@ -568,9 +575,21 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine, > static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine, > struct i915_wa_list *wal) > { > + u32 val; > + > /* Wa_1409142259:tgl */ > WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3, > GEN12_DISABLE_CPS_AWARE_COLOR_PIPE); > + > + /* Wa_1604555607:tgl */ > + val = intel_uncore_read(engine->uncore, FF_MODE2); > + val &= ~FF_MODE2_TDS_TIMER_MASK; > + val |= FF_MODE2_TDS_TIMER_128; > + if (IS_TGL_REVID(engine->i915, 0, TGL_REVID_A0)) > + __wa_write_masked_or(wal, FF_MODE2, > + FF_MODE2_TDS_TIMER_MASK, val, 0); > + else > + wa_write_masked_or(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val); Hm a hybrid solution. I was thinking: __wa_write_masked_or(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val, IS_TGL_REVID(engine->i915, 0, TGL_REVID_A0) ? 0 : FF_MODE2_TDS_TIMER_MASK); But yours works as well. Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko > } > > static void > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 94d0f593eeb7..a99fdf8ea53b 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7922,6 +7922,10 @@ enum { > #define PIXEL_ROUNDING_TRUNC_FB_PASSTHRU (1 << 15) > #define PER_PIXEL_ALPHA_BYPASS_EN (1 << 7) > > +#define FF_MODE2 _MMIO(0x6604) > +#define FF_MODE2_TDS_TIMER_MASK REG_GENMASK(23, 16) > +#define FF_MODE2_TDS_TIMER_128 REG_FIELD_PREP(FF_MODE2_TDS_TIMER_MASK, 4) > + > /* PCH */ > > #define PCH_DISPLAY_BASE 0xc0000u >
On 2019-11-21 at 10:19:13 +0000, Chris Wilson wrote: > Quoting Ramalingam C (2019-11-21 10:12:26) > > From: Michel Thierry <michel.thierry@intel.com> > > > > Implement Wa_1604555607 (set the DS pairing timer to 128 cycles). > > FF_MODE2 is part of the register state context, that's why it is > > implemented here. > > > > At TGL A0 stepping, FF_MODE2 register read back is broken, hence > > disabling the WA verification. > > > > v2: Rebased on top of the WA refactoring (Oscar) > > v3: Correctly add to ctx_workarounds_init (Michel) > > v4: > > uncore read is used [Tvrtko] > > Macros as used for MASK definition [Chris] > > v5: > > Skip the Wa_1604555607 verification [Ram] > > i915 ptr retrieved from engine. [Tvrtko] > > > > BSpec: 19363 > > HSDES: 1604555607 > > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > > Signed-off-by: Ramalingam C <ramlingam.c@intel.com> > > --- > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 25 ++++++++++++++++++--- > > drivers/gpu/drm/i915/i915_reg.h | 4 ++++ > > 2 files changed, 26 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > index 399acae2f33f..7cfd2442b736 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > > @@ -147,19 +147,26 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) > > } > > > > static void > > -wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, > > - u32 val) > > +__wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, > > + u32 val, u32 read_mask) > > { > > struct i915_wa wa = { > > .reg = reg, > > .mask = mask, > > .val = val, > > - .read = mask, > > + .read = read_mask, > > }; > > > > _wa_add(wal, &wa); > > You might as well call it wa_add() since it takes all the arguments to > _wa_add() and wraps them up into struct that we then copy. > Something like this? static void wa_add(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, u32 val, u32 read_mask) { struct i915_wa wa = { .reg = reg, .mask = mask, .val = val, .read = read_mask, }; _wa_add(wal, &wa); } static void __wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, u32 val, u32 read_mask) { wa_add(wal, reg, mask, val, read_mask); } -Ram > > } > > > > +static void > > +wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, > > + u32 val) > > +{ > > + __wa_write_masked_or(wal, reg, mask, val, mask); > > +} > > + > > static void > > wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val) > > { > > @@ -568,9 +575,21 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine, > > static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine, > > struct i915_wa_list *wal) > > { > > + u32 val; > > + > > /* Wa_1409142259:tgl */ > > WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3, > > GEN12_DISABLE_CPS_AWARE_COLOR_PIPE); > > + > > + /* Wa_1604555607:tgl */ > > + val = intel_uncore_read(engine->uncore, FF_MODE2); > > + val &= ~FF_MODE2_TDS_TIMER_MASK; > > + val |= FF_MODE2_TDS_TIMER_128; > > + if (IS_TGL_REVID(engine->i915, 0, TGL_REVID_A0)) > > + __wa_write_masked_or(wal, FF_MODE2, > > + FF_MODE2_TDS_TIMER_MASK, val, 0); > > + else > > + wa_write_masked_or(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val); > > I still have this plan to do this as MI_MATH ops... But it's not a > blocker. > -Chris
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index 399acae2f33f..7cfd2442b736 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -147,19 +147,26 @@ static void _wa_add(struct i915_wa_list *wal, const struct i915_wa *wa) } static void -wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, - u32 val) +__wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, + u32 val, u32 read_mask) { struct i915_wa wa = { .reg = reg, .mask = mask, .val = val, - .read = mask, + .read = read_mask, }; _wa_add(wal, &wa); } +static void +wa_write_masked_or(struct i915_wa_list *wal, i915_reg_t reg, u32 mask, + u32 val) +{ + __wa_write_masked_or(wal, reg, mask, val, mask); +} + static void wa_masked_en(struct i915_wa_list *wal, i915_reg_t reg, u32 val) { @@ -568,9 +575,21 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine, static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) { + u32 val; + /* Wa_1409142259:tgl */ WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3, GEN12_DISABLE_CPS_AWARE_COLOR_PIPE); + + /* Wa_1604555607:tgl */ + val = intel_uncore_read(engine->uncore, FF_MODE2); + val &= ~FF_MODE2_TDS_TIMER_MASK; + val |= FF_MODE2_TDS_TIMER_128; + if (IS_TGL_REVID(engine->i915, 0, TGL_REVID_A0)) + __wa_write_masked_or(wal, FF_MODE2, + FF_MODE2_TDS_TIMER_MASK, val, 0); + else + wa_write_masked_or(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val); } static void diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 94d0f593eeb7..a99fdf8ea53b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7922,6 +7922,10 @@ enum { #define PIXEL_ROUNDING_TRUNC_FB_PASSTHRU (1 << 15) #define PER_PIXEL_ALPHA_BYPASS_EN (1 << 7) +#define FF_MODE2 _MMIO(0x6604) +#define FF_MODE2_TDS_TIMER_MASK REG_GENMASK(23, 16) +#define FF_MODE2_TDS_TIMER_128 REG_FIELD_PREP(FF_MODE2_TDS_TIMER_MASK, 4) + /* PCH */ #define PCH_DISPLAY_BASE 0xc0000u