Re: [PATCH 1/2] drm/i915/tgl: Implement Wa_1604555607

From: Ramalingam C <ramalingam.c@intel.com>
To: Lucas De Marchi <lucas.de.marchi@gmail.com>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 1/2] drm/i915/tgl: Implement Wa_1604555607
Date: Wed, 2 Oct 2019 11:01:28 +0530	[thread overview]
Message-ID: <20191002053127.GA27814@intel.com> (raw)
In-Reply-To: <CAKi4VA+e6PM_qRYtyZcJ50WEcUvN=JFw82oLPUWT-+eYyk_07g@mail.gmail.com>

On 2019-10-01 at 13:16:11 -0700, Lucas De Marchi wrote:
> On Tue, Oct 1, 2019 at 10:36 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > Quoting Ramalingam C (2019-10-01 18:26:23)
> > > 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.
> > >
> > > v2: Rebased on top of the WA refactoring (Oscar)
> > > v3: Correctly add to ctx_workarounds_init (Michel)
> > >
> > > 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 | 9 +++++++++
> > >  drivers/gpu/drm/i915/i915_reg.h             | 5 +++++
> > >  2 files changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > index ba65e5018978..4049b876492a 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > @@ -567,9 +567,18 @@ 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)
> > >  {
> > > +       struct drm_i915_private *dev_priv = engine->i915;
> > > +       u32 val;
> > > +
> > >         /* Wa_1409142259 */
> > >         WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3,
> > >                           GEN12_DISABLE_CPS_AWARE_COLOR_PIPE);
> > > +
> > > +       /* Wa_1604555607:tgl */
> > > +       val = I915_READ(FF_MODE2);
> >
> > No, you can't use indiscriminate mmio access that may not match the engine
> > (engine->gt->uncore).
> >
> > But really consider doing the rmw as part of the wa.
> 
> And:
> https://patchwork.freedesktop.org/patch/319952/?series=64274&rev=1
> https://patchwork.freedesktop.org/patch/317654/?series=63670&rev=2
> 
> Please don't simply resend patches that were already reviewed.
Happy if it already getting reviewed. Before sending it, I could have confirmed
with owner of the patch. Sorry for the inconvenience. Lets drop this patch.

-Ram
> 
> Lucas De Marchi
> 
> >
> > > +       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);
> > >  }
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx