On Wed, Feb 14, 2018 at 03:53:19PM -0800, Nanley Chery wrote:
> On Wed, Feb 14, 2018 at 12:16:33PM -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.
> > ---
> > src/intel/vulkan/genX_cmd_buffer.c | 62
> > +++++++++++++++++++++++---------------
> > 1 file changed, 38 insertions(+), 24 deletions(-)
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > b/src/intel/vulkan/genX_cmd_buffer.c
> > index 7f06441..453aea8 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -3335,6 +3335,12 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> > *cmd_buffer,
> > target_layout = subpass->attachments[i].layout;
> > }
> >
> > + /* 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 (image->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV) {
> > assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> >
> > @@ -3348,30 +3354,8 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> > *cmd_buffer,
> > 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);
> > - }
> > - 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;
> > -
> > - if (att_state->fast_clear) {
> > + if ((att_state->pending_clear_aspects &
> > VK_IMAGE_ASPECT_COLOR_BIT) &&
> > + att_state->fast_clear) {
> > /* 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);
> > @@ -3381,6 +3365,13 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> > *cmd_buffer,
> > 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.
> > + */
>
> Why not do the same optimization for depth?
>
I did some performance testing on this patch and couldn't measure any
improvement in the Sascha Willems Vulkan demos. It also decreases the
readability of this function, so I'm no longer seeing this as net win.
I think we should instead do this for the depth case. By doing this
optimization in the depth case I noticed a 10fps increase in at least 3
out of 5 demos.
The diff of the quick hack I did for depth is included below:
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -3353,8 +3353,18 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
*cmd_buffer,
base_layer, layer_count,
att_state->current_layout, target_layout);
} else if (image->aspects & VK_IMAGE_ASPECT_DEPTH_BIT) {
+
+ if (att_state->fast_clear && (
+ render_area.offset.x == 0 ||
+ render_area.offset.y == 0 ||
+ render_area.extent.width == iview->extent.width ||
+ render_area.extent.height == iview->extent.height)) {
+ printf("Avoiding transition before fast-clearing depth\n");
+ } else {
+
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);
-Nanley
> -Nanley
>
> > + base_layer++;
> > + layer_count--;
> > +
> > genX(copy_fast_clear_dwords)(cmd_buffer,
> > att_state->color.state,
> > image, VK_IMAGE_ASPECT_COLOR_BIT,
> > true /* copy from ss */);
> > @@ -3401,6 +3392,29 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> > *cmd_buffer,
> > }
> > }
> >
> > + 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) {
> > + 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,
> > --
> > 2.5.0.400.gff86faf
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
