<p dir="ltr"></p>
<p dir="ltr">On Sep 26, 2016 12:26 PM, "Nanley Chery" <<a href="mailto:nanleychery@gmail.com">nanleychery@gmail.com</a>> wrote:<br>
><br>
> On Mon, Sep 26, 2016 at 12:12:32PM -0700, Jason Ekstrand wrote:<br>
> > On Sep 26, 2016 11:16 AM, "Nanley Chery" <<a href="mailto:nanleychery@gmail.com">nanleychery@gmail.com</a>> wrote:<br>
> > ><br>
> > > On Sun, Sep 25, 2016 at 09:59:00AM -0700, Jason Ekstrand wrote:<br>
> > > > Signed-off-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br>
> > > > ---<br>
> > > >  src/intel/vulkan/anv_blorp.c      | 106<br>
> > +++++++++++++++++++++++++++++----<br>
> > > >  src/intel/vulkan/anv_meta_clear.c | 120<br>
> > --------------------------------------<br>
> > > >  2 files changed, 96 insertions(+), 130 deletions(-)<br>
> > > ><br>
> > > > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c<br>
> > > > index cb61070..f5a6c40 100644<br>
> > > > --- a/src/intel/vulkan/anv_blorp.c<br>
> > > > +++ b/src/intel/vulkan/anv_blorp.c<br>
> > > > @@ -480,6 +480,20 @@ void anv_CmdBlitImage(<br>
> > > >     blorp_batch_finish(&batch);<br>
> > > >  }<br>
> > > ><br>
> > > > +static enum isl_format<br>
> > > > +isl_format_for_size(unsigned size_B)<br>
> > > > +{<br>
> > > > +   switch (size_B) {<br>
> > > > +   case 1:  return ISL_FORMAT_R8_UINT;<br>
> > > > +   case 2:  return ISL_FORMAT_R8G8_UINT;<br>
> > > > +   case 4:  return ISL_FORMAT_R8G8B8A8_UINT;<br>
> > > > +   case 8:  return ISL_FORMAT_R16G16B16A16_UINT;<br>
> > > > +   case 16: return ISL_FORMAT_R32G32B32A32_UINT;<br>
> > > > +   default:<br>
> > > > +      unreachable("Not a power-of-two format size");<br>
> > > > +   }<br>
> > > > +}<br>
> > > > +<br>
> > > >  static void<br>
> > > >  do_buffer_copy(struct blorp_batch *batch,<br>
> > > >                 struct anv_bo *src, uint64_t src_offset,<br>
> > > > @@ -491,16 +505,7 @@ do_buffer_copy(struct blorp_batch *batch,<br>
> > > >     /* The actual format we pick doesn't matter as blorp will throw it<br>
> > away.<br>
> > > >      * The only thing that actually matters is the size.<br>
> > > >      */<br>
> > > > -   enum isl_format format;<br>
> > > > -   switch (block_size) {<br>
> > > > -   case 1:  format = ISL_FORMAT_R8_UINT;              break;<br>
> > > > -   case 2:  format = ISL_FORMAT_R8G8_UINT;            break;<br>
> > > > -   case 4:  format = ISL_FORMAT_R8G8B8A8_UNORM;       break;<br>
> > > > -   case 8:  format = ISL_FORMAT_R16G16B16A16_UNORM;   break;<br>
> > > > -   case 16: format = ISL_FORMAT_R32G32B32A32_UINT;    break;<br>
> > > > -   default:<br>
> > > > -      unreachable("Not a power-of-two format size");<br>
> > > > -   }<br>
> > > > +   enum isl_format format = isl_format_for_size(block_size);<br>
> > > ><br>
> > > >     struct isl_surf surf;<br>
> > > >     isl_surf_init(&device->isl_dev, &surf,<br>
> > > > @@ -667,6 +672,87 @@ void anv_CmdUpdateBuffer(<br>
> > > >     blorp_batch_finish(&batch);<br>
> > > >  }<br>
> > > ><br>
> > > > +void anv_CmdFillBuffer(<br>
> > > > +    VkCommandBuffer                             commandBuffer,<br>
> > > > +    VkBuffer                                    dstBuffer,<br>
> > > > +    VkDeviceSize                                dstOffset,<br>
> > > > +    VkDeviceSize                                fillSize,<br>
> > > > +    uint32_t                                    data)<br>
> > > > +{<br>
> > > > +   ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer);<br>
> > > > +   ANV_FROM_HANDLE(anv_buffer, dst_buffer, dstBuffer);<br>
> > > > +   struct blorp_surf surf;<br>
> > > > +   struct isl_surf isl_surf;<br>
> > > > +<br>
> > > > +   struct blorp_batch batch;<br>
> > > > +   blorp_batch_init(&cmd_buffer->device->blorp, &batch, cmd_buffer);<br>
> > > > +<br>
> > > > +   if (fillSize == VK_WHOLE_SIZE) {<br>
> > > > +      fillSize = dst_buffer->size - dstOffset;<br>
> > > > +      /* Make sure fillSize is a multiple of 4 */<br>
> > > > +      fillSize &= ~3ull;<br>
> > > > +   }<br>
> > > > +<br>
> > > > +   /* First, we compute the biggest format that can be used with the<br>
> > > > +    * given offsets and size.<br>
> > > > +    */<br>
> > > > +   int bs = 16;<br>
> > > > +   bs = gcd_pow2_u64(bs, dstOffset);<br>
> > > > +   bs = gcd_pow2_u64(bs, fillSize);<br>
> > > > +   enum isl_format isl_format = isl_format_for_size(bs);<br>
> > > > +<br>
> > > > +   union isl_color_value color = {<br>
> > > > +      .u32 = { data, data, data, data },<br>
> > > > +   };<br>
> > > > +<br>
> > > > +   const uint64_t max_fill_size = MAX_SURFACE_DIM * MAX_SURFACE_DIM *<br>
> > bs;<br>
> > > > +   while (fillSize > max_fill_size) {<br>
> > >                       ^<br>
> > >                       This should be '>='.<br>
> ><br>
> > Sure.  Both work but >= is a bit clearer.  Fixed locally.<br>
> ><br>
><br>
> I don't see how both could work. Wouldn't the assertion for height below fail<br>
> if fillSize == max_fill_size?</p>
<p dir="ltr">Right. The assertion would trigger but the release build logic is correct.  In any case, it doesn't matter since I made the change.</p>
<p dir="ltr">> > > > +      get_blorp_surf_for_anv_buffer(cmd_buffer->device,<br>
> > > > +                                    dst_buffer, dstOffset,<br>
> > > > +                                    MAX_SURFACE_DIM, MAX_SURFACE_DIM,<br>
> > > > +                                    MAX_SURFACE_DIM * bs, isl_format,<br>
> > > > +                                    &surf, &isl_surf);<br>
> > > > +<br>
> > > > +      blorp_clear(&batch, &surf, isl_format, ISL_SWIZZLE_IDENTITY,<br>
> > > > +                  0, 0, 1, 0, 0, MAX_SURFACE_DIM, MAX_SURFACE_DIM,<br>
> > > > +                  color, NULL);<br>
> > > > +      fillSize -= max_fill_size;<br>
> > > > +      dstOffset += max_fill_size;<br>
> > > > +   }<br>
> > > > +<br>
> > > > +   uint64_t height = fillSize / (MAX_SURFACE_DIM * bs);<br>
> > > > +   assert(height < MAX_SURFACE_DIM);<br>
> > > > +   if (height != 0) {<br>
> > > > +      const uint64_t rect_fill_size = height * MAX_SURFACE_DIM * bs;<br>
> > > > +      get_blorp_surf_for_anv_buffer(cmd_buffer->device,<br>
> > > > +                                    dst_buffer, dstOffset,<br>
> > > > +                                    MAX_SURFACE_DIM, height,<br>
> > > > +                                    MAX_SURFACE_DIM * bs, isl_format,<br>
> > > > +                                    &surf, &isl_surf);<br>
> > > > +<br>
> > > > +      blorp_clear(&batch, &surf, isl_format, ISL_SWIZZLE_IDENTITY,<br>
> > > > +                  0, 0, 1, 0, 0, MAX_SURFACE_DIM, height,<br>
> > > > +                  color, NULL);<br>
> > > > +      fillSize -= rect_fill_size;<br>
> > > > +      dstOffset += rect_fill_size;<br>
> > > > +   }<br>
> > > > +<br>
> > > > +   if (fillSize != 0) {<br>
> > > > +      const uint32_t width = fillSize / bs;<br>
> > > > +      get_blorp_surf_for_anv_buffer(cmd_buffer->device,<br>
> > > > +                                    dst_buffer, dstOffset,<br>
> > > > +                                    width, 1,<br>
> > > > +                                    width * bs, isl_format,<br>
> > >                                        ^<br>
> > >                                        fillSize could be used instead.<br>
> ><br>
> > Sure. I mostly did it that way to match the other two callers of<br>
> > get_blorp_surf_for_anv_buffer.<br>
> ><br>
><br>
> Gotcha.<br>
><br>
> -Nanley<br>
><br>
> > > -Nanley<br>
> > ><br>
> > > > +                                    &surf, &isl_surf);<br>
> > > > +<br>
> > > > +      blorp_clear(&batch, &surf, isl_format, ISL_SWIZZLE_IDENTITY,<br>
> > > > +                  0, 0, 1, 0, 0, width, 1,<br>
> > > > +                  color, NULL);<br>
> > > > +   }<br>
> > > > +<br>
> > > > +   blorp_batch_finish(&batch);<br>
> > > > +}<br>
> > > > +<br>
> > > >  void anv_CmdClearColorImage(<br>
> > > >      VkCommandBuffer                             commandBuffer,<br>
> > > >      VkImage                                     _image,<br>
> > > > diff --git a/src/intel/vulkan/anv_meta_clear.c<br>
> > b/src/intel/vulkan/anv_meta_clear.c<br>
> > > > index fd0797f..5579454 100644<br>
> > > > --- a/src/intel/vulkan/anv_meta_clear.c<br>
> > > > +++ b/src/intel/vulkan/anv_meta_clear.c<br>
> > > > @@ -944,123 +944,3 @@ void anv_CmdClearAttachments(<br>
> > > ><br>
> > > >     meta_clear_end(&saved_state, cmd_buffer);<br>
> > > >  }<br>
> > > > -<br>
> > > > -static void<br>
> > > > -do_buffer_fill(struct anv_cmd_buffer *cmd_buffer,<br>
> > > > -               struct anv_bo *dest, uint64_t dest_offset,<br>
> > > > -               int width, int height, VkFormat fill_format, uint32_t<br>
> > data)<br>
> > > > -{<br>
> > > > -   VkDevice vk_device = anv_device_to_handle(cmd_buffer->device);<br>
> > > > -<br>
> > > > -   VkImageCreateInfo image_info = {<br>
> > > > -      .sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO,<br>
> > > > -      .imageType = VK_IMAGE_TYPE_2D,<br>
> > > > -      .format = fill_format,<br>
> > > > -      .extent = {<br>
> > > > -         .width = width,<br>
> > > > -         .height = height,<br>
> > > > -         .depth = 1,<br>
> > > > -      },<br>
> > > > -      .mipLevels = 1,<br>
> > > > -      .arrayLayers = 1,<br>
> > > > -      .samples = 1,<br>
> > > > -      .tiling = VK_IMAGE_TILING_LINEAR,<br>
> > > > -      .usage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT,<br>
> > > > -      .flags = 0,<br>
> > > > -   };<br>
> > > > -<br>
> > > > -   VkImage dest_image;<br>
> > > > -   image_info.usage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT;<br>
> > > > -   anv_CreateImage(vk_device, &image_info,<br>
> > > > -                   &cmd_buffer->pool->alloc, &dest_image);<br>
> > > > -<br>
> > > > -   /* We could use a vk call to bind memory, but that would require<br>
> > > > -    * creating a dummy memory object etc. so there's really no point.<br>
> > > > -    */<br>
> > > > -   anv_image_from_handle(dest_image)->bo = dest;<br>
> > > > -   anv_image_from_handle(dest_image)->offset = dest_offset;<br>
> > > > -<br>
> > > > -   const VkClearValue clear_value = {<br>
> > > > -      .color = {<br>
> > > > -         .uint32 = { data, data, data, data }<br>
> > > > -      }<br>
> > > > -   };<br>
> > > > -<br>
> > > > -   const VkImageSubresourceRange range = {<br>
> > > > -      .aspectMask = VK_IMAGE_ASPECT_COLOR_BIT,<br>
> > > > -      .baseMipLevel = 0,<br>
> > > > -      .levelCount = 1,<br>
> > > > -      .baseArrayLayer = 0,<br>
> > > > -      .layerCount = 1,<br>
> > > > -   };<br>
> > > > -<br>
> > > > -   anv_cmd_clear_image(cmd_buffer, anv_image_from_handle(dest_image),<br>
> > > > -                       VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL,<br>
> > > > -                       clear_value, 1, &range);<br>
> > > > -}<br>
> > > > -<br>
> > > > -void anv_CmdFillBuffer(<br>
> > > > -    VkCommandBuffer                             commandBuffer,<br>
> > > > -    VkBuffer                                    dstBuffer,<br>
> > > > -    VkDeviceSize                                dstOffset,<br>
> > > > -    VkDeviceSize                                fillSize,<br>
> > > > -    uint32_t                                    data)<br>
> > > > -{<br>
> > > > -   ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer);<br>
> > > > -   ANV_FROM_HANDLE(anv_buffer, dst_buffer, dstBuffer);<br>
> > > > -   struct anv_meta_saved_state saved_state;<br>
> > > > -<br>
> > > > -   meta_clear_begin(&saved_state, cmd_buffer);<br>
> > > > -<br>
> > > > -   if (fillSize == VK_WHOLE_SIZE) {<br>
> > > > -      fillSize = dst_buffer->size - dstOffset;<br>
> > > > -      /* Make sure fillSize is a multiple of 4 */<br>
> > > > -      fillSize -= fillSize & 3;<br>
> > > > -   }<br>
> > > > -<br>
> > > > -   VkFormat format;<br>
> > > > -   int bs;<br>
> > > > -   if ((fillSize & 15) == 0 && (dstOffset & 15) == 0) {<br>
> > > > -      format = VK_FORMAT_R32G32B32A32_UINT;<br>
> > > > -      bs = 16;<br>
> > > > -   } else if ((fillSize & 7) == 0 && (dstOffset & 15) == 0) {<br>
> > > > -      format = VK_FORMAT_R32G32_UINT;<br>
> > > > -      bs = 8;<br>
> > > > -   } else {<br>
> > > > -      assert((fillSize & 3) == 0 && (dstOffset & 3) == 0);<br>
> > > > -      format = VK_FORMAT_R32_UINT;<br>
> > > > -      bs = 4;<br>
> > > > -   }<br>
> > > > -<br>
> > > > -   /* This is maximum possible width/height our HW can handle */<br>
> > > > -   const uint64_t max_surface_dim = 1 << 14;<br>
> > > > -<br>
> > > > -   /* First, we make a bunch of max-sized copies */<br>
> > > > -   const uint64_t max_fill_size = max_surface_dim * max_surface_dim *<br>
> > bs;<br>
> > > > -   while (fillSize > max_fill_size) {<br>
> > > > -      do_buffer_fill(cmd_buffer, dst_buffer->bo,<br>
> > > > -                     dst_buffer->offset + dstOffset,<br>
> > > > -                     max_surface_dim, max_surface_dim, format, data);<br>
> > > > -      fillSize -= max_fill_size;<br>
> > > > -      dstOffset += max_fill_size;<br>
> > > > -   }<br>
> > > > -<br>
> > > > -   uint64_t height = fillSize / (max_surface_dim * bs);<br>
> > > > -   assert(height < max_surface_dim);<br>
> > > > -   if (height != 0) {<br>
> > > > -      const uint64_t rect_fill_size = height * max_surface_dim * bs;<br>
> > > > -      do_buffer_fill(cmd_buffer, dst_buffer->bo,<br>
> > > > -                     dst_buffer->offset + dstOffset,<br>
> > > > -                     max_surface_dim, height, format, data);<br>
> > > > -      fillSize -= rect_fill_size;<br>
> > > > -      dstOffset += rect_fill_size;<br>
> > > > -   }<br>
> > > > -<br>
> > > > -   if (fillSize != 0) {<br>
> > > > -      do_buffer_fill(cmd_buffer, dst_buffer->bo,<br>
> > > > -                     dst_buffer->offset + dstOffset,<br>
> > > > -                     fillSize / bs, 1, format, data);<br>
> > > > -   }<br>
> > > > -<br>
> > > > -   meta_clear_end(&saved_state, cmd_buffer);<br>
> > > > -}<br>
> > > > --<br>
> > > > 2.5.0.400.gff86faf<br>
> > > ><br>
> > > > _______________________________________________<br>
> > > > mesa-dev mailing list<br>
> > > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> > > > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br></p>