Should this be i965/gen7? (Seems to be a no-op for gen8.) Series Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com>
Speaking of xfb and gen8, I rebased a ~week old branch today, and saw two regressions: spec/EXT_transform_feedback/primgen-query transform-feedback-disabled: pass fail spec/ARB_transform_feedback2/counting with pause: pass fail -Jordan On Tue, Jul 1, 2014 at 5:25 PM, Kenneth Graunke <kenn...@whitecape.org> wrote: > 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 <kenn...@whitecape.org> > Cc: Iago Toral Quiroga <ito...@igalia.com> > Cc: Chris Forbes <chr...@ijw.co.nz> > Cc: Steven Newbury <st...@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 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev