[Mesa-dev] [PATCH 2/2] i965: Disable SOL buffers and decls when not doing transform feedback.
Jordan Justen
jljusten at gmail.com
Tue Jul 1 20:26:48 PDT 2014
Should this be i965/gen7? (Seems to be a no-op for gen8.)
Series Reviewed-by: Jordan Justen <jordan.l.justen at 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 <kenneth at 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 <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
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list