Re: [Mesa-dev] [PATCH 2/2] Implement WaClearTDRRegBeforeEOTForNonPS.

2017-12-01 Thread Rafael Antognolli
On Fri, Dec 01, 2017 at 10:33:26AM +0200, Pohjolainen, Topi wrote:
> On Thu, Nov 30, 2017 at 04:50:19PM -0800, Rafael Antognolli wrote:
> > It looks like I forgot to prefix the subject with "intel/compiler:".
> > Fixed locally.
> > 
> > On Thu, Nov 30, 2017 at 04:42:48PM -0800, Rafael Antognolli wrote:
> > > The bspec describes:
> > > 
> > >"WA: Clear tdr register before send EOT in all non-PS shader kernels
> > > 
> > >mov(8) tdr0:ud 0x0:ud {NoMask}"
> > > 
> > > Signed-off-by: Rafael Antognolli 
> > > ---
> > >  src/intel/compiler/brw_fs_generator.cpp | 7 +++
> > >  src/intel/compiler/brw_reg.h| 6 ++
> > >  2 files changed, 13 insertions(+)
> > > 
> > > diff --git a/src/intel/compiler/brw_fs_generator.cpp 
> > > b/src/intel/compiler/brw_fs_generator.cpp
> > > index 28790c86a64..78aa764fe73 100644
> > > --- a/src/intel/compiler/brw_fs_generator.cpp
> > > +++ b/src/intel/compiler/brw_fs_generator.cpp
> > > @@ -573,6 +573,13 @@ fs_generator::generate_urb_write(fs_inst *inst, 
> > > struct brw_reg payload)
> > >  {
> > > brw_inst *insn;
> > >  
> 
> I think it would be nice to have the bspec quote here.

Good point, will do that.

> > > +   if (inst->eot) {
> > > +  brw_push_insn_state(p);
> > > +  brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> > > +  brw_MOV(p, brw_tdr_reg(), brw_imm_uw(0));
> > > +  brw_pop_insn_state(p);
> > > +   }
> > > +
> > > insn = brw_next_insn(p, BRW_OPCODE_SEND);
> > >  
> > > brw_set_dest(p, insn, brw_null_reg());
> > > diff --git a/src/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
> > > index ec1045b612a..a039c6f676c 100644
> > > --- a/src/intel/compiler/brw_reg.h
> > > +++ b/src/intel/compiler/brw_reg.h
> > > @@ -774,6 +774,12 @@ brw_address_reg(unsigned subnr)
> > > return brw_uw1_reg(BRW_ARCHITECTURE_REGISTER_FILE, BRW_ARF_ADDRESS, 
> > > subnr);
> > >  }
> > >  
> > > +static inline struct brw_reg
> > > +brw_tdr_reg(void)
> > > +{
> > > +   return brw_uw1_reg(BRW_ARCHITECTURE_REGISTER_FILE, BRW_ARF_TDR, 0);
> > > +}
> > > +
> > >  /* If/else instructions break in align16 mode if writemask & swizzle
> > >   * aren't xyzw.  This goes against the convention for other scalar
> > >   * regs:
> > > -- 
> > > 2.13.6
> > > 
> > > ___
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] Implement WaClearTDRRegBeforeEOTForNonPS.

2017-12-01 Thread Pohjolainen, Topi
On Thu, Nov 30, 2017 at 04:50:19PM -0800, Rafael Antognolli wrote:
> It looks like I forgot to prefix the subject with "intel/compiler:".
> Fixed locally.
> 
> On Thu, Nov 30, 2017 at 04:42:48PM -0800, Rafael Antognolli wrote:
> > The bspec describes:
> > 
> >"WA: Clear tdr register before send EOT in all non-PS shader kernels
> > 
> >mov(8) tdr0:ud 0x0:ud {NoMask}"
> > 
> > Signed-off-by: Rafael Antognolli 
> > ---
> >  src/intel/compiler/brw_fs_generator.cpp | 7 +++
> >  src/intel/compiler/brw_reg.h| 6 ++
> >  2 files changed, 13 insertions(+)
> > 
> > diff --git a/src/intel/compiler/brw_fs_generator.cpp 
> > b/src/intel/compiler/brw_fs_generator.cpp
> > index 28790c86a64..78aa764fe73 100644
> > --- a/src/intel/compiler/brw_fs_generator.cpp
> > +++ b/src/intel/compiler/brw_fs_generator.cpp
> > @@ -573,6 +573,13 @@ fs_generator::generate_urb_write(fs_inst *inst, struct 
> > brw_reg payload)
> >  {
> > brw_inst *insn;
> >  

I think it would be nice to have the bspec quote here.

> > +   if (inst->eot) {
> > +  brw_push_insn_state(p);
> > +  brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> > +  brw_MOV(p, brw_tdr_reg(), brw_imm_uw(0));
> > +  brw_pop_insn_state(p);
> > +   }
> > +
> > insn = brw_next_insn(p, BRW_OPCODE_SEND);
> >  
> > brw_set_dest(p, insn, brw_null_reg());
> > diff --git a/src/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
> > index ec1045b612a..a039c6f676c 100644
> > --- a/src/intel/compiler/brw_reg.h
> > +++ b/src/intel/compiler/brw_reg.h
> > @@ -774,6 +774,12 @@ brw_address_reg(unsigned subnr)
> > return brw_uw1_reg(BRW_ARCHITECTURE_REGISTER_FILE, BRW_ARF_ADDRESS, 
> > subnr);
> >  }
> >  
> > +static inline struct brw_reg
> > +brw_tdr_reg(void)
> > +{
> > +   return brw_uw1_reg(BRW_ARCHITECTURE_REGISTER_FILE, BRW_ARF_TDR, 0);
> > +}
> > +
> >  /* If/else instructions break in align16 mode if writemask & swizzle
> >   * aren't xyzw.  This goes against the convention for other scalar
> >   * regs:
> > -- 
> > 2.13.6
> > 
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] Implement WaClearTDRRegBeforeEOTForNonPS.

2017-11-30 Thread Kenneth Graunke
On Thursday, November 30, 2017 4:42:48 PM PST Rafael Antognolli wrote:
> The bspec describes:
> 
>"WA: Clear tdr register before send EOT in all non-PS shader kernels
> 
>mov(8) tdr0:ud 0x0:ud {NoMask}"
> 
> Signed-off-by: Rafael Antognolli 
> ---
>  src/intel/compiler/brw_fs_generator.cpp | 7 +++
>  src/intel/compiler/brw_reg.h| 6 ++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/src/intel/compiler/brw_fs_generator.cpp 
> b/src/intel/compiler/brw_fs_generator.cpp
> index 28790c86a64..78aa764fe73 100644
> --- a/src/intel/compiler/brw_fs_generator.cpp
> +++ b/src/intel/compiler/brw_fs_generator.cpp
> @@ -573,6 +573,13 @@ fs_generator::generate_urb_write(fs_inst *inst, struct 
> brw_reg payload)
>  {
> brw_inst *insn;
>  
> +   if (inst->eot) {
> +  brw_push_insn_state(p);
> +  brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> +  brw_MOV(p, brw_tdr_reg(), brw_imm_uw(0));
> +  brw_pop_insn_state(p);
> +   }
> +
> insn = brw_next_insn(p, BRW_OPCODE_SEND);
>  
> brw_set_dest(p, insn, brw_null_reg());
> diff --git a/src/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
> index ec1045b612a..a039c6f676c 100644
> --- a/src/intel/compiler/brw_reg.h
> +++ b/src/intel/compiler/brw_reg.h
> @@ -774,6 +774,12 @@ brw_address_reg(unsigned subnr)
> return brw_uw1_reg(BRW_ARCHITECTURE_REGISTER_FILE, BRW_ARF_ADDRESS, 
> subnr);
>  }
>  
> +static inline struct brw_reg
> +brw_tdr_reg(void)
> +{
> +   return brw_uw1_reg(BRW_ARCHITECTURE_REGISTER_FILE, BRW_ARF_TDR, 0);
> +}
> +
>  /* If/else instructions break in align16 mode if writemask & swizzle
>   * aren't xyzw.  This goes against the convention for other scalar
>   * regs:
> 

Still not clear whether this is needed for compute shaders or not.

Looks good for the other stages.

My gut feeling is that this only matters if you're doing preemption
(possibly mid-draw preemption), but I have no data to back that up.
We may as well do it...

Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] Implement WaClearTDRRegBeforeEOTForNonPS.

2017-11-30 Thread Rafael Antognolli
It looks like I forgot to prefix the subject with "intel/compiler:".
Fixed locally.

On Thu, Nov 30, 2017 at 04:42:48PM -0800, Rafael Antognolli wrote:
> The bspec describes:
> 
>"WA: Clear tdr register before send EOT in all non-PS shader kernels
> 
>mov(8) tdr0:ud 0x0:ud {NoMask}"
> 
> Signed-off-by: Rafael Antognolli 
> ---
>  src/intel/compiler/brw_fs_generator.cpp | 7 +++
>  src/intel/compiler/brw_reg.h| 6 ++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/src/intel/compiler/brw_fs_generator.cpp 
> b/src/intel/compiler/brw_fs_generator.cpp
> index 28790c86a64..78aa764fe73 100644
> --- a/src/intel/compiler/brw_fs_generator.cpp
> +++ b/src/intel/compiler/brw_fs_generator.cpp
> @@ -573,6 +573,13 @@ fs_generator::generate_urb_write(fs_inst *inst, struct 
> brw_reg payload)
>  {
> brw_inst *insn;
>  
> +   if (inst->eot) {
> +  brw_push_insn_state(p);
> +  brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> +  brw_MOV(p, brw_tdr_reg(), brw_imm_uw(0));
> +  brw_pop_insn_state(p);
> +   }
> +
> insn = brw_next_insn(p, BRW_OPCODE_SEND);
>  
> brw_set_dest(p, insn, brw_null_reg());
> diff --git a/src/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
> index ec1045b612a..a039c6f676c 100644
> --- a/src/intel/compiler/brw_reg.h
> +++ b/src/intel/compiler/brw_reg.h
> @@ -774,6 +774,12 @@ brw_address_reg(unsigned subnr)
> return brw_uw1_reg(BRW_ARCHITECTURE_REGISTER_FILE, BRW_ARF_ADDRESS, 
> subnr);
>  }
>  
> +static inline struct brw_reg
> +brw_tdr_reg(void)
> +{
> +   return brw_uw1_reg(BRW_ARCHITECTURE_REGISTER_FILE, BRW_ARF_TDR, 0);
> +}
> +
>  /* If/else instructions break in align16 mode if writemask & swizzle
>   * aren't xyzw.  This goes against the convention for other scalar
>   * regs:
> -- 
> 2.13.6
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] Implement WaClearTDRRegBeforeEOTForNonPS.

2017-11-30 Thread Rafael Antognolli
The bspec describes:

   "WA: Clear tdr register before send EOT in all non-PS shader kernels

   mov(8) tdr0:ud 0x0:ud {NoMask}"

Signed-off-by: Rafael Antognolli 
---
 src/intel/compiler/brw_fs_generator.cpp | 7 +++
 src/intel/compiler/brw_reg.h| 6 ++
 2 files changed, 13 insertions(+)

diff --git a/src/intel/compiler/brw_fs_generator.cpp 
b/src/intel/compiler/brw_fs_generator.cpp
index 28790c86a64..78aa764fe73 100644
--- a/src/intel/compiler/brw_fs_generator.cpp
+++ b/src/intel/compiler/brw_fs_generator.cpp
@@ -573,6 +573,13 @@ fs_generator::generate_urb_write(fs_inst *inst, struct 
brw_reg payload)
 {
brw_inst *insn;
 
+   if (inst->eot) {
+  brw_push_insn_state(p);
+  brw_set_default_mask_control(p, BRW_MASK_DISABLE);
+  brw_MOV(p, brw_tdr_reg(), brw_imm_uw(0));
+  brw_pop_insn_state(p);
+   }
+
insn = brw_next_insn(p, BRW_OPCODE_SEND);
 
brw_set_dest(p, insn, brw_null_reg());
diff --git a/src/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
index ec1045b612a..a039c6f676c 100644
--- a/src/intel/compiler/brw_reg.h
+++ b/src/intel/compiler/brw_reg.h
@@ -774,6 +774,12 @@ brw_address_reg(unsigned subnr)
return brw_uw1_reg(BRW_ARCHITECTURE_REGISTER_FILE, BRW_ARF_ADDRESS, subnr);
 }
 
+static inline struct brw_reg
+brw_tdr_reg(void)
+{
+   return brw_uw1_reg(BRW_ARCHITECTURE_REGISTER_FILE, BRW_ARF_TDR, 0);
+}
+
 /* If/else instructions break in align16 mode if writemask & swizzle
  * aren't xyzw.  This goes against the convention for other scalar
  * regs:
-- 
2.13.6

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev