i965: Disable depth writes when depth test is GL_EQUAL.

There's no point in performing depth writes when the depth test
comparison function is set to GL_EQUAL - it would just write out
the same value that's already there (if it is written at all).  While
this is harmless from a functional perspective, it hurts performance.
Obviously, writing to memory is not free, but there's another more
subtle impact as well: it can prevent early depth optimizations.

Depth writes aren't supposed to happen for pixels that are killed
by fragment shader discard statements or the alpha test.  So, with
depth writes enabled and either of those, the pixel shader must be
invoked to determine whether or not to perform the write.  This is
fairly stupid in the EQUAL case - we're running a shader to decide
whether to replace the existing depth value with itself.

By disabling these pointless writes, we allow early depth even with
discards and alpha testing, allowing the hardware to skip the pixel
shader altogether if the depth test fails.

Improves performance of Unigine Valley:

- Skylake GT2:    +17.8%
- Broadwell GT3e: +11.5%
- Cherrytrail:    +19.4%

Huge thanks to Mark Janes for building frameretrace [1], the performance
analysis tool that helped us find this issue, and to Robert Bragg for
providing us performance metrics on Linux.  Mark also spent the time to
analyze Valley performance on Windows vs. Linux and discovered a
discrepancy in early depth test metrics.  Once he had isolated a draw
call and drawn attention to the problem, fixing it was pretty simple.

[1] https://github.com/janesma/apitrace/wiki/frameretrace-branch

Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>

diff --git a/src/mesa/drivers/dri/i965/brw_cc.c b/src/mesa/drivers/dri/i965/brw_cc.c
index b11d7c85ca9e51625fec617e331b41fe05d41298..c3b00e1b605efd5c2fdcf7346a47fa5bd4b6fd8a 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_cc.c
+++ b/src/mesa/drivers/dri/i965/brw_cc.c
@@ -225,7 +225,7 @@ static void upload_cc_unit(struct brw_context *brw)
       cc->cc2.depth_test = 1;
       cc->cc2.depth_test_function =
         intel_translate_compare_func(ctx->Depth.Func);
-      cc->cc2.depth_write_enable = ctx->Depth.Mask;
+      cc->cc2.depth_write_enable = brw_depth_writes_enabled(brw);
    }
 
    if (brw->stats_wm || unlikely(INTEL_DEBUG & DEBUG_STATS))

diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index be59e3b0d16549d258b08cabe313e9b7c5fa67bb..550eefedcc29433c0223a42a888da44fae3a6716 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1713,6 +1713,26 @@ bool brw_lower_texture_gradients(struct brw_context *brw,
 extern const char * const conditional_modifier[16];
 extern const char *const pred_ctrl_align16[16];
 
+static inline bool
+brw_depth_writes_enabled(const struct brw_context *brw)
+{
+   const struct gl_context *ctx = &brw->ctx;
+
+   /* We consider depth writes disabled if the depth function is GL_EQUAL,
+    * because it would just overwrite the existing depth value with itself.
+    *
+    * These bonus depth writes not only use bandwidth, but they also can
+    * prevent early depth processing.  For example, if the pixel shader
+    * discards, the hardware must invoke the to determine whether or not
+    * to do the depth write.  If writes are disabled, we may still be able
+    * to do the depth test before the shader, and skip the shader execution.
+    *
+    * The Broadwell 3DSTATE_WM_DEPTH_STENCIL documentation also contains
+    * a programming note saying to disable depth writes for EQUAL.
+    */
+   return ctx->Depth.Test && ctx->Depth.Mask && ctx->Depth.Func != GL_EQUAL;
+}
+
 void
 brw_emit_depthbuffer(struct brw_context *brw);
 

diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c
index b09302006b2838347d547cb01508237a1c77962a..7904ef5c81869c0cea523aa31a9664a8c7657b82 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -372,7 +372,7 @@ brw_postdraw_set_buffers_need_resolve(struct brw_context *brw)
       front_irb->need_downsample = true;
    if (back_irb)
       back_irb->need_downsample = true;
-   if (depth_irb && ctx->Depth.Mask) {
+   if (depth_irb && brw_depth_writes_enabled(brw)) {
       intel_renderbuffer_att_set_needs_depth_resolve(depth_att);
       brw_render_cache_set_add_bo(brw, depth_irb->mt->bo);
    }

diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
index e6f68c46f8b9a94ae3b5fac1e27345fc31a95a9b..dab2e3305836b31d0dc86a3de34c5c14da8a9a0b 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_wm.c
+++ b/src/mesa/drivers/dri/i965/brw_wm.c
@@ -460,7 +460,7 @@ brw_wm_populate_key(struct brw_context *brw, struct brw_wm_prog_key *key)
       if (ctx->Depth.Test)
          lookup |= IZ_DEPTH_TEST_ENABLE_BIT;
 
-      if (ctx->Depth.Test && ctx->Depth.Mask) /* ?? */
+      if (brw_depth_writes_enabled(brw))
          lookup |= IZ_DEPTH_WRITE_ENABLE_BIT;
 
       /* _NEW_STENCIL | _NEW_BUFFERS */

diff --git a/src/mesa/drivers/dri/i965/gen6_depthstencil.c b/src/mesa/drivers/dri/i965/gen6_depthstencil.c
index a3de8448336816a29db6dccdd68f156648b63c4a..79d4d5da162a571e80b6f243a12919dbb34f945b 100644 (file)
--- a/src/mesa/drivers/dri/i965/gen6_depthstencil.c
+++ b/src/mesa/drivers/dri/i965/gen6_depthstencil.c
@@ -83,7 +83,7 @@ gen6_upload_depth_stencil_state(struct brw_context *brw)
    if (ctx->Depth.Test && depth_irb) {
       ds->ds2.depth_test_enable = ctx->Depth.Test;
       ds->ds2.depth_test_func = intel_translate_compare_func(ctx->Depth.Func);
-      ds->ds2.depth_write_enable = ctx->Depth.Mask;
+      ds->ds2.depth_write_enable = brw_depth_writes_enabled(brw);
    }
 
    /* Point the GPU at the new indirect state. */

diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c b/src/mesa/drivers/dri/i965/gen7_misc_state.c
index 7bd5cd5c0534d6565b95445dd0793eb134dfc354..af9be66ff8e8431a61c5a92a102fc9e80cea56f3 100644 (file)
--- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
@@ -109,7 +109,7 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
              (depthbuffer_format << 18) |
              ((hiz ? 1 : 0) << 22) |
              ((stencil_mt != NULL && ctx->Stencil._WriteEnabled) << 27) |
-             ((ctx->Depth.Mask != 0) << 28) |
+             (brw_depth_writes_enabled(brw) << 28) |
              (surftype << 29));
 
    /* 3DSTATE_DEPTH_BUFFER dw2 */

diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c b/src/mesa/drivers/dri/i965/gen8_depth_state.c
index 3604aeef1df991a5162620b53f89f02d6384c45c..14689f400fb7c43bd155de4e7a0b0d57d4030061 100644 (file)
--- a/src/mesa/drivers/dri/i965/gen8_depth_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c
@@ -218,7 +218,7 @@ gen8_emit_depth_stencil_hiz(struct brw_context *brw,
    }
 
    emit_depth_packets(brw, depth_mt, brw_depthbuffer_format(brw), surftype,
-                      ctx->Depth.Mask != 0,
+                      brw_depth_writes_enabled(brw),
                       stencil_mt, ctx->Stencil._WriteEnabled,
                       hiz, width, height, depth, lod, min_array_element);
 }
@@ -280,7 +280,7 @@ pma_fix_enable(const struct brw_context *brw)
     * 3DSTATE_WM_DEPTH_STENCIL::DepthWriteEnable &&
     * 3DSTATE_DEPTH_BUFFER::DEPTH_WRITE_ENABLE.
     */
-   const bool depth_writes_enabled = ctx->Depth.Mask;
+   const bool depth_writes_enabled = brw_depth_writes_enabled(brw);
 
    /* _NEW_STENCIL:
     * !DEPTH_STENCIL_STATE::Stencil Buffer Write Enable ||

diff --git a/src/mesa/drivers/dri/i965/gen8_wm_depth_stencil.c b/src/mesa/drivers/dri/i965/gen8_wm_depth_stencil.c
index e49103c7cb5d4bc549ccbe138add4c787b23fc5c..9a6c9e060bb2ec268f380b13642e6ee6dd2ef1f2 100644 (file)
--- a/src/mesa/drivers/dri/i965/gen8_wm_depth_stencil.c
+++ b/src/mesa/drivers/dri/i965/gen8_wm_depth_stencil.c
@@ -90,7 +90,7 @@ gen8_upload_wm_depth_stencil(struct brw_context *brw)
          GEN8_WM_DS_DEPTH_TEST_ENABLE |
          FUNC(ctx->Depth.Func) << GEN8_WM_DS_DEPTH_FUNC_SHIFT;
 
-      if (ctx->Depth.Mask)
+      if (brw_depth_writes_enabled(brw))
          dw1 |= GEN8_WM_DS_DEPTH_BUFFER_WRITE_ENABLE;
    }