[Mesa-stable] [PATCH v2] i965/gen7_urb: Re-emit PUSH_CONSTANT_ALLOC on some gen9

Nanley Chery nanleychery at gmail.com
Wed Aug 29 20:38:51 UTC 2018


According to internal docs, some gen9 platforms have a pixel shader push
constant synchronization issue. Although not listed among said
platforms, this issue seems to be present on the GeminiLake 2x6's we've
tested.

We consider the available workarounds to be too detrimental on
performance. Instead, we mitigate the issue by applying part of one of
the workarounds. Re-emit PUSH_CONSTANT_ALLOC at the top of every batch
(as suggested by Ken).

Fixes ext_framebuffer_multisample-accuracy piglit test failures with the
following options:
* 6 depth_draw small depthstencil
* 8 stencil_draw small depthstencil
* 6 stencil_draw small depthstencil
* 8 depth_resolve small
* 6 stencil_resolve small depthstencil
* 4 stencil_draw small depthstencil
* 16 stencil_draw small depthstencil
* 16 depth_draw small depthstencil
* 2 stencil_resolve small depthstencil
* 6 stencil_draw small
* all_samples stencil_draw small
* 2 depth_draw small depthstencil
* all_samples depth_draw small depthstencil
* all_samples stencil_resolve small
* 4 depth_draw small depthstencil
* all_samples depth_draw small
* all_samples stencil_draw small depthstencil
* 4 stencil_resolve small depthstencil
* 4 depth_resolve small depthstencil
* all_samples stencil_resolve small depthstencil

v2: Include more platforms in WA (Ken).

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106865
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93355
Cc: <mesa-stable at lists.freedesktop.org>
Tested-by: Mark Janes <mark.a.janes at intel.com>
---
 src/mesa/drivers/dri/i965/gen7_urb.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

I'm not sure I have enough information about what's happening in the HW
to create a piglit test for this issue.

diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c b/src/mesa/drivers/dri/i965/gen7_urb.c
index 2e5f8e60ba9..e7259fc1b8d 100644
--- a/src/mesa/drivers/dri/i965/gen7_urb.c
+++ b/src/mesa/drivers/dri/i965/gen7_urb.c
@@ -118,6 +118,33 @@ gen7_emit_push_constant_state(struct brw_context *brw, unsigned vs_size,
    const struct gen_device_info *devinfo = &brw->screen->devinfo;
    unsigned offset = 0;
 
+   /* From the SKL PRM, Workarounds section (#878):
+    *
+    *    Push constant buffer corruption possible. WA: Insert 2 zero-length
+    *    PushConst_PS before every intended PushConst_PS update, issue a
+    *    NULLPRIM after each of the zero len PC update to make sure CS commits
+    *    them.
+    *
+    * This workaround is attempting to solve a pixel shader push constant
+    * synchronization issue.
+    *
+    * There's an unpublished WA that involves re-emitting
+    * 3DSTATE_PUSH_CONSTANT_ALLOC_PS for every 500-ish 3DSTATE_CONSTANT_PS
+    * packets. Since our counting methods may not be reliable due to
+    * context-switching and pre-emption, we instead choose to approximate this
+    * behavior by re-emitting the packet at the top of the batch.
+    */
+   if (brw->ctx.NewDriverState == BRW_NEW_BATCH) {
+       /* SKL GT2 and GLK 2x6 have reliably demonstrated this issue thus far.
+        * We've also seen some intermittent failures from SKL GT4 and BXT in
+        * the past.
+        */
+      if (!devinfo->is_skylake &&
+          !devinfo->is_broxton &&
+          !devinfo->is_geminilake)
+         return;
+   }
+
    BEGIN_BATCH(10);
    OUT_BATCH(_3DSTATE_PUSH_CONSTANT_ALLOC_VS << 16 | (2 - 2));
    OUT_BATCH(vs_size | offset << GEN7_PUSH_CONSTANT_BUFFER_OFFSET_SHIFT);
@@ -154,6 +181,7 @@ const struct brw_tracked_state gen7_push_constant_space = {
    .dirty = {
       .mesa = 0,
       .brw = BRW_NEW_CONTEXT |
+             BRW_NEW_BATCH | /* Push constant workaround */
              BRW_NEW_GEOMETRY_PROGRAM |
              BRW_NEW_TESS_PROGRAMS,
    },
-- 
2.18.0



More information about the mesa-stable mailing list