Re: [Mesa-dev] [PATCH 1/2] anv/blit2d: Add a format parameter to bind_dst and create_iview

[Nanley Chery]

On Wed, Aug 10, 2016 at 10:12:17AM -0700, Nanley Chery wrote:
> On Tue, Aug 02, 2016 at 10:00:29AM -0700, Jason Ekstrand wrote:
> > Signed-off-by: Jasosn Ekstrand 
> > Cc: "12.0" 

I just noticed a bug in this patch. With that fixed the Reviewed-by
will hold. Please see the note inline.

> > ---
> >  src/intel/vulkan/anv_meta_blit2d.c | 15 ++-
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/intel/vulkan/anv_meta_blit2d.c 
> > b/src/intel/vulkan/anv_meta_blit2d.c
> > index 9d4c2fc..30bc6ed 100644
> > --- a/src/intel/vulkan/anv_meta_blit2d.c
> > +++ b/src/intel/vulkan/anv_meta_blit2d.c
> > @@ -99,6 +99,7 @@ create_iview(struct anv_cmd_buffer *cmd_buffer,
> >   VkImageUsageFlags usage,
> >   uint32_t width,
> >   uint32_t height,
> > + VkFormat format,
> >   VkImage *img,
> >   struct anv_image_view *iview)
> >  {
> > @@ -106,8 +107,7 @@ create_iview(struct anv_cmd_buffer *cmd_buffer,
> >.sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO,
> >.imageType = VK_IMAGE_TYPE_2D,
> >/* W-tiled images must be stencil-formatted. */
> > -  .format = surf->tiling == ISL_TILING_W ?
> > -VK_FORMAT_S8_UINT : vk_format_for_size(surf->bs),
> > +  .format = format,
> >.extent = {
> >   .width = width,
> >   .height = height,
> > @@ -190,6 +190,8 @@ blit2d_bind_src(struct anv_cmd_buffer *cmd_buffer,
> >  
> >create_iview(cmd_buffer, src, offset, usage,
> > rect->src_x + rect->width, rect->src_y + rect->height,
> > +   src->tiling == ISL_TILING_W ?
> > +  VK_FORMAT_S8_UINT : vk_format_for_size(src->bs),
> > >image, >iview);
> >  
> >anv_CreateDescriptorPool(vk_device,
> > @@ -339,10 +341,11 @@ blit2d_bind_dst(struct anv_cmd_buffer *cmd_buffer,
> >  uint64_t offset,
> >  uint32_t width,
> >  uint32_t height,
> > +enum isl_format format,

This should be:

VkFormat format,

- Nanley

> >  struct blit2d_dst_temps *tmp)
> >  {
> > create_iview(cmd_buffer, dst, offset, 
> > VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT,
> > -width, height, >image, >iview);
> > +width, height, format, >image, >iview);
> >  
> > anv_CreateFramebuffer(anv_device_to_handle(cmd_buffer->device),
> >&(VkFramebufferCreateInfo) {
> > @@ -417,7 +420,8 @@ anv_meta_blit2d_normal_dst(struct anv_cmd_buffer 
> > *cmd_buffer,
> >  
> >struct blit2d_dst_temps dst_temps;
> >blit2d_bind_dst(cmd_buffer, dst, offset, rects[r].dst_x + 
> > rects[r].width,
> > -  rects[r].dst_y + rects[r].height, _temps);
> > +  rects[r].dst_y + rects[r].height,
> > +  vk_format_for_size(dst->bs), _temps);
> >  
> >struct blit_vb_data {
> >   float pos[2];
> > @@ -555,7 +559,8 @@ anv_meta_blit2d_w_tiled_dst(struct anv_cmd_buffer 
> > *cmd_buffer,
> >};
> >  
> >struct blit2d_dst_temps dst_temps;
> > -  blit2d_bind_dst(cmd_buffer, _Y, offset, xmax_Y, ymax_Y, 
> > _temps);
> > +  blit2d_bind_dst(cmd_buffer, _Y, offset, xmax_Y, ymax_Y,
> > +  vk_format_for_size(dst->bs), _temps);
> 
> We know the format block size here, so I think we should replace dst->bs with
> 1. There actually isn't any other usage of dst->bs (other than the assert) for
> this reason.
> 
> With that change this patch is,
> Reviewed-by: Nanley Chery 
> 
> >  
> >struct blit_vb_header {
> >   struct anv_vue_header vue;
> > -- 
> > 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

Re: [Mesa-dev] [PATCH 1/2] anv/blit2d: Add a format parameter to bind_dst and create_iview

[Nanley Chery]

On Tue, Aug 02, 2016 at 10:00:29AM -0700, Jason Ekstrand wrote:
> Signed-off-by: Jasosn Ekstrand 
> Cc: "12.0" 
> ---
>  src/intel/vulkan/anv_meta_blit2d.c | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_meta_blit2d.c 
> b/src/intel/vulkan/anv_meta_blit2d.c
> index 9d4c2fc..30bc6ed 100644
> --- a/src/intel/vulkan/anv_meta_blit2d.c
> +++ b/src/intel/vulkan/anv_meta_blit2d.c
> @@ -99,6 +99,7 @@ create_iview(struct anv_cmd_buffer *cmd_buffer,
>   VkImageUsageFlags usage,
>   uint32_t width,
>   uint32_t height,
> + VkFormat format,
>   VkImage *img,
>   struct anv_image_view *iview)
>  {
> @@ -106,8 +107,7 @@ create_iview(struct anv_cmd_buffer *cmd_buffer,
>.sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO,
>.imageType = VK_IMAGE_TYPE_2D,
>/* W-tiled images must be stencil-formatted. */
> -  .format = surf->tiling == ISL_TILING_W ?
> -VK_FORMAT_S8_UINT : vk_format_for_size(surf->bs),
> +  .format = format,
>.extent = {
>   .width = width,
>   .height = height,
> @@ -190,6 +190,8 @@ blit2d_bind_src(struct anv_cmd_buffer *cmd_buffer,
>  
>create_iview(cmd_buffer, src, offset, usage,
> rect->src_x + rect->width, rect->src_y + rect->height,
> +   src->tiling == ISL_TILING_W ?
> +  VK_FORMAT_S8_UINT : vk_format_for_size(src->bs),
> >image, >iview);
>  
>anv_CreateDescriptorPool(vk_device,
> @@ -339,10 +341,11 @@ blit2d_bind_dst(struct anv_cmd_buffer *cmd_buffer,
>  uint64_t offset,
>  uint32_t width,
>  uint32_t height,
> +enum isl_format format,
>  struct blit2d_dst_temps *tmp)
>  {
> create_iview(cmd_buffer, dst, offset, VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT,
> -width, height, >image, >iview);
> +width, height, format, >image, >iview);
>  
> anv_CreateFramebuffer(anv_device_to_handle(cmd_buffer->device),
>&(VkFramebufferCreateInfo) {
> @@ -417,7 +420,8 @@ anv_meta_blit2d_normal_dst(struct anv_cmd_buffer 
> *cmd_buffer,
>  
>struct blit2d_dst_temps dst_temps;
>blit2d_bind_dst(cmd_buffer, dst, offset, rects[r].dst_x + 
> rects[r].width,
> -  rects[r].dst_y + rects[r].height, _temps);
> +  rects[r].dst_y + rects[r].height,
> +  vk_format_for_size(dst->bs), _temps);
>  
>struct blit_vb_data {
>   float pos[2];
> @@ -555,7 +559,8 @@ anv_meta_blit2d_w_tiled_dst(struct anv_cmd_buffer 
> *cmd_buffer,
>};
>  
>struct blit2d_dst_temps dst_temps;
> -  blit2d_bind_dst(cmd_buffer, _Y, offset, xmax_Y, ymax_Y, 
> _temps);
> +  blit2d_bind_dst(cmd_buffer, _Y, offset, xmax_Y, ymax_Y,
> +  vk_format_for_size(dst->bs), _temps);

We know the format block size here, so I think we should replace dst->bs with
1. There actually isn't any other usage of dst->bs (other than the assert) for
this reason.

With that change this patch is,
Reviewed-by: Nanley Chery 

>  
>struct blit_vb_header {
>   struct anv_vue_header vue;
> -- 
> 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