i965: Reduce code duplication in handling of depth, stencil, and HiZ.

This patch consolidates duplicate code in the brw_depthbuffer and
gen7_depthbuffer state atoms.  Previously, these state atoms contained
5 chunks of code for emitting the _3DSTATE_DEPTH_BUFFER packet (3 for
Gen4-6 and 2 for Gen7).  Also a lot of logic for determining the
appropriate buffer setup was duplicated between the Gen4-6 and Gen7
functions.

This refactor splits the code into three separate functions:
brw_emit_depthbuffer(), which determines the appropriate buffer setup
in a mostly generation-independent way, brw_emit_depth_stencil_hiz(),
which emits the appropriate state packets for Gen4-6, and
gen7_emit_depth_stencil_hiz(), which emits the appropriate state
packets for Gen7.

Tested using Piglit on Gen5-7 (no regressions).

v2: Re-word some comments.  Fix an assertion that incorrectly
prohibited packed depth/stencil formats on Gen6 (these are allowed
provided that HiZ is disabled).

Reviewed-by: Chad Versace <chad.versace@linux.intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

src/mesa/drivers/dri/i965/brw_context.h

@@ -1352,6 +1352,29 @@ struct opcode_desc {
 
 extern const struct opcode_desc opcode_descs[128];
 
+void
+brw_emit_depthbuffer(struct brw_context *brw);
+
+void
+brw_emit_depth_stencil_hiz(struct brw_context *brw,
+                           struct intel_mipmap_tree *depth_mt,
+                           uint32_t depth_offset, uint32_t depthbuffer_format,
+                           uint32_t depth_surface_type,
+                           struct intel_mipmap_tree *stencil_mt,
+                           struct intel_mipmap_tree *hiz_mt,
+                           bool separate_stencil, uint32_t width,
+                           uint32_t height, uint32_t tile_x, uint32_t tile_y);
+
+void
+gen7_emit_depth_stencil_hiz(struct brw_context *brw,
+                            struct intel_mipmap_tree *depth_mt,
+                            uint32_t depth_offset, uint32_t depthbuffer_format,
+                            uint32_t depth_surface_type,
+                            struct intel_mipmap_tree *stencil_mt,
+                            struct intel_mipmap_tree *hiz_mt,
+                            bool separate_stencil, uint32_t width,
+                            uint32_t height, uint32_t tile_x, uint32_t tile_y);
+
 #ifdef __cplusplus
 }
 #endif

src/mesa/drivers/dri/i965/brw_misc_state.c

@@ -561,7 +561,8 @@ brw_workaround_depthstencil_alignment(struct brw_context *brw,
    }
 }
 
