On Mon, Nov 27, 2017 at 07:06:15PM -0800, Jason Ekstrand wrote:
> Previously, we would always apply the layout transition at the beginning
> of the subpass and then do the clear whether fast or slow.  This meant
> that there were some cases, specifically when the initial layout is
> VK_IMAGE_LAYOUT_UNDEFINED, where we would end up doing a fast-clear or
> ambiguate followed immediately by a fast-clear.  This probably isn't
> terribly expensive, but it is a waste that we can avoid easily enough
> now that we're doing everything at the same time in begin_subpass.

Had to read it thru a few times but couldn't spot anything amiss:

Reviewed-by: Topi Pohjolainen <[email protected]>

> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 146 
> +++++++++++++++++++++++++++----------
>  1 file changed, 106 insertions(+), 40 deletions(-)
> 
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 2c4ab38..3473fdd 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -3009,6 +3009,101 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer 
> *cmd_buffer,
>     VkRect2D render_area = cmd_buffer->state.render_area;
>     struct anv_framebuffer *fb = cmd_buffer->state.framebuffer;
>  
> +   /* First, walk through the attachments and handle any full-surface
> +    * fast-clears.  These are special because they do an implicit layout
> +    * transition and allow us to potentially avoid a resolve later.
> +    */
> +   for (uint32_t i = 0; i < subpass->attachment_count; ++i) {
> +      const uint32_t a = subpass->attachments[i].attachment;
> +      if (a == VK_ATTACHMENT_UNUSED)
> +         continue;
> +
> +      assert(a < cmd_state->pass->attachment_count);
> +      struct anv_attachment_state *att_state = &cmd_state->attachments[a];
> +
> +      struct anv_image_view *iview = fb->attachments[a];
> +      const struct anv_image *image = iview->image;
> +
> +      if (!att_state->pending_clear_aspects)
> +         continue;
> +
> +      if (!att_state->fast_clear)
> +         continue;
> +
> +      if (render_area.offset.x != 0 ||
> +          render_area.offset.y != 0 ||
> +          render_area.extent.width != iview->extent.width ||
> +          render_area.extent.height != iview->extent.height)
> +         continue;
> +
> +      if (att_state->pending_clear_aspects & VK_IMAGE_ASPECT_COLOR_BIT) {
> +         assert(att_state->pending_clear_aspects == 
> VK_IMAGE_ASPECT_COLOR_BIT);
> +
> +         /* Multi-planar images are not supported as attachments */
> +         assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> +         assert(image->n_planes == 1);
> +
> +         anv_image_ccs_op(cmd_buffer, image, VK_IMAGE_ASPECT_COLOR_BIT,
> +                          iview->planes[0].isl.base_level,
> +                          iview->planes[0].isl.base_array_layer,
> +                          fb->layers,
> +                          ISL_AUX_OP_FAST_CLEAR, false);
> +
> +         genX(copy_fast_clear_dwords)(cmd_buffer, att_state->color.state,
> +                                      image, VK_IMAGE_ASPECT_COLOR_BIT,
> +                                      iview->planes[0].isl.base_level,
> +                                      true /* copy from ss */);
> +
> +         /* Fast-clears impact whether or not a resolve will be necessary. */
> +         if (image->planes[0].aux_usage == ISL_AUX_USAGE_CCS_E &&
> +             att_state->clear_color_is_zero) {
> +            /* This image always has the auxiliary buffer enabled. We can
> +             * mark the subresource as not needing a resolve because the
> +             * clear color will match what's in every RENDER_SURFACE_STATE
> +             * object when it's being used for sampling.
> +             */
> +            clear_image_needs_resolve_bits(cmd_buffer, iview->image,
> +                                           VK_IMAGE_ASPECT_COLOR_BIT,
> +                                           iview->planes[0].isl.base_level,
> +                                           ANV_IMAGE_HAS_FAST_CLEAR_BIT);
> +         } else {
> +            set_image_needs_resolve_bits(cmd_buffer, iview->image,
> +                                         VK_IMAGE_ASPECT_COLOR_BIT,
> +                                         iview->planes[0].isl.base_level,
> +                                         ANV_IMAGE_HAS_FAST_CLEAR_BIT);
> +         }
> +
> +         att_state->pending_clear_aspects = 0;
> +         att_state->current_layout = 
> VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
> +      }
> +
> +      /* We only key the HiZ clear off of DEPTH_BIT because a fast stencil
> +       * clear doesn't reset the HiZ buffer contents the way we want.
> +       */
> +      if (att_state->pending_clear_aspects & VK_IMAGE_ASPECT_DEPTH_BIT) {
> +         /* 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);
> +         assert(fb->layers == 1);
> +
> +         anv_image_hiz_clear(cmd_buffer, image,
> +                             att_state->pending_clear_aspects,
> +                             iview->planes[0].isl.base_level,
> +                             iview->planes[0].isl.base_array_layer,
> +                             fb->layers, render_area,
> +                             att_state->clear_value.depthStencil.stencil);
> +
> +         att_state->pending_clear_aspects = 0;
> +         att_state->current_layout =
> +            VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL;
> +      }
> +   }
> +
> +
> +   /* Now that we've handled any implicit transitions due to full-surface 
> fast
> +    * clears, we walk the attachments again and handle all of the other 
> clears
> +    * and transitions.
> +    */
>     for (uint32_t i = 0; i < subpass->attachment_count; ++i) {
>        const uint32_t a = subpass->attachments[i].attachment;
>        if (a == VK_ATTACHMENT_UNUSED)
> @@ -3060,46 +3155,17 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer 
> *cmd_buffer,
>           assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
>           assert(image->n_planes == 1);
>  
> -         if (att_state->fast_clear) {
> -            anv_image_ccs_op(cmd_buffer, image, VK_IMAGE_ASPECT_COLOR_BIT,
> -                             iview->planes[0].isl.base_level,
> -                             iview->planes[0].isl.base_array_layer,
> -                             fb->layers,
> -                             ISL_AUX_OP_FAST_CLEAR, false);
> -
> -            genX(copy_fast_clear_dwords)(cmd_buffer, att_state->color.state,
> -                                         image, VK_IMAGE_ASPECT_COLOR_BIT,
> -                                         iview->planes[0].isl.base_level,
> -                                         true /* copy from ss */);
> -
> -            /* Fast-clears impact whether or not a resolve will be 
> necessary. */
> -            if (image->planes[0].aux_usage == ISL_AUX_USAGE_CCS_E &&
> -                att_state->clear_color_is_zero) {
> -               /* This image always has the auxiliary buffer enabled. We can
> -                * mark the subresource as not needing a resolve because the
> -                * clear color will match what's in every RENDER_SURFACE_STATE
> -                * object when it's being used for sampling.
> -                */
> -               clear_image_needs_resolve_bits(cmd_buffer, iview->image,
> -                                              VK_IMAGE_ASPECT_COLOR_BIT,
> -                                              
> iview->planes[0].isl.base_level,
> -                                              ANV_IMAGE_HAS_FAST_CLEAR_BIT);
> -            } else {
> -               set_image_needs_resolve_bits(cmd_buffer, iview->image,
> -                                            VK_IMAGE_ASPECT_COLOR_BIT,
> -                                            iview->planes[0].isl.base_level,
> -                                            ANV_IMAGE_HAS_FAST_CLEAR_BIT);
> -            }
> -         } else {
> -            anv_image_clear_color(cmd_buffer, image, 
> VK_IMAGE_ASPECT_COLOR_BIT,
> -                                  att_state->aux_usage,
> -                                  iview->planes[0].isl.format,
> -                                  iview->planes[0].isl.swizzle,
> -                                  iview->planes[0].isl.base_level,
> -                                  iview->planes[0].isl.base_array_layer,
> -                                  fb->layers, render_area,
> -                                  
> vk_to_isl_color(att_state->clear_value.color));
> -         }
> +         /* Color fast-clears are handled earlier */
> +         assert(!att_state->fast_clear);
> +
> +         anv_image_clear_color(cmd_buffer, image, VK_IMAGE_ASPECT_COLOR_BIT,
> +                               att_state->aux_usage,
> +                               iview->planes[0].isl.format,
> +                               iview->planes[0].isl.swizzle,
> +                               iview->planes[0].isl.base_level,
> +                               iview->planes[0].isl.base_array_layer,
> +                               fb->layers, render_area,
> +                               
> vk_to_isl_color(att_state->clear_value.color));
>        } else if (att_state->pending_clear_aspects & 
> (VK_IMAGE_ASPECT_DEPTH_BIT |
>                                                       
> VK_IMAGE_ASPECT_STENCIL_BIT)) {
>           if (att_state->fast_clear) {
> -- 
> 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

Reply via email to