On 09/26/2011 01:15 PM, Eric Anholt wrote:
On Fri, 23 Sep 2011 17:37:46 -0700, Chad Versace<c...@chad-versace.us>  wrote:

That's a pretty terse commit message :)

Signed-off-by: Chad Versace<c...@chad-versace.us>
---
  src/mesa/drivers/dri/i965/brw_draw.c          |    3 ++
  src/mesa/drivers/dri/i965/gen6_clip_state.c   |   17 +++++++++++++++
  src/mesa/drivers/dri/i965/gen6_depthstencil.c |   22 +++++++++++++++++-
  src/mesa/drivers/dri/i965/gen6_sf_state.c     |   16 ++++++++++++-
  src/mesa/drivers/dri/i965/gen6_wm_state.c     |   28 +++++++++++++++++++++---
  5 files changed, 78 insertions(+), 8 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/gen6_depthstencil.c 
b/src/mesa/drivers/dri/i965/gen6_depthstencil.c
index 5d14147..59f3fe4 100644
--- a/src/mesa/drivers/dri/i965/gen6_depthstencil.c
+++ b/src/mesa/drivers/dri/i965/gen6_depthstencil.c
@@ -77,10 +77,28 @@ gen6_prepare_depth_stencil_state(struct brw_context *brw)
     }

     /* _NEW_DEPTH */
-   if (ctx->Depth.Test) {
-      ds->ds2.depth_test_enable = 1;
+   if (ctx->Depth.Test || brw->hiz.op != 0) {
        ds->ds2.depth_test_func = intel_translate_compare_func(ctx->Depth.Func);
        ds->ds2.depth_write_enable = ctx->Depth.Mask;
+
+      /* See the following sections of the Sandy Bridge PRM, Volume 1, Part2:
+       *   - 7.5.3.1 Depth Buffer Clear
+       *   - 7.5.3.2 Depth Buffer Resolve
+       *   - 7.5.3.3 Hierarchical Depth Buffer Resolve
+       */
+      switch (brw->hiz.op) {
+      case BRW_HIZ_OP_NONE:
+      case BRW_HIZ_OP_DEPTH_RESOLVE:
+         ds->ds2.depth_test_enable = 1;
+         break;
+      case BRW_HIZ_OP_DEPTH_CLEAR:
+      case BRW_HIZ_OP_HIZ_RESOLVE:
+         ds->ds2.depth_test_enable = 0;
+         break;
+      default:
+         assert(0);
+         break;
+      }

Isn't the metaop already ensuring that the depth test is
enabled/disabled as appropriate?

Yes. I wrote this commit long before writing the meta-ops, and I failed to 
thoroughly
clean this commit up afterwards.


diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c 
b/src/mesa/drivers/dri/i965/gen6_sf_state.c
index 5cbfe78..bdd02ed 100644
--- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
@@ -149,8 +149,20 @@ upload_sf_state(struct brw_context *brw)
        num_outputs<<  GEN6_SF_NUM_OUTPUTS_SHIFT |
        urb_entry_read_length<<  GEN6_SF_URB_ENTRY_READ_LENGTH_SHIFT |
        urb_entry_read_offset<<  GEN6_SF_URB_ENTRY_READ_OFFSET_SHIFT;
-   dw2 = GEN6_SF_VIEWPORT_TRANSFORM_ENABLE |
-      GEN6_SF_STATISTICS_ENABLE;
+
+   dw2 = GEN6_SF_STATISTICS_ENABLE;
+
+   /* Enable viewport transform only if no HiZ operation is progress
+    *
+    * From page 11 of the SandyBridge PRM, Volume 2, Part 1, Section 1.3, "3D
+    * Primitives Overview":
+    *     RECTLIST: Viewport Mapping must be DISABLED (as is typical with the
+    *     use of screen- space coordinates).
+    */
+   if (brw->hiz.op == 0) {
+      dw2 |= GEN6_SF_VIEWPORT_TRANSFORM_ENABLE;
+   }

diff --git a/src/mesa/drivers/dri/i965/gen6_wm_state.c 
b/src/mesa/drivers/dri/i965/gen6_wm_state.c
index 07e9995..bbc421f 100644
--- a/src/mesa/drivers/dri/i965/gen6_wm_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_wm_state.c
@@ -144,6 +144,23 @@ upload_wm_state(struct brw_context *brw)
     dw4 |= (brw->wm.prog_data->first_curbe_grf_16<<
        GEN6_WM_DISPATCH_START_GRF_SHIFT_2);

+   switch (brw->hiz.op) {
+   case BRW_HIZ_OP_NONE:
+      break;
+   case BRW_HIZ_OP_DEPTH_CLEAR:
+      dw4 |= GEN6_WM_DEPTH_CLEAR;
+      break;
+   case BRW_HIZ_OP_DEPTH_RESOLVE:
+      dw4 |= GEN6_WM_DEPTH_RESOLVE;
+      break;
+   case BRW_HIZ_OP_HIZ_RESOLVE:
+      dw4 |= GEN6_WM_HIERARCHICAL_DEPTH_RESOLVE;
+      break;
+   default:
+      assert(0);
+      break;
+   }
+
     dw5 |= (brw->wm_max_threads - 1)<<  GEN6_WM_MAX_THREADS_SHIFT;

     /* CACHE_NEW_WM_PROG */
@@ -166,15 +183,18 @@ upload_wm_state(struct brw_context *brw)
     /* BRW_NEW_FRAGMENT_PROGRAM */
     if (fp->program.Base.InputsRead&  (1<<  FRAG_ATTRIB_WPOS))
        dw5 |= GEN6_WM_USES_SOURCE_DEPTH | GEN6_WM_USES_SOURCE_W;
-   if (fp->program.Base.OutputsWritten&  BITFIELD64_BIT(FRAG_RESULT_DEPTH))
+   if ((fp->program.Base.OutputsWritten&  BITFIELD64_BIT(FRAG_RESULT_DEPTH))&&
+       !brw->hiz.op)
        dw5 |= GEN6_WM_COMPUTED_DEPTH;

     /* _NEW_COLOR */
-   if (fp->program.UsesKill || ctx->Color.AlphaEnabled)
+   if ((fp->program.UsesKill || ctx->Color.AlphaEnabled)&&
+       !brw->hiz.op)
        dw5 |= GEN6_WM_KILL_ENABLE;

-   if (brw_color_buffer_write_enabled(brw) ||
-       dw5&  (GEN6_WM_KILL_ENABLE | GEN6_WM_COMPUTED_DEPTH)) {
+   if ((brw_color_buffer_write_enabled(brw) ||
+        (dw5&  (GEN6_WM_KILL_ENABLE | GEN6_WM_COMPUTED_DEPTH)))&&
+       !brw->hiz.op) {
        dw5 |= GEN6_WM_DISPATCH_ENABLE;

It looks to me like if the meta code just set up an empty fragment
shader with its VS, none of the hiz.op checks here would be needed.  I
think that would be cleaner.

I agree. I'll test that idea and, if it works, this commit can be deleted.

--
Chad Versace
c...@chad-versace.us
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to