<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Sep 9, 2016 at 2:35 PM, Nanley Chery <<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed, Aug 31, 2016 at 02:22:48PM -0700, Jason Ekstrand wrote: > --- > src/intel/vulkan/anv_blorp.c | 67 +++++++++++++++++ > src/intel/vulkan/anv_meta_copy.c | 158 --------------------------------------- > 2 files changed, 67 insertions(+), 158 deletions(-) > > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c > index 5fa6699..89ff3b3 100644 > --- a/src/intel/vulkan/anv_blorp.c > +++ b/src/intel/vulkan/anv_blorp.c > @@ -168,6 +168,73 @@ get_blorp_surf_for_anv_image(const struct anv_image *image, > }; > } > > +void anv_CmdCopyImage( > + VkCommandBuffer commandBuffer, > + VkImage srcImage, > + VkImageLayout srcImageLayout, > + VkImage dstImage, > + VkImageLayout dstImageLayout, > + uint32_t regionCount, > + const VkImageCopy* pRegions) > +{ > + ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer); > + ANV_FROM_HANDLE(anv_image, src_image, srcImage); > + ANV_FROM_HANDLE(anv_image, dst_image, dstImage); > + > + struct blorp_batch batch; > + blorp_batch_init(&cmd_buffer->device->blorp, &batch, cmd_buffer); > + > + for (unsigned r = 0; r < regionCount; r++) { > + VkOffset3D srcOffset = > + anv_sanitize_image_offset(src_image->type, pRegions[r].srcOffset); > + VkOffset3D dstOffset = > + anv_sanitize_image_offset(src_image->type, pRegions[r].dstOffset); > + VkExtent3D extent = > + anv_sanitize_image_extent(src_image->type, pRegions[r].extent); > + > + unsigned dst_base_layer, layer_count; > + if (dst_image->type == VK_IMAGE_TYPE_3D) { > + dst_base_layer = dstOffset.z; > + layer_count = extent.depth; </div></div>In the patch titled, "anv/blorp: Use the correct coordinates in CmdCopyImage," you replace extent.depth with pRegions[r].extent.depth. I think it also makes sense to change the assignment of src_base_layer and dest_base_layer with the offsets straight from user when we know the image type is VK_IMAGE_TYPE_3D. When the type is 3D, the sanitize functions do nothing. </blockquote><div> </div><div>Sounds reasonable. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> With or without that change, and assuming the fixup patch is amended, this patch is, Reviewed-by: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>> </blockquote><div> </div><div>Thanks! </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> > + } else { > + dst_base_layer = pRegions[r].dstSubresource.baseArrayLayer; > + layer_count = pRegions[r].dstSubresource.layerCount; > + } > + > + unsigned src_base_layer; > + if (src_image->type == VK_IMAGE_TYPE_3D) { > + src_base_layer = srcOffset.z; > + } else { > + src_base_layer = pRegions[r].srcSubresource.baseArrayLayer; > + assert(pRegions[r].srcSubresource.layerCount == layer_count); > + } > + > + assert(pRegions[r].srcSubresource.aspectMask == > + pRegions[r].dstSubresource.aspectMask); > + > + uint32_t a; > + for_each_bit(a, pRegions[r].dstSubresource.aspectMask) { > + VkImageAspectFlagBits aspect = (1 << a); > + > + struct blorp_surf src_surf, dst_surf; > + get_blorp_surf_for_anv_image(src_image, aspect, &src_surf); > + get_blorp_surf_for_anv_image(dst_image, aspect, &dst_surf); > + > + for (unsigned i = 0; i < layer_count; i++) { > + blorp_copy(&batch, &src_surf, pRegions[r].srcSubresource.mipLevel, > + src_base_layer + i, > + &dst_surf, pRegions[r].dstSubresource.mipLevel, > + dst_base_layer + i, > + srcOffset.x, srcOffset.y, > + dstOffset.x, dstOffset.y, > + extent.width, extent.height); > + } > + } > + } > + > + blorp_batch_finish(&batch); > +} > + > static void > copy_buffer_to_image(struct anv_cmd_buffer *cmd_buffer, > struct anv_buffer *anv_buffer, > diff --git a/src/intel/vulkan/anv_meta_copy.c b/src/intel/vulkan/anv_meta_copy.c > index 5df04e6..b33273e 100644 > --- a/src/intel/vulkan/anv_meta_copy.c > +++ b/src/intel/vulkan/anv_meta_copy.c > @@ -23,63 +23,6 @@ > > #include "anv_meta.h" > > -static VkExtent3D > -meta_image_block_size(const struct anv_image *image) > -{ > - if (image->aspects == VK_IMAGE_ASPECT_COLOR_BIT) { > - const struct isl_format_layout *isl_layout = > - isl_format_get_layout(image->color_surface.isl.format); > - return (VkExtent3D) { isl_layout->bw, isl_layout->bh, isl_layout->bd }; > - } else { > - return (VkExtent3D) { 1, 1, 1 }; > - } > -} > - > -/* Returns the user-provided VkBufferImageCopy::imageExtent in units of > - * elements rather than texels. One element equals one texel or one block > - * if Image is uncompressed or compressed, respectively. > - */ > -static struct VkExtent3D > -meta_region_extent_el(const struct anv_image *image, > - const struct VkExtent3D *extent) > -{ > - const VkExtent3D block = meta_image_block_size(image); > - return anv_sanitize_image_extent(image->type, (VkExtent3D) { > - .width = DIV_ROUND_UP(extent->width , block.width), > - .height = DIV_ROUND_UP(extent->height, block.height), > - .depth = DIV_ROUND_UP(extent->depth , block.depth), > - }); > -} > - > -/* Returns the user-provided VkBufferImageCopy::imageOffset in units of > - * elements rather than texels. One element equals one texel or one block > - * if Image is uncompressed or compressed, respectively. > - */ > -static struct VkOffset3D > -meta_region_offset_el(const struct anv_image *image, > - const struct VkOffset3D *offset) > -{ > - const VkExtent3D block = meta_image_block_size(image); > - return anv_sanitize_image_offset(image->type, (VkOffset3D) { > - .x = offset->x / block.width, > - .y = offset->y / block.height, > - .z = offset->z / block.depth, > - }); > -} > - > -static struct anv_meta_blit2d_surf > -blit_surf_for_image(const struct anv_image* image, > - const struct anv_surface *surf) > -{ > - return (struct anv_meta_blit2d_surf) { > - .bo = image->bo, > - .tiling = surf->isl.tiling, > - .base_offset = image->offset + surf->offset, > - .bs = isl_format_get_layout(surf->isl.format)->bpb / 8, > - .pitch = isl_surf_get_row_pitch(&surf->isl), > - }; > -} > - > static void > do_buffer_copy(struct anv_cmd_buffer *cmd_buffer, > struct anv_bo *src, uint64_t src_offset, > @@ -107,107 +50,6 @@ do_buffer_copy(struct anv_cmd_buffer *cmd_buffer, > anv_meta_blit2d(cmd_buffer, &b_src, &b_dst, 1, &rect); > } > > -void anv_CmdCopyImage( > - VkCommandBuffer commandBuffer, > - VkImage srcImage, > - VkImageLayout srcImageLayout, > - VkImage destImage, > - VkImageLayout destImageLayout, > - uint32_t regionCount, > - const VkImageCopy* pRegions) > -{ > - ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer); > - ANV_FROM_HANDLE(anv_image, src_image, srcImage); > - ANV_FROM_HANDLE(anv_image, dest_image, destImage); > - struct anv_meta_saved_state saved_state; > - > - /* From the Vulkan 1.0 spec: > - * > - * vkCmdCopyImage can be used to copy image data between multisample > - * images, but both images must have the same number of samples. > - */ > - assert(src_image->samples == dest_image->samples); > - > - anv_meta_begin_blit2d(cmd_buffer, &saved_state); > - > - for (unsigned r = 0; r < regionCount; r++) { > - assert(pRegions[r].srcSubresource.aspectMask == > - pRegions[r].dstSubresource.aspectMask); > - > - VkImageAspectFlags aspect = pRegions[r].srcSubresource.aspectMask; > - > - /* Create blit surfaces */ > - const struct anv_surface *src_surf = > - anv_image_get_surface_for_aspect_mask(src_image, aspect); > - const struct anv_surface *dst_surf = > - anv_image_get_surface_for_aspect_mask(dest_image, aspect); > - struct anv_meta_blit2d_surf b_src = > - blit_surf_for_image(src_image, src_surf); > - struct anv_meta_blit2d_surf b_dst = > - blit_surf_for_image(dest_image, dst_surf); > - > - /** > - * From the Vulkan 1.0.6 spec: 18.4 Copying Data Between Buffers and Images > - * imageExtent is the size in texels of the image to copy in width, height > - * and depth. 1D images use only x and width. 2D images use x, y, width > - * and height. 3D images use x, y, z, width, height and depth. > - * > - * Also, convert the offsets and extent from units of texels to units of > - * blocks - which is the highest resolution accessible in this command. > - */ > - const VkOffset3D dst_offset_el = > - meta_region_offset_el(dest_image, &pRegions[r].dstOffset); > - const VkOffset3D src_offset_el = > - meta_region_offset_el(src_image, &pRegions[r].srcOffset); > - const VkExtent3D img_extent_el = > - meta_region_extent_el(src_image, &pRegions[r].extent); > - > - /* Start creating blit rect */ > - struct anv_meta_blit2d_rect rect = { > - .width = img_extent_el.width, > - .height = img_extent_el.height, > - }; > - > - /* Loop through each 3D or array slice */ > - unsigned num_slices_3d = img_extent_el.depth; > - unsigned num_slices_array = pRegions[r].dstSubresource.layerCount; > - unsigned slice_3d = 0; > - unsigned slice_array = 0; > - while (slice_3d < num_slices_3d && slice_array < num_slices_array) { > - > - /* Finish creating blit rect */ > - isl_surf_get_image_offset_el(&dst_surf->isl, > - pRegions[r].dstSubresource.mipLevel, > - pRegions[r].dstSubresource.baseArrayLayer > - + slice_array, > - dst_offset_el.z + slice_3d, > - &rect.dst_x, > - &rect.dst_y); > - isl_surf_get_image_offset_el(&src_surf->isl, > - pRegions[r].srcSubresource.mipLevel, > - pRegions[r].srcSubresource.baseArrayLayer > - + slice_array, > - src_offset_el.z + slice_3d, > - &rect.src_x, > - &rect.src_y); > - rect.dst_x += dst_offset_el.x; > - rect.dst_y += dst_offset_el.y; > - rect.src_x += src_offset_el.x; > - rect.src_y += src_offset_el.y; > - > - /* Perform Blit */ > - anv_meta_blit2d(cmd_buffer, &b_src, &b_dst, 1, &rect); > - > - if (dest_image->type == VK_IMAGE_TYPE_3D) > - slice_3d++; > - else > - slice_array++; > - } > - } > - > - anv_meta_end_blit2d(cmd_buffer, &saved_state); > -} > - > void anv_CmdCopyBuffer( > VkCommandBuffer commandBuffer, > VkBuffer srcBuffer, > -- > 2.5.0.400.gff86faf > </div></div>> _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>