[Mesa-dev] [PATCH] anv/cmd_buffer: Avoid unnecessary transitions before fast clears
Jason Ekstrand
jason at jlekstrand.net
Mon Feb 26 16:05:11 UTC 2018
On Sun, Feb 25, 2018 at 9:57 PM, Michael Schellenberger Costa <
mschellenbergercosta at googlemail.com> wrote:
> HI Jason,
>
> -----Ursprüngliche Nachricht-----
> Von: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] Im Auftrag
> von Jason Ekstrand
> Gesendet: Montag, 26. Februar 2018 03:33
> An: mesa-dev at lists.freedesktop.org
> Cc: Jason Ekstrand <jason.ekstrand at intel.com>
> Betreff: [Mesa-dev] [PATCH] anv/cmd_buffer: Avoid unnecessary transitions
> before fast clears
>
> 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 isn't usually
> 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.
>
> This improves the performance of the Sascha multisampling demo on my
> laptop by around 10% by avoiding a duplicate MCS clear.
> ---
> src/intel/vulkan/genX_cmd_buffer.c | 158 ++++++++++++++++++++++++------
> -------
> 1 file changed, 101 insertions(+), 57 deletions(-)
>
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 98e58ca..743c25c0 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -3436,43 +3436,24 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> *cmd_buffer,
> target_layout = subpass->attachments[i].layout;
> }
>
> - if (image->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV) {
> - assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> -
> - uint32_t base_layer, layer_count;
> - if (image->type == VK_IMAGE_TYPE_3D) {
> - base_layer = 0;
> - layer_count = anv_minify(iview->image->extent.depth,
> - iview->planes[0].isl.base_level);
> - } else {
> - base_layer = iview->planes[0].isl.base_array_layer;
> - layer_count = fb->layers;
> - }
> -
> - transition_color_buffer(cmd_buffer, image,
> VK_IMAGE_ASPECT_COLOR_BIT,
> - iview->planes[0].isl.base_level, 1,
> - base_layer, layer_count,
> - att_state->current_layout,
> target_layout);
> - } else if (image->aspects & VK_IMAGE_ASPECT_DEPTH_BIT) {
> - transition_depth_buffer(cmd_buffer, image,
> - att_state->current_layout,
> target_layout);
> - att_state->aux_usage =
> - anv_layout_to_aux_usage(&cmd_buffer->device->info, image,
> - VK_IMAGE_ASPECT_DEPTH_BIT,
> target_layout);
> + uint32_t base_layer, layer_count;
> + if (image->type == VK_IMAGE_TYPE_3D) {
> + base_layer = 0;
> + layer_count = anv_minify(iview->image->extent.depth,
> + iview->planes[0].isl.base_level);
> + } else {
> + base_layer = iview->planes[0].isl.base_array_layer;
> + layer_count = fb->layers;
> }
> - att_state->current_layout = target_layout;
> -
> - 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);
>
> - uint32_t base_clear_layer = iview->planes[0].isl.base_
> array_layer;
> - uint32_t clear_layer_count = fb->layers;
> + /* Clears are based on the image view for 3D surfaces but
> transitions
> + * are done on an entire miplevel at a time.
> + */
> + uint32_t base_clear_layer = iview->planes[0].isl.base_array_layer;
> + uint32_t clear_layer_count = fb->layers;
>
> - if (att_state->fast_clear) {
> + if (att_state->fast_clear) {
> + if (att_state->pending_clear_aspects &
> VK_IMAGE_ASPECT_COLOR_BIT) {
> /* We only support fast-clears on the first layer */
> assert(iview->planes[0].isl.base_level == 0);
> assert(iview->planes[0].isl.base_array_layer == 0);
> @@ -3484,9 +3465,17 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> *cmd_buffer,
> anv_image_mcs_op(cmd_buffer, image,
> VK_IMAGE_ASPECT_COLOR_BIT,
> 0, 1, ISL_AUX_OP_FAST_CLEAR, false);
> }
> +
> base_clear_layer++;
> clear_layer_count--;
>
> + /* Performing a fast clear takes care of all our transition
> needs
> + * for the first slice. Increment the base layer and layer
> count
> + * so that later transitions don't touch layer 0.
> + */
> + base_layer++;
> + layer_count--;
> Code and Comment do not match here. Given the original code do you mean
> Increment base_layer and decrement layer_count?
>
Yup. Fixed.
> --Michael
> +
> genX(copy_fast_clear_dwords)(cmd_buffer,
> att_state->color.state,
> image, VK_IMAGE_ASPECT_COLOR_BIT,
> true /* copy from ss */);
> @@ -3507,6 +3496,59 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> *cmd_buffer,
> }
> }
>
> + if ((att_state->pending_clear_aspects &
> VK_IMAGE_ASPECT_DEPTH_BIT) &&
> + render_area.offset.x == 0 && render_area.offset.y == 0 &&
> + render_area.extent.width == iview->image->extent.width &&
> + render_area.extent.height == iview->image->extent.height) {
> + /* We currently only support HiZ for single-layer images */
> + assert(iview->image->planes[0].aux_usage ==
> ISL_AUX_USAGE_HIZ);
> + 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,
> + 0, 1, render_area,
> + att_state->clear_value.
> depthStencil.stencil);
> +
> + /* Performing a fast clear takes care of all our transition
> needs
> + * for the attachment. Set clear_layer_count and layer_count
> to
> + * indicate that no clears or transitions need to be done.
> + */
> + clear_layer_count = 0;
> + layer_count = 0;
> + }
> + }
> +
> + if (image->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV) {
> + assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> +
> + if (layer_count > 0) {
> + transition_color_buffer(cmd_buffer, image,
> + VK_IMAGE_ASPECT_COLOR_BIT,
> + iview->planes[0].isl.base_level, 1,
> + base_layer, layer_count,
> + att_state->current_layout,
> target_layout);
> + }
> + } else if (image->aspects & VK_IMAGE_ASPECT_DEPTH_BIT) {
> + if (layer_count > 0) {
> + transition_depth_buffer(cmd_buffer, image,
> + att_state->current_layout,
> target_layout);
> + }
> + att_state->aux_usage =
> + anv_layout_to_aux_usage(&cmd_buffer->device->info, image,
> + VK_IMAGE_ASPECT_DEPTH_BIT,
> target_layout);
> + }
> + att_state->current_layout = target_layout;
> +
> + 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);
> +
> if (clear_layer_count > 0) {
> assert(image->n_planes == 1);
> anv_image_clear_color(cmd_buffer, image,
> VK_IMAGE_ASPECT_COLOR_BIT,
> @@ -3520,30 +3562,32 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> *cmd_buffer,
> }
> } else if (att_state->pending_clear_aspects &
> (VK_IMAGE_ASPECT_DEPTH_BIT |
>
> VK_IMAGE_ASPECT_STENCIL_BIT)) {
> - if (att_state->fast_clear) {
> - /* We currently only support HiZ for single-layer images */
> - if (att_state->pending_clear_aspects &
> VK_IMAGE_ASPECT_DEPTH_BIT) {
> - assert(iview->image->planes[0].aux_usage ==
> ISL_AUX_USAGE_HIZ);
> - assert(iview->planes[0].isl.base_level == 0);
> - assert(iview->planes[0].isl.base_array_layer == 0);
> - assert(fb->layers == 1);
> - }
> + if (clear_layer_count > 0) {
> + if (att_state->fast_clear) {
> + /* We currently only support HiZ for single-layer images */
> + if (att_state->pending_clear_aspects &
> VK_IMAGE_ASPECT_DEPTH_BIT) {
> + assert(iview->image->planes[0].aux_usage ==
> ISL_AUX_USAGE_HIZ);
> + 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);
> - } else {
> - anv_image_clear_depth_stencil(cmd_buffer, image,
> - att_state->pending_clear_
> aspects,
> - att_state->aux_usage,
> - iview->planes[0].isl.base_
> level,
> - iview->planes[0].isl.base_
> array_layer,
> - fb->layers, render_area,
> - att_state->clear_value.
> depthStencil.depth,
> - att_state->clear_value.
> depthStencil.stencil);
> + anv_image_hiz_clear(cmd_buffer, image,
> + att_state->pending_clear_aspects,
> + iview->planes[0].isl.base_level,
> + base_clear_layer, clear_layer_count,
> + render_area,
> + att_state->clear_value.
> depthStencil.stencil);
> + } else {
> + anv_image_clear_depth_stencil(cmd_buffer, image,
> + att_state->pending_clear_
> aspects,
> + att_state->aux_usage,
> + iview->planes[0].isl.base_
> level,
> + base_clear_layer,
> clear_layer_count,
> + render_area,
> + att_state->clear_value.
> depthStencil.depth,
> + att_state->clear_value.
> depthStencil.stencil);
> + }
> }
> } else {
> assert(att_state->pending_clear_aspects == 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/20180226/166a240a/attachment-0001.html>
More information about the mesa-dev
mailing list