[Mesa-dev] [PATCH] [v2] i965: Split out gen8 push constant state upload

Ben Widawsky benjamin.widawsky at intel.com
Thu Jul 9 09:44:52 PDT 2015


While implementing the workaround in the previous patch I noticed things were
starting to get a bit messy. Since gen8 works differently enough from gen7, I
thought splitting it out with be good.

While here, get rid of gen8 MOCS which does nothing and was in the wrong place
anyway.

This patch is totally optional. I'd be willing to just always use buffer #2 on
gen8+. Pre-HSW this wasn't allowed, but it looks like it's okay for gen8 too.

v2: Move inactive batch generation to the top of the function in order to make
the rest of the code easier to read.

Jenkins results (still a bunch of spurious failures, I miss Mark):
http://otc-mesa-ci.jf.intel.com/job/bwidawsk/169/

Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com> (v1)
---

I had a minor bug in v1 which prevented me from pushing this sooner. I'd like to
merge this patch unless anyone has complaints?

---
 src/mesa/drivers/dri/i965/brw_state.h     |  6 +-
 src/mesa/drivers/dri/i965/gen6_gs_state.c |  2 +-
 src/mesa/drivers/dri/i965/gen6_vs_state.c |  3 +-
 src/mesa/drivers/dri/i965/gen6_wm_state.c |  3 +-
 src/mesa/drivers/dri/i965/gen7_vs_state.c | 93 ++++++++++++++++++++-----------
 5 files changed, 68 insertions(+), 39 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h
index 987672f..f45459d 100644
--- a/src/mesa/drivers/dri/i965/brw_state.h
+++ b/src/mesa/drivers/dri/i965/brw_state.h
@@ -368,9 +368,9 @@ brw_upload_pull_constants(struct brw_context *brw,
 
 /* gen7_vs_state.c */
 void
-gen7_upload_constant_state(struct brw_context *brw,
-                           const struct brw_stage_state *stage_state,
-                           bool active, unsigned opcode);
+brw_upload_constant_state(struct brw_context *brw,
+                          const struct brw_stage_state *stage_state,
+                          bool active, unsigned opcode);
 
 #ifdef __cplusplus
 }
diff --git a/src/mesa/drivers/dri/i965/gen6_gs_state.c b/src/mesa/drivers/dri/i965/gen6_gs_state.c
index eb4c586..19568b0 100644
--- a/src/mesa/drivers/dri/i965/gen6_gs_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_gs_state.c
@@ -48,7 +48,7 @@ gen6_upload_gs_push_constants(struct brw_context *brw)
    }
 
    if (brw->gen >= 7)
-      gen7_upload_constant_state(brw, stage_state, gp, _3DSTATE_CONSTANT_GS);
+      brw_upload_constant_state(brw, stage_state, gp, _3DSTATE_CONSTANT_GS);
 }
 
 const struct brw_tracked_state gen6_gs_push_constants = {
diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c b/src/mesa/drivers/dri/i965/gen6_vs_state.c
index 35d10ef..c33607d 100644
--- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
@@ -140,8 +140,7 @@ gen6_upload_vs_push_constants(struct brw_context *brw)
       if (brw->gen == 7 && !brw->is_haswell && !brw->is_baytrail)
          gen7_emit_vs_workaround_flush(brw);
 
-      gen7_upload_constant_state(brw, stage_state, true /* active */,
-                                 _3DSTATE_CONSTANT_VS);
+      brw_upload_constant_state(brw, stage_state, true, _3DSTATE_CONSTANT_VS);
    }
 }
 
diff --git a/src/mesa/drivers/dri/i965/gen6_wm_state.c b/src/mesa/drivers/dri/i965/gen6_wm_state.c
index d1748ba..ced4ad6 100644
--- a/src/mesa/drivers/dri/i965/gen6_wm_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_wm_state.c
@@ -50,8 +50,7 @@ gen6_upload_wm_push_constants(struct brw_context *brw)
                               stage_state, AUB_TRACE_WM_CONSTANTS);
 
    if (brw->gen >= 7) {
-      gen7_upload_constant_state(brw, &brw->wm.base, true,
-                                 _3DSTATE_CONSTANT_PS);
+      brw_upload_constant_state(brw, &brw->wm.base, true, _3DSTATE_CONSTANT_PS);
    }
 }
 
diff --git a/src/mesa/drivers/dri/i965/gen7_vs_state.c b/src/mesa/drivers/dri/i965/gen7_vs_state.c
index 4b17d06..6a51934 100644
--- a/src/mesa/drivers/dri/i965/gen7_vs_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_vs_state.c
@@ -29,20 +29,23 @@
 #include "program/prog_statevars.h"
 #include "intel_batchbuffer.h"
 
