[Mesa-dev] [PATCH 03/18] anv/cmd_buffer: Replace layout_to_hiz_usage()

Jason Ekstrand jason at jlekstrand.net
Tue Feb 28 16:16:43 UTC 2017


On Tue, Feb 28, 2017 at 5:54 AM, Nanley Chery <nanleychery at gmail.com> wrote:

> On Mon, Feb 27, 2017 at 08:41:56PM -0800, Jason Ekstrand wrote:
> > On Feb 27, 2017 5:21 PM, "Nanley Chery" <nanleychery at gmail.com> wrote:
> >
> > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > ---
> >  src/intel/vulkan/genX_cmd_buffer.c | 57 ++++++++++--------------------
> > --------
> >  1 file changed, 15 insertions(+), 42 deletions(-)
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > b/src/intel/vulkan/genX_cmd_buffer.c
> > index 5171e6f587..60230bf14b 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -326,27 +326,6 @@ need_input_attachment_state(const struct
> > anv_render_pass_attachment *att)
> >     return vk_format_is_color(att->format);
> >  }
> >
> > -static enum isl_aux_usage
> > -layout_to_hiz_usage(VkImageLayout layout, uint8_t samples)
> > -{
> > -   switch (layout) {
> > -   case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL:
> > -      return ISL_AUX_USAGE_HIZ;
> > -   case VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL:
> > -   case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL:
> > -      if (anv_can_sample_with_hiz(GEN_GEN, samples))
> > -         return ISL_AUX_USAGE_HIZ;
> > -      /* Fall-through */
> > -   case VK_IMAGE_LAYOUT_GENERAL:
> > -      /* This buffer could be used as a source or destination in a
> transfer
> > -       * operation. Transfer operations current don't perform
> HiZ-enabled
> > reads
> > -       * and writes.
> > -       */
> > -   default:
> > -      return ISL_AUX_USAGE_NONE;
> > -   }
> > -}
> > -
> >  /* Transitions a HiZ-enabled depth buffer from one layout to another.
> > Unless
> >   * the initial layout is undefined, the HiZ buffer and depth buffer will
> >   * represent the same data at the end of this operation.
> > @@ -362,21 +341,15 @@ transition_depth_buffer(struct anv_cmd_buffer
> > *cmd_buffer,
> >     if (image->aux_usage != ISL_AUX_USAGE_HIZ || final_layout ==
> > initial_layout)
> >        return;
> >
> > -   const bool hiz_enabled = layout_to_hiz_usage(initial_layout,
> > image->samples) ==
> > -                            ISL_AUX_USAGE_HIZ;
> > -   const bool enable_hiz = layout_to_hiz_usage(final_layout,
> > image->samples) ==
> > -                           ISL_AUX_USAGE_HIZ;
> > +   const bool hiz_enabled = ISL_AUX_USAGE_HIZ ==
> > +      anv_layout_to_aux_usage(GEN_GEN, image, image->aspects,
> > +                              initial_layout, final_layout);
> > +   const bool enable_hiz = ISL_AUX_USAGE_HIZ ==
> > +      anv_layout_to_aux_usage(GEN_GEN, image, image->aspects,
> > +                              final_layout, final_layout);
> >
> >     enum blorp_hiz_op hiz_op;
> > -   if (initial_layout == VK_IMAGE_LAYOUT_UNDEFINED) {
> >
> >
> > I'm not sure how I feel about handling this in layout_to_aux_usage.  It
> > seems like a conflation of two things.  If this is the only reason for
> the
> > future_layout parameter, then it seems like it would be better handled
> > here.  In any case, I'll keep reading
> >
>
> Yes, that is the only reason for the future_layout parameter. I can
> remove it if you'd like.
>
> Currently the layout_to_aux_usage function determines the optimal
> aux_usage for all device accesses: sampling, rendering, transferring,
> and resolves. Removing the future_layout parameter would remove its
> responsibility for the last access type.
>

In my brain, a transition from LAYOUT_UNDEFINED to LAYOUT_* is a transition
from AUX_USAGE_NONE to AUX_USAGE_*.  However, we know based on some other
bits of the driver, that we can skip that particular resolve because the
HiZ surface is in a non-hanging state.  Also, the second parameter makes
the function far less obvious and every other call sight of the function,
even though it doesn't care, has to think about that parmeter.


> -Nanley
>
> > -      /* We've already initialized the aux HiZ buffer at BindImageMemory
> > time,
> > -       * so there's no need to perform a HIZ resolve or clear to avoid
> GPU
> > hangs.
> > -       * This initial layout indicates that the user doesn't care about
> > the data
> > -       * that's currently in the buffer, so resolves are not necessary
> > except
> > -       * for the special case noted below.
> > -       */
> > -      hiz_op = BLORP_HIZ_OP_NONE;
> > -   } else if (hiz_enabled && !enable_hiz) {
> > +   if (hiz_enabled && !enable_hiz) {
> >        hiz_op = BLORP_HIZ_OP_DEPTH_RESOLVE;
> >     } else if (!hiz_enabled && enable_hiz) {
> >        hiz_op = BLORP_HIZ_OP_HIZ_RESOLVE;
> > @@ -556,12 +529,11 @@ genX(cmd_buffer_setup_attachments)(struct
> > anv_cmd_buffer *cmd_buffer,
> >                                    state->attachments[i].aux_usage,
> >                                    state->attachments[i].color_
> rt_state);
> >           } else {
> > -            if (iview->image->aux_usage == ISL_AUX_USAGE_HIZ) {
> > -               state->attachments[i].aux_usage =
> > -                  layout_to_hiz_usage(att->initial_layout,
> > iview->image->samples);
> > -            } else {
> > -               state->attachments[i].aux_usage = ISL_AUX_USAGE_NONE;
> > -            }
> > +            /* This field will be initialized after the first subpass
> > +             * transition.
> > +             */
> > +            state->attachments[i].aux_usage = ISL_AUX_USAGE_NONE;
> > +
> >              state->attachments[i].input_aux_usage = ISL_AUX_USAGE_NONE;
> >           }
> >
> > @@ -2399,8 +2371,9 @@ genX(cmd_buffer_set_subpass)(struct anv_cmd_buffer
> > *cmd_buffer,
> >        cmd_buffer->state.attachments[ds].current_layout =
> >           cmd_buffer->state.subpass->depth_stencil_layout;
> >        cmd_buffer->state.attachments[ds].aux_usage =
> > -         layout_to_hiz_usage(cmd_buffer->state.subpass->depth_stenci
> > l_layout,
> > -                             iview->image->samples);
> > +         anv_layout_to_aux_usage(GEN_GEN, iview->image,
> iview->aspect_mask,
> > +            cmd_buffer->state.subpass->depth_stencil_layout,
> > +            cmd_buffer->state.subpass->depth_stencil_layout);
> >     }
> >
> >     cmd_buffer_emit_depth_stencil(cmd_buffer);
> > --
> > 2.11.1
> >
> > _______________________________________________
> > 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/20170228/6c48a6e1/attachment.html>


More information about the mesa-dev mailing list