[Mesa-dev] [PATCH 2/3] intel: Eliminate unneeded hiz resolves

[Paul Berry]

On 14 August 2012 16:58, Chad Versace <chad.versace at linux.intel.com> wrote:

> On creating a hiz miptree, we conservatively marked that each miptree
> slice needed a hiz resolve. But the resolves are unneeded when creating
> a non-texture miptree, so this patch removes them.
>
> This eliminates one hiz resolve per each creation of a non-texture depth
> miptree.  Hence, this eliminates many resolves when resizing a window.
>

So, with this change, are the contents of the HiZ buffer uninitialized for
a newly created (or resized) non-texture miptree?  The HW docs don't
specify the exact format of the HiZ buffer, so it's possible that there may
be some possible states of the HiZ buffer that are invalid.  If there are,
and the hardware doesn't deal with those invalid states well, then we run
the risk of sporadic incorrect results (if subsequent rendering fails to
bring the HiZ buffer into a valid state) or possibly sporadic GPU hangs (if
the hardware is unable to cope with the invalid states).

Alternatively, perhaps we could initialize non-texture HiZ buffers to a
state that indicates that they need a fast depth clear rather than a HiZ
resolve.  Clears are likely to be much faster than HiZ resolves, because
they don't require reading from the depth buffer.  So we would still get
most of the benefit of avoiding the HiZ resolve, and no risk of undefined
behaviour.  In fact, we might conceivably get even more benefit, since
after a fast depth clear, the HiZ buffer is in a state where future
rendering is very efficient (since the depth buffer doesn't need to be
read).


>
> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
> ---
>  src/mesa/drivers/dri/intel/intel_fbo.c         |  4 ++-
>  src/mesa/drivers/dri/intel/intel_mipmap_tree.c | 39
> +++++++++++++++++---------
>  src/mesa/drivers/dri/intel/intel_mipmap_tree.h |  3 +-
>  3 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c
> b/src/mesa/drivers/dri/intel/intel_fbo.c
> index 3a610c2..ada0f69 100644
> --- a/src/mesa/drivers/dri/intel/intel_fbo.c
> +++ b/src/mesa/drivers/dri/intel/intel_fbo.c
> @@ -538,7 +538,9 @@ intel_renderbuffer_update_wrapper(struct intel_context
> *intel,
>
>     if (mt->hiz_mt == NULL &&
>         intel->vtbl.is_hiz_depth_format(intel, rb->Format)) {
> -      intel_miptree_alloc_hiz(intel, mt, 0 /* num_samples */);
> +      intel_miptree_alloc_hiz(intel, mt,
> +                              0 /*num_samples*/,
> +                              true /*for_texture*/);
>        if (!mt->hiz_mt)
>          return false;
>     }
> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> index 24cd9e9..143f2e3 100644
> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> @@ -467,7 +467,8 @@ intel_miptree_create_for_renderbuffer(struct
> intel_context *intel,
>        goto fail;
>
>     if (intel->vtbl.is_hiz_depth_format(intel, format)) {
> -      ok = intel_miptree_alloc_hiz(intel, mt, num_samples);
> +      ok = intel_miptree_alloc_hiz(intel, mt, num_samples,
> +                                   false /*for_texture*/);
>        if (!ok)
>           goto fail;
>     }
> @@ -825,7 +826,8 @@ intel_miptree_alloc_mcs(struct intel_context *intel,
>  bool
>  intel_miptree_alloc_hiz(struct intel_context *intel,
>                         struct intel_mipmap_tree *mt,
> -                        GLuint num_samples)
> +                        GLuint num_samples,
> +                        bool for_texture)
>  {
>     assert(mt->hiz_mt == NULL);
>     /* MSAA HiZ surfaces always use IMS layout. */
> @@ -844,18 +846,27 @@ intel_miptree_alloc_hiz(struct intel_context *intel,
>     if (!mt->hiz_mt)
>        return false;
>
> -   /* Mark that all slices need a HiZ resolve. */
> -   struct intel_resolve_map *head = &mt->hiz_map;
> -   for (int level = mt->first_level; level <= mt->last_level; ++level) {
> -      for (int layer = 0; layer < mt->level[level].depth; ++layer) {
> -        head->next = malloc(sizeof(*head->next));
> -        head->next->prev = head;
> -        head->next->next = NULL;
> -        head = head->next;
> -
> -        head->level = level;
> -        head->layer = layer;
> -        head->need = GEN6_HIZ_OP_HIZ_RESOLVE;
> +   if (for_texture) {
> +      /* Mark that all slices need a HiZ resolve. This is necessary for
> +       * renderbuffers that wrap textures because the user may have
> previously
> +       * uploaded texture data into the parent depth miptree.
> +       *
> +       * This is skipped for non-texture miptrees. In the non-texture
> case,
> +       * the depth miptree and the hiz miptree are created together, and
> hence
> +       * the content of each is undefined here.
> +       */
> +      struct intel_resolve_map *head = &mt->hiz_map;
> +      for (int level = mt->first_level; level <= mt->last_level; ++level)
> {
> +         for (int layer = 0; layer < mt->level[level].depth; ++layer) {
> +            head->next = malloc(sizeof(*head->next));
> +            head->next->prev = head;
> +            head->next->next = NULL;
> +            head = head->next;
> +
> +            head->level = level;
> +            head->layer = layer;
> +            head->need = GEN6_HIZ_OP_HIZ_RESOLVE;
> +         }
>        }
>     }
>
> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> index 0d0e757..65743fd 100644
> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
> @@ -503,7 +503,8 @@ intel_miptree_alloc_mcs(struct intel_context *intel,
>  bool
>  intel_miptree_alloc_hiz(struct intel_context *intel,
>                         struct intel_mipmap_tree *mt,
> -                        GLuint num_samples);
> +                        GLuint num_samples,
> +                        bool for_texture);
>
>  void
>  intel_miptree_slice_set_needs_hiz_resolve(struct intel_mipmap_tree *mt,
> --
> 1.7.11.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120817/b3fb7abc/attachment.html>