-static void emit_depthbuffer(struct brw_context *brw)
+void
+brw_emit_depthbuffer(struct brw_context *brw)
 {
    struct intel_context *intel = &brw->intel;
    struct gl_context *ctx = &intel->ctx;
@@ -574,18 +575,17 @@ static void emit_depthbuffer(struct brw_context *brw)
    struct intel_mipmap_tree *hiz_mt = brw->depthstencil.hiz_mt;
    uint32_t tile_x = brw->depthstencil.tile_x;
    uint32_t tile_y = brw->depthstencil.tile_y;
-   unsigned int len;
    bool separate_stencil = false;
+   uint32_t depth_surface_type = BRW_SURFACE_NULL;
+   uint32_t depthbuffer_format = BRW_DEPTHFORMAT_D32_FLOAT;
+   uint32_t depth_offset = 0;
+   uint32_t width = 1, height = 1;
 
-   if (stencil_mt && stencil_mt->format == MESA_FORMAT_S8)
-      separate_stencil = true;
+   if (stencil_mt) {
+      separate_stencil = stencil_mt->format == MESA_FORMAT_S8;
 
-   /* 3DSTATE_DEPTH_BUFFER, 3DSTATE_STENCIL_BUFFER are both
-    * non-pipelined state that will need the PIPE_CONTROL workaround.
-    */
-   if (intel->gen == 6) {
-      intel_emit_post_sync_nonzero_flush(intel);
-      intel_emit_depth_stall_flushes(intel);
+      /* Gen7 supports only separate stencil */
+      assert(separate_stencil || intel->gen < 7);
    }
 
    /* If there's a packed depth/stencil bound to stencil only, we need to
@@ -596,31 +596,33 @@ static void emit_depthbuffer(struct brw_context *brw)
       depth_mt = stencil_mt;
    }
 
-   if (intel->gen >= 6)
-      len = 7;
-   else if (intel->is_g4x || intel->gen == 5)
-      len = 6;
-   else
-      len = 5;
-
-   if (!depth_irb && !separate_stencil) {
-      BEGIN_BATCH(len);
-      OUT_BATCH(_3DSTATE_DEPTH_BUFFER << 16 | (len - 2));
-      OUT_BATCH((BRW_DEPTHFORMAT_D32_FLOAT << 18) |
-		(BRW_SURFACE_NULL << 29));
-      OUT_BATCH(0);
-      OUT_BATCH(0);
-      OUT_BATCH(0);
+   if (depth_irb) {
+      struct intel_region *region = depth_mt->region;
 
-      if (intel->is_g4x || intel->gen >= 5)
-         OUT_BATCH(0);
+      /* When 3DSTATE_DEPTH_BUFFER.Separate_Stencil_Enable is set, then
+       * 3DSTATE_DEPTH_BUFFER.Surface_Format is not permitted to be a packed
+       * depthstencil format.
+       *
+       * Gens prior to 7 require that HiZ_Enable and Separate_Stencil_Enable be
+       * set to the same value. Gens after 7 implicitly always set
+       * Separate_Stencil_Enable; software cannot disable it.
+       */
+      if ((intel->gen < 7 && depth_mt->hiz_mt) || intel->gen >= 7) {
+         assert(!_mesa_is_format_packed_depth_stencil(depth_mt->format));
+      }
 
-      if (intel->gen >= 6)
-	 OUT_BATCH(0);
+      /* Prior to Gen7, if using separate stencil, hiz must be enabled. */
+      assert(intel->gen >= 7 || !separate_stencil || hiz_mt);
 
-      ADVANCE_BATCH();
+      assert(intel->gen < 6 || region->tiling == I915_TILING_Y);
+      assert(!hiz_mt || region->tiling == I915_TILING_Y);
 
-   } else if (!depth_irb && separate_stencil) {
+      depthbuffer_format = brw_depthbuffer_format(brw);
+      depth_surface_type = BRW_SURFACE_2D;
+      depth_offset = brw->depthstencil.depth_offset;
+      width = depth_irb->Base.Base.Width;
+      height = depth_irb->Base.Base.Height;
+   } else if (separate_stencil) {
       /*
        * There exists a separate stencil buffer but no depth buffer.
        *
@@ -628,80 +630,95 @@ static void emit_depthbuffer(struct brw_context *brw)
        * 3DSTATE_DEPTH_BUFFER: namely the tile walk, surface type, width, and
        * height.
        *
-       * Enable the hiz bit because it and the separate stencil bit must have
-       * the same value. From Section 2.11.5.6.1.1 3DSTATE_DEPTH_BUFFER, Bit
-       * 1.21 "Separate Stencil Enable":
-       *     [DevIL]: If this field is enabled, Hierarchical Depth Buffer
-       *     Enable must also be enabled.
-       *
-       *     [DevGT]: This field must be set to the same value (enabled or
-       *     disabled) as Hierarchical Depth Buffer Enable
-       *
        * The tiled bit must be set. From the Sandybridge PRM, Volume 2, Part 1,
        * Section 7.5.5.1.1 3DSTATE_DEPTH_BUFFER, Bit 1.27 Tiled Surface:
        *     [DevGT+]: This field must be set to TRUE.
        */
       assert(intel->has_separate_stencil);
 
-      BEGIN_BATCH(len);
-      OUT_BATCH(_3DSTATE_DEPTH_BUFFER << 16 | (len - 2));
-      OUT_BATCH((BRW_DEPTHFORMAT_D32_FLOAT << 18) |
-	        (1 << 21) | /* separate stencil enable */
-	        (1 << 22) | /* hiz enable */
-	        (BRW_TILEWALK_YMAJOR << 26) |
-	        (1 << 27) | /* tiled surface */
-	        (BRW_SURFACE_2D << 29));
-      OUT_BATCH(0);
-      OUT_BATCH(((stencil_irb->Base.Base.Width + tile_x - 1) << 6) |
-	         (stencil_irb->Base.Base.Height + tile_y - 1) << 19);
-      OUT_BATCH(0);
+      depth_surface_type = BRW_SURFACE_2D;
+      width = stencil_irb->Base.Base.Width;
+      height = stencil_irb->Base.Base.Height;
+   }
 
-      if (intel->is_g4x || intel->gen >= 5)
-         OUT_BATCH(tile_x | (tile_y << 16));
-      else
-	 assert(tile_x == 0 && tile_y == 0);
+   intel->vtbl.emit_depth_stencil_hiz(brw, depth_mt, depth_offset,
+                                      depthbuffer_format, depth_surface_type,
+                                      stencil_mt, hiz_mt, separate_stencil,
+                                      width, height, tile_x, tile_y);
+}
 
