Re: [Mesa-dev] [PATCH v2 2/4] i965/gen10: Implement WaForceRCPFEHangWorkaround

2017-11-02 Thread Anuj Phogat 
On Thu, Nov 2, 2017 at 11:18 AM, Nanley Chery  wrote:
> On Wed, Nov 01, 2017 at 03:49:52PM -0700, Anuj Phogat wrote:
>> V2: Add the check for Post Sync Operation.
>> Update the workaround comment.
>>
>
> Did you mean to put the V2 hunk after the messsage?
>
>> This workaround doesn't fix any of the piglit hangs we've seen
>> on CNL. But it might be fixing something we haven't tested yet.
>>
>> Cc: Nanley Chery 
>> Signed-off-by: Anuj Phogat 
>> Reviewed-by: Rafael Antognolli 
>> ---
>>  src/mesa/drivers/dri/i965/brw_pipe_control.c | 21 +
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_pipe_control.c 
>> b/src/mesa/drivers/dri/i965/brw_pipe_control.c
>> index 6ebe1443d5..b1975ce533 100644
>> --- a/src/mesa/drivers/dri/i965/brw_pipe_control.c
>> +++ b/src/mesa/drivers/dri/i965/brw_pipe_control.c
>> @@ -89,6 +89,24 @@ gen7_cs_stall_every_four_pipe_controls(struct brw_context 
>> *brw, uint32_t flags)
>> return 0;
>>  }
>>
>> +/* #1130 from gen10 workarounds page in h/w specs:
>> + * "Enable Depth Stall on every Post Sync Op if Render target Cache Flush is
>> + *  not enabled in same PIPE CONTROL and Enable Pixel score board stall if
>> + *  Render target cache flush is enabled."
>> + *
>> + * Applicable to CNL B0 and C0 steppings only.
>> + */
>> +static void
>> +gen10_add_rcpfe_workaround_bits(uint32_t *flags)
>> +{
>> +   if (*flags & PIPE_CONTROL_RENDER_TARGET_FLUSH)
>> +  *flags = *flags | PIPE_CONTROL_STALL_AT_SCOREBOARD;
>> +   else if ((*flags & PIPE_CONTROL_WRITE_IMMEDIATE) ||
>> +(*flags & PIPE_CONTROL_WRITE_DEPTH_COUNT) ||
>> +(*flags & PIPE_CONTROL_WRITE_TIMESTAMP))
>> +  *flags = *flags | PIPE_CONTROL_DEPTH_STALL;
>
> I think we usually use braces in multi-line if-statements.
>
> How about:
>} else if (*flags & (PIPE_CONTROL_WRITE_IMMEDIATE |
> PIPE_CONTROL_WRITE_DEPTH_COUNT |
> PIPE_CONTROL_WRITE_TIMESTAMP)) {
>
>
>
> With the commit message fixed and braces included, this patch is
> Reviewed-by: Nanley Chery 
>

All suggestions fixed locally.
>> +}
>> +
>>  static void
>>  brw_emit_pipe_control(struct brw_context *brw, uint32_t flags,
>>struct brw_bo *bo, uint32_t offset, uint64_t imm)
>> @@ -109,6 +127,9 @@ brw_emit_pipe_control(struct brw_context *brw, uint32_t 
>> flags,
>>   brw_emit_pipe_control_flush(brw, 0);
>>}
>>
>> +  if (devinfo->gen == 10)
>> + gen10_add_rcpfe_workaround_bits();
>> +
>>BEGIN_BATCH(6);
>>OUT_BATCH(_3DSTATE_PIPE_CONTROL | (6 - 2));
>>OUT_BATCH(flags);
>> --
>> 2.13.5
>>
>> ___
>> 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 v2 2/4] i965/gen10: Implement WaForceRCPFEHangWorkaround

2017-11-02 Thread Nanley Chery 
On Wed, Nov 01, 2017 at 03:49:52PM -0700, Anuj Phogat wrote:
> V2: Add the check for Post Sync Operation.
> Update the workaround comment.
> 

Did you mean to put the V2 hunk after the messsage?

