Re: [Intel-gfx] [PATCH] drm/i915/tgl: Add Wa_1608008084
On Mon, Feb 24, 2020 at 09:10:34PM +, Chris Wilson wrote:
Quoting Souza, Jose (2020-02-22 00:36:53)
+ CCing people involved in the patch fixed.
On Fri, 2020-02-21 at 16:28 -0800, Lucas De Marchi wrote:
> Wa_1608008084 is an additional WA that applies to writes on FF_MODE2
> register. We can't read it back either from CPU or GPU. Since the
> other
> bits should be 0, recommendation to handle Wa_1604555607 is to
> actually
> just write the timer value.
>
> Do a write only and don't try to read it, neither before or after
> the WA is applied.
>
> Fixes: ff690b2111ba ("drm/i915/tgl: Implement Wa_1604555607")
> Signed-off-by: Lucas De Marchi
> ---
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 22 ++-
> --
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 887e0dc701f7..0d76e1d6ec87 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -580,24 +580,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_1604555607:gen12
> + * FF_MODE2 register is not readable till TGL B0, either by CPU
> or GPU.
The line above could be removed as the comments above explain it
better, also BSpec don't say that it will be fixed in B0.
The HW guys on discovering the bug promised it would be fixed for B0.
-Chris
So... we have 2 different things here: setting the timer in FF_MODE2
is the Wa_1604555607. According to the comments on the spec it could
even be treated as a general "gen12 programming mode" rather that a WA.
The fact that we can't read the register since it's tied to another
clock is the Wa_1608008084. I don't see it documented anywhere that it is
fixed in B0. Not even as a comment to the issue. And it's in fact even
marked as permanent.
Lucas De Marchi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/tgl: Add Wa_1608008084
Quoting Souza, Jose (2020-02-22 00:36:53)
> + CCing people involved in the patch fixed.
>
> On Fri, 2020-02-21 at 16:28 -0800, Lucas De Marchi wrote:
> > Wa_1608008084 is an additional WA that applies to writes on FF_MODE2
> > register. We can't read it back either from CPU or GPU. Since the
> > other
> > bits should be 0, recommendation to handle Wa_1604555607 is to
> > actually
> > just write the timer value.
> >
> > Do a write only and don't try to read it, neither before or after
> > the WA is applied.
> >
> > Fixes: ff690b2111ba ("drm/i915/tgl: Implement Wa_1604555607")
> > Signed-off-by: Lucas De Marchi
> > ---
> > drivers/gpu/drm/i915/gt/intel_workarounds.c | 22 ++-
> > --
> > 1 file changed, 10 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > index 887e0dc701f7..0d76e1d6ec87 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > @@ -580,24 +580,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_1604555607:gen12
> > + * FF_MODE2 register is not readable till TGL B0, either by CPU
> > or GPU.
>
> The line above could be removed as the comments above explain it
> better, also BSpec don't say that it will be fixed in B0.
The HW guys on discovering the bug promised it would be fixed for B0.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/tgl: Add Wa_1608008084
Wa_1608008084 is an additional WA that applies to writes on FF_MODE2
register. We can't read it back either from CPU or GPU. Since the other
bits should be 0, recommendation to handle Wa_1604555607 is to actually
just write the timer value.
Do a write only and don't try to read it, neither before or after
the WA is applied.
Fixes: ff690b2111ba ("drm/i915/tgl: Implement Wa_1604555607")
Signed-off-by: Lucas De Marchi
Reviewed-by: José Roberto de Souza
---
drivers/gpu/drm/i915/gt/intel_workarounds.c | 19 +++
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 887e0dc701f7..06cef3c18f26 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -580,24 +580,19 @@ 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_1604555607:gen12 and Wa_1608008084:gen12
+* FF_MODE2 register will return the wrong value when read. The default
+* value for this register is zero for all fields and there are no bit
+* masks. So instead of doing a RMW we should just write the TDS timer
+* value for Wa_1604555607.
*/
- wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
- IS_TGL_REVID(engine->i915, TGL_REVID_A0, TGL_REVID_A0) ? 0 :
- FF_MODE2_TDS_TIMER_MASK);
+ wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK,
+ FF_MODE2_TDS_TIMER_128, 0);
}
static void
--
2.24.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/tgl: Add Wa_1608008084
On Fri, Feb 21, 2020 at 04:36:53PM -0800, Jose Souza wrote:
+ CCing people involved in the patch fixed.
On Fri, 2020-02-21 at 16:28 -0800, Lucas De Marchi wrote:
Wa_1608008084 is an additional WA that applies to writes on FF_MODE2
register. We can't read it back either from CPU or GPU. Since the
other
bits should be 0, recommendation to handle Wa_1604555607 is to
actually
just write the timer value.
Do a write only and don't try to read it, neither before or after
the WA is applied.
Fixes: ff690b2111ba ("drm/i915/tgl: Implement Wa_1604555607")
Signed-off-by: Lucas De Marchi
---
drivers/gpu/drm/i915/gt/intel_workarounds.c | 22 ++-
--
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 887e0dc701f7..0d76e1d6ec87 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -580,24 +580,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_1604555607:gen12
+* FF_MODE2 register is not readable till TGL B0, either by CPU
or GPU.
The line above could be removed as the comments above explain it
better, also BSpec don't say that it will be fixed in B0.
1604555607 was documented to be fixed in B0. Just saw it's not anymore.
With that:
Reviewed-by: José Roberto de Souza
thanks
Lucas De Marchi
+*
+* Wa_1608008084:gen12
+* FF_MODE2 register will return the wrong value when read. The
default
+ * value for this register is zero for all fields and there are
no bit
+* masks. So instead of doing a RMW we should just write the
TDS timer
+* value for Wa_1604555607.
*/
- wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
- IS_TGL_REVID(engine->i915, TGL_REVID_A0, TGL_REVID_A0) ?
0 :
- FF_MODE2_TDS_TIMER_MASK);
+ wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK,
+ FF_MODE2_TDS_TIMER_128, 0);
}
static void
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/tgl: Add Wa_1608008084
+ CCing people involved in the patch fixed.
On Fri, 2020-02-21 at 16:28 -0800, Lucas De Marchi wrote:
> Wa_1608008084 is an additional WA that applies to writes on FF_MODE2
> register. We can't read it back either from CPU or GPU. Since the
> other
> bits should be 0, recommendation to handle Wa_1604555607 is to
> actually
> just write the timer value.
>
> Do a write only and don't try to read it, neither before or after
> the WA is applied.
>
> Fixes: ff690b2111ba ("drm/i915/tgl: Implement Wa_1604555607")
> Signed-off-by: Lucas De Marchi
> ---
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 22 ++-
> --
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 887e0dc701f7..0d76e1d6ec87 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -580,24 +580,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_1604555607:gen12
> + * FF_MODE2 register is not readable till TGL B0, either by CPU
> or GPU.
The line above could be removed as the comments above explain it
better, also BSpec don't say that it will be fixed in B0.
With that:
Reviewed-by: José Roberto de Souza
> + *
> + * Wa_1608008084:gen12
> + * FF_MODE2 register will return the wrong value when read. The
> default
> + * value for this register is zero for all fields and there are
> no bit
> + * masks. So instead of doing a RMW we should just write the
> TDS timer
> + * value for Wa_1604555607.
>*/
> - wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
> -IS_TGL_REVID(engine->i915, TGL_REVID_A0, TGL_REVID_A0) ?
> 0 :
> - FF_MODE2_TDS_TIMER_MASK);
> + wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK,
> +FF_MODE2_TDS_TIMER_128, 0);
> }
>
> static void
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/tgl: Add Wa_1608008084
Wa_1608008084 is an additional WA that applies to writes on FF_MODE2
register. We can't read it back either from CPU or GPU. Since the other
bits should be 0, recommendation to handle Wa_1604555607 is to actually
just write the timer value.
Do a write only and don't try to read it, neither before or after
the WA is applied.
Fixes: ff690b2111ba ("drm/i915/tgl: Implement Wa_1604555607")
Signed-off-by: Lucas De Marchi
---
drivers/gpu/drm/i915/gt/intel_workarounds.c | 22 ++---
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 887e0dc701f7..0d76e1d6ec87 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -580,24 +580,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_1604555607:gen12
+* FF_MODE2 register is not readable till TGL B0, either by CPU or GPU.
+*
+* Wa_1608008084:gen12
+* FF_MODE2 register will return the wrong value when read. The default
+* value for this register is zero for all fields and there are no bit
+* masks. So instead of doing a RMW we should just write the TDS timer
+* value for Wa_1604555607.
*/
- wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
- IS_TGL_REVID(engine->i915, TGL_REVID_A0, TGL_REVID_A0) ? 0 :
- FF_MODE2_TDS_TIMER_MASK);
+ wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK,
+ FF_MODE2_TDS_TIMER_128, 0);
}
static void
--
2.24.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
