[Mesa-dev] [PATCH] i965: Avoid extraneous fast depth clears

Ilia Mirkin imirkin at alum.mit.edu
Wed May 11 08:56:19 UTC 2016


You're dropping depth_irb->mt_layer. Is that right? What if there's a
texture view, or some other mechanism to attach a layer subrange?
On May 11, 2016 2:31 AM, "Kenneth Graunke" <kenneth at whitecape.org> wrote:

> From: Chia-I Wu <olvaffe at gmail.com>
>
> When the depth buffer is already cleared, skip GEN6_HIZ_OP_DEPTH_CLEAR.
> This is made possible by tracking which slices have been cleared in
> "struct intel_mipmap_level".  The hiz_cleared flag is unset when the
> depth buffer is rendered to or when a HiZ resolve is needed.
>
> Improves performance in Manhattan on Skylake GT3e at 1280x720 by
> 0.621245% +/- 0.103831%.
>
> v2:
> - unset hiz_cleared automatically in
> intel_miptree_slice_set_needs_hiz_resolve
> - set/unset hiz_cleared with intel_renderbuffer_att_set_needs_depth_resolve
> v3: (Ken) Rebase forward 8 months.
> v4: (Ken) Rebase forward 2 years; benchmark again.
>
> Signed-off-by: Chia-I Wu <olv at lunarg.com>
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_clear.c         | 55
> +++++++++++++++++++--------
>  src/mesa/drivers/dri/i965/brw_draw.c          |  2 +-
>  src/mesa/drivers/dri/i965/intel_fbo.c         | 18 ++++++++-
>  src/mesa/drivers/dri/i965/intel_fbo.h         |  4 +-
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 39 +++++++++++++++++++
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 21 ++++++++++
>  6 files changed, 118 insertions(+), 21 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clear.c
> b/src/mesa/drivers/dri/i965/brw_clear.c
> index d57b677..5b76263 100644
> --- a/src/mesa/drivers/dri/i965/brw_clear.c
> +++ b/src/mesa/drivers/dri/i965/brw_clear.c
> @@ -166,34 +166,57 @@ brw_fast_clear_depth(struct gl_context *ctx)
>        break;
>     }
>
> +   unsigned num_layers_cleared = 0;
> +   bool clear_all_layers = false;
> +
>     /* If we're clearing to a new clear value, then we need to resolve any
> clear
>      * flags out of the HiZ buffer into the real depth buffer.
>      */
>     if (mt->depth_clear_value != depth_clear_value) {
>        intel_miptree_all_slices_resolve_depth(brw, mt);
>        mt->depth_clear_value = depth_clear_value;
> -   }
>
> -   /* From the Sandy Bridge PRM, volume 2 part 1, page 313:
> -    *
> -    *     "If other rendering operations have preceded this clear, a
> -    *      PIPE_CONTROL with write cache flush enabled and Z-inhibit
> disabled
> -    *      must be issued before the rectangle primitive used for the
> depth
> -    *      buffer clear operation.
> -    */
> -   brw_emit_mi_flush(brw);
> +      clear_all_layers = true;
> +   }
>
>     if (fb->MaxNumLayers > 0) {
>        for (unsigned layer = 0; layer < depth_irb->layer_count; layer++) {
> -         intel_hiz_exec(brw, mt, depth_irb->mt_level,
> -                        depth_irb->mt_layer + layer,
> -                        GEN6_HIZ_OP_DEPTH_CLEAR);
> +         if (clear_all_layers ||
> +             !intel_miptree_slice_get_hiz_cleared(mt,
> +                                                  depth_irb->mt_level,
> +                                                  layer)) {
> +            /* From the Sandy Bridge PRM, volume 2 part 1, page 313:
> +             *
> +             *     "If other rendering operations have preceded this
> clear, a
> +             *      PIPE_CONTROL with write cache flush enabled and
> Z-inhibit
> +             *      disabled must be issued before the rectangle
> primitive used
> +             *      for the depth buffer clear operation.
> +             */
> +            if (num_layers_cleared == 0)
> +               brw_emit_mi_flush(brw);
> +
> +            intel_hiz_exec(brw, mt, depth_irb->mt_level, layer,
> +                           GEN6_HIZ_OP_DEPTH_CLEAR);
> +
> +            num_layers_cleared++;
> +         }
>        }
>     } else {
> -      intel_hiz_exec(brw, mt, depth_irb->mt_level, depth_irb->mt_layer,
> -                     GEN6_HIZ_OP_DEPTH_CLEAR);
> +      if (clear_all_layers ||
> +          !intel_miptree_slice_get_hiz_cleared(mt,
> +                                               depth_irb->mt_level,
> +                                               depth_irb->mt_layer)) {
> +         brw_emit_mi_flush(brw);
> +         intel_hiz_exec(brw, mt, depth_irb->mt_level, depth_irb->mt_layer,
> +                        GEN6_HIZ_OP_DEPTH_CLEAR);
> +
> +         num_layers_cleared = 1;
> +      }
>     }
>
> +   if (num_layers_cleared == 0)
> +      return true;
> +
>     if (brw->gen == 6) {
>        /* From the Sandy Bridge PRM, volume 2 part 1, page 314:
>         *
> @@ -205,9 +228,9 @@ brw_fast_clear_depth(struct gl_context *ctx)
>     }
>
>     /* Now, the HiZ buffer contains data that needs to be resolved to the
> depth
> -    * buffer.
> +    * buffer.  And set its cleared state to avoid unnecessary clears.
>      */
> -   intel_renderbuffer_att_set_needs_depth_resolve(depth_att);
> +   intel_renderbuffer_att_set_needs_depth_resolve(depth_att, true);
>
>     return true;
>  }
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
> b/src/mesa/drivers/dri/i965/brw_draw.c
> index 9d034cf..2f77251 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -373,7 +373,7 @@ brw_postdraw_set_buffers_need_resolve(struct
> brw_context *brw)
>     if (back_irb)
>        back_irb->need_downsample = true;
>     if (depth_irb && ctx->Depth.Mask) {
> -      intel_renderbuffer_att_set_needs_depth_resolve(depth_att);
> +      intel_renderbuffer_att_set_needs_depth_resolve(depth_att, false);
>        brw_render_cache_set_add_bo(brw, depth_irb->mt->bo);
>     }
>
> diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c
> b/src/mesa/drivers/dri/i965/intel_fbo.c
> index 7eb21ac..7588a1b 100644
> --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> @@ -969,17 +969,31 @@ intel_renderbuffer_resolve_hiz(struct brw_context
> *brw,
>     return false;
>  }
>
> +/**
> + * Set that the depth buffer needs to be resolved.  This should be called
> when
> + * the renderbuffer is rendered or cleared, and we want to distinguish
> them so
> + * that we know if the HiZ is in a "cleared" state.
> + */
>  void
> -intel_renderbuffer_att_set_needs_depth_resolve(struct
> gl_renderbuffer_attachment *att)
> +intel_renderbuffer_att_set_needs_depth_resolve(struct
> gl_renderbuffer_attachment *att,
> +                                               bool cleared)
>  {
>     struct intel_renderbuffer *irb = intel_renderbuffer(att->Renderbuffer);
>     if (irb->mt) {
>        if (att->Layered) {
> -         intel_miptree_set_all_slices_need_depth_resolve(irb->mt,
> irb->mt_level);
> +         intel_miptree_set_all_slices_need_depth_resolve(irb->mt,
> +                                                         irb->mt_level);
> +         intel_miptree_set_all_slices_hiz_cleared(irb->mt,
> +                                                  irb->mt_level,
> +                                                  cleared);
>        } else {
>           intel_miptree_slice_set_needs_depth_resolve(irb->mt,
>                                                       irb->mt_level,
>                                                       irb->mt_layer);
> +         intel_miptree_slice_set_hiz_cleared(irb->mt,
> +                                             irb->mt_level,
> +                                             irb->mt_layer,
> +                                             cleared);
>        }
>     }
>  }
> diff --git a/src/mesa/drivers/dri/i965/intel_fbo.h
> b/src/mesa/drivers/dri/i965/intel_fbo.h
> index 89894cd..bab6924 100644
> --- a/src/mesa/drivers/dri/i965/intel_fbo.h
> +++ b/src/mesa/drivers/dri/i965/intel_fbo.h
> @@ -198,8 +198,8 @@ bool
>  intel_renderbuffer_has_hiz(struct intel_renderbuffer *irb);
>
>  void
> -intel_renderbuffer_att_set_needs_depth_resolve(struct
> gl_renderbuffer_attachment *att);
> -
> +intel_renderbuffer_att_set_needs_depth_resolve(struct
> gl_renderbuffer_attachment *att,
> +                                               bool cleared);
>
>  /**
>   * \brief Perform a HiZ resolve on the renderbuffer.
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 94f6333..e612ef9 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -1933,6 +1933,42 @@ intel_miptree_level_has_hiz(struct
> intel_mipmap_tree *mt, uint32_t level)
>  }
>
>  void
> +intel_miptree_slice_set_hiz_cleared(struct intel_mipmap_tree *mt,
> +                                    uint32_t level,
> +                                    uint32_t layer,
> +                                    bool cleared)
> +{
> +   if (!intel_miptree_level_has_hiz(mt, level))
> +      return;
> +
> +   mt->level[level].slice[layer].hiz_cleared = cleared;
> +}
> +
> +void
> +intel_miptree_set_all_slices_hiz_cleared(struct intel_mipmap_tree *mt,
> +                                         uint32_t level,
> +                                         bool cleared)
> +{
> +   uint32_t layer;
> +   uint32_t end_layer = mt->level[level].depth;
> +
> +   for (layer = 0; layer < end_layer; layer++) {
> +      intel_miptree_slice_set_hiz_cleared(mt, level, layer, cleared);
> +   }
> +}
> +
> +bool
> +intel_miptree_slice_get_hiz_cleared(struct intel_mipmap_tree *mt,
> +                                    uint32_t level,
> +                                    uint32_t layer)
> +{
> +   if (!intel_miptree_level_has_hiz(mt, level))
> +      return false;
> +
> +   return mt->level[level].slice[layer].hiz_cleared;
> +}
> +
> +void
>  intel_miptree_slice_set_needs_hiz_resolve(struct intel_mipmap_tree *mt,
>                                           uint32_t level,
>                                           uint32_t layer)
> @@ -1942,6 +1978,9 @@ intel_miptree_slice_set_needs_hiz_resolve(struct
> intel_mipmap_tree *mt,
>
>     intel_resolve_map_set(&mt->hiz_map,
>                          level, layer, GEN6_HIZ_OP_HIZ_RESOLVE);
> +
> +   /* HiZ should no longer be considered cleared when it needs a resolve
> */
> +   intel_miptree_slice_set_hiz_cleared(mt, level, layer, false);
>  }
>
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 7862152..261dcf3 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -155,6 +155,11 @@ struct intel_mipmap_level
>         * intel_miptree_map/unmap on this slice.
>         */
>        struct intel_miptree_map *map;
> +
> +      /**
> +       * Is HiZ cleared for this slice?
> +       */
> +      bool hiz_cleared;
>     } *slice;
>  };
>
> @@ -835,6 +840,22 @@ bool
>  intel_miptree_level_has_hiz(struct intel_mipmap_tree *mt, uint32_t level);
>
>  void
> +intel_miptree_slice_set_hiz_cleared(struct intel_mipmap_tree *mt,
> +                                    uint32_t level,
> +                                    uint32_t layer,
> +                                    bool cleared);
> +
> +void
> +intel_miptree_set_all_slices_hiz_cleared(struct intel_mipmap_tree *mt,
> +                                         uint32_t level,
> +                                         bool cleared);
> +
> +bool
> +intel_miptree_slice_get_hiz_cleared(struct intel_mipmap_tree *mt,
> +                                    uint32_t level,
> +                                    uint32_t layer);
> +
> +void
>  intel_miptree_slice_set_needs_hiz_resolve(struct intel_mipmap_tree *mt,
>                                            uint32_t level,
>                                           uint32_t depth);
> --
> 2.8.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160511/26b47e6f/attachment-0001.html>


More information about the mesa-dev mailing list