From 996cd238b81814f81cf94fdd6107faf26d9d0a4b Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Fri, 18 Aug 2017 16:10:39 -0700 Subject: [PATCH] i965: Stop looking at NewDriverState when emitting 3DSTATE_URB Looking at NewDriverState is not safe in general. The state atom system is set up to ensure that new bits that get added to NewDriverState get accumulated into the set of bits used when emitting atoms but it doesn't go the other way. If we read NewDriverState, we may not get the full picture because the per-pipeline state (3D or compute) does not get added to NewDriverState before state emit is done. It's especially dangerous to do this from BLORP (either explicitly or implicitly when BLORP calls gen7_upload_urb) because that does not happen during one of the normal state upload paths. This commit solves the problem by whacking all of the per-shader-stage URB sizes to zero whenever we change the total URB size. We still have to flag BRW_NEW_URB_SIZE to ensure that the gen7_urb atom triggers but the actual decision in gen7_upload_urb can now be based entirely on URB sizes rather than on state atoms. This also makes BLORP correct because it just asks for a new URB config whenever the vsize is too small and so any change to the total URB size will trigger blorp to re-emit as well because 0 < vs_entry_size. Reviewed-by: Kenneth Graunke Reviewed-by: Jordan Justen Bugzilla: https://bugs.freedesktop.org/102289 Cc: mesa-stable@lists.freedesktop.org (cherry picked from commit d5e217dbfda2a87e149bdc8586c25143fc0ac183) --- src/mesa/drivers/dri/i965/gen7_l3_state.c | 9 +++++++++ src/mesa/drivers/dri/i965/gen7_urb.c | 4 +--- src/mesa/drivers/dri/i965/genX_blorp_exec.c | 3 +-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/mesa/drivers/dri/i965/gen7_l3_state.c b/src/mesa/drivers/dri/i965/gen7_l3_state.c index 536c00c4543..53638eb9b83 100644 --- a/src/mesa/drivers/dri/i965/gen7_l3_state.c +++ b/src/mesa/drivers/dri/i965/gen7_l3_state.c @@ -204,6 +204,15 @@ update_urb_size(struct brw_context *brw, const struct gen_l3_config *cfg) if (brw->urb.size != sz) { brw->urb.size = sz; brw->ctx.NewDriverState |= BRW_NEW_URB_SIZE; + + /* If we change the total URB size, reset the individual stage sizes to + * zero so that, even if there is no URB size change, gen7_upload_urb + * still re-emits 3DSTATE_URB_*. + */ + brw->urb.vsize = 0; + brw->urb.gsize = 0; + brw->urb.hsize = 0; + brw->urb.dsize = 0; } } diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c b/src/mesa/drivers/dri/i965/gen7_urb.c index 028161df395..0da44e116f6 100644 --- a/src/mesa/drivers/dri/i965/gen7_urb.c +++ b/src/mesa/drivers/dri/i965/gen7_urb.c @@ -197,9 +197,7 @@ gen7_upload_urb(struct brw_context *brw, unsigned vs_size, /* If we're just switching between programs with the same URB requirements, * skip the rest of the logic. */ - if (!(brw->ctx.NewDriverState & BRW_NEW_CONTEXT) && - !(brw->ctx.NewDriverState & BRW_NEW_URB_SIZE) && - brw->urb.vsize == entry_size[MESA_SHADER_VERTEX] && + if (brw->urb.vsize == entry_size[MESA_SHADER_VERTEX] && brw->urb.gs_present == gs_present && brw->urb.gsize == entry_size[MESA_SHADER_GEOMETRY] && brw->urb.tess_present == tess_present && diff --git a/src/mesa/drivers/dri/i965/genX_blorp_exec.c b/src/mesa/drivers/dri/i965/genX_blorp_exec.c index 00720f853c4..efe75f381ef 100644 --- a/src/mesa/drivers/dri/i965/genX_blorp_exec.c +++ b/src/mesa/drivers/dri/i965/genX_blorp_exec.c @@ -161,8 +161,7 @@ blorp_emit_urb_config(struct blorp_batch *batch, unsigned vs_entry_size) struct brw_context *brw = batch->driver_batch; #if GEN_GEN >= 7 - if (!(brw->ctx.NewDriverState & (BRW_NEW_CONTEXT | BRW_NEW_URB_SIZE)) && - brw->urb.vsize >= vs_entry_size) + if (brw->urb.vsize >= vs_entry_size) return; brw->ctx.NewDriverState |= BRW_NEW_URB_SIZE; -- 2.11.0