[Mesa-dev] [PATCH 07/14] anv/cmd_buffer: Decide whether or not to HiZ clear up-front

Jason Ekstrand jason at jlekstrand.net
Wed Feb 14 02:37:37 UTC 2018


You're going to want to re-review this one when I send the v2.  It changed
quite a bit.

On Tue, Feb 13, 2018 at 2:44 PM, Jason Ekstrand <jason at jlekstrand.net>
wrote:

> On Tue, Feb 13, 2018 at 11:02 AM, Nanley Chery <nanleychery at gmail.com>
> wrote:
>
>> On Mon, Feb 05, 2018 at 02:34:56PM -0800, Jason Ekstrand wrote:
>> > This moves the decision out of begin_subpass and into BeginRenderPass
>> > like the decision for color clears.  We use a similar name for the
>> > function for depth/stencil as for color even though no aux usage is
>> > really getting computed.
>> > ---
>> >  src/intel/vulkan/genX_cmd_buffer.c | 84 +++++++++++++++++++++++-------
>> --------
>> >  1 file changed, 50 insertions(+), 34 deletions(-)
>> >
>> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
>> b/src/intel/vulkan/genX_cmd_buffer.c
>> > index 21fdc6b..ab79fbf 100644
>> > --- a/src/intel/vulkan/genX_cmd_buffer.c
>> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
>> > @@ -350,6 +350,52 @@ color_attachment_compute_aux_usage(struct
>> anv_device * device,
>> >     }
>> >  }
>> >
>> > +static void
>> > +depth_stencil_attachment_compute_aux_usage(struct anv_device *device,
>> > +                                           struct anv_cmd_state
>> *cmd_state,
>> > +                                           uint32_t att, VkRect2D
>> render_area)
>> > +{
>> > +   struct anv_attachment_state *att_state =
>> &cmd_state->attachments[att];
>> > +   struct anv_image_view *iview = cmd_state->framebuffer->attach
>> ments[att];
>> > +
>> > +   /* These will be initialized after the first subpass transition. */
>> > +   att_state->aux_usage = ISL_AUX_USAGE_NONE;
>> > +   att_state->input_aux_usage = ISL_AUX_USAGE_NONE;
>> > +
>> > +   if (att_state->aux_usage != ISL_AUX_USAGE_HIZ) {
>>
>> We set this to NONE 3 lines above. I think you meant to have your
>> variable be: iview->image->planes[plane].aux_usage ?
>>
>
> Yup.  You're right.  Fixed locally.
>
> This means I accidentally disabled HiZ clears entirely. :(  This is going
> to need another jenkins run.
>
>
>> This is a nice cleanup. With the above fixed, this patch is
>> Reviewed-by: Nanley Chery <nanley.g.chery at intel.com>
>>
>>
>> > +      att_state->fast_clear = false;
>> > +      return;
>> > +   } else if (!(att_state->pending_clear_aspects &
>> VK_IMAGE_ASPECT_DEPTH_BIT)) {
>> > +      /* If we're just clearing stencil, we can always HiZ clear */
>> > +      att_state->fast_clear = true;
>> > +      return;
>> > +   }
>> > +
>> > +   if (!blorp_can_hiz_clear_depth(GEN_GEN,
>> > +                                  iview->planes[0].isl.format,
>> > +                                  iview->image->samples,
>> > +                                  render_area.offset.x,
>> > +                                  render_area.offset.y,
>> > +                                  render_area.offset.x +
>> > +                                  render_area.extent.width,
>> > +                                  render_area.offset.y +
>> > +                                  render_area.extent.height)) {
>> > +      att_state->fast_clear = false;
>> > +   } else if (att_state->clear_value.depthStencil.depth !=
>> ANV_HZ_FC_VAL) {
>> > +      att_state->fast_clear = false;
>> > +   } else if (GEN_GEN == 8 &&
>> > +              anv_can_sample_with_hiz(&device->info, iview->image)) {
>> > +      /* Only gen9+ supports returning ANV_HZ_FC_VAL when sampling a
>> > +       * fast-cleared portion of a HiZ buffer. Testing has revealed
>> that Gen8
>> > +       * only supports returning 0.0f. Gens prior to gen8 do not
>> support this
>> > +       * feature at all.
>> > +       */
>> > +      att_state->fast_clear = false;
>> > +   } else {
>> > +      att_state->fast_clear = true;
>> > +   }
>> > +}
>> > +
>> >  static bool
>> >  need_input_attachment_state(const struct anv_render_pass_attachment
>> *att)
>> >  {
>> > @@ -1125,12 +1171,9 @@ genX(cmd_buffer_setup_attachments)(struct
>> anv_cmd_buffer *cmd_buffer,
>> >              add_image_view_relocs(cmd_buffer, iview, 0,
>> >                                    state->attachments[i].color);
>> >           } else {
>> > -            /* 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;
>> > +            depth_stencil_attachment_compu
>> te_aux_usage(cmd_buffer->device,
>> > +                                                       state, i,
>> > +
>>  begin->renderArea);
>> >           }
>> >
>> >           if (need_input_attachment_state(&pass->attachments[i])) {
>> > @@ -3541,34 +3584,7 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
>> *cmd_buffer,
>> >                                 VK_IMAGE_ASPECT_STENCIL_BIT));
>> >
>> >        if (att_state->pending_clear_aspects) {
>> > -         bool clear_with_hiz = att_state->aux_usage ==
>> ISL_AUX_USAGE_HIZ;
>> > -         if (clear_with_hiz &&
>> > -             (att_state->pending_clear_aspects &
>> VK_IMAGE_ASPECT_DEPTH_BIT)) {
>> > -            if (!blorp_can_hiz_clear_depth(GEN_GEN,
>> > -                                           iview->planes[0].isl.format,
>> > -                                           iview->image->samples,
>> > -                                           render_area.offset.x,
>> > -                                           render_area.offset.y,
>> > -                                           render_area.offset.x +
>> > -                                           render_area.extent.width,
>> > -                                           render_area.offset.y +
>> > -                                           render_area.extent.height))
>> {
>> > -               clear_with_hiz = false;
>> > -            } else if (att_state->clear_value.depthStencil.depth !=
>> ANV_HZ_FC_VAL) {
>> > -               clear_with_hiz = false;
>> > -            } else if (GEN_GEN == 8 &&
>> > -                       anv_can_sample_with_hiz(&cmd_
>> buffer->device->info,
>> > -                                               iview->image)) {
>> > -               /* Only gen9+ supports returning ANV_HZ_FC_VAL when
>> sampling a
>> > -                * fast-cleared portion of a HiZ buffer. Testing has
>> revealed
>> > -                * that Gen8 only supports returning 0.0f. Gens prior
>> to gen8
>> > -                * do not support this feature at all.
>> > -                */
>> > -               clear_with_hiz = false;
>> > -            }
>> > -         }
>> > -
>> > -         if (clear_with_hiz) {
>> > +         if (att_state->fast_clear) {
>> >              /* We currently only support HiZ for single-layer images */
>> >              assert(iview->planes[0].isl.base_level == 0);
>> >              assert(iview->planes[0].isl.base_array_layer == 0);
>> > --
>> > 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/20180213/2d8fd349/attachment-0001.html>


More information about the mesa-dev mailing list