Reviewed-by: Bas Nieuwenhuizen <b...@basnieuwenhuizen.nl>

I'm wondering though, doesn't this result in more saves/restores, as
we now do it for each part of a subpass clear separately?

On Mon, Apr 9, 2018 at 11:10 PM, Samuel Pitoiset
<samuel.pitoi...@gmail.com> wrote:
> This removes the emission of unnecessary states, for example
> when performing a fast depth stencil clear (ie. clearing htile),
> we don't need to save/restore the graphics pipeline.
>
> For GFX9 chips that have the scissor bug workaround, that
> should also reduce the number of partial flushes.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com>
> ---
>  src/amd/vulkan/radv_meta_bufimage.c |  8 +++++
>  src/amd/vulkan/radv_meta_clear.c    | 47 +++++++++--------------------
>  2 files changed, 22 insertions(+), 33 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_meta_bufimage.c 
> b/src/amd/vulkan/radv_meta_bufimage.c
> index 69e15d3213..5018ce1f2e 100644
> --- a/src/amd/vulkan/radv_meta_bufimage.c
> +++ b/src/amd/vulkan/radv_meta_bufimage.c
> @@ -1242,8 +1242,14 @@ radv_meta_clear_image_cs(struct radv_cmd_buffer 
> *cmd_buffer,
>  {
>         VkPipeline pipeline = cmd_buffer->device->meta_state.cleari.pipeline;
>         struct radv_device *device = cmd_buffer->device;
> +       struct radv_meta_saved_state saved_state;
>         struct radv_image_view dst_iview;
>
> +       radv_meta_save(&saved_state, cmd_buffer,
> +                      RADV_META_SAVE_COMPUTE_PIPELINE |
> +                      RADV_META_SAVE_CONSTANTS |
> +                      RADV_META_SAVE_DESCRIPTORS);
> +
>         create_iview(cmd_buffer, dst, &dst_iview);
>         cleari_bind_descriptors(cmd_buffer, &dst_iview);
>
> @@ -1268,4 +1274,6 @@ radv_meta_clear_image_cs(struct radv_cmd_buffer 
> *cmd_buffer,
>                               push_constants);
>
>         radv_unaligned_dispatch(cmd_buffer, dst->image->info.width, 
> dst->image->info.height, 1);
> +
> +       radv_meta_restore(&saved_state, cmd_buffer);
>  }
> diff --git a/src/amd/vulkan/radv_meta_clear.c 
> b/src/amd/vulkan/radv_meta_clear.c
> index 016c1ee296..833e3cebab 100644
> --- a/src/amd/vulkan/radv_meta_clear.c
> +++ b/src/amd/vulkan/radv_meta_clear.c
> @@ -342,6 +342,7 @@ emit_color_clear(struct radv_cmd_buffer *cmd_buffer,
>         unsigned fs_key = radv_format_meta_fs_key(iview->vk_format);
>         VkClearColorValue clear_value = clear_att->clearValue.color;
>         VkCommandBuffer cmd_buffer_h = radv_cmd_buffer_to_handle(cmd_buffer);
> +       struct radv_meta_saved_state saved_state;
>         VkPipeline pipeline;
>
>         if (fs_key == -1) {
> @@ -359,6 +360,10 @@ emit_color_clear(struct radv_cmd_buffer *cmd_buffer,
>         assert(clear_att->aspectMask == VK_IMAGE_ASPECT_COLOR_BIT);
>         assert(clear_att->colorAttachment < subpass->color_count);
>
> +       radv_meta_save(&saved_state, cmd_buffer,
> +                      RADV_META_SAVE_GRAPHICS_PIPELINE |
> +                      RADV_META_SAVE_CONSTANTS);
> +
>         radv_CmdPushConstants(radv_cmd_buffer_to_handle(cmd_buffer),
>                               device->meta_state.clear_color_p_layout,
>                               VK_SHADER_STAGE_FRAGMENT_BIT, 0, 16,
> @@ -397,6 +402,8 @@ emit_color_clear(struct radv_cmd_buffer *cmd_buffer,
>         }
>
>         radv_cmd_buffer_set_subpass(cmd_buffer, subpass, false);
> +
> +       radv_meta_restore(&saved_state, cmd_buffer);
>  }
>
>
> @@ -613,12 +620,17 @@ emit_depthstencil_clear(struct radv_cmd_buffer 
> *cmd_buffer,
>         const uint32_t samples = iview->image->info.samples;
>         const uint32_t samples_log2 = ffs(samples) - 1;
>         VkCommandBuffer cmd_buffer_h = radv_cmd_buffer_to_handle(cmd_buffer);
> +       struct radv_meta_saved_state saved_state;
>
>         assert(pass_att != VK_ATTACHMENT_UNUSED);
>
>         if (!(aspects & VK_IMAGE_ASPECT_DEPTH_BIT))
>                 clear_value.depth = 1.0f;
>
> +       radv_meta_save(&saved_state, cmd_buffer,
> +                      RADV_META_SAVE_GRAPHICS_PIPELINE |
> +                      RADV_META_SAVE_CONSTANTS);
> +
>         radv_CmdPushConstants(radv_cmd_buffer_to_handle(cmd_buffer),
>                               device->meta_state.clear_depth_p_layout,
>                               VK_SHADER_STAGE_VERTEX_BIT, 0, 4,
> @@ -664,6 +676,8 @@ emit_depthstencil_clear(struct radv_cmd_buffer 
> *cmd_buffer,
>                 radv_CmdSetStencilReference(cmd_buffer_h, 
> VK_STENCIL_FACE_FRONT_BIT,
>                                                   prev_reference);
>         }
> +
> +       radv_meta_restore(&saved_state, cmd_buffer);
>  }
>
>  static bool
> @@ -1165,17 +1179,12 @@ void
>  radv_cmd_buffer_clear_subpass(struct radv_cmd_buffer *cmd_buffer)
>  {
>         struct radv_cmd_state *cmd_state = &cmd_buffer->state;
> -       struct radv_meta_saved_state saved_state;
>         enum radv_cmd_flush_bits pre_flush = 0;
>         enum radv_cmd_flush_bits post_flush = 0;
>
>         if (!radv_subpass_needs_clear(cmd_buffer))
>                 return;
>
> -       radv_meta_save(&saved_state, cmd_buffer,
> -                      RADV_META_SAVE_GRAPHICS_PIPELINE |
> -                      RADV_META_SAVE_CONSTANTS);
> -
>         for (uint32_t i = 0; i < cmd_state->subpass->color_count; ++i) {
>                 uint32_t a = 
> cmd_state->subpass->color_attachments[i].attachment;
>
> @@ -1210,7 +1219,6 @@ radv_cmd_buffer_clear_subpass(struct radv_cmd_buffer 
> *cmd_buffer)
>                                               &post_flush);
>         }
>
> -       radv_meta_restore(&saved_state, cmd_buffer);
>         cmd_buffer->state.flush_bits |= post_flush;
>  }
>
> @@ -1405,25 +1413,11 @@ void radv_CmdClearColorImage(
>  {
>         RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
>         RADV_FROM_HANDLE(radv_image, image, image_h);
> -       struct radv_meta_saved_state saved_state;
>         bool cs = cmd_buffer->queue_family_index == RADV_QUEUE_COMPUTE;
>
> -       if (cs) {
> -               radv_meta_save(&saved_state, cmd_buffer,
> -                              RADV_META_SAVE_COMPUTE_PIPELINE |
> -                              RADV_META_SAVE_CONSTANTS |
> -                              RADV_META_SAVE_DESCRIPTORS);
> -       } else {
> -               radv_meta_save(&saved_state, cmd_buffer,
> -                              RADV_META_SAVE_GRAPHICS_PIPELINE |
> -                              RADV_META_SAVE_CONSTANTS);
> -       }
> -
>         radv_cmd_clear_image(cmd_buffer, image, imageLayout,
>                              (const VkClearValue *) pColor,
>                              rangeCount, pRanges, cs);
> -
> -       radv_meta_restore(&saved_state, cmd_buffer);
>  }
>
>  void radv_CmdClearDepthStencilImage(
> @@ -1436,17 +1430,10 @@ void radv_CmdClearDepthStencilImage(
>  {
>         RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
>         RADV_FROM_HANDLE(radv_image, image, image_h);
> -       struct radv_meta_saved_state saved_state;
> -
> -       radv_meta_save(&saved_state, cmd_buffer,
> -                      RADV_META_SAVE_GRAPHICS_PIPELINE |
> -                      RADV_META_SAVE_CONSTANTS);
>
>         radv_cmd_clear_image(cmd_buffer, image, imageLayout,
>                              (const VkClearValue *) pDepthStencil,
>                              rangeCount, pRanges, false);
> -
> -       radv_meta_restore(&saved_state, cmd_buffer);
>  }
>
>  void radv_CmdClearAttachments(
> @@ -1457,17 +1444,12 @@ void radv_CmdClearAttachments(
>         const VkClearRect*                          pRects)
>  {
>         RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> -       struct radv_meta_saved_state saved_state;
>         enum radv_cmd_flush_bits pre_flush = 0;
>         enum radv_cmd_flush_bits post_flush = 0;
>
>         if (!cmd_buffer->state.subpass)
>                 return;
>
> -       radv_meta_save(&saved_state, cmd_buffer,
> -                      RADV_META_SAVE_GRAPHICS_PIPELINE |
> -                      RADV_META_SAVE_CONSTANTS);
> -
>         /* FINISHME: We can do better than this dumb loop. It thrashes too 
> much
>          * state.
>          */
> @@ -1478,6 +1460,5 @@ void radv_CmdClearAttachments(
>                 }
>         }
>
> -       radv_meta_restore(&saved_state, cmd_buffer);
>         cmd_buffer->state.flush_bits |= post_flush;
>  }
> --
> 2.17.0
>
> _______________________________________________
> 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

Reply via email to