[Mesa-dev] [PATCH 2/2] i965: Disable SOL buffers and decls when not doing transform feedback.

[Kenneth Graunke]

Previously, we only emitted 3DSTATE_SO_BUFFER and 3DSTATE_SO_DECL_LIST
when transform feedback was active.  When it was inactive, we disabled
the SOL stage in 3DSTATE_SOL so the other state would be ignored.

In commit 3178d2474ae5bdd1102fb3d76a60d1d63c961ff5, Iago enabled the SOL
stage universally, so we could implement the GL_PRIMITIVES_GENERATED
statistics counter.  This caused every Piglit test to trigger assertions
in the simulator, and apparently caused GPU hangs for some users.

Apparently, we're supposed to program 3DSTATE_SO_DECL_LIST to zero when
output streams are inactive, but we hadn't been doing so.  Now that SOL
is on, we need to do that properly.

Experimentally, it looks like we also need to program 3DSTATE_SO_BUFFER
to zero as well, or else I get many GPU hangs on Haswell.

Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
Cc: Iago Toral Quiroga <itoral at igalia.com>
Cc: Chris Forbes <chrisf at ijw.co.nz>
Cc: Steven Newbury <steve at snewbury.org.uk>
---
 src/mesa/drivers/dri/i965/brw_state.h      |  3 ++-
 src/mesa/drivers/dri/i965/gen7_sol_state.c | 34 ++++++++++++++++++++++--------
 src/mesa/drivers/dri/i965/gen8_sol_state.c |  2 +-
 3 files changed, 28 insertions(+), 11 deletions(-)

Iago,

I noticed that pretty much every Piglit test broke in our simulation
environment after your patch to turn on SOL by default.  Something about
SO_DECLs not being right.  This patch seems to fix the issue.  Does it
look reasonable to you?

Steven,

Does this fix your GPU hangs on Ivybridge?

Thanks!
 --Ken

diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h
index c52a977..2c1fc87 100644
--- a/src/mesa/drivers/dri/i965/brw_state.h
+++ b/src/mesa/drivers/dri/i965/brw_state.h
@@ -237,7 +237,8 @@ void gen7_init_vtable_surface_functions(struct brw_context *brw);
 
 /* gen7_sol_state.c */
 void gen7_upload_3dstate_so_decl_list(struct brw_context *brw,
-                                      const struct brw_vue_map *vue_map);
+                                      const struct brw_vue_map *vue_map,
+                                      bool active);
 
 /* gen8_surface_state.c */
 void gen8_init_vtable_surface_functions(struct brw_context *brw);
diff --git a/src/mesa/drivers/dri/i965/gen7_sol_state.c b/src/mesa/drivers/dri/i965/gen7_sol_state.c
index eccd5a5..e2eb334 100644
--- a/src/mesa/drivers/dri/i965/gen7_sol_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_sol_state.c
@@ -36,7 +36,7 @@
 #include "main/transformfeedback.h"
 
 static void
-upload_3dstate_so_buffers(struct brw_context *brw)
+upload_3dstate_so_buffers(struct brw_context *brw, bool active)
 {
    struct gl_context *ctx = &brw->ctx;
    /* BRW_NEW_TRANSFORM_FEEDBACK */
@@ -56,7 +56,7 @@ upload_3dstate_so_buffers(struct brw_context *brw)
       uint32_t start, end;
       uint32_t stride;
 
-      if (!xfb_obj->Buffers[i]) {
+      if (!xfb_obj->Buffers[i] || !active) {
 	 /* The pitch of 0 in this command indicates that the buffer is
 	  * unbound and won't be written to.
 	  */
@@ -96,10 +96,28 @@ upload_3dstate_so_buffers(struct brw_context *brw)
  */
 void
 gen7_upload_3dstate_so_decl_list(struct brw_context *brw,
-                                 const struct brw_vue_map *vue_map)
+                                 const struct brw_vue_map *vue_map,
+                                 bool active)
 {
-   struct gl_context *ctx = &brw->ctx;
+   if (!active) {
+      /* If inactive, disable all SO outputs.
+       *
+       * From the Ivybridge PRM, Volume 2, Part 1, page 202
+       * (3DSTATE_SO_DECL_LIST packet definition), DWord 1, bits 3:0:
+       * "Note: For 'inactive' streams, software must program this field to
+       *  all zero (no buffers written to) and the corresponding Num Entries
+       *  field to zero (no valid SO_DECLs)."
+       */
+      BEGIN_BATCH(3);
+      OUT_BATCH(_3DSTATE_SO_DECL_LIST << 16 | (3 - 2));
+      OUT_BATCH(0);
+      OUT_BATCH(0);
+      ADVANCE_BATCH();
+      return;
+   }
+
    /* BRW_NEW_TRANSFORM_FEEDBACK */
+   struct gl_context *ctx = &brw->ctx;
    struct gl_transform_feedback_object *xfb_obj =
       ctx->TransformFeedback.CurrentObject;
    const struct gl_transform_feedback_info *linked_xfb_info =
@@ -289,11 +307,9 @@ upload_sol_state(struct brw_context *brw)
    /* BRW_NEW_TRANSFORM_FEEDBACK */
    bool active = _mesa_is_xfb_active_and_unpaused(ctx);
 
-   if (active) {
-      upload_3dstate_so_buffers(brw);
-      /* BRW_NEW_VUE_MAP_GEOM_OUT */
-      gen7_upload_3dstate_so_decl_list(brw, &brw->vue_map_geom_out);
-   }
+   upload_3dstate_so_buffers(brw, active);
+   /* BRW_NEW_VUE_MAP_GEOM_OUT */
+   gen7_upload_3dstate_so_decl_list(brw, &brw->vue_map_geom_out, active);
 
    /* Finally, set up the SOL stage.  This command must always follow updates to
     * the nonpipelined SOL state (3DSTATE_SO_BUFFER, 3DSTATE_SO_DECL_LIST) or
diff --git a/src/mesa/drivers/dri/i965/gen8_sol_state.c b/src/mesa/drivers/dri/i965/gen8_sol_state.c
index ebcdaf8..ff85bf2 100644
--- a/src/mesa/drivers/dri/i965/gen8_sol_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_sol_state.c
@@ -157,7 +157,7 @@ upload_sol_state(struct brw_context *brw)
    if (active) {
       gen8_upload_3dstate_so_buffers(brw);
       /* BRW_NEW_VUE_MAP_GEOM_OUT */
-      gen7_upload_3dstate_so_decl_list(brw, &brw->vue_map_geom_out);
+      gen7_upload_3dstate_so_decl_list(brw, &brw->vue_map_geom_out, active);
    }
 
    gen8_upload_3dstate_streamout(brw, active, &brw->vue_map_geom_out);
-- 
2.0.0