-
-void
-gen7_upload_constant_state(struct brw_context *brw,
+static void
+gen8_upload_constant_state(struct brw_context *brw,
                            const struct brw_stage_state *stage_state,
                            bool active, unsigned opcode)
 {
-   uint32_t mocs = brw->gen < 8 ? GEN7_MOCS_L3 : 0;
 
-   /* Disable if the shader stage is inactive or there are no push constants. */
-   active = active && stage_state->push_const_size != 0;
+   /* FINISHME: determine if we should use mocs on gen9 */
 
-   int dwords = brw->gen >= 8 ? 11 : 7;
-   BEGIN_BATCH(dwords);
-   OUT_BATCH(opcode << 16 | (dwords - 2));
+   BEGIN_BATCH(11);
+   OUT_BATCH(opcode << 16 | (11 - 2));
+
+   if (!active) {
+      for (int i = 0; i < 11; i++)
+         OUT_BATCH(0);
+
+      return;
+   }
 
    /* Workaround for SKL+ (we use option #2 until we have a need for more
     * constant buffers). This comes from the documentation for 3DSTATE_CONSTANT_*
@@ -55,44 +58,42 @@ gen7_upload_constant_state(struct brw_context *brw,
     *     1. always force buffer 3 to have a non zero read length
     *     2. always force buffer 0 to a zero read length
     */
-   if (brw->gen >= 9 && active) {
+   if (brw->gen >= 9) {
       OUT_BATCH(0);
-      OUT_BATCH(stage_state->push_const_size);
+      OUT_BATCH(stage_state->push_const_size); /* buffer 3, 2 */
    } else {
-      OUT_BATCH(active ? stage_state->push_const_size : 0);
+      OUT_BATCH(active ? stage_state->push_const_size : 0); /* buffer 1, 0 */
       OUT_BATCH(0);
    }
-   /* Pointer to the constant buffer.  Covered by the set of state flags
-    * from gen6_prepare_wm_contants
-    */
-   if (brw->gen >= 9 && active) {
-      OUT_BATCH(0);
+
+   /* Push constant buffer #0 */
+   if (brw->gen >= 9) {
       OUT_BATCH(0);
       OUT_BATCH(0);
+   } else {
+      OUT_BATCH(stage_state->push_const_offset);
       OUT_BATCH(0);
+   }
+
+   /* Push constant buffer #1 */
+   OUT_BATCH(0);
+   OUT_BATCH(0);
+
+   /* Push constant buffer #2 */
+   if (brw->gen >= 9) {
       /* XXX: When using buffers other than 0, you need to specify the
-       * graphics virtual address regardless of INSPM/debug bits
+       * graphics virtual address regardless of INSTPM/debug bits
        */
       OUT_RELOC64(brw->batch.bo, I915_GEM_DOMAIN_RENDER, 0,
                   stage_state->push_const_offset);
-      OUT_BATCH(0);
-      OUT_BATCH(0);
-   } else if (brw->gen>= 8) {
-      OUT_BATCH(active ? (stage_state->push_const_offset | mocs) : 0);
-      OUT_BATCH(0);
-      OUT_BATCH(0);
-      OUT_BATCH(0);
-      OUT_BATCH(0);
-      OUT_BATCH(0);
-      OUT_BATCH(0);
-      OUT_BATCH(0);
    } else {
-      OUT_BATCH(active ? (stage_state->push_const_offset | mocs) : 0);
-      OUT_BATCH(0);
       OUT_BATCH(0);
       OUT_BATCH(0);
    }
 
+   /* Push constant buffer #3 */
+   OUT_BATCH(0);
+   OUT_BATCH(0);
    ADVANCE_BATCH();
 
   /* On SKL+ the new constants don't take effect until the next corresponding
@@ -103,6 +104,36 @@ gen7_upload_constant_state(struct brw_context *brw,
       brw->ctx.NewDriverState |= BRW_NEW_SURFACES;
 }
 
+static void
+gen7_upload_constant_state(struct brw_context *brw,
+                           const struct brw_stage_state *stage_state,
+                           bool active, unsigned opcode)
+{
+   BEGIN_BATCH(7);
+   OUT_BATCH(opcode << 16 | (7 - 2));
+   OUT_BATCH(active ? stage_state->push_const_size : 0);
+   OUT_BATCH(0);
+   OUT_BATCH(active ? (stage_state->push_const_offset | GEN7_MOCS_L3) : 0);
+   OUT_BATCH(0);
+   OUT_BATCH(0);
+   OUT_BATCH(0);
+   ADVANCE_BATCH();
+}
+
+void
+brw_upload_constant_state(struct brw_context *brw,
+                          const struct brw_stage_state *stage_state,
+                          bool active, unsigned opcode)
+{
+   /* Disable if the shader stage is inactive or there are no push constants. */
+   active = active && stage_state->push_const_size != 0;
+
+   if (brw->gen >= 8)
+      gen8_upload_constant_state(brw, stage_state, active, opcode);
+   else
+      gen7_upload_constant_state(brw, stage_state, active, opcode);
+}
+
 
 static void
 upload_vs_state(struct brw_context *brw)
-- 
2.4.4



More information about the mesa-dev mailing list