[Mesa-dev] [PATCH V2 05/11] anv: Allocate hiz surface

[Jason Ekstrand]

On Tue, Oct 4, 2016 at 3:48 PM, Nanley Chery <nanleychery at gmail.com> wrote:

> On Mon, Oct 03, 2016 at 04:27:53PM -0700, Jason Ekstrand wrote:
> > On Mon, Sep 26, 2016 at 5:10 PM, Nanley Chery <nanleychery at gmail.com>
> wrote:
> >
> > > From: Chad Versace <chad.versace at intel.com>
> > >
> > > Nanley Chery:
> > > (rebase)
> > >  - Use isl_surf_get_hiz_surf()
> > > (amend)
> > >  - Only add a HiZ surface onto a depth/stencil attachment
> > >  - Add comment above HiZ surface addition
> > >  - Hide HiZ behind INTEL_VK_HIZ prior to BDW
> > >  - Disable HiZ for untested cases
> > >  - Remove DISABLE_AUX_BIT instead of preventing it from being added
> > >
> > > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > > Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
> > > Reviewed-by: Chad Versace <chadversary at chromium.org> (v1)
> > >
> > > ---
> > >
> > > v2: Disable certain HiZ cases here (Jason)
> > >
> > >  src/intel/vulkan/anv_image.c | 39 ++++++++++++++++++++++++++++++
> ++++++---
> > >  1 file changed, 36 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/src/intel/vulkan/anv_image.c
> b/src/intel/vulkan/anv_image.c
> > > index f6e8672..d408819 100644
> > > --- a/src/intel/vulkan/anv_image.c
> > > +++ b/src/intel/vulkan/anv_image.c
> > > @@ -28,6 +28,7 @@
> > >  #include <fcntl.h>
> > >
> > >  #include "anv_private.h"
> > > +#include "util/debug.h"
> > >
> > >  #include "vk_format_info.h"
> > >
> > > @@ -60,6 +61,7 @@ choose_isl_surf_usage(VkImageUsageFlags vk_usage,
> > >        default:
> > >           unreachable("bad VkImageAspect");
> > >        case VK_IMAGE_ASPECT_DEPTH_BIT:
> > > +         isl_usage &= ~ISL_SURF_USAGE_DISABLE_AUX_BIT;
> > >           isl_usage |= ISL_SURF_USAGE_DEPTH_BIT;
> > >           break;
> > >        case VK_IMAGE_ASPECT_STENCIL_BIT:
> > > @@ -99,6 +101,16 @@ get_surface(struct anv_image *image,
> > > VkImageAspectFlags aspect)
> > >     }
> > >  }
> > >
> > > +static void
> > > +add_surface(struct anv_image *image, struct anv_surface *surf)
> > > +{
> > > +   assert(surf->isl.size > 0); /* isl surface must be initialized */
> > > +
> > > +   surf->offset = align_u32(image->size, surf->isl.alignment);
> > > +   image->size = surf->offset + surf->isl.size;
> > > +   image->alignment = MAX(image->alignment, surf->isl.alignment);
> > > +}
> > > +
> > >  /**
> > >   * Initialize the anv_image::*_surface selected by \a aspect. Then
> update
> > > the
> > >   * image's memory requirements (that is, the image's size and
> alignment).
> > > @@ -160,9 +172,30 @@ make_surface(const struct anv_device *dev,
> > >      */
> > >     assert(ok);
> > >
> > > -   anv_surf->offset = align_u32(image->size, anv_surf->isl.alignment);
> > > -   image->size = anv_surf->offset + anv_surf->isl.size;
> > > -   image->alignment = MAX(image->alignment, anv_surf->isl.alignment);
> > > +   add_surface(image, anv_surf);
> > >
> >
> > In my CCS series, I split this in two and had a precursor patch that
> added
> > the add_surface helper.  Do with that information what you will.  I'm
> fine
> > with having it all in one patch.
> >
> >
> > > +
> > > +   /* Allow the user to control HiZ enabling. Disable by default on
> gen7
> > > +    * because resolves are not currently implemented pre-BDW.
> > > +    */
> > > +   if (!env_var_as_boolean("INTEL_VK_HIZ", dev->info.gen >= 8)) {
> > > +      anv_finishme("Implement gen7 HiZ");
> > > +      return VK_SUCCESS;
> > > +   } else if (vk_info->mipLevels > 1) {
> > > +      anv_finishme("Test multi-LOD HiZ");
> > > +      return VK_SUCCESS;
> > > +   } else if (dev->info.gen == 8 && vk_info->samples > 1) {
> > > +      anv_finishme("Test gen8 multisampled HiZ");
> > > +      return VK_SUCCESS;
> > > +   }
> > >
> >
> > It may be better to pull this (and the usage check below) into an
> > image_supports_hiz helper.  The early returns work for now but as soon as
> > we have multiple kinds of aux surfaces, we'll have to do that refactor
> > anyway.
> >
> >
>
> I'll nest the hunk above into the following if statement. That should
> prevent conflicts with other aux surfaces.
>

I suppose that works.  If it's a problem, we can always do that refactor
later.


> > > +
> > > +   /* Add a HiZ surface to a depth buffer that will be used for
> rendering.
> > > +    */
> > > +   if (aspect == VK_IMAGE_ASPECT_DEPTH_BIT &&
> > > +       (image->usage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT))
> {
>
> -Nanley
>
> > > +      isl_surf_get_hiz_surf(&dev->isl_dev, &image->depth_surface.isl,
> > > +                            &image->hiz_surface.isl);
> > > +      add_surface(image, &image->hiz_surface);
> > > +   }
> > >
> > >     return VK_SUCCESS;
> > >  }
> > > --
> > > 2.10.0
> > >
> > > _______________________________________________
> > > 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/20161004/95e9a8e0/attachment-0001.html>