Re: [Mesa-dev] [PATCH v2] workarounds: Update workaround names and platforms

[Kibey, Sameer]

> -Original Message-
> From: Ben Widawsky [mailto:b...@bwidawsk.net]
> Sent: Monday, February 08, 2016 5:41 PM
> To: Kibey, Sameer
> Cc: mesa-dev@lists.freedesktop.org; Sharp, Sarah A; Widawsky, Benjamin
> Subject: Re: [Mesa-dev] [PATCH v2] workarounds: Update workaround
> names and platforms
> 
> On Fri, Feb 05, 2016 at 01:59:23PM -0800, Sameer Kibey wrote:
> > Update the format in which workarounds are documented in the source
> > code. This allows mesa to be parsed by the list-workarounds utility in
> > intel-gpu-tools.
> >
> > Signed-off-by: Sameer Kibey <sameer.ki...@intel.com>
> > ---
> > changed byt to vlv for consistency.
> >  src/mesa/drivers/dri/i965/brw_binding_tables.c | 2 +-
> >  src/mesa/drivers/dri/i965/brw_blorp.cpp| 2 ++
> >  src/mesa/drivers/dri/i965/brw_defines.h| 3 ++-
> >  src/mesa/drivers/dri/i965/brw_eu_emit.c| 3 ++-
> >  src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 9 ++---
> >  src/mesa/drivers/dri/i965/brw_pipe_control.c   | 4 +++-
> >  src/mesa/drivers/dri/i965/gen6_queryobj.c  | 2 +-
> >  src/mesa/drivers/dri/i965/gen8_depth_state.c   | 3 ++-
> >  src/mesa/drivers/dri/i965/intel_batchbuffer.c  | 2 +-
> >  9 files changed, 20 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_binding_tables.c
> > b/src/mesa/drivers/dri/i965/brw_binding_tables.c
> > index f3a0310..bcf6422 100644
> > --- a/src/mesa/drivers/dri/i965/brw_binding_tables.c
> > +++ b/src/mesa/drivers/dri/i965/brw_binding_tables.c
> > @@ -54,7 +54,7 @@ static uint32_t
> >  reserve_hw_bt_space(struct brw_context *brw, unsigned bytes)  {
> > /* From the Broadwell PRM, Volume 16, "Workarounds",
> > -* WaStateBindingTableOverfetch:
> > +* WaStateBindingTableOverfetch:hsw,bdw,chv,bxt
> >  * "HW over-fetches two cache lines of binding table indices.  When
> >  *  using the resource streamer, SW needs to pad binding table pointer
> >  *  updates with an additional two cache lines."
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > index 1bc6d15..dd01ea8 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > @@ -304,6 +304,8 @@ brw_hiz_op_params::brw_hiz_op_params(struct
> intel_mipmap_tree *mt,
> >  * aligned to an 8x4 pixel block relative to the upper left corner
> >  * of the depth buffer [...]
> >  *
> > +* WaHizAmbiguate8x4Aligned:hsw
> > +*
> >  * For hiz resolves, the rectangle must also be 8x4 aligned. Item
> >  * WaHizAmbiguate8x4Aligned from the Haswell workarounds page and
> the
> >  * Ivybridge simulator require the alignment.
> > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
> > b/src/mesa/drivers/dri/i965/brw_defines.h
> > index 01e0c99..5410a1d 100644
> > --- a/src/mesa/drivers/dri/i965/brw_defines.h
> > +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> > @@ -1756,7 +1756,8 @@ enum brw_message_target {
> >  /* Dataport special binding table indices: */
> >  #define BRW_BTI_STATELESS255
> >  #define GEN7_BTI_SLM 254
> > -/* Note that on Gen8+ BTI 255 was redefined to be IA-coherent
> > according to the
> > +/* WaForceEnableNonCoherent:bdw,chv,skl,kbl
> > + * Note that on Gen8+ BTI 255 was redefined to be IA-coherent
> > +according to the
> >   * hardware spec, however because the DRM sets bit 4 of HDC_CHICKEN0
> on BDW,
> >   * CHV and at least some pre-production steppings of SKL due to
> >   * WaForceEnableNonCoherent, HDC memory access may have been
> > overridden by the diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > index 35d8039..918d69e 100644
> > --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > @@ -1885,7 +1885,8 @@ void brw_CMP(struct brw_codegen *p,
> > brw_set_src0(p, insn, src0);
> > brw_set_src1(p, insn, src1);
> >
> > -   /* Item WaCMPInstNullDstForcesThreadSwitch in the Haswell Bspec
> workarounds
> > +   /* WaCMPInstNullDstForcesThreadSwitch:ivb,hsw,vlv
> > +* Item WaCMPInstNullDstForcesThreadSwitch in the Haswell Bspec
> > + workarounds
> >  * page says:
> >  *"Any CMP instruction with a null destination must use a {switch}."
> >  *
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > b/src/mesa/drive

Re: [Mesa-dev] [PATCH v2] workarounds: Update workaround names and platforms

[Ben Widawsky]

On Fri, Feb 05, 2016 at 01:59:23PM -0800, Sameer Kibey wrote:
> Update the format in which workarounds are documented
> in the source code. This allows mesa to be parsed
> by the list-workarounds utility in intel-gpu-tools.
> 
> Signed-off-by: Sameer Kibey 
> ---
> changed byt to vlv for consistency.
>  src/mesa/drivers/dri/i965/brw_binding_tables.c | 2 +-
>  src/mesa/drivers/dri/i965/brw_blorp.cpp| 2 ++
>  src/mesa/drivers/dri/i965/brw_defines.h| 3 ++-
>  src/mesa/drivers/dri/i965/brw_eu_emit.c| 3 ++-
>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 9 ++---
>  src/mesa/drivers/dri/i965/brw_pipe_control.c   | 4 +++-
>  src/mesa/drivers/dri/i965/gen6_queryobj.c  | 2 +-
>  src/mesa/drivers/dri/i965/gen8_depth_state.c   | 3 ++-
>  src/mesa/drivers/dri/i965/intel_batchbuffer.c  | 2 +-
>  9 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_binding_tables.c 
> b/src/mesa/drivers/dri/i965/brw_binding_tables.c
> index f3a0310..bcf6422 100644
> --- a/src/mesa/drivers/dri/i965/brw_binding_tables.c
> +++ b/src/mesa/drivers/dri/i965/brw_binding_tables.c
> @@ -54,7 +54,7 @@ static uint32_t
>  reserve_hw_bt_space(struct brw_context *brw, unsigned bytes)
>  {
> /* From the Broadwell PRM, Volume 16, "Workarounds",
> -* WaStateBindingTableOverfetch:
> +* WaStateBindingTableOverfetch:hsw,bdw,chv,bxt
>  * "HW over-fetches two cache lines of binding table indices.  When
>  *  using the resource streamer, SW needs to pad binding table pointer
>  *  updates with an additional two cache lines."
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp 
> b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> index 1bc6d15..dd01ea8 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> @@ -304,6 +304,8 @@ brw_hiz_op_params::brw_hiz_op_params(struct 
> intel_mipmap_tree *mt,
>  * aligned to an 8x4 pixel block relative to the upper left corner
>  * of the depth buffer [...]
>  *
> +* WaHizAmbiguate8x4Aligned:hsw
> +*
>  * For hiz resolves, the rectangle must also be 8x4 aligned. Item
>  * WaHizAmbiguate8x4Aligned from the Haswell workarounds page and the
>  * Ivybridge simulator require the alignment.
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
> b/src/mesa/drivers/dri/i965/brw_defines.h
> index 01e0c99..5410a1d 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -1756,7 +1756,8 @@ enum brw_message_target {
>  /* Dataport special binding table indices: */
>  #define BRW_BTI_STATELESS255
>  #define GEN7_BTI_SLM 254
> -/* Note that on Gen8+ BTI 255 was redefined to be IA-coherent according to 
> the
> +/* WaForceEnableNonCoherent:bdw,chv,skl,kbl
> + * Note that on Gen8+ BTI 255 was redefined to be IA-coherent according to 
> the
>   * hardware spec, however because the DRM sets bit 4 of HDC_CHICKEN0 on BDW,
>   * CHV and at least some pre-production steppings of SKL due to
>   * WaForceEnableNonCoherent, HDC memory access may have been overridden by 
> the
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index 35d8039..918d69e 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -1885,7 +1885,8 @@ void brw_CMP(struct brw_codegen *p,
> brw_set_src0(p, insn, src0);
> brw_set_src1(p, insn, src1);
>  
> -   /* Item WaCMPInstNullDstForcesThreadSwitch in the Haswell Bspec 
> workarounds
> +   /* WaCMPInstNullDstForcesThreadSwitch:ivb,hsw,vlv
> +* Item WaCMPInstNullDstForcesThreadSwitch in the Haswell Bspec 
> workarounds
>  * page says:
>  *"Any CMP instruction with a null destination must use a {switch}."
>  *
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index 1916a99..24d4a9d 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -1836,7 +1836,8 @@ fs_generator::generate_code(const cfg_t *cfg, int 
> dispatch_width)
>   brw_F16TO32(p, dst, src[0]);
>   break;
>case BRW_OPCODE_CMP:
> - /* The Ivybridge/BayTrail WaCMPInstFlagDepClearedEarly workaround 
> says
> + /* WaCMPInstFlagDepClearedEarly:ivb,hsw,vlv
> +  * The Ivybridge/BayTrail WaCMPInstFlagDepClearedEarly workaround 
> says
>* that when the destination is a GRF that the dependency-clear bit 
> on
>* the flag register is cleared early.
>*
> @@ -1928,7 +1929,8 @@ fs_generator::generate_code(const cfg_t *cfg, int 
> dispatch_width)
>  
>case BRW_OPCODE_BFI1:
>   assert(devinfo->gen >= 7);
> - /* The Haswell WaForceSIMD8ForBFIInstruction workaround says that we
> + /* 

Re: [Mesa-dev] [PATCH v2] workarounds: Update workaround names and platforms

[Kibey, Sameer]

> -Original Message-
> From: Widawsky, Benjamin
> Sent: Saturday, February 06, 2016 10:30 AM
> To: Kibey, Sameer
> Cc: mesa-dev@lists.freedesktop.org; Sharp, Sarah A
> Subject: Re: [PATCH v2] workarounds: Update workaround names and
> platforms
> 
> On Fri, Feb 05, 2016 at 01:59:23PM -0800, Sameer Kibey wrote:
> > Update the format in which workarounds are documented in the source
> > code. This allows mesa to be parsed by the list-workarounds utility in
> > intel-gpu-tools.
> >
> > Signed-off-by: Sameer Kibey 
> 
> Do you have any plan for updating these as we add new platforms to mesa? I
> foresee a problem of these getting stale. I wonder how the drm-intel devs
> deals with that.

To update this for the new platforms should be a trivial patch. I do not see 
any issues with that.
 
> [snip]
> 
> --
> Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] workarounds: Update workaround names and platforms

[Ben Widawsky]

On Mon, Feb 08, 2016 at 09:09:30AM -0800, Kibey, Sameer wrote:
> 
> 
> > -Original Message-
> > From: Widawsky, Benjamin
> > Sent: Saturday, February 06, 2016 10:30 AM
> > To: Kibey, Sameer
> > Cc: mesa-dev@lists.freedesktop.org; Sharp, Sarah A
> > Subject: Re: [PATCH v2] workarounds: Update workaround names and
> > platforms
> > 
> > On Fri, Feb 05, 2016 at 01:59:23PM -0800, Sameer Kibey wrote:
> > > Update the format in which workarounds are documented in the source
> > > code. This allows mesa to be parsed by the list-workarounds utility in
> > > intel-gpu-tools.
> > >
> > > Signed-off-by: Sameer Kibey 
> > 
> > Do you have any plan for updating these as we add new platforms to mesa? I
> > foresee a problem of these getting stale. I wonder how the drm-intel devs
> > deals with that.
> 
> To update this for the new platforms should be a trivial patch. I do not see 
> any issues with that.

The issue is remembering to do it. But I take it that we have no good solution
for that.

>  
> > [snip]
> > 
> > --
> > Ben Widawsky, Intel Open Source Technology Center

-- 
Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] workarounds: Update workaround names and platforms

[Ben Widawsky]

On Fri, Feb 05, 2016 at 01:59:23PM -0800, Sameer Kibey wrote:
> Update the format in which workarounds are documented
> in the source code. This allows mesa to be parsed
> by the list-workarounds utility in intel-gpu-tools.
> 
> Signed-off-by: Sameer Kibey 

Do you have any plan for updating these as we add new platforms to mesa? I
foresee a problem of these getting stale. I wonder how the drm-intel devs deals
with that.

[snip]

-- 
Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2] workarounds: Update workaround names and platforms

[Sameer Kibey]

Update the format in which workarounds are documented
in the source code. This allows mesa to be parsed
by the list-workarounds utility in intel-gpu-tools.

Signed-off-by: Sameer Kibey 
---
changed byt to vlv for consistency.
 src/mesa/drivers/dri/i965/brw_binding_tables.c | 2 +-
 src/mesa/drivers/dri/i965/brw_blorp.cpp| 2 ++
 src/mesa/drivers/dri/i965/brw_defines.h| 3 ++-
 src/mesa/drivers/dri/i965/brw_eu_emit.c| 3 ++-
 src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 9 ++---
 src/mesa/drivers/dri/i965/brw_pipe_control.c   | 4 +++-
 src/mesa/drivers/dri/i965/gen6_queryobj.c  | 2 +-
 src/mesa/drivers/dri/i965/gen8_depth_state.c   | 3 ++-
 src/mesa/drivers/dri/i965/intel_batchbuffer.c  | 2 +-
 9 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_binding_tables.c 
b/src/mesa/drivers/dri/i965/brw_binding_tables.c
index f3a0310..bcf6422 100644
--- a/src/mesa/drivers/dri/i965/brw_binding_tables.c
+++ b/src/mesa/drivers/dri/i965/brw_binding_tables.c
@@ -54,7 +54,7 @@ static uint32_t
 reserve_hw_bt_space(struct brw_context *brw, unsigned bytes)
 {
/* From the Broadwell PRM, Volume 16, "Workarounds",
-* WaStateBindingTableOverfetch:
+* WaStateBindingTableOverfetch:hsw,bdw,chv,bxt
 * "HW over-fetches two cache lines of binding table indices.  When
 *  using the resource streamer, SW needs to pad binding table pointer
 *  updates with an additional two cache lines."
diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp 
b/src/mesa/drivers/dri/i965/brw_blorp.cpp
index 1bc6d15..dd01ea8 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp
@@ -304,6 +304,8 @@ brw_hiz_op_params::brw_hiz_op_params(struct 
intel_mipmap_tree *mt,
 * aligned to an 8x4 pixel block relative to the upper left corner
 * of the depth buffer [...]
 *
+* WaHizAmbiguate8x4Aligned:hsw
+*
 * For hiz resolves, the rectangle must also be 8x4 aligned. Item
 * WaHizAmbiguate8x4Aligned from the Haswell workarounds page and the
 * Ivybridge simulator require the alignment.
diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
b/src/mesa/drivers/dri/i965/brw_defines.h
index 01e0c99..5410a1d 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -1756,7 +1756,8 @@ enum brw_message_target {
 /* Dataport special binding table indices: */
 #define BRW_BTI_STATELESS255
 #define GEN7_BTI_SLM 254
-/* Note that on Gen8+ BTI 255 was redefined to be IA-coherent according to the
+/* WaForceEnableNonCoherent:bdw,chv,skl,kbl
+ * Note that on Gen8+ BTI 255 was redefined to be IA-coherent according to the
  * hardware spec, however because the DRM sets bit 4 of HDC_CHICKEN0 on BDW,
  * CHV and at least some pre-production steppings of SKL due to
  * WaForceEnableNonCoherent, HDC memory access may have been overridden by the
diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index 35d8039..918d69e 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -1885,7 +1885,8 @@ void brw_CMP(struct brw_codegen *p,
brw_set_src0(p, insn, src0);
brw_set_src1(p, insn, src1);
 
-   /* Item WaCMPInstNullDstForcesThreadSwitch in the Haswell Bspec workarounds
+   /* WaCMPInstNullDstForcesThreadSwitch:ivb,hsw,vlv
+* Item WaCMPInstNullDstForcesThreadSwitch in the Haswell Bspec workarounds
 * page says:
 *"Any CMP instruction with a null destination must use a {switch}."
 *
diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
index 1916a99..24d4a9d 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
@@ -1836,7 +1836,8 @@ fs_generator::generate_code(const cfg_t *cfg, int 
dispatch_width)
  brw_F16TO32(p, dst, src[0]);
  break;
   case BRW_OPCODE_CMP:
- /* The Ivybridge/BayTrail WaCMPInstFlagDepClearedEarly workaround says
+ /* WaCMPInstFlagDepClearedEarly:ivb,hsw,vlv
+  * The Ivybridge/BayTrail WaCMPInstFlagDepClearedEarly workaround says
   * that when the destination is a GRF that the dependency-clear bit on
   * the flag register is cleared early.
   *
@@ -1928,7 +1929,8 @@ fs_generator::generate_code(const cfg_t *cfg, int 
dispatch_width)
 
   case BRW_OPCODE_BFI1:
  assert(devinfo->gen >= 7);
- /* The Haswell WaForceSIMD8ForBFIInstruction workaround says that we
+ /* WaForceSIMD8ForBFIInstruction:hsw
+  * The Haswell WaForceSIMD8ForBFIInstruction workaround says that we
   * should
   *
   *"Force BFI instructions to be executed always in SIMD8."
@@ -1947,7 +1949,8 @@ fs_generator::generate_code(const cfg_t *cfg, int 
dispatch_width)