[Mesa-dev] [PATCH 1/2] radv: handle depth/stencil image copy with layouts better. (v3)

Bas Nieuwenhuizen bas at basnieuwenhuizen.nl
Thu Dec 21 22:00:42 UTC 2017


Just a stylistic nit.

On Thu, Dec 21, 2017 at 8:25 AM, Dave Airlie <airlied at gmail.com> wrote:
> From: Dave Airlie <airlied at redhat.com>
>
> If we are doing a general->general transfer with HIZ enabled,
> we want to hit the tile surface disable bits in radv_emit_fb_ds_state,
> however we never get the current layout to know we are in general
> and meta hardcoded the transfer layout which is always tile enabled.
>
> This fixes:
> dEQP-VK.api.copy_and_blit.core.image_to_image.all_formats.depth_stencil.d32_sfloat_s8_uint_d32_sfloat_s8_uint.optimal_general
> dEQP-VK.api.copy_and_blit.core.image_to_image.all_formats.depth_stencil.d32_sfloat_s8_uint_d32_sfloat_s8_uint.general_general
>
> v2: refactor some shared helpers for blit patches
> v3: we only need multiple render passes as they should be compatible.
>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  src/amd/vulkan/radv_meta.h        |   1 +
>  src/amd/vulkan/radv_meta_blit2d.c | 130 ++++++++++++++++++++------------------
>  src/amd/vulkan/radv_meta_copy.c   |  20 +++++-
>  src/amd/vulkan/radv_private.h     |  18 +++++-
>  4 files changed, 104 insertions(+), 65 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_meta.h b/src/amd/vulkan/radv_meta.h
> index d10ec99413..3edf5fa646 100644
> --- a/src/amd/vulkan/radv_meta.h
> +++ b/src/amd/vulkan/radv_meta.h
> @@ -109,6 +109,7 @@ struct radv_meta_blit2d_surf {
>         unsigned level;
>         unsigned layer;
>         VkImageAspectFlags aspect_mask;
> +       VkImageLayout current_layout;
>  };
>
>  struct radv_meta_blit2d_buffer {
> diff --git a/src/amd/vulkan/radv_meta_blit2d.c b/src/amd/vulkan/radv_meta_blit2d.c
> index 08a1bae7c6..31570656ef 100644
> --- a/src/amd/vulkan/radv_meta_blit2d.c
> +++ b/src/amd/vulkan/radv_meta_blit2d.c
> @@ -278,10 +278,11 @@ radv_meta_blit2d_normal_dst(struct radv_cmd_buffer *cmd_buffer,
>
>                                 bind_pipeline(cmd_buffer, src_type, fs_key);
>                         } else if (aspect_mask == VK_IMAGE_ASPECT_DEPTH_BIT) {
> +                               int idx = radv_meta_blit_ds_to_type(dst->current_layout);
>                                 radv_CmdBeginRenderPass(radv_cmd_buffer_to_handle(cmd_buffer),
>                                                         &(VkRenderPassBeginInfo) {
>                                                                 .sType = VK_STRUCTURE_TYPE_RENDER_PASS_BEGIN_INFO,
> -                                                                       .renderPass = device->meta_state.blit2d.depth_only_rp,
> +                                                                       .renderPass = device->meta_state.blit2d.depth_only_rp[idx],
>                                                                         .framebuffer = dst_temps.fb,
>                                                                         .renderArea = {
>                                                                         .offset = { rects[r].dst_x, rects[r].dst_y, },
> @@ -295,10 +296,11 @@ radv_meta_blit2d_normal_dst(struct radv_cmd_buffer *cmd_buffer,
>                                 bind_depth_pipeline(cmd_buffer, src_type);
>
>                         } else if (aspect_mask == VK_IMAGE_ASPECT_STENCIL_BIT) {
> +                               int idx = radv_meta_blit_ds_to_type(dst->current_layout);
>                                 radv_CmdBeginRenderPass(radv_cmd_buffer_to_handle(cmd_buffer),
>                                                         &(VkRenderPassBeginInfo) {
>                                                                 .sType = VK_STRUCTURE_TYPE_RENDER_PASS_BEGIN_INFO,
> -                                                                       .renderPass = device->meta_state.blit2d.stencil_only_rp,
> +                                                                       .renderPass = device->meta_state.blit2d.stencil_only_rp[idx],
>                                                                         .framebuffer = dst_temps.fb,
>                                                                         .renderArea = {
>                                                                         .offset = { rects[r].dst_x, rects[r].dst_y, },
> @@ -614,10 +616,12 @@ radv_device_finish_meta_blit2d_state(struct radv_device *device)
>                                        &state->alloc);
>         }
>
> -       radv_DestroyRenderPass(radv_device_to_handle(device),
> -                              state->blit2d.depth_only_rp, &state->alloc);
> -       radv_DestroyRenderPass(radv_device_to_handle(device),
> -                              state->blit2d.stencil_only_rp, &state->alloc);
> +       for (unsigned j = 0; j < RADV_BLIT_DS_NUM_LAYOUTS; j++) {
> +               radv_DestroyRenderPass(radv_device_to_handle(device),
> +                                      state->blit2d.depth_only_rp[j], &state->alloc);
> +               radv_DestroyRenderPass(radv_device_to_handle(device),
> +                                      state->blit2d.stencil_only_rp[j], &state->alloc);
> +       }
>
>         for (unsigned src = 0; src < BLIT2D_NUM_SRC_TYPES; src++) {
>                 radv_DestroyPipelineLayout(radv_device_to_handle(device),
> @@ -859,34 +863,37 @@ blit2d_init_depth_only_pipeline(struct radv_device *device,
>                 },
>         };
>
> -       if (!device->meta_state.blit2d.depth_only_rp) {
> -               result = radv_CreateRenderPass(radv_device_to_handle(device),
> -                                              &(VkRenderPassCreateInfo) {
> -                                                      .sType = VK_STRUCTURE_TYPE_RENDER_PASS_CREATE_INFO,
> +       for (unsigned idx = 0; idx < RADV_BLIT_DS_NUM_LAYOUTS; idx++) {
> +               if (!device->meta_state.blit2d.depth_only_rp[idx]) {
> +                       VkImageLayout layout = radv_meta_blit_ds_to_layout(idx);
> +                       result = radv_CreateRenderPass(radv_device_to_handle(device),
> +                                                      &(VkRenderPassCreateInfo) {
> +                                                              .sType = VK_STRUCTURE_TYPE_RENDER_PASS_CREATE_INFO,
>                                                                .attachmentCount = 1,
>                                                                .pAttachments = &(VkAttachmentDescription) {
> -                                                              .format = VK_FORMAT_D32_SFLOAT,
> -                                                              .loadOp = VK_ATTACHMENT_LOAD_OP_LOAD,
> -                                                              .storeOp = VK_ATTACHMENT_STORE_OP_STORE,
> -                                                              .initialLayout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
> -                                                              .finalLayout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
> -                                                      },
> -                                                      .subpassCount = 1,
> -                                                      .pSubpasses = &(VkSubpassDescription) {
> -                                                      .pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS,
> -                                                      .inputAttachmentCount = 0,
> -                                                      .colorAttachmentCount = 0,
> -                                                      .pColorAttachments = NULL,
> -                                                      .pResolveAttachments = NULL,
> -                                                      .pDepthStencilAttachment = &(VkAttachmentReference) {
> -                                                              .attachment = 0,
> -                                                              .layout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
> -                                                      },
> -                                                      .preserveAttachmentCount = 1,
> -                                                      .pPreserveAttachments = (uint32_t[]) { 0 },
> -                                              },
> -                                                               .dependencyCount = 0,
> -                                                }, &device->meta_state.alloc, &device->meta_state.blit2d.depth_only_rp);
> +                                                                      .format = VK_FORMAT_D32_SFLOAT,
> +                                                                      .loadOp = VK_ATTACHMENT_LOAD_OP_LOAD,
> +                                                                      .storeOp = VK_ATTACHMENT_STORE_OP_STORE,
> +                                                                      .initialLayout = layout,
> +                                                                      .finalLayout = layout,
> +                                                              },
> +                                                              .subpassCount = 1,
> +                                                              .pSubpasses = &(VkSubpassDescription) {
> +                                                                      .pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS,
> +                                                                      .inputAttachmentCount = 0,
> +                                                                      .colorAttachmentCount = 0,
> +                                                                      .pColorAttachments = NULL,
> +                                                                      .pResolveAttachments = NULL,
> +                                                                      .pDepthStencilAttachment = &(VkAttachmentReference) {
> +                                                                              .attachment = 0,
> +                                                                              .layout = layout,
> +                                                                      },
> +                                                                      .preserveAttachmentCount = 1,
> +                                                                      .pPreserveAttachments = (uint32_t[]) { 0 },
> +                                                              },
> +                                                              .dependencyCount = 0,
> +                                                       }, &device->meta_state.alloc, &device->meta_state.blit2d.depth_only_rp[idx]);
> +               }
>         }
>
>         const VkGraphicsPipelineCreateInfo vk_pipeline_info = {
> @@ -945,7 +952,7 @@ blit2d_init_depth_only_pipeline(struct radv_device *device,
>                 },
>                 .flags = 0,
>                 .layout = device->meta_state.blit2d.p_layouts[src_type],
> -               .renderPass = device->meta_state.blit2d.depth_only_rp,
> +               .renderPass = device->meta_state.blit2d.depth_only_rp[0],
>                 .subpass = 0,
>         };
>
> @@ -1018,34 +1025,37 @@ blit2d_init_stencil_only_pipeline(struct radv_device *device,
>                 },
>         };
>
> -       if (!device->meta_state.blit2d.stencil_only_rp) {
> -               result = radv_CreateRenderPass(radv_device_to_handle(device),
> -                                              &(VkRenderPassCreateInfo) {
> -                                                      .sType = VK_STRUCTURE_TYPE_RENDER_PASS_CREATE_INFO,
> +       for (unsigned idx = 0; idx < RADV_BLIT_DS_NUM_LAYOUTS; idx++) {
> +               if (!device->meta_state.blit2d.stencil_only_rp[idx]) {
> +                       VkImageLayout layout = radv_meta_blit_ds_to_layout(idx);
> +                       result = radv_CreateRenderPass(radv_device_to_handle(device),
> +                                                      &(VkRenderPassCreateInfo) {
> +                                                              .sType = VK_STRUCTURE_TYPE_RENDER_PASS_CREATE_INFO,
>                                                                .attachmentCount = 1,
>                                                                .pAttachments = &(VkAttachmentDescription) {
> -                                                              .format = VK_FORMAT_S8_UINT,
> -                                                              .loadOp = VK_ATTACHMENT_LOAD_OP_LOAD,
> -                                                              .storeOp = VK_ATTACHMENT_STORE_OP_STORE,
> -                                                              .initialLayout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
> -                                                              .finalLayout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
> -                                                      },
> -                                                      .subpassCount = 1,
> -                                                      .pSubpasses = &(VkSubpassDescription) {
> -                                                      .pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS,
> -                                                      .inputAttachmentCount = 0,
> -                                                      .colorAttachmentCount = 0,
> -                                                      .pColorAttachments = NULL,
> -                                                      .pResolveAttachments = NULL,
> -                                                      .pDepthStencilAttachment = &(VkAttachmentReference) {
> -                                                              .attachment = 0,
> -                                                              .layout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
> -                                                      },
> -                                                      .preserveAttachmentCount = 1,
> -                                                      .pPreserveAttachments = (uint32_t[]) { 0 },
> -                                              },
> -                                                               .dependencyCount = 0,
> -                                                }, &device->meta_state.alloc, &device->meta_state.blit2d.stencil_only_rp);
> +                                                                      .format = VK_FORMAT_S8_UINT,
> +                                                                      .loadOp = VK_ATTACHMENT_LOAD_OP_LOAD,
> +                                                                      .storeOp = VK_ATTACHMENT_STORE_OP_STORE,
> +                                                                      .initialLayout = layout,
> +                                                                      .finalLayout = layout,
> +                                                              },
> +                                                              .subpassCount = 1,
> +                                                              .pSubpasses = &(VkSubpassDescription) {
> +                                                                      .pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS,
> +                                                                      .inputAttachmentCount = 0,
> +                                                                      .colorAttachmentCount = 0,
> +                                                                      .pColorAttachments = NULL,
> +                                                                      .pResolveAttachments = NULL,
> +                                                                      .pDepthStencilAttachment = &(VkAttachmentReference) {
> +                                                                              .attachment = 0,
> +                                                                              .layout = layout,
> +                                                                      },
> +                                                                      .preserveAttachmentCount = 1,
> +                                                                      .pPreserveAttachments = (uint32_t[]) { 0 },
> +                                                              },
> +                                                              .dependencyCount = 0,
> +                                                      }, &device->meta_state.alloc, &device->meta_state.blit2d.stencil_only_rp[idx]);
> +               }
>         }
>
>         const VkGraphicsPipelineCreateInfo vk_pipeline_info = {
> @@ -1120,7 +1130,7 @@ blit2d_init_stencil_only_pipeline(struct radv_device *device,
>                 },
>                 .flags = 0,
>                 .layout = device->meta_state.blit2d.p_layouts[src_type],
> -               .renderPass = device->meta_state.blit2d.stencil_only_rp,
> +               .renderPass = device->meta_state.blit2d.stencil_only_rp[0],
>                 .subpass = 0,
>         };
>
> diff --git a/src/amd/vulkan/radv_meta_copy.c b/src/amd/vulkan/radv_meta_copy.c
> index a42b15a01a..de784d5305 100644
> --- a/src/amd/vulkan/radv_meta_copy.c
> +++ b/src/amd/vulkan/radv_meta_copy.c
> @@ -79,6 +79,7 @@ vk_format_for_size(int bs)
>
>  static struct radv_meta_blit2d_surf
>  blit_surf_for_image_level_layer(struct radv_image *image,
> +                               VkImageLayout layout,
>                                 const VkImageSubresourceLayers *subres)
>  {
>         VkFormat format = image->vk_format;
> @@ -97,6 +98,7 @@ blit_surf_for_image_level_layer(struct radv_image *image,
>                 .layer = subres->baseArrayLayer,
>                 .image = image,
>                 .aspect_mask = subres->aspectMask,
> +               .current_layout = layout,
>         };
>  }
>
> @@ -104,6 +106,7 @@ static void
>  meta_copy_buffer_to_image(struct radv_cmd_buffer *cmd_buffer,
>                            struct radv_buffer* buffer,
>                            struct radv_image* image,
> +                         VkImageLayout layout,
>                            uint32_t regionCount,
>                            const VkBufferImageCopy* pRegions)
>  {
> @@ -155,6 +158,7 @@ meta_copy_buffer_to_image(struct radv_cmd_buffer *cmd_buffer,
>                 /* Create blit surfaces */
>                 struct radv_meta_blit2d_surf img_bsurf =
>                         blit_surf_for_image_level_layer(image,
> +                                                       layout,
>                                                         &pRegions[r].imageSubresource);
>
>                 struct radv_meta_blit2d_buffer buf_bsurf = {
> @@ -214,7 +218,7 @@ void radv_CmdCopyBufferToImage(
>         RADV_FROM_HANDLE(radv_image, dest_image, destImage);
>         RADV_FROM_HANDLE(radv_buffer, src_buffer, srcBuffer);
>
> -       meta_copy_buffer_to_image(cmd_buffer, src_buffer, dest_image,
> +       meta_copy_buffer_to_image(cmd_buffer, src_buffer, dest_image, destImageLayout,
>                                   regionCount, pRegions);
>  }
>
> @@ -222,6 +226,7 @@ static void
>  meta_copy_image_to_buffer(struct radv_cmd_buffer *cmd_buffer,
>                            struct radv_buffer* buffer,
>                            struct radv_image* image,
> +                         VkImageLayout layout,
>                            uint32_t regionCount,
>                            const VkBufferImageCopy* pRegions)
>  {
> @@ -266,6 +271,7 @@ meta_copy_image_to_buffer(struct radv_cmd_buffer *cmd_buffer,
>                 /* Create blit surfaces */
>                 struct radv_meta_blit2d_surf img_info =
>                         blit_surf_for_image_level_layer(image,
> +                                                       layout,
>                                                         &pRegions[r].imageSubresource);
>
>                 struct radv_meta_blit2d_buffer buf_info = {
> @@ -318,13 +324,16 @@ void radv_CmdCopyImageToBuffer(
>         RADV_FROM_HANDLE(radv_buffer, dst_buffer, destBuffer);
>
>         meta_copy_image_to_buffer(cmd_buffer, dst_buffer, src_image,
> +                                 srcImageLayout,
>                                   regionCount, pRegions);
>  }
>
>  static void
>  meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
>                 struct radv_image *src_image,
> +               VkImageLayout src_image_layout,
>                 struct radv_image *dest_image,
> +               VkImageLayout dest_image_layout,
>                 uint32_t regionCount,
>                 const VkImageCopy *pRegions)
>  {
> @@ -351,10 +360,12 @@ meta_copy_image(struct radv_cmd_buffer *cmd_buffer,
>                 /* Create blit surfaces */
>                 struct radv_meta_blit2d_surf b_src =
>                         blit_surf_for_image_level_layer(src_image,
> +                                                       src_image_layout,
>                                                         &pRegions[r].srcSubresource);
>
>                 struct radv_meta_blit2d_surf b_dst =
>                         blit_surf_for_image_level_layer(dest_image,
> +                                                       dest_image_layout,
>                                                         &pRegions[r].dstSubresource);
>
>                 /* for DCC */
> @@ -429,7 +440,9 @@ void radv_CmdCopyImage(
>         RADV_FROM_HANDLE(radv_image, src_image, srcImage);
>         RADV_FROM_HANDLE(radv_image, dest_image, destImage);
>
> -       meta_copy_image(cmd_buffer, src_image, dest_image,
> +       meta_copy_image(cmd_buffer,
> +                       src_image, srcImageLayout,
> +                       dest_image, destImageLayout,
>                         regionCount, pRegions);
>  }
>
> @@ -449,6 +462,7 @@ void radv_blit_to_prime_linear(struct radv_cmd_buffer *cmd_buffer,
>         image_copy.extent.height = image->info.height;
>         image_copy.extent.depth = 1;
>
> -       meta_copy_image(cmd_buffer, image, linear_image,
> +       meta_copy_image(cmd_buffer, image, VK_IMAGE_LAYOUT_GENERAL, linear_image,
> +                       VK_IMAGE_LAYOUT_GENERAL,
>                         1, &image_copy);
>  }
> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
> index 47ae0342ce..a93f606672 100644
> --- a/src/amd/vulkan/radv_private.h
> +++ b/src/amd/vulkan/radv_private.h
> @@ -353,6 +353,20 @@ radv_pipeline_cache_insert_shaders(struct radv_device *device,
>                                    const void *const *codes,
>                                    const unsigned *code_sizes);
>
> +#define RADV_BLIT_DS_LAYOUT_TILE_ENABLE 0
> +#define RADV_BLIT_DS_LAYOUT_TILE_DISABLE 1
> +#define RADV_BLIT_DS_NUM_LAYOUTS 2

Can we make this an enum?

Reviewed-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>

for the series.

> +
> +static inline int radv_meta_blit_ds_to_type(VkImageLayout layout)
> +{
> +       return (layout == VK_IMAGE_LAYOUT_GENERAL) ? RADV_BLIT_DS_LAYOUT_TILE_DISABLE : RADV_BLIT_DS_LAYOUT_TILE_ENABLE;
> +}
> +
> +static inline VkImageLayout radv_meta_blit_ds_to_layout(int idx)
> +{
> +       return idx == RADV_BLIT_DS_LAYOUT_TILE_ENABLE ? VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL : VK_IMAGE_LAYOUT_GENERAL;
> +}
> +
>  struct radv_meta_state {
>         VkAllocationCallbacks alloc;
>
> @@ -405,10 +419,10 @@ struct radv_meta_state {
>                 VkDescriptorSetLayout ds_layouts[3];
>                 VkPipeline pipelines[3][NUM_META_FS_KEYS];
>
> -               VkRenderPass depth_only_rp;
> +               VkRenderPass depth_only_rp[RADV_BLIT_DS_NUM_LAYOUTS];
>                 VkPipeline depth_only_pipeline[3];
>
> -               VkRenderPass stencil_only_rp;
> +               VkRenderPass stencil_only_rp[RADV_BLIT_DS_NUM_LAYOUTS];
>                 VkPipeline stencil_only_pipeline[3];
>         } blit2d;
>
> --
> 2.14.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list