Mesa (master): i965: Don' t reallocate push constant URB space on new VS programs.

Kenneth Graunke kwg at kemper.freedesktop.org
Wed Jan 11 20:14:43 UTC 2012


Module: Mesa
Branch: master
Commit: 28cfa1fa213fe7ba6e5b57e61da663a6c3bf0c13
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=28cfa1fa213fe7ba6e5b57e61da663a6c3bf0c13

Author: Kenneth Graunke <kenneth at whitecape.org>
Date:   Wed Jan 11 11:44:02 2012 -0800

i965: Don't reallocate push constant URB space on new VS programs.

The gen7_urb atom depends on CACHE_NEW_VS_PROG and CACHE_NEW_GS_PROG,
causing gen7_upload_urb() to be called when switching to a new VS
program.

In addition to partitioning the URB space between the VS and GS,
gen7_upload_urb() also allocated space for VS and PS push constants.
Unfortunately, this meant that whenever CACHE_NEW_VS was flagged, we'd
reallocate the space for the PS push constants.  According to the BSpec,
after sending 3DSTATE_PUSH_CONSTANT_ALLOC_PS, we must reprogram
3DSTATE_CONSTANT_PS prior to the next 3DPRIMITIVE.

Since our URB allocation for push constants is entirely static, it makes
sense to split it out into its own atom that only subscribes to
BRW_NEW_CONTEXT.  This avoids reallocating the space and trashing
constants.

Fixes a rendering artifact in Extreme Tuxracer, where instead of a snow
trail, you'd get a bright red streak (affectionately known as the
"bloody penguin bug").

This also explains why adding VS-related dirty bits to gen7_ps_state
made the problem disappear: it made 3DSTATE_CONSTANT_PS be emitted after
every 3DSTATE_PUSH_CONSTANT_ALLOC_PS packet.

NOTE: This is a candidate for the 7.11 branch.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38868
Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
Reviewed-by: Paul Berry <stereotype441 at gmail.com>

---

 src/mesa/drivers/dri/i965/brw_state.h        |    1 +
 src/mesa/drivers/dri/i965/brw_state_upload.c |    1 +
 src/mesa/drivers/dri/i965/gen7_urb.c         |   34 ++++++++++++++++++-------
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h
index fcfe79f..2dd5665 100644
--- a/src/mesa/drivers/dri/i965/brw_state.h
+++ b/src/mesa/drivers/dri/i965/brw_state.h
@@ -110,6 +110,7 @@ extern const struct brw_tracked_state gen7_clip_state;
 extern const struct brw_tracked_state gen7_depth_stencil_state_pointer;
 extern const struct brw_tracked_state gen7_disable_stages;
 extern const struct brw_tracked_state gen7_ps_state;
+extern const struct brw_tracked_state gen7_push_constant_alloc;
 extern const struct brw_tracked_state gen7_samplers;
 extern const struct brw_tracked_state gen7_sbe_state;
 extern const struct brw_tracked_state gen7_sf_clip_viewport;
diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c b/src/mesa/drivers/dri/i965/brw_state_upload.c
index c7b796c..d071f87 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -188,6 +188,7 @@ const struct brw_tracked_state *gen7_atoms[] =
 
    /* Command packets: */
    &brw_invariant_state,
+   &gen7_push_constant_alloc,
 
    /* must do before binding table pointers, cc state ptrs */
    &brw_state_base_address,
diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c b/src/mesa/drivers/dri/i965/gen7_urb.c
index e53fcb7..e6cf1eb 100644
--- a/src/mesa/drivers/dri/i965/gen7_urb.c
+++ b/src/mesa/drivers/dri/i965/gen7_urb.c
@@ -51,6 +51,30 @@
  * See "Volume 2a: 3D Pipeline," section 1.8.
  */
 static void
+gen7_allocate_push_constants(struct brw_context *brw)
+{
+   struct intel_context *intel = &brw->intel;
+   BEGIN_BATCH(2);
+   OUT_BATCH(_3DSTATE_PUSH_CONSTANT_ALLOC_VS << 16 | (2 - 2));
+   OUT_BATCH(8);
+   ADVANCE_BATCH();
+
+   BEGIN_BATCH(2);
+   OUT_BATCH(_3DSTATE_PUSH_CONSTANT_ALLOC_PS << 16 | (2 - 2));
+   OUT_BATCH(8 | 8 << GEN7_PUSH_CONSTANT_BUFFER_OFFSET_SHIFT);
+   ADVANCE_BATCH();
+}
+
+const struct brw_tracked_state gen7_push_constant_alloc = {
+   .dirty = {
+      .mesa = 0,
+      .brw = BRW_NEW_CONTEXT,
+      .cache = 0,
+   },
+   .emit = gen7_allocate_push_constants,
+};
+
+static void
 gen7_upload_urb(struct brw_context *brw)
 {
    struct intel_context *intel = &brw->intel;
@@ -76,16 +100,6 @@ gen7_upload_urb(struct brw_context *brw)
    assert(!brw->gs.prog_active);
 
    BEGIN_BATCH(2);
-   OUT_BATCH(_3DSTATE_PUSH_CONSTANT_ALLOC_VS << 16 | (2 - 2));
-   OUT_BATCH(8);
-   ADVANCE_BATCH();
-
-   BEGIN_BATCH(2);
-   OUT_BATCH(_3DSTATE_PUSH_CONSTANT_ALLOC_PS << 16 | (2 - 2));
-   OUT_BATCH(8 | 8 << GEN7_PUSH_CONSTANT_BUFFER_OFFSET_SHIFT);
-   ADVANCE_BATCH();
-
-   BEGIN_BATCH(2);
    OUT_BATCH(_3DSTATE_URB_VS << 16 | (2 - 2));
    OUT_BATCH(brw->urb.nr_vs_entries |
              ((brw->urb.vs_size - 1) << GEN7_URB_ENTRY_SIZE_SHIFT) |




More information about the mesa-commit mailing list