On Fri, Sep 9, 2016 at 2:35 PM, Nanley Chery <nanleych...@gmail.com> wrote:
> 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;
>
> 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.
>
Sounds reasonable.
> With or without that change, and assuming the fixup patch is amended,
> this patch is,
>
> Reviewed-by: Nanley Chery <nanley.g.ch...@intel.com>
>
Thanks!
> + } 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
> >
> > _______________________________________________
> > 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
