[Mesa-stable] [PATCH 3/3] anv/blorp: Properly handle VK_ATTACHMENT_UNUSED

[Juan A. Suarez Romero]

On Tue, 2017-04-11 at 07:54 -0700, Jason Ekstrand wrote:
> The Vulkan driver was originally written under the assumption that
> VK_ATTACHMENT_UNUSED was basically just for depth-stencil attachments.
> However, the way things fell together, VK_ATTACHMENT_UNUSED can be used
> anywhere in the subpass description.  The blorp-based clear and resolve
> code has a bunch of places where we walk lists of attachments and we
> weren't handling VK_ATTACHMENT_UNUSED everywhere.  This commit should
> fix all of them.
> 
> Cc: "13.0 17.0" <mesa-stable at lists.freedesktop.org>
> ---
>  src/intel/vulkan/anv_blorp.c | 30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index 72a468a..d27132a 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -1148,6 +1148,9 @@ anv_cmd_buffer_flush_attachments(struct anv_cmd_buffer *cmd_buffer,
>  
>     for (uint32_t i = 0; i < subpass->color_count; ++i) {
>        uint32_t att = subpass->color_attachments[i].attachment;

Seems this relies on commit 608d17b80e617b0052 (anv: Store the user's
VkAttachmentReference).

At least, without that patch, att would be

uint32_t att = subpass->color_attachments[i];


Should we propose the above commit to stable? Or should we keep as it
is, and use later att.attachment?


BTW, this very same happens in couple of places more in this patch.



> +      if (att == VK_ATTACHMENT_UNUSED)
> +         continue;
> +
>        assert(att < pass->attachment_count);
>        if (attachment_needs_flush(cmd_buffer, &pass->attachments[att], stage)) {
>           cmd_buffer->state.pending_pipe_bits |=
> @@ -1175,14 +1178,19 @@ subpass_needs_clear(const struct anv_cmd_buffer *cmd_buffer)
>  
>     for (uint32_t i = 0; i < cmd_state->subpass->color_count; ++i) {
>        uint32_t a = cmd_state->subpass->color_attachments[i].attachment;
> +      if (a == VK_ATTACHMENT_UNUSED)
> +         continue;
> +
> +      assert(a < cmd_state->pass->attachment_count);
>        if (cmd_state->attachments[a].pending_clear_aspects) {
>           return true;
>        }
>     }
>  
> -   if (ds != VK_ATTACHMENT_UNUSED &&
> -       cmd_state->attachments[ds].pending_clear_aspects) {
> -      return true;
> +   if (ds != VK_ATTACHMENT_UNUSED) {
> +      assert(ds < cmd_state->pass->attachment_count);
> +      if (cmd_state->attachments[ds].pending_clear_aspects)
> +         return true;
>     }
>  
>     return false;
> @@ -1214,6 +1222,10 @@ anv_cmd_buffer_clear_subpass(struct anv_cmd_buffer *cmd_buffer)
>     struct anv_framebuffer *fb = cmd_buffer->state.framebuffer;
>     for (uint32_t i = 0; i < cmd_state->subpass->color_count; ++i) {
>        const uint32_t a = cmd_state->subpass->color_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)
> @@ -1273,6 +1285,7 @@ anv_cmd_buffer_clear_subpass(struct anv_cmd_buffer *cmd_buffer)
>     }
>  
>     const uint32_t ds = cmd_state->subpass->depth_stencil_attachment.attachment;
> +   assert(ds == VK_ATTACHMENT_UNUSED || ds < cmd_state->pass->attachment_count);
>  
>     if (ds != VK_ATTACHMENT_UNUSED &&
>         cmd_state->attachments[ds].pending_clear_aspects) {
> @@ -1578,8 +1591,12 @@ anv_cmd_buffer_resolve_subpass(struct anv_cmd_buffer *cmd_buffer)
>     blorp_batch_init(&cmd_buffer->device->blorp, &batch, cmd_buffer, 0);
>  
>     for (uint32_t i = 0; i < subpass->color_count; ++i) {
> -      ccs_resolve_attachment(cmd_buffer, &batch,
> -                             subpass->color_attachments[i].attachment);
> +      const uint32_t att = subpass->color_attachments[i].attachment;
> +      if (att == VK_ATTACHMENT_UNUSED)
> +         continue;
> +
> +      assert(att < cmd_buffer->state.pass->attachment_count);
> +      ccs_resolve_attachment(cmd_buffer, &batch, att);
>     }
>  
>     anv_cmd_buffer_flush_attachments(cmd_buffer, SUBPASS_STAGE_DRAW);
> @@ -1592,6 +1609,9 @@ anv_cmd_buffer_resolve_subpass(struct anv_cmd_buffer *cmd_buffer)
>           if (dst_att == VK_ATTACHMENT_UNUSED)
>              continue;
>  
> +         assert(src_att < cmd_buffer->state.pass->attachment_count);
> +         assert(dst_att < cmd_buffer->state.pass->attachment_count);
> +
>           if (cmd_buffer->state.attachments[dst_att].pending_clear_aspects) {
>              /* From the Vulkan 1.0 spec:
>               *