[Mesa-dev] [PATCH 27/33] anv: Use blorp for CopyImageToBuffer

Nanley Chery nanleychery at gmail.com
Mon Sep 12 19:30:44 UTC 2016


On Wed, Aug 31, 2016 at 02:22:46PM -0700, Jason Ekstrand wrote:
> ---
>  src/intel/vulkan/anv_blorp.c     | 134 +++++++++++++++++++++++++++++++++++++++
>  src/intel/vulkan/anv_meta_copy.c |  16 -----
>  2 files changed, 134 insertions(+), 16 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index 5d715fc..a838b55 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -120,6 +120,38 @@ anv_device_finish_blorp(struct anv_device *device)
>  }
>  
>  static void
> +get_blorp_surf_for_anv_buffer(struct anv_device *device,
> +                              struct anv_buffer *buffer, uint64_t offset,
> +                              uint32_t width, uint32_t height,
> +                              uint32_t row_pitch, enum isl_format format,
> +                              struct blorp_surf *blorp_surf,
> +                              struct isl_surf *isl_surf)
> +{
> +   *blorp_surf = (struct blorp_surf) {
> +      .surf = isl_surf,
> +      .addr = {
> +         .buffer = buffer->bo,
> +         .offset = buffer->offset + offset,
> +      },
> +   };
> +
> +   isl_surf_init(&device->isl_dev, isl_surf,
> +                 .dim = ISL_SURF_DIM_2D,
> +                 .format = format,
> +                 .width = width,
> +                 .height = height,
> +                 .depth = 1,
> +                 .levels = 1,
> +                 .array_len = 1,
> +                 .samples = 1,
> +                 .min_pitch = row_pitch,

I don't think we want to set the min_pitch field. If ISL creates a
surface with a pitch smaller than the required pitch, I think we'd
want to know about it rather than silently aligning up. If it does
get aligned up I wouldn't be confident that the HW will render or
texture from the image correctly. Just having the assertion would
provide such confidence whoever.

> +                 .usage = ISL_SURF_USAGE_TEXTURE_BIT |
> +                          ISL_SURF_USAGE_RENDER_TARGET_BIT,
> +                 .tiling_flags = ISL_TILING_LINEAR_BIT);
> +   assert(isl_surf->row_pitch == row_pitch);
> +}
> +
> +static void
>  get_blorp_surf_for_anv_image(const struct anv_image *image,
>                               VkImageAspectFlags aspect,
>                               struct blorp_surf *blorp_surf)
> @@ -136,6 +168,108 @@ get_blorp_surf_for_anv_image(const struct anv_image *image,
>     };
>  }
>  
> +static void
> +copy_buffer_to_image(struct anv_cmd_buffer *cmd_buffer,
> +                     struct anv_buffer *anv_buffer,
> +                     struct anv_image *anv_image,
> +                     uint32_t regionCount,
> +                     const VkBufferImageCopy* pRegions,
> +                     bool buffer_to_image)
> +{
> +   struct blorp_batch batch;
> +   blorp_batch_init(&cmd_buffer->device->blorp, &batch, cmd_buffer);
> +
> +   struct {
> +      struct blorp_surf surf;
> +      uint32_t level;
> +      VkOffset3D offset;
> +   } image, buffer, *src, *dst;
> +
> +   buffer.level = 0;
> +   buffer.offset = (VkOffset3D) { 0, 0, 0 };
> +
> +   if (buffer_to_image) {
> +      src = &buffer;
> +      dst = ℑ
> +   } else {
> +      src = ℑ
> +      dst = &buffer;
> +   }
> +
> +   for (unsigned r = 0; r < regionCount; r++) {
> +      const VkImageAspectFlags aspect = pRegions[r].imageSubresource.aspectMask;
> +
> +      get_blorp_surf_for_anv_image(anv_image, aspect, &image.surf);
> +      image.offset =
> +         anv_sanitize_image_offset(anv_image->type, pRegions[r].imageOffset);
> +      image.level = pRegions[r].imageSubresource.mipLevel;
> +
> +      VkExtent3D extent =
> +         anv_sanitize_image_extent(anv_image->type, pRegions[r].imageExtent);
> +      if (anv_image->type != VK_IMAGE_TYPE_3D) {
> +         image.offset.z = pRegions[r].imageSubresource.baseArrayLayer;
> +         extent.depth = pRegions[r].imageSubresource.layerCount;
> +      }

Always making the image 3D is a great idea.

> +
> +      const enum isl_format buffer_format =
> +         anv_get_isl_format(&cmd_buffer->device->info, anv_image->vk_format,
> +                            aspect, VK_IMAGE_TILING_LINEAR);
> +
> +      const VkExtent3D bufferImageExtent = {
> +         .width  = pRegions[r].bufferRowLength ?
> +                   pRegions[r].bufferRowLength : extent.width,
> +         .height = pRegions[r].bufferImageHeight ?
> +                   pRegions[r].bufferImageHeight : extent.height,
> +      };
> +
> +      const struct isl_format_layout *buffer_fmtl =
> +         isl_format_get_layout(buffer_format);
> +
> +      const uint32_t buffer_row_pitch =
> +         DIV_ROUND_UP(bufferImageExtent.width, buffer_fmtl->bw) *
> +         (buffer_fmtl->bpb / 8);
> +
> +      const uint32_t buffer_layer_stride =
> +         DIV_ROUND_UP(bufferImageExtent.height, buffer_fmtl->bh) *
> +         buffer_row_pitch;
> +
> +      struct isl_surf buffer_isl_surf;
> +      get_blorp_surf_for_anv_buffer(cmd_buffer->device,
> +                                    anv_buffer, pRegions[r].bufferOffset,
> +                                    extent.width, extent.height,
> +                                    buffer_row_pitch, buffer_format,
> +                                    &buffer.surf, &buffer_isl_surf);

I think this would be clearer if buffer_isl_surf was a struct local to 
get_blorp_surf_for_anv_buffer(). It's not used anywhere outside of that
function and it gets initialized from inside it.

- Nanley

> +
> +      for (unsigned z = 0; z < extent.depth; z++) {
> +         blorp_copy(&batch, &src->surf, src->level, src->offset.z,
> +                    &dst->surf, dst->level, dst->offset.z,
> +                    src->offset.x, src->offset.y, dst->offset.x, dst->offset.y,
> +                    extent.width, extent.height);
> +
> +         image.offset.z++;
> +         buffer.surf.addr.offset += buffer_layer_stride;
> +      }
> +   }
> +
> +   blorp_batch_finish(&batch);
> +}
> +
> +void anv_CmdCopyImageToBuffer(
> +    VkCommandBuffer                             commandBuffer,
> +    VkImage                                     srcImage,
> +    VkImageLayout                               srcImageLayout,
> +    VkBuffer                                    dstBuffer,
> +    uint32_t                                    regionCount,
> +    const VkBufferImageCopy*                    pRegions)
> +{
> +   ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer);
> +   ANV_FROM_HANDLE(anv_image, src_image, srcImage);
> +   ANV_FROM_HANDLE(anv_buffer, dst_buffer, dstBuffer);
> +
> +   copy_buffer_to_image(cmd_buffer, dst_buffer, src_image,
> +                        regionCount, pRegions, false);
> +}
> +
>  static bool
>  flip_coords(unsigned *src0, unsigned *src1, unsigned *dst0, unsigned *dst1)
>  {
> diff --git a/src/intel/vulkan/anv_meta_copy.c b/src/intel/vulkan/anv_meta_copy.c
> index 3f548e6..a17dd63 100644
> --- a/src/intel/vulkan/anv_meta_copy.c
> +++ b/src/intel/vulkan/anv_meta_copy.c
> @@ -232,22 +232,6 @@ void anv_CmdCopyBufferToImage(
>                               regionCount, pRegions, true);
>  }
>  
> -void anv_CmdCopyImageToBuffer(
> -    VkCommandBuffer                             commandBuffer,
> -    VkImage                                     srcImage,
> -    VkImageLayout                               srcImageLayout,
> -    VkBuffer                                    destBuffer,
> -    uint32_t                                    regionCount,
> -    const VkBufferImageCopy*                    pRegions)
> -{
> -   ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer);
> -   ANV_FROM_HANDLE(anv_image, src_image, srcImage);
> -   ANV_FROM_HANDLE(anv_buffer, dst_buffer, destBuffer);
> -
> -   meta_copy_buffer_to_image(cmd_buffer, dst_buffer, src_image,
> -                             regionCount, pRegions, false);
> -}
> -
>  void anv_CmdCopyImage(
>      VkCommandBuffer                             commandBuffer,
>      VkImage                                     srcImage,
> -- 
> 2.5.0.400.gff86faf
> 
> _______________________________________________
> 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