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