[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;
> +      if (att == VK_ATTACHMENT_UNUSED)
> +         continue;
> +


I'm realizing that the above hunk is not present in the master branch,
though it was also in the patch submitted to the mailing list.


Didn't see any reference to remove it. 

Jason, can I ignore that hunk in the stable branch too?




>        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:
>               *