> This workaround doesn't fix any of the piglit hangs we've seen
> on CNL. But it might be fixing something we haven't tested yet.
> 
> Cc: Nanley Chery 
> Signed-off-by: Anuj Phogat 
> Reviewed-by: Rafael Antognolli 
> ---
>  src/mesa/drivers/dri/i965/brw_pipe_control.c | 21 +
>  1 file changed, 21 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_pipe_control.c 
> b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> index 6ebe1443d5..b1975ce533 100644
> --- a/src/mesa/drivers/dri/i965/brw_pipe_control.c
> +++ b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> @@ -89,6 +89,24 @@ gen7_cs_stall_every_four_pipe_controls(struct brw_context 
> *brw, uint32_t flags)
> return 0;
>  }
>  
> +/* #1130 from gen10 workarounds page in h/w specs:
> + * "Enable Depth Stall on every Post Sync Op if Render target Cache Flush is
> + *  not enabled in same PIPE CONTROL and Enable Pixel score board stall if
> + *  Render target cache flush is enabled."
> + *
> + * Applicable to CNL B0 and C0 steppings only.
> + */
> +static void
> +gen10_add_rcpfe_workaround_bits(uint32_t *flags)
> +{
> +   if (*flags & PIPE_CONTROL_RENDER_TARGET_FLUSH)
> +  *flags = *flags | PIPE_CONTROL_STALL_AT_SCOREBOARD;
> +   else if ((*flags & PIPE_CONTROL_WRITE_IMMEDIATE) ||
> +(*flags & PIPE_CONTROL_WRITE_DEPTH_COUNT) ||
> +(*flags & PIPE_CONTROL_WRITE_TIMESTAMP))
> +  *flags = *flags | PIPE_CONTROL_DEPTH_STALL;

I think we usually use braces in multi-line if-statements.

How about:
   } else if (*flags & (PIPE_CONTROL_WRITE_IMMEDIATE |
PIPE_CONTROL_WRITE_DEPTH_COUNT |
PIPE_CONTROL_WRITE_TIMESTAMP)) {



With the commit message fixed and braces included, this patch is
Reviewed-by: Nanley Chery 

> +}
> +
>  static void
>  brw_emit_pipe_control(struct brw_context *brw, uint32_t flags,
>struct brw_bo *bo, uint32_t offset, uint64_t imm)
> @@ -109,6 +127,9 @@ brw_emit_pipe_control(struct brw_context *brw, uint32_t 
> flags,
>   brw_emit_pipe_control_flush(brw, 0);
>}
>  
> +  if (devinfo->gen == 10)
> + gen10_add_rcpfe_workaround_bits();
> +
>BEGIN_BATCH(6);
>OUT_BATCH(_3DSTATE_PIPE_CONTROL | (6 - 2));
>OUT_BATCH(flags);
> -- 
> 2.13.5
> 
> ___
> 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 v2 2/4] i965/gen10: Implement WaForceRCPFEHangWorkaround

2017-11-01 Thread Anuj Phogat 
V2: Add the check for Post Sync Operation.
Update the workaround comment.

This workaround doesn't fix any of the piglit hangs we've seen
on CNL. But it might be fixing something we haven't tested yet.

Cc: Nanley Chery 
Signed-off-by: Anuj Phogat 
Reviewed-by: Rafael Antognolli 
---
 src/mesa/drivers/dri/i965/brw_pipe_control.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_pipe_control.c 
b/src/mesa/drivers/dri/i965/brw_pipe_control.c
index 6ebe1443d5..b1975ce533 100644
--- a/src/mesa/drivers/dri/i965/brw_pipe_control.c
+++ b/src/mesa/drivers/dri/i965/brw_pipe_control.c
@@ -89,6 +89,24 @@ gen7_cs_stall_every_four_pipe_controls(struct brw_context 
*brw, uint32_t flags)
return 0;
 }
 
+/* #1130 from gen10 workarounds page in h/w specs:
+ * "Enable Depth Stall on every Post Sync Op if Render target Cache Flush is
+ *  not enabled in same PIPE CONTROL and Enable Pixel score board stall if
+ *  Render target cache flush is enabled."
+ *
+ * Applicable to CNL B0 and C0 steppings only.
+ */
+static void
+gen10_add_rcpfe_workaround_bits(uint32_t *flags)
+{
+   if (*flags & PIPE_CONTROL_RENDER_TARGET_FLUSH)
+  *flags = *flags | PIPE_CONTROL_STALL_AT_SCOREBOARD;
+   else if ((*flags & PIPE_CONTROL_WRITE_IMMEDIATE) ||
+(*flags & PIPE_CONTROL_WRITE_DEPTH_COUNT) ||
+(*flags & PIPE_CONTROL_WRITE_TIMESTAMP))
+  *flags = *flags | PIPE_CONTROL_DEPTH_STALL;
+}
+
 static void
 brw_emit_pipe_control(struct brw_context *brw, uint32_t flags,
   struct brw_bo *bo, uint32_t offset, uint64_t imm)
@@ -109,6 +127,9 @@ brw_emit_pipe_control(struct brw_context *brw, uint32_t 
flags,
  brw_emit_pipe_control_flush(brw, 0);
   }
 
+  if (devinfo->gen == 10)
+ gen10_add_rcpfe_workaround_bits();
+
   BEGIN_BATCH(6);
   OUT_BATCH(_3DSTATE_PIPE_CONTROL | (6 - 2));
   OUT_BATCH(flags);
-- 
2.13.5

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