Re: [Intel-gfx] [PATCH 1/2] drm/i915/tgl: Implement Wa_1604555607
On 2019-10-01 at 13:16:11 -0700, Lucas De Marchi wrote:
> On Tue, Oct 1, 2019 at 10:36 AM Chris Wilson wrote:
> >
> > Quoting Ramalingam C (2019-10-01 18:26:23)
> > > From: Michel Thierry
> > >
> > > 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
> > > Signed-off-by: Ramalingam C
> > > ---
> > > 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=1
> https://patchwork.freedesktop.org/patch/317654/?series=63670=2
>
> Please don't simply resend patches that were already reviewed.
Lucas,
Are you planning pursue the merge of these patches.
Verification is not fixed at B Stepping too. And we need this WA for the
performance.
Thanks,
-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
Re: [Intel-gfx] [PATCH 1/2] drm/i915/tgl: Implement Wa_1604555607
On 01/10/2019 18:35, Chris Wilson wrote:
Quoting Ramalingam C (2019-10-01 18:26:23)
From: Michel Thierry
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
Signed-off-by: Ramalingam C
---
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.
You are suggesting going via the context image after all? Or MI_MATH?
Regards,
Tvrtko
+ 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
Re: [Intel-gfx] [PATCH 1/2] drm/i915/tgl: Implement Wa_1604555607
On 2019-10-01 at 13:16:11 -0700, Lucas De Marchi wrote:
> On Tue, Oct 1, 2019 at 10:36 AM Chris Wilson wrote:
> >
> > Quoting Ramalingam C (2019-10-01 18:26:23)
> > > From: Michel Thierry
> > >
> > > 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
> > > Signed-off-by: Ramalingam C
> > > ---
> > > 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=1
> https://patchwork.freedesktop.org/patch/317654/?series=63670=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
Re: [Intel-gfx] [PATCH 1/2] drm/i915/tgl: Implement Wa_1604555607
On Tue, Oct 1, 2019 at 10:36 AM Chris Wilson wrote:
>
> Quoting Ramalingam C (2019-10-01 18:26:23)
> > From: Michel Thierry
> >
> > 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
> > Signed-off-by: Ramalingam C
> > ---
> > 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=1
https://patchwork.freedesktop.org/patch/317654/?series=63670=2
Please don't simply resend patches that were already reviewed.
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
Re: [Intel-gfx] [PATCH 1/2] drm/i915/tgl: Implement Wa_1604555607
Quoting Ramalingam C (2019-10-01 18:26:23)
> From: Michel Thierry
>
> 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
> Signed-off-by: Ramalingam C
> ---
> 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.
> + 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
