| Message ID | 20190726000226.26914-3-lucas.demarchi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Tiger Lake: add workarounds | expand |
Quoting Lucas De Marchi (2019-07-26 01:02:25) > 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. > > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 7 +++++++ > drivers/gpu/drm/i915/i915_reg.h | 5 +++++ > 2 files changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > index a6eb9c6e87ec..3235ef355dfd 100644 > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > @@ -572,6 +572,13 @@ 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_1604555607:tgl */ > + val = intel_uncore_read(engine->uncore, FF_MODE2); > + val &= ~FF_MODE2_TDS_TIMER_MASK; > + val |= FF_MODE2_TDS_TIMER_128; > + wa_write_masked_or(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val); It will do a rmw on application, so you just need wa_write_masked_or(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, FF_MODE2_TDS_TIMER_128); > } > > static void > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 54ea250000be..fbbb89f6ca2f 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7771,6 +7771,11 @@ 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_SHIFT (16) > +#define FF_MODE2_TDS_TIMER_128 (4 << FF_MODE2_TDS_TIMER_SHIFT) > +#define FF_MODE2_TDS_TIMER_MASK (0xff << FF_MODE2_TDS_TIMER_SHIFT) #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) -Chris
On 26/07/2019 01:10, Chris Wilson wrote: > Quoting Lucas De Marchi (2019-07-26 01:02:25) >> 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. >> >> Signed-off-by: Michel Thierry <michel.thierry@intel.com> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >> --- >> drivers/gpu/drm/i915/gt/intel_workarounds.c | 7 +++++++ >> drivers/gpu/drm/i915/i915_reg.h | 5 +++++ >> 2 files changed, 12 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c >> index a6eb9c6e87ec..3235ef355dfd 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c >> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c >> @@ -572,6 +572,13 @@ 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_1604555607:tgl */ >> + val = intel_uncore_read(engine->uncore, FF_MODE2); >> + val &= ~FF_MODE2_TDS_TIMER_MASK; >> + val |= FF_MODE2_TDS_TIMER_128; >> + wa_write_masked_or(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val); > > It will do a rmw on application, so you just need > wa_write_masked_or(wal, FF_MODE2, > FF_MODE2_TDS_TIMER_MASK, FF_MODE2_TDS_TIMER_128); Not with ctx was unfortunately, no rmw there, just lri. To be less misleading perhaps: wa_write(wal, FF_MODE2, val); ? It only affects verification, do we want to verify the whole register or just the FF_MODE2_TDS_TIMER_MASK bits. Since the code reads it and sets it, it may want to check it whole. Regards, Tvrtko > >> } >> >> static void >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index 54ea250000be..fbbb89f6ca2f 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -7771,6 +7771,11 @@ 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_SHIFT (16) >> +#define FF_MODE2_TDS_TIMER_128 (4 << FF_MODE2_TDS_TIMER_SHIFT) >> +#define FF_MODE2_TDS_TIMER_MASK (0xff << FF_MODE2_TDS_TIMER_SHIFT) > > #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) > -Chris > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
Quoting Tvrtko Ursulin (2019-07-26 14:15:51) > > On 26/07/2019 01:10, Chris Wilson wrote: > > Quoting Lucas De Marchi (2019-07-26 01:02:25) > >> 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. > >> > >> Signed-off-by: Michel Thierry <michel.thierry@intel.com> > >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > >> --- > >> drivers/gpu/drm/i915/gt/intel_workarounds.c | 7 +++++++ > >> drivers/gpu/drm/i915/i915_reg.h | 5 +++++ > >> 2 files changed, 12 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c > >> index a6eb9c6e87ec..3235ef355dfd 100644 > >> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c > >> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c > >> @@ -572,6 +572,13 @@ 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_1604555607:tgl */ > >> + val = intel_uncore_read(engine->uncore, FF_MODE2); > >> + val &= ~FF_MODE2_TDS_TIMER_MASK; > >> + val |= FF_MODE2_TDS_TIMER_128; > >> + wa_write_masked_or(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val); > > > > It will do a rmw on application, so you just need > > wa_write_masked_or(wal, FF_MODE2, > > FF_MODE2_TDS_TIMER_MASK, FF_MODE2_TDS_TIMER_128); > > Not with ctx was unfortunately, no rmw there, just lri. Odd that we read from outside the ctx then, no? We can do rmw inside ctx_wa if we have to thanks to MI_MATH. Should we start preparing for that nightmare? :) -Chris
On 26/07/2019 14:20, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2019-07-26 14:15:51) >> >> On 26/07/2019 01:10, Chris Wilson wrote: >>> Quoting Lucas De Marchi (2019-07-26 01:02:25) >>>> 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. >>>> >>>> Signed-off-by: Michel Thierry <michel.thierry@intel.com> >>>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >>>> --- >>>> drivers/gpu/drm/i915/gt/intel_workarounds.c | 7 +++++++ >>>> drivers/gpu/drm/i915/i915_reg.h | 5 +++++ >>>> 2 files changed, 12 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c >>>> index a6eb9c6e87ec..3235ef355dfd 100644 >>>> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c >>>> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c >>>> @@ -572,6 +572,13 @@ 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_1604555607:tgl */ >>>> + val = intel_uncore_read(engine->uncore, FF_MODE2); >>>> + val &= ~FF_MODE2_TDS_TIMER_MASK; >>>> + val |= FF_MODE2_TDS_TIMER_128; >>>> + wa_write_masked_or(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val); >>> >>> It will do a rmw on application, so you just need >>> wa_write_masked_or(wal, FF_MODE2, >>> FF_MODE2_TDS_TIMER_MASK, FF_MODE2_TDS_TIMER_128); >> >> Not with ctx was unfortunately, no rmw there, just lri. > > Odd that we read from outside the ctx then, no? Perhaps. We have to get the default value from somewhere. There is one like this already, GEN8_L3CNTLREG, from not too long ago. > We can do rmw inside ctx_wa if we have to thanks to MI_MATH. Should we > start preparing for that nightmare? :) When you say preparing it makes it sounds complicated so I want to say no. But sure, if you have extra time go for it. Regards, Tvrtko
Quoting Tvrtko Ursulin (2019-07-26 14:34:56) > > On 26/07/2019 14:20, Chris Wilson wrote: > > We can do rmw inside ctx_wa if we have to thanks to MI_MATH. Should we > > start preparing for that nightmare? :) > > When you say preparing it makes it sounds complicated so I want to say > no. But sure, if you have extra time go for it. The quick-and-dirty solution is to write the assembly routines by hand, but Lionel mentioned that mesa might have a dsl we could use to create MI_MATH routines. -Chris
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index a6eb9c6e87ec..3235ef355dfd 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -572,6 +572,13 @@ 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_1604555607:tgl */ + val = intel_uncore_read(engine->uncore, FF_MODE2); + val &= ~FF_MODE2_TDS_TIMER_MASK; + val |= FF_MODE2_TDS_TIMER_128; + 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 54ea250000be..fbbb89f6ca2f 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7771,6 +7771,11 @@ 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_SHIFT (16) +#define FF_MODE2_TDS_TIMER_128 (4 << FF_MODE2_TDS_TIMER_SHIFT) +#define FF_MODE2_TDS_TIMER_MASK (0xff << FF_MODE2_TDS_TIMER_SHIFT) + /* PCH */ #define PCH_DISPLAY_BASE 0xc0000u