[Mesa-dev] [PATCH v2 07/24] anv/image: Support color aspects in layout_to_aux_usage

[Jason Ekstrand]

On Mon, Jan 22, 2018 at 12:07 AM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:

> On Fri, Jan 19, 2018 at 03:47:24PM -0800, Jason Ekstrand wrote:
> > Reviewed-by: Nanley Chery <nanley.g.chery at intel.com>
> > ---
> >  src/intel/vulkan/anv_image.c | 48 ++++++++++++++++++++++++++----
> --------------
> >  1 file changed, 29 insertions(+), 19 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> > index 84e4b96..e34ac95 100644
> > --- a/src/intel/vulkan/anv_image.c
> > +++ b/src/intel/vulkan/anv_image.c
> > @@ -774,12 +774,6 @@ anv_layout_to_aux_usage(const struct
> gen_device_info * const devinfo,
> >     /* Stencil has no aux */
> >     assert(aspect != VK_IMAGE_ASPECT_STENCIL_BIT);
> >
> > -   /* The following switch currently only handles depth stencil aspects.
> > -    * TODO: Handle the color aspect.
> > -    */
> > -   if (image->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV)
> > -      return image->planes[plane].aux_usage;
> > -
> >     switch (layout) {
> >
> >     /* Invalid Layouts */
> > @@ -799,28 +793,38 @@ anv_layout_to_aux_usage(const struct
> gen_device_info * const devinfo,
> >
> >
> >     /* Transfer Layouts
> > -    *
> > -    * This buffer could be a depth buffer used in a transfer operation.
> BLORP
> > -    * currently doesn't use HiZ for transfer operations so we must use
> the main
> > -    * buffer for this layout. TODO: Enable HiZ in BLORP.
> >      */
> >     case VK_IMAGE_LAYOUT_GENERAL:
> >     case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL:
> >     case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL:
> > -      return ISL_AUX_USAGE_NONE;
> > +      if (aspect == VK_IMAGE_ASPECT_DEPTH_BIT) {
> > +         /* This buffer could be a depth buffer used in a transfer
> operation.
> > +          * BLORP currently doesn't use HiZ for transfer operations so
> we must
> > +          * use the main buffer for this layout. TODO: Enable HiZ in
> BLORP.
> > +          */
> > +         assert(image->planes[plane].aux_usage == ISL_AUX_USAGE_HIZ);
> > +         return ISL_AUX_USAGE_NONE;
> > +      } else {
> > +         assert(image->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV);
> > +         return image->planes[plane].aux_usage;
> > +      }
> >
> >
> >     /* Sampling Layouts */
> >     case VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL:
> > +   case VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_STENCIL_ATTACHMENT_OPTIMAL_KHR:
> >        assert((image->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV) ==
> 0);
> >        /* Fall-through */
> >     case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL:
> > -   case VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_STENCIL_ATTACHMENT_OPTIMAL_KHR:
> > -      assert(aspect == VK_IMAGE_ASPECT_DEPTH_BIT);
> > -      if (anv_can_sample_with_hiz(devinfo, image))
> > -         return ISL_AUX_USAGE_HIZ;
> > -      else
> > -         return ISL_AUX_USAGE_NONE;
> > +      if (aspect == VK_IMAGE_ASPECT_DEPTH_BIT) {
> > +         if (anv_can_sample_with_hiz(devinfo, image))
> > +            return ISL_AUX_USAGE_HIZ;
> > +         else
> > +            return ISL_AUX_USAGE_NONE;
> > +      } else {
> > +         return image->planes[plane].aux_usage;
> > +      }
> > +
> >
> >     case VK_IMAGE_LAYOUT_PRESENT_SRC_KHR:
> >        assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> > @@ -845,8 +849,14 @@ anv_layout_to_aux_usage(const struct
> gen_device_info * const devinfo,
> >
> >     /* Rendering Layouts */
> >     case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL:
> > -      assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> > -      unreachable("Color images are not yet supported.");
> > +      assert(aspect & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV);
> > +      if (image->planes[plane].aux_usage == ISL_AUX_USAGE_NONE) {
> > +         assert(image->samples == 1);
> > +         return ISL_AUX_USAGE_CCS_D;
>
> Just checking that I understand what is going on. Earlier in this function
> there is early return for "image->planes[plane].aux_surface.isl.size ==
> 0".
> That means that we end up here only if there is auxiliary. Moreover if
> CCS_E
> should be used then image->planes[plane].aux_usage is set already?
>

Yes and no.  Yes, we only get here if we actually have an aux surface.  For
color images, aux_usage is the "default" aux_usage.  Specifically, it's
CCS_E for images where we can enable proper compression and NONE when we
cannot.  It's a bit of an artifact of history and we should probably change
it at some point.  For aux_usage == NONE, we can still enable CCS_D for
color attachments and we do a full resolve when we transition from
COLOR_ATTACHMENT_OPTIMAL to basically any other layout.

We could also enable CCS_E if we wanted in certain cases, but it's not
clear that doing so would actually be faster since a full resolve of a
CCS_E image can actually be more expensive than a full resolve of CCS_D.
(For CCS_D the hardware can just ignore any blocks that aren't clear
whereas a full resolve with CCS_E will touch almost every pixel.)


> > +      } else {
> > +         assert(image->planes[plane].aux_usage != ISL_AUX_USAGE_CCS_D);
> > +         return image->planes[plane].aux_usage;
> > +      }
> >
> >     case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL:
> >     case VK_IMAGE_LAYOUT_DEPTH_ATTACHMENT_STENCIL_READ_ONLY_OPTIMAL_KHR:
> > --
> > 2.5.0.400.gff86faf
> >
> > _______________________________________________
> > 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/20180122/532b343d/attachment.html>