-      if (intel->gen >= 6)
-	 OUT_BATCH(0);
+void
+brw_emit_depth_stencil_hiz(struct brw_context *brw,
+                           struct intel_mipmap_tree *depth_mt,
+                           uint32_t depth_offset, uint32_t depthbuffer_format,
+                           uint32_t depth_surface_type,
+                           struct intel_mipmap_tree *stencil_mt,
+                           struct intel_mipmap_tree *hiz_mt,
+                           bool separate_stencil, uint32_t width,
+                           uint32_t height, uint32_t tile_x, uint32_t tile_y)
+{
+   struct intel_context *intel = &brw->intel;
 
-      ADVANCE_BATCH();
+   /* Enable the hiz bit if we're doing separate stencil, because it and the
+    * separate stencil bit must have the same value. From Section 2.11.5.6.1.1
+    * 3DSTATE_DEPTH_BUFFER, Bit 1.21 "Separate Stencil Enable":
+    *     [DevIL]: If this field is enabled, Hierarchical Depth Buffer
+    *     Enable must also be enabled.
+    *
+    *     [DevGT]: This field must be set to the same value (enabled or
+    *     disabled) as Hierarchical Depth Buffer Enable
+    */
+   bool enable_hiz_ss = hiz_mt || separate_stencil;
 
-   } else {
-      struct intel_region *region = depth_mt->region;
 
-      /* If using separate stencil, hiz must be enabled. */
-      assert(!separate_stencil || hiz_mt);
+   /* 3DSTATE_DEPTH_BUFFER, 3DSTATE_STENCIL_BUFFER are both
+    * non-pipelined state that will need the PIPE_CONTROL workaround.
+    */
+   if (intel->gen == 6) {
+      intel_emit_post_sync_nonzero_flush(intel);
+      intel_emit_depth_stall_flushes(intel);
+   }
 
-      assert(intel->gen < 6 || region->tiling == I915_TILING_Y);
-      assert(!hiz_mt || region->tiling == I915_TILING_Y);
+   unsigned int len;
+   if (intel->gen >= 6)
+      len = 7;
+   else if (intel->is_g4x || intel->gen == 5)
+      len = 6;
+   else
+      len = 5;
 
-      BEGIN_BATCH(len);
-      OUT_BATCH(_3DSTATE_DEPTH_BUFFER << 16 | (len - 2));
-      OUT_BATCH((region->pitch - 1) |
-		(brw_depthbuffer_format(brw) << 18) |
-		((hiz_mt ? 1 : 0) << 21) | /* separate stencil enable */
-		((hiz_mt ? 1 : 0) << 22) | /* hiz enable */
-		(BRW_TILEWALK_YMAJOR << 26) |
-		((region->tiling != I915_TILING_NONE) << 27) |
-		(BRW_SURFACE_2D << 29));
-      OUT_RELOC(region->bo,
+   BEGIN_BATCH(len);
+   OUT_BATCH(_3DSTATE_DEPTH_BUFFER << 16 | (len - 2));
+   OUT_BATCH((depth_mt ? depth_mt->region->pitch - 1 : 0) |
+             (depthbuffer_format << 18) |
+             ((enable_hiz_ss ? 1 : 0) << 21) | /* separate stencil enable */
+             ((enable_hiz_ss ? 1 : 0) << 22) | /* hiz enable */
+             (BRW_TILEWALK_YMAJOR << 26) |
+             ((depth_mt ? depth_mt->region->tiling != I915_TILING_NONE : 1)
+              << 27) |
+             (depth_surface_type << 29));
+
+   if (depth_mt) {
+      OUT_RELOC(depth_mt->region->bo,
 		I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
-		brw->depthstencil.depth_offset);
-      OUT_BATCH((BRW_SURFACE_MIPMAPLAYOUT_BELOW << 1) |
-		(((depth_irb->Base.Base.Width + tile_x) - 1) << 6) |
-		(((depth_irb->Base.Base.Height + tile_y) - 1) << 19));
+		depth_offset);
+   } else {
       OUT_BATCH(0);
+   }
 
-      if (intel->is_g4x || intel->gen >= 5)
-         OUT_BATCH(tile_x | (tile_y << 16));
-      else
-	 assert(tile_x == 0 && tile_y == 0);
+   OUT_BATCH(((width + tile_x - 1) << 6) |
+             ((height + tile_y - 1) << 19));
+   OUT_BATCH(0);
 
-      if (intel->gen >= 6)
-	 OUT_BATCH(0);
+   if (intel->is_g4x || intel->gen >= 5)
+      OUT_BATCH(tile_x | (tile_y << 16));
+   else
+      assert(tile_x == 0 && tile_y == 0);
 
-      ADVANCE_BATCH();
-   }
+   if (intel->gen >= 6)
+      OUT_BATCH(0);
+
+   ADVANCE_BATCH();
 
    if (hiz_mt || separate_stencil) {
       /*
@@ -770,7 +787,7 @@ static void emit_depthbuffer(struct brw_context *brw)
       OUT_BATCH(_3DSTATE_CLEAR_PARAMS << 16 |
 		GEN5_DEPTH_CLEAR_VALID |
 		(2 - 2));
-      OUT_BATCH(depth_irb ? depth_irb->mt->depth_clear_value : 0);
+      OUT_BATCH(depth_mt ? depth_mt->depth_clear_value : 0);
       ADVANCE_BATCH();
    }
 }
@@ -781,7 +798,7 @@ const struct brw_tracked_state brw_depthbuffer = {
       .brw = BRW_NEW_BATCH,
       .cache = 0,
    },
-   .emit = emit_depthbuffer,
+   .emit = brw_emit_depthbuffer,
 };
 
 

src/mesa/drivers/dri/i965/brw_vtbl.c

@@ -267,7 +267,9 @@ void brwInitVtbl( struct brw_context *brw )
    assert(brw->intel.gen >= 4);
    if (brw->intel.gen >= 7) {
       gen7_init_vtable_surface_functions(brw);
+      brw->intel.vtbl.emit_depth_stencil_hiz = gen7_emit_depth_stencil_hiz;
    } else if (brw->intel.gen >= 4) {
       gen4_init_vtable_surface_functions(brw);
+      brw->intel.vtbl.emit_depth_stencil_hiz = brw_emit_depth_stencil_hiz;
    }
 }

src/mesa/drivers/dri/i965/gen7_misc_state.c

@@ -29,77 +29,46 @@
 #include "brw_state.h"
 #include "brw_defines.h"
 
-static void emit_depthbuffer(struct brw_context *brw)
+void
+gen7_emit_depth_stencil_hiz(struct brw_context *brw,
+                            struct intel_mipmap_tree *depth_mt,
+                            uint32_t depth_offset, uint32_t depthbuffer_format,
+                            uint32_t depth_surface_type,
+                            struct intel_mipmap_tree *stencil_mt,
+                            struct intel_mipmap_tree *hiz_mt,
+                            bool separate_stencil, uint32_t width,
+                            uint32_t height, uint32_t tile_x, uint32_t tile_y)
 {
    struct intel_context *intel = &brw->intel;
    struct gl_context *ctx = &intel->ctx;
-   struct gl_framebuffer *fb = ctx->DrawBuffer;
-
-   /* _NEW_BUFFERS */
-   struct intel_renderbuffer *drb = intel_get_renderbuffer(fb, BUFFER_DEPTH);
-   struct intel_renderbuffer *srb = intel_get_renderbuffer(fb, BUFFER_STENCIL);
-   struct intel_mipmap_tree *depth_mt = brw->depthstencil.depth_mt;
-   struct intel_mipmap_tree *stencil_mt = brw->depthstencil.stencil_mt;
-   struct intel_mipmap_tree *hiz_mt = brw->depthstencil.hiz_mt;
-   uint32_t tile_x = brw->depthstencil.tile_x;
-   uint32_t tile_y = brw->depthstencil.tile_y;
-
-   /* Gen7 only supports separate stencil */
-   assert(!stencil_mt || stencil_mt->format == MESA_FORMAT_S8);
-   assert(!depth_mt || !_mesa_is_format_packed_depth_stencil(depth_mt->format));
 
    intel_emit_depth_stall_flushes(intel);
 
-   if (depth_mt == NULL) {
-      uint32_t dw1 = BRW_DEPTHFORMAT_D32_FLOAT << 18;
-      uint32_t dw3 = 0;
-
-      if (stencil_mt == NULL) {
-	 dw1 |= (BRW_SURFACE_NULL << 29);
-      } else {
-	 /* _NEW_STENCIL: enable stencil buffer writes */
-	 dw1 |= ((ctx->Stencil.WriteMask != 0) << 27);
-
-	 /* 3DSTATE_STENCIL_BUFFER inherits surface type and dimensions. */
-	 dw1 |= (BRW_SURFACE_2D << 29);
-	 dw3 = ((srb->Base.Base.Width + tile_x - 1) << 4) |
-	       ((srb->Base.Base.Height + tile_y - 1) << 18);
-      }
-
-      BEGIN_BATCH(7);
-      OUT_BATCH(GEN7_3DSTATE_DEPTH_BUFFER << 16 | (7 - 2));
-      OUT_BATCH(dw1);
-      OUT_BATCH(0);
-      OUT_BATCH(dw3);
-      OUT_BATCH(0);
-      OUT_BATCH(tile_x | (tile_y << 16));
-      OUT_BATCH(0);
-      ADVANCE_BATCH();
-   } else {
-      struct intel_region *region = depth_mt->region;
-
-      assert(region->tiling == I915_TILING_Y);
-
-      /* _NEW_DEPTH, _NEW_STENCIL */
-      BEGIN_BATCH(7);
-      OUT_BATCH(GEN7_3DSTATE_DEPTH_BUFFER << 16 | (7 - 2));
-      OUT_BATCH((region->pitch - 1) |
-		(brw_depthbuffer_format(brw) << 18) |
-		((hiz_mt ? 1 : 0) << 22) | /* hiz enable */
-		((stencil_mt != NULL && ctx->Stencil.WriteMask != 0) << 27) |
-		((ctx->Depth.Mask != 0) << 28) |
-		(BRW_SURFACE_2D << 29));
-      OUT_RELOC(region->bo,
+   /* _NEW_DEPTH, _NEW_STENCIL */
+   BEGIN_BATCH(7);
+   OUT_BATCH(GEN7_3DSTATE_DEPTH_BUFFER << 16 | (7 - 2));
+   OUT_BATCH((depth_mt ? depth_mt->region->pitch - 1 : 0) |
+             (depthbuffer_format << 18) |
+             ((hiz_mt ? 1 : 0) << 22) |
+             ((stencil_mt != NULL && ctx->Stencil.WriteMask != 0) << 27) |
+             ((ctx->Depth.Mask != 0) << 28) |
+             (depth_surface_type << 29));
+
+   if (depth_mt) {
+      OUT_RELOC(depth_mt->region->bo,
 	        I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
-		brw->depthstencil.depth_offset);
-      OUT_BATCH((((drb->Base.Base.Width + tile_x) - 1) << 4) |
-                (((drb->Base.Base.Height + tile_y) - 1) << 18));
-      OUT_BATCH(0);
-      OUT_BATCH(tile_x | (tile_y << 16));
+		depth_offset);
+   } else {
       OUT_BATCH(0);
-      ADVANCE_BATCH();
    }
 
+   OUT_BATCH(((width + tile_x - 1) << 4) |
+             ((height + tile_y - 1) << 18));
+   OUT_BATCH(0);
+   OUT_BATCH(tile_x | (tile_y << 16));
+   OUT_BATCH(0);
+   ADVANCE_BATCH();
+
    if (hiz_mt == NULL) {
       BEGIN_BATCH(3);
       OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (3 - 2));
@@ -166,5 +135,5 @@ const struct brw_tracked_state gen7_depthbuffer = {
       .brw = BRW_NEW_BATCH,
       .cache = 0,
    },
-   .emit = emit_depthbuffer,
+   .emit = brw_emit_depthbuffer,
 };

src/mesa/drivers/dri/intel/intel_context.h

@@ -206,6 +206,22 @@ struct intel_context
 				      uint32_t *out_offset,
                                       bool dword_pitch);
       /** \} */
+
+      /**
+       * Send the appropriate state packets to configure depth, stencil, and
+       * HiZ buffers (i965+ only)
+       */
+      void (*emit_depth_stencil_hiz)(struct brw_context *brw,
+                                     struct intel_mipmap_tree *depth_mt,
+                                     uint32_t depth_offset,
+                                     uint32_t depthbuffer_format,
+                                     uint32_t depth_surface_type,
+                                     struct intel_mipmap_tree *stencil_mt,
+                                     struct intel_mipmap_tree *hiz_mt,
+                                     bool separate_stencil,
+                                     uint32_t width, uint32_t height,
+                                     uint32_t tile_x, uint32_t tile_y);
+
    } vtbl;
 
    GLbitfield Fallback;  /**< mask of INTEL_FALLBACK_x bits */