[Mesa-dev] [PATCH] i965/gen7: Fix depth buffer rendering to tile offsets.

[Chad Versace]

On 01/11/2012 03:40 PM, Eric Anholt wrote:
> Previously, we were saying that everything from the starting tile to
> region width+height was part of the limits of our depthbuffer, even if
> the tile was near the bottom of the depthbuffer.  This mean that our
> range was not clipping to buffer buonds if the start tile was anything
> but the start of the buffer.
> 
> In bebc91f0f3a1f2d19d36a7f1a4f7c992ace064e9, this was changed to
> saying that we're just rendering to a region of the size of the
> renderbuffer.  This is great -- we get a range that should actually
> match what we want.  However, the hardware's range checking occurs
> after the X/Y offset addition, so we were clipping out rendering to
> small depth mip levels when an X/Y offset was present.  Just add
> tile_x/y to the width in that case -- the WM won't produce negative
> x/y values pre-offset, so we just need to get the left/bottom sides of
> the region to cover our buffer.
> 
> Fixes piglit fbo-clear-formats GL_ARB_depth_buffer_float on gen7.
> (Full test run not completed yet because of memory leaks in master)
> ---
>  src/mesa/drivers/dri/i965/brw_misc_state.c  |    4 ++--
>  src/mesa/drivers/dri/i965/gen7_misc_state.c |    4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/dri/i965/brw_misc_state.c
> index f9652df..b6bca4b 100644
> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> @@ -376,8 +376,8 @@ static void emit_depthbuffer(struct brw_context *brw)
>  		I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
>  		offset);
>        OUT_BATCH((BRW_SURFACE_MIPMAPLAYOUT_BELOW << 1) |
> -		((depth_irb->Base.Width - 1) << 6) |
> -		((depth_irb->Base.Height - 1) << 19));
> +		(((depth_irb->Base.Width + tile_x)- 1) << 6) |
> +		(((depth_irb->Base.Height + tile_y) - 1) << 19));
>        OUT_BATCH(0);
>  
>        if (intel->is_g4x || intel->gen >= 5)
> diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> index 8a383f5..c2f58d5 100644
> --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> @@ -107,8 +107,8 @@ static void emit_depthbuffer(struct brw_context *brw)
>        OUT_RELOC(region->bo,
>  	        I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
>  		offset);
> -      OUT_BATCH(((drb->Base.Width - 1) << 4) |
> -                ((drb->Base.Height - 1) << 18));
> +      OUT_BATCH((((drb->Base.Width + tile_x) - 1) << 4) |
> +                (((drb->Base.Height + tile_y) - 1) << 18));
>        OUT_BATCH(0);
>        OUT_BATCH(tile_x | (tile_y << 16));
>        OUT_BATCH(0);

The patch needs to do the same for the stencil buffer. I think the hunks below do the trick.
I did a full piglit run on gen7 with the extra hunks; regressions are fixed and no new ones
are introduced.

I don't believe we have any tests for mipmapped depthstencil buffers that use the stencil bits, but we should.

diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/dri/i965/brw_misc_state.c
index 0c8525a..300b3d8 100644
--- a/src/mesa/drivers/dri/i965/brw_misc_state.c
+++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
@@ -332,6 +332,9 @@ static void emit_depthbuffer(struct brw_context *brw)
        */
       assert(intel->has_separate_stencil);
 
+      uint32_t tile_x, tile_y, offset;
+      offset = intel_renderbuffer_tile_offsets(stencil_irb, &tile_x, &tile_y);
+
       BEGIN_BATCH(len);
       OUT_BATCH(_3DSTATE_DEPTH_BUFFER << 16 | (len - 2));
       OUT_BATCH((BRW_DEPTHFORMAT_D32_FLOAT << 18) |
@@ -341,10 +344,11 @@ static void emit_depthbuffer(struct brw_context *brw)
                (1 << 27) | /* tiled surface */
                (BRW_SURFACE_2D << 29));
       OUT_BATCH(0);
-      OUT_BATCH(((stencil_irb->Base.Width - 1) << 6) |
-                (stencil_irb->Base.Height - 1) << 19);
-      OUT_BATCH(0);
+      OUT_BATCH(((stencil_irb->Base.Width + tile_x - 1) << 6) |
+                (stencil_irb->Base.Height + tile_y - 1) << 19);
       OUT_BATCH(0);
+      OUT_BATCH((tile_x) |
+                (tile_y << 16));
 
       if (intel->gen >= 6)
         OUT_BATCH(0);
diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c b/src/mesa/drivers/dri/i965/gen7_misc_state.c
index 03133ff..d085ba1 100644
--- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
@@ -65,17 +65,23 @@ static void emit_depthbuffer(struct brw_context *brw)
    if (depth_mt == NULL) {
       uint32_t dw1 = BRW_DEPTHFORMAT_D32_FLOAT << 18;
       uint32_t dw3 = 0;
+      uint32_t dw5 = 0;
 
       if (stencil_mt == NULL) {
         dw1 |= (BRW_SURFACE_NULL << 29);
       } else {
+        uint32_t tile_x, tile_y, offset;
+        offset = intel_renderbuffer_tile_offsets(srb, &tile_x, &tile_y);
+
         /* _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.Width - 1) << 4) |
-              ((srb->Base.Height - 1) << 18);
+        dw3 = ((srb->Base.Width + tile_x - 1) << 4) |
+              ((srb->Base.Height + tile_y - 1) << 18);
+        dw5 = (tile_x) |
+              (tile_y << 16);
       }
 
       BEGIN_BATCH(7);
@@ -84,7 +90,7 @@ static void emit_depthbuffer(struct brw_context *brw)
       OUT_BATCH(0);
       OUT_BATCH(dw3);
       OUT_BATCH(0);
-      OUT_BATCH(0);
+      OUT_BATCH(dw5);
       OUT_BATCH(0);
       ADVANCE_BATCH();
    } else {


----
Chad Versace
chad.versace at linux.intel.com