i965: Stop looking at NewDriverState when emitting 3DSTATE_URB
authorJason Ekstrand <jason.ekstrand@intel.com>
Fri, 18 Aug 2017 23:10:39 +0000 (16:10 -0700)
committerJason Ekstrand <jason.ekstrand@intel.com>
Sat, 19 Aug 2017 00:30:55 +0000 (17:30 -0700)
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 <kenneth@whitecape.org>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Bugzilla: https://bugs.freedesktop.org/102289
Cc: mesa-stable@lists.freedesktop.org
src/mesa/drivers/dri/i965/gen7_l3_state.c
src/mesa/drivers/dri/i965/gen7_urb.c
src/mesa/drivers/dri/i965/genX_blorp_exec.c

index 536c00c4543368196327f26b14c4e10e44d012d2..53638eb9b83cd84f9977dbe72231edae6bbb8824 100644 (file)
@@ -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;
    }
 }
 
index d5b03ef44fcb314848f40656bed9038de8e06ed5..06113fa572c9d2fe9420728b4fec5d60ef9802d0 100644 (file)
@@ -201,9 +201,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 &&
index f2c9f3f1f820ab9b2dba367964156c9b7921d417..2b531cbcd12e941bc82fac4350ed1444ec8e6312 100644 (file)
@@ -176,8 +176,7 @@ blorp_emit_urb_config(struct blorp_batch *batch,
    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;
 
    gen7_upload_urb(brw, vs_entry_size, false, false);