[Intel-gfx] [PATCH 1/1] drm/i915/tgl: Implement Wa_1604555607

[Ramalingam C]

On 2019-11-22 at 09:29:43 +0000, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-11-22 09:21:45)
> > 
> > On 22/11/2019 04:02, Ramalingam C wrote:
> > > @@ -568,9 +581,22 @@ 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;
> > > +     /*
> > > +      * FIXME: FF_MODE2 register is not readable till TGL B0. We can
> > > +      * enable verification of WA from the later steppings, which enables
> > > +      * the read of FF_MODE2.
> > > +      */
> > > +     __wa_write_masked_or(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val, 0);
> > 
> > If I was a betting man I'd bet no one will ever remember to add the 
> > verification back. So I have to say I disagree with Lucas on this point. 
> > Someone do a casting vote please. :)
> 
> I would go with IS_TGL_REVID(A0, A0) as we expect it to be picked up by
> the selftests if we have a new stepping that is unfixed -- and a blip in
> CI is a much clearer reminder to come back and revisit this code. We
> should be able to go "oops, live_workarounds is red, failing on ctx:0xf00"
> and from there find this fixme. And so update for a new stepping in the
> course of a day (because that's how long it takes for CI to approve a
> patch).
Tvrtko and Chris,

So I take it as, we want to exclude the WA verification for the current
steppings alone that is A0 alone, when new stepping comes with
readability broken then we will excude the verification at that time.

Shall I revert to the previous version of patch, just to exclude the WA
verification for A0 alone.?

-Ram
> -Chris