Sorry for all the list spam, but I'm sort of thinking out-loud and writing
it on the list for all to read.

I'm thinking that what we want this list to return is not a bool but an enum

/* The ordering of this enum is important */
enum anv_fast_clear_support {
   ANV_FAST_CLEAR_NONE = 0,
   ANV_FAST_CLEAR_ZERO_ONLY = 1,
   ANV_FAST_CLEAR_ANY = 2,
};

And then the predicate for whether or not to do the resolve becomes
(has_compression && !supports_compression) || (image_fast_clear >
supported_fast_clear).  I still haven't quite figured out what to do with
MI_PREDICATE to make this work out.  I'm sure it's possible with MI math
but maybe I can come up with something clever that doesn't require that.
In the worst case, we only have to deal with this complexity on gen9+ where
we have MI_MATH so it'll be ok.

Does this seem like a reasonable plan?

On Fri, Jan 12, 2018 at 5:23 PM, Jason Ekstrand <ja...@jlekstrand.net>
wrote:

> I made a table to help visualize all the different cases:
>
> Layout                      | compression | zero clear | non-zero clear
> ============================+=============+============+================
> GENERAL                     |      N      |      N     |       N
> ----------------------------+-------------+------------+----------------
> COLOR_ATTACHMENT            |      Y      |      Y     |       Y
> ----------------------------+-------------+------------+----------------
> SHADER_READ_ONLY            |      Y      |      Y     |       N
> ----------------------------+-------------+------------+----------------
> GENERAL (RT/tex/blit only)  |      Y      |      Y     |       N
> ----------------------------+-------------+------------+----------------
> PRESENT_SRC (Y_TILED)       |      N      |      N     |       N
> ----------------------------+-------------+------------+----------------
> PRESENT_SRC (Y_TILED_CCS)   |      Y      |      N     |       N
> ----------------------------+-------------+------------+----------------
>
> We will need to think about what to do with this.  Having non-zero clears
> be different from zero clears is tricky.  We may need to have even more
> bits in our aux tracking. :-/
>
> On January 12, 2018 16:05:12 Jason Ekstrand <ja...@jlekstrand.net> wrote:
>
>> On Wed, Dec 13, 2017 at 11:26 AM, Nanley Chery <nanleych...@gmail.com>
>> wrote:
>>
>>> On Mon, Nov 27, 2017 at 07:05:56PM -0800, Jason Ekstrand wrote:
>>> > ---
>>> >  src/intel/vulkan/anv_image.c   | 58 ++++++++++++++++++++++++++++++
>>> ++++++++++++
>>> >  src/intel/vulkan/anv_private.h |  5 ++++
>>> >  2 files changed, 63 insertions(+)
>>> >
>>> > diff --git a/src/intel/vulkan/anv_image.c
>>> b/src/intel/vulkan/anv_image..c
>>>
>>> > index a872149..561da28 100644
>>> > --- a/src/intel/vulkan/anv_image.c
>>> > +++ b/src/intel/vulkan/anv_image.c
>>> > @@ -837,6 +837,64 @@ anv_layout_to_aux_usage(const struct
>>> gen_device_info * const devinfo,
>>> >     unreachable("layout is not a VkImageLayout enumeration member.");
>>> >  }
>>> >
>>> > +/**
>>> > + * This function returns true if the given image in the given
>>> VkImageLayout
>>> > + * supports unresolved fast-clears.
>>> > + *
>>> > + * @param devinfo The device information of the Intel GPU.
>>> > + * @param image The image that may contain a collection of buffers.
>>> > + * @param aspect The aspect of the image to be accessed.
>>> > + * @param layout The current layout of the image aspect(s).
>>> > + */
>>> > +bool
>>> > +anv_layout_supports_fast_clear(const struct gen_device_info * const
>>> devinfo,
>>> > +                               const struct anv_image * const image,
>>> > +                               const VkImageAspectFlagBits aspect,
>>> > +                               const VkImageLayout layout)
>>> > +{
>>> > +   /* The aspect must be exactly one of the image aspects. */
>>> > +   assert(_mesa_bitcount(aspect) == 1 && (aspect & image->aspects));
>>> > +
>>> > +   uint32_t plane = anv_image_aspect_to_plane(image->aspects,
>>> aspect);
>>> > +
>>> > +   /* If there is no auxiliary surface allocated, there are no
>>> fast-clears */
>>> > +   if (image->planes[plane].aux_surface.isl.size == 0)
>>> > +      return false;
>>> > +
>>> > +   /* All images that use an auxiliary surface are required to be
>>> tiled. */
>>> > +   assert(image->tiling == VK_IMAGE_TILING_OPTIMAL);
>>> > +
>>> > +   /* Stencil has no aux */
>>> > +   assert(aspect != VK_IMAGE_ASPECT_STENCIL_BIT);
>>> > +
>>> > +   if (aspect == VK_IMAGE_ASPECT_DEPTH_BIT) {
>>> > +      /* For depth images (with HiZ), the layout supports fast-clears
>>> if and
>>> > +       * only if it supports HiZ.
>>> > +       */
>>> > +      enum isl_aux_usage aux_usage =
>>> > +         anv_layout_to_aux_usage(devinfo, image, aspect, layout);
>>> > +      return aux_usage == ISL_AUX_USAGE_HIZ;
>>> > +   }
>>> > +
>>> > +   assert(image->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV);
>>> > +
>>> > +   /* Multisample fast-clear is not yet supported. */
>>> > +   if (image->samples > 1)
>>> > +      return false;
>>> > +
>>> > +   /* The only layout which actually supports fast-clears today is
>>> > +    * VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL.  Some day in the
>>> future
>>> > +    * this may change if our ability to track clear colors improves.
>>> > +    */
>>> > +   switch (layout) {
>>> > +   case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL:
>>> > +      return true;
>>> > +
>>>
>>> This is kind of tricky. We actually allow fast-clears for CCS_E textures
>>> in the GENERAL layout if the clear color is all zeros. When this
>>> happens, we set the needs_resolve predicate to false. This means that
>>> unresolved fast-clears is potentially in use for CCS_E images in any
>>> layout.
>>>
>>
>> Hrm...  This is going to get interesting.  This works but only if we
>> assume that the thing using it in the general layout is rendering or
>> texturing.  If it's storage, this doesn't work.  That said, storage images
>> are CCS_D-only so that's not a problem now.  The bigger problem is that
>> this means the resolve won't happen for window-system images and that's
>> bad.  I'm not sure what the right path forward is; I'll have to think on it.
>>
>> Do we have any benchmark numbers to justify this clever trick?
>>
>>
>>> -Nanley
>>>
>>> > +   default:
>>> > +      return false;
>>> > +   }
>>> > +}
>>> > +
>>> >
>>> >  static struct anv_state
>>> >  alloc_surface_state(struct anv_device *device)
>>> > diff --git a/src/intel/vulkan/anv_private.h
>>> b/src/intel/vulkan/anv_private.h
>>> > index 5dd95a3..461bfed 100644
>>> > --- a/src/intel/vulkan/anv_private.h
>>> > +++ b/src/intel/vulkan/anv_private.h
>>> > @@ -2559,6 +2559,11 @@ anv_layout_to_aux_usage(const struct
>>> gen_device_info * const devinfo,
>>> >                          const struct anv_image *image,
>>> >                          const VkImageAspectFlagBits aspect,
>>> >                          const VkImageLayout layout);
>>> > +bool
>>> > +anv_layout_supports_fast_clear(const struct gen_device_info * const
>>> devinfo,
>>> > +                               const struct anv_image * const image,
>>> > +                               const VkImageAspectFlagBits aspect,
>>> > +                               const VkImageLayout layout);
>>> >
>>> >  /* This is defined as a macro so that it works for both
>>> >   * VkImageSubresourceRange and VkImageSubresourceLayers
>>> > --
>>> > 2.5.0.400.gff86faf
>>> >
>>> > _______________________________________________
>>> > mesa-dev mailing list
>>> > mesa-dev@lists.freedesktop.org
>>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>
>>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to