[Mesa-dev] [PATCH 11/17] i965/gen6 depth surface: program 3DSTATE_DEPTH_BUFFER to top of surface

Jordan Justen jljusten at gmail.com
Thu Jul 31 01:09:17 PDT 2014


On Tue, Jul 22, 2014 at 3:09 AM, Pohjolainen, Topi
<topi.pohjolainen at intel.com> wrote:
> On Fri, Jul 18, 2014 at 02:16:46PM -0700, Jordan Justen wrote:
>> (bf25ee2 for gen6)
>>
>> Previously we would always find the 2D sub-surface of interest,
>> and then program the surface to this location. Now we always
>> program the 3DSTATE_DEPTH_BUFFER at the start of the surface.
>> To select the lod/slice, we utilize the lod & minimum array
>> element fields.
>>
>> We also must disable brw_workaround_depthstencil_alignment for
>> gen >= 6. Now the hardware will handle alignment when rendering
>> to additional slices/LODs.
>>
>> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=56127
>> ---
>>  src/mesa/drivers/dri/i965/brw_misc_state.c   |  4 +-
>>  src/mesa/drivers/dri/i965/gen6_blorp.cpp     | 71 +++++++++-------------------
>>  src/mesa/drivers/dri/i965/gen6_depth_state.c | 32 ++++++++++---
>>  3 files changed, 49 insertions(+), 58 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/dri/i965/brw_misc_state.c
>> index 76e22bd..e3980fc 100644
>> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
>> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
>> @@ -261,10 +261,10 @@ brw_workaround_depthstencil_alignment(struct brw_context *brw,
>>     if (stencil_irb)
>>        brw->depthstencil.stencil_mt = get_stencil_miptree(stencil_irb);
>>
>> -   /* Gen7+ doesn't require the workarounds, since we always program the
>> +   /* Gen6+ doesn't require the workarounds, since we always program the
>>      * surface state at the start of the whole surface.
>>      */
>> -   if (brw->gen >= 7)
>> +   if (brw->gen >= 6)
>>        return;
>>
>>     /* Check if depth buffer is in depth/stencil format.  If so, then it's only
>> diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
>> index dca6bfc..5a56442 100644
>> --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
>> +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
>> @@ -787,10 +787,6 @@ static void
>>  gen6_blorp_emit_depth_stencil_config(struct brw_context *brw,
>>                                       const brw_blorp_params *params)
>>  {
>> -   struct gl_context *ctx = &brw->ctx;
>> -   uint32_t draw_x = params->depth.x_offset;
>> -   uint32_t draw_y = params->depth.y_offset;
>> -   uint32_t tile_mask_x, tile_mask_y;
>>     uint32_t surfwidth, surfheight;
>>     uint32_t surftype;
>>     unsigned int depth = MAX2(params->depth.mt->logical_depth0, 1);
>> @@ -814,12 +810,6 @@ gen6_blorp_emit_depth_stencil_config(struct brw_context *brw,
>>        break;
>>     }
>>
>> -   brw_get_depthstencil_tile_masks(params->depth.mt,
>> -                                   params->depth.level,
>> -                                   params->depth.layer,
>> -                                   NULL,
>> -                                   &tile_mask_x, &tile_mask_y);
>> -
>>     min_array_element = params->depth.layer;
>>
>>     lod = params->depth.level - params->depth.mt->first_level;
>> @@ -838,55 +828,42 @@ gen6_blorp_emit_depth_stencil_config(struct brw_context *brw,
>>
>>     /* 3DSTATE_DEPTH_BUFFER */
>>     {
>> -      uint32_t tile_x = draw_x & tile_mask_x;
>> -      uint32_t tile_y = draw_y & tile_mask_y;
>> -      uint32_t offset =
>> -         intel_miptree_get_aligned_offset(params->depth.mt,
>> -                                          draw_x & ~tile_mask_x,
>> -                                          draw_y & ~tile_mask_y, false);
>> -
>> -      /* According to the Sandy Bridge PRM, volume 2 part 1, pp326-327
>> -       * (3DSTATE_DEPTH_BUFFER dw5), in the documentation for "Depth
>> -       * Coordinate Offset X/Y":
>> -       *
>> -       *   "The 3 LSBs of both offsets must be zero to ensure correct
>> -       *   alignment"
>> -       *
>> -       * We have no guarantee that tile_x and tile_y are correctly aligned,
>> -       * since they are determined by the mipmap layout, which is only aligned
>> -       * to multiples of 4.
>> -       *
>> -       * So, to avoid hanging the GPU, just smash the low order 3 bits of
>> -       * tile_x and tile_y to 0.  This is a temporary workaround until we come
>> -       * up with a better solution.
>> -       */
>> -      WARN_ONCE((tile_x & 7) || (tile_y & 7),
>> -                "Depth/stencil buffer needs alignment to 8-pixel boundaries.\n"
>> -                "Truncating offset, bad rendering may occur.\n");
>> -      tile_x &= ~7;
>> -      tile_y &= ~7;
>> -
>>        intel_emit_post_sync_nonzero_flush(brw);
>>        intel_emit_depth_stall_flushes(brw);
>>
>>        BEGIN_BATCH(7);
>> +      /* 3DSTATE_DEPTH_BUFFER dw0 */
>>        OUT_BATCH(_3DSTATE_DEPTH_BUFFER << 16 | (7 - 2));
>> +
>> +      /* 3DSTATE_DEPTH_BUFFER dw1 */
>>        OUT_BATCH((params->depth.mt->pitch - 1) |
>>                  params->depth_format << 18 |
>>                  1 << 21 | /* separate stencil enable */
>>                  1 << 22 | /* hiz enable */
>>                  BRW_TILEWALK_YMAJOR << 26 |
>>                  1 << 27 | /* y-tiled */
>> -                BRW_SURFACE_2D << 29);
>> +                surftype << 29);
>> +
>> +      /* 3DSTATE_DEPTH_BUFFER dw2 */
>>        OUT_RELOC(params->depth.mt->bo,
>>                  I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
>> -                offset);
>> +                0);
>> +
>> +      /* 3DSTATE_DEPTH_BUFFER dw3 */
>>        OUT_BATCH(BRW_SURFACE_MIPMAPLAYOUT_BELOW << 1 |
>> -                (params->depth.width + tile_x - 1) << 6 |
>> -                (params->depth.height + tile_y - 1) << 19);
>> +                (surfwidth - 1) << 6 |
>> +                (surfheight - 1) << 19 |
>> +                lod << 2);
>> +
>> +      /* 3DSTATE_DEPTH_BUFFER dw4 */
>> +      OUT_BATCH((depth - 1) << 21 |
>> +                min_array_element << 10 |
>> +                (depth - 1) << 1);
>> +
>> +      /* 3DSTATE_DEPTH_BUFFER dw5 */
>>        OUT_BATCH(0);
>> -      OUT_BATCH(tile_x |
>> -                tile_y << 16);
>> +
>> +      /* 3DSTATE_DEPTH_BUFFER dw6 */
>>        OUT_BATCH(0);
>>        ADVANCE_BATCH();
>>     }
>> @@ -894,17 +871,13 @@ gen6_blorp_emit_depth_stencil_config(struct brw_context *brw,
>>     /* 3DSTATE_HIER_DEPTH_BUFFER */
>>     {
>>        struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_mt;
>> -      uint32_t hiz_offset =
>> -         intel_miptree_get_aligned_offset(hiz_mt,
>> -                                          draw_x & ~tile_mask_x,
>> -                                          (draw_y & ~tile_mask_y) / 2, false);
>>
>>        BEGIN_BATCH(3);
>>        OUT_BATCH((_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
>>        OUT_BATCH(hiz_mt->pitch - 1);
>>        OUT_RELOC(hiz_mt->bo,
>>                  I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
>> -                hiz_offset);
>> +                0);
>>        ADVANCE_BATCH();
>>     }
>>
>> diff --git a/src/mesa/drivers/dri/i965/gen6_depth_state.c b/src/mesa/drivers/dri/i965/gen6_depth_state.c
>> index abb2124..b58f970 100644
>> --- a/src/mesa/drivers/dri/i965/gen6_depth_state.c
>> +++ b/src/mesa/drivers/dri/i965/gen6_depth_state.c
>> @@ -51,6 +51,7 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
>>     unsigned int min_array_element;
>>     GLenum gl_target = GL_TEXTURE_2D;
>>     unsigned int lod;
>> +   const struct intel_mipmap_tree *mt = depth_mt ? depth_mt : stencil_mt;
>>     const struct intel_renderbuffer *irb = NULL;
>>     const struct gl_renderbuffer *rb = NULL;
>>
>> @@ -112,6 +113,11 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
>>
>>     lod = irb ? irb->mt_level - irb->mt->first_level : 0;
>>
>> +   if (mt) {
>> +      width = mt->logical_width0;
>> +      height = mt->logical_height0;
>> +   }
>> +
>>     unsigned int len;
>>     if (brw->gen >= 6)
>>        len = 7;
>> @@ -121,7 +127,10 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
>>        len = 5;
>>
>>     BEGIN_BATCH(len);
>> +   /* 3DSTATE_DEPTH_BUFFER dw0 */
>>     OUT_BATCH(_3DSTATE_DEPTH_BUFFER << 16 | (len - 2));
>> +
>> +   /* 3DSTATE_DEPTH_BUFFER dw1 */
>>     OUT_BATCH((depth_mt ? depth_mt->pitch - 1 : 0) |
>>               (depthbuffer_format << 18) |
>>               ((enable_hiz_ss ? 1 : 0) << 21) | /* separate stencil enable */
>> @@ -129,8 +138,9 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
>>               (BRW_TILEWALK_YMAJOR << 26) |
>>               ((depth_mt ? depth_mt->tiling != I915_TILING_NONE : 1)
>>                << 27) |
>> -             (depth_surface_type << 29));
>> +             (surftype << 29));
>>
>> +   /* 3DSTATE_DEPTH_BUFFER dw2 */
>>     if (depth_mt) {
>>        OUT_RELOC(depth_mt->bo,
>>               I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
>
> You don't force the depth offset here to zero, is that on purpose?

I think you meant on the very next line that just got cut off in the
diff. (The last param to OUT_RELOC.)

You are right. I missed that.

This patch r-b with that fixed?

-Jordan

>> @@ -139,15 +149,23 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
>>        OUT_BATCH(0);
>>     }
>>
>> -   OUT_BATCH(((width + tile_x - 1) << 6) |
>> -             ((height + tile_y - 1) << 19));
>> -   OUT_BATCH(0);
>> +   /* 3DSTATE_DEPTH_BUFFER dw3 */
>> +   OUT_BATCH(((width - 1) << 6) |
>> +             ((height - 1) << 19) |
>> +             lod << 2);
>>
>> +   /* 3DSTATE_DEPTH_BUFFER dw4 */
>> +   OUT_BATCH((depth - 1) << 21 |
>> +             min_array_element << 10 |
>> +             (depth - 1) << 1);
>> +
>> +   /* 3DSTATE_DEPTH_BUFFER dw5 */
>>     if (brw->is_g4x || brw->gen >= 5)
>> -      OUT_BATCH(tile_x | (tile_y << 16));
>> +      OUT_BATCH(0);
>>     else
>>        assert(tile_x == 0 && tile_y == 0);
>>
>> +   /* 3DSTATE_DEPTH_BUFFER dw6 */
>>     if (brw->gen >= 6)
>>        OUT_BATCH(0);
>>
>> @@ -170,7 +188,7 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
>>        OUT_BATCH(hiz_mt->pitch - 1);
>>        OUT_RELOC(hiz_mt->bo,
>>                  I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
>> -                brw->depthstencil.hiz_offset);
>> +                0);
>>        ADVANCE_BATCH();
>>        } else {
>>        BEGIN_BATCH(3);
>> @@ -192,7 +210,7 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
>>        OUT_BATCH(2 * stencil_mt->pitch - 1);
>>        OUT_RELOC(stencil_mt->bo,
>>                  I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
>> -                brw->depthstencil.stencil_offset);
>> +                0);
>>        ADVANCE_BATCH();
>>        } else {
>>        BEGIN_BATCH(3);
>> --
>> 2.0.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list