Re: [Mesa-dev] [PATCH 08/14] anv/cmd_buffer: Iterate all subpass attachments when clearing

2018-02-13 Thread Jason Ekstrand
On Tue, Feb 13, 2018 at 11:29 AM, Nanley Chery 
wrote:

> On Mon, Feb 05, 2018 at 02:34:57PM -0800, Jason Ekstrand wrote:
> > This unifies things a bit because we now handle depth and stencil at the
> > same time.  It also ensures that clears happen for input attachments.
>
> As we discussed in another patch, clears are always guaranteed to happen
> for input attachments on LOAD_OP_CLEAR, so we can get rid of that last
> sentence.


Done.


> With that change, this patch is
> Reviewed-by: Nanley Chery 
>
> > ---
> >  src/intel/vulkan/genX_cmd_buffer.c | 77 --
> 
> >  1 file changed, 32 insertions(+), 45 deletions(-)
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> > index ab79fbf..608f5ee 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -3524,66 +3524,51 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> *cmd_buffer,
> >
> > VkRect2D render_area = cmd_buffer->state.render_area;
> > struct anv_framebuffer *fb = cmd_buffer->state.framebuffer;
> > -   for (uint32_t i = 0; i < subpass->color_count; ++i) {
> > -  const uint32_t a = subpass->color_attachments[i].attachment;
> > +
> > +   for (uint32_t i = 0; i < subpass->attachment_count; ++i) {
> > +  const uint32_t a = subpass->attachments[i].attachment;
> >if (a == VK_ATTACHMENT_UNUSED)
> >   continue;
> >
> >assert(a < cmd_state->pass->attachment_count);
> >struct anv_attachment_state *att_state =
> &cmd_state->attachments[a];
> >
> > -  if (!att_state->pending_clear_aspects)
> > - continue;
> > -
> > -  assert(att_state->pending_clear_aspects ==
> VK_IMAGE_ASPECT_COLOR_BIT);
> > -
> >struct anv_image_view *iview = fb->attachments[a];
> >const struct anv_image *image = iview->image;
> >
> > -  /* Multi-planar images are not supported as attachments */
> > -  assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> > -  assert(image->n_planes == 1);
> > -
> > -  uint32_t base_layer = iview->planes[0].isl.base_array_layer;
> > -  uint32_t layer_count = fb->layers;
> > +  if (att_state->pending_clear_aspects &
> VK_IMAGE_ASPECT_COLOR_BIT) {
> > + assert(att_state->pending_clear_aspects ==
> VK_IMAGE_ASPECT_COLOR_BIT);
> >
> > -  if (att_state->fast_clear) {
> > - /* We only support fast-clears on the first layer */
> > - assert(iview->planes[0].isl.base_level == 0);
> > - assert(iview->planes[0].isl.base_array_layer == 0);
> > -
> > - anv_image_ccs_op(cmd_buffer, image, VK_IMAGE_ASPECT_COLOR_BIT,
> > -  0, 0, 1, ISL_AUX_OP_FAST_CLEAR, false);
> > - base_layer++;
> > - layer_count--;
> > -  }
> > -
> > -  if (layer_count > 0) {
> > + /* Multi-planar images are not supported as attachments */
> > + assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> >   assert(image->n_planes == 1);
> > - anv_image_clear_color(cmd_buffer, image,
> VK_IMAGE_ASPECT_COLOR_BIT,
> > -   att_state->aux_usage,
> > -   iview->planes[0].isl.format,
> > -   iview->planes[0].isl.swizzle,
> > -   iview->planes[0].isl.base_level,
> > -   base_layer, layer_count, render_area,
> > -   vk_to_isl_color(att_state->
> clear_value.color));
> > -  }
> > -
> > -  att_state->pending_clear_aspects = 0;
> > -   }
> >
> > -   if (subpass->depth_stencil_attachment.attachment !=
> VK_ATTACHMENT_UNUSED) {
> > -  const uint32_t a = subpass->depth_stencil_attachment.attachment;
> > + uint32_t base_layer = iview->planes[0].isl.base_array_layer;
> > + uint32_t layer_count = fb->layers;
> >
> > -  assert(a < cmd_state->pass->attachment_count);
> > -  struct anv_attachment_state *att_state =
> &cmd_state->attachments[a];
> > -  struct anv_image_view *iview = fb->attachments[a];
> > -  const struct anv_image *image = iview->image;
> > + if (att_state->fast_clear) {
> > +/* We only support fast-clears on the first layer */
> > +assert(iview->planes[0].isl.base_level == 0);
> > +assert(iview->planes[0].isl.base_array_layer == 0);
> >
> > -  assert(image->aspects & (VK_IMAGE_ASPECT_DEPTH_BIT |
> > -   VK_IMAGE_ASPECT_STENCIL_BIT));
> > +anv_image_ccs_op(cmd_buffer, image,
> VK_IMAGE_ASPECT_COLOR_BIT,
> > + 0, 0, 1, ISL_AUX_OP_FAST_CLEAR, false);
> > +base_layer++;
> > +layer_count--;
> > + }
> >
> > -  if (att_state->pending_clear_aspects) {
> > + if (layer_count > 0) {
> > +assert(image->n_planes == 1);
> > +anv_image_clear_color(cmd_buffer, image,
> VK_IMAGE_ASPECT_

Re: [Mesa-dev] [PATCH 08/14] anv/cmd_buffer: Iterate all subpass attachments when clearing

2018-02-13 Thread Nanley Chery
On Mon, Feb 05, 2018 at 02:34:57PM -0800, Jason Ekstrand wrote:
> This unifies things a bit because we now handle depth and stencil at the
> same time.  It also ensures that clears happen for input attachments.

As we discussed in another patch, clears are always guaranteed to happen
for input attachments on LOAD_OP_CLEAR, so we can get rid of that last
sentence. With that change, this patch is
Reviewed-by: Nanley Chery 

> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 77 
> --
>  1 file changed, 32 insertions(+), 45 deletions(-)
> 
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
> b/src/intel/vulkan/genX_cmd_buffer.c
> index ab79fbf..608f5ee 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -3524,66 +3524,51 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer 
> *cmd_buffer,
>  
> VkRect2D render_area = cmd_buffer->state.render_area;
> struct anv_framebuffer *fb = cmd_buffer->state.framebuffer;
> -   for (uint32_t i = 0; i < subpass->color_count; ++i) {
> -  const uint32_t a = subpass->color_attachments[i].attachment;
> +
> +   for (uint32_t i = 0; i < subpass->attachment_count; ++i) {
> +  const uint32_t a = subpass->attachments[i].attachment;
>if (a == VK_ATTACHMENT_UNUSED)
>   continue;
>  
>assert(a < cmd_state->pass->attachment_count);
>struct anv_attachment_state *att_state = &cmd_state->attachments[a];
>  
> -  if (!att_state->pending_clear_aspects)
> - continue;
> -
> -  assert(att_state->pending_clear_aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> -
>struct anv_image_view *iview = fb->attachments[a];
>const struct anv_image *image = iview->image;
>  
> -  /* Multi-planar images are not supported as attachments */
> -  assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
> -  assert(image->n_planes == 1);
> -
> -  uint32_t base_layer = iview->planes[0].isl.base_array_layer;
> -  uint32_t layer_count = fb->layers;
> +  if (att_state->pending_clear_aspects & VK_IMAGE_ASPECT_COLOR_BIT) {
> + assert(att_state->pending_clear_aspects == 
> VK_IMAGE_ASPECT_COLOR_BIT);
>  
> -  if (att_state->fast_clear) {
> - /* We only support fast-clears on the first layer */
> - assert(iview->planes[0].isl.base_level == 0);
> - assert(iview->planes[0].isl.base_array_layer == 0);
> -
> - anv_image_ccs_op(cmd_buffer, image, VK_IMAGE_ASPECT_COLOR_BIT,
> -  0, 0, 1, ISL_AUX_OP_FAST_CLEAR, false);
> - base_layer++;
> - layer_count--;
> -  }
> -
> -  if (layer_count > 0) {
> + /* Multi-planar images are not supported as attachments */
> + assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
>   assert(image->n_planes == 1);
> - anv_image_clear_color(cmd_buffer, image, VK_IMAGE_ASPECT_COLOR_BIT,
> -   att_state->aux_usage,
> -   iview->planes[0].isl.format,
> -   iview->planes[0].isl.swizzle,
> -   iview->planes[0].isl.base_level,
> -   base_layer, layer_count, render_area,
> -   
> vk_to_isl_color(att_state->clear_value.color));
> -  }
> -
> -  att_state->pending_clear_aspects = 0;
> -   }
>  
> -   if (subpass->depth_stencil_attachment.attachment != VK_ATTACHMENT_UNUSED) 
> {
> -  const uint32_t a = subpass->depth_stencil_attachment.attachment;
> + uint32_t base_layer = iview->planes[0].isl.base_array_layer;
> + uint32_t layer_count = fb->layers;
>  
> -  assert(a < cmd_state->pass->attachment_count);
> -  struct anv_attachment_state *att_state = &cmd_state->attachments[a];
> -  struct anv_image_view *iview = fb->attachments[a];
> -  const struct anv_image *image = iview->image;
> + if (att_state->fast_clear) {
> +/* We only support fast-clears on the first layer */
> +assert(iview->planes[0].isl.base_level == 0);
> +assert(iview->planes[0].isl.base_array_layer == 0);
>  
> -  assert(image->aspects & (VK_IMAGE_ASPECT_DEPTH_BIT |
> -   VK_IMAGE_ASPECT_STENCIL_BIT));
> +anv_image_ccs_op(cmd_buffer, image, VK_IMAGE_ASPECT_COLOR_BIT,
> + 0, 0, 1, ISL_AUX_OP_FAST_CLEAR, false);
> +base_layer++;
> +layer_count--;
> + }
>  
> -  if (att_state->pending_clear_aspects) {
> + if (layer_count > 0) {
> +assert(image->n_planes == 1);
> +anv_image_clear_color(cmd_buffer, image, 
> VK_IMAGE_ASPECT_COLOR_BIT,
> +  att_state->aux_usage,
> +  iview->planes[0].isl.format,
> +  iview->planes[0].isl.swizzle,
> +  iview->planes[0].i