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 <[email protected]>
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 <[email protected]> wrote:
>
>> On Wed, Dec 13, 2017 at 11:26 AM, Nanley Chery <[email protected]>
>> 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
>>> > [email protected]
>>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>
>>
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev