[Mesa-dev] [PATCH 10/14] anv/pass: Store usage in each subpass attachment

Jason Ekstrand jason at jlekstrand.net
Tue Feb 13 23:23:42 UTC 2018


On Tue, Feb 13, 2018 at 3:18 PM, Nanley Chery <nanleychery at gmail.com> wrote:

> On Mon, Feb 05, 2018 at 02:34:59PM -0800, Jason Ekstrand wrote:
> > This requires us to ditch the VkAttachmentReference struct in favor of
> > an anv-specific struct.  However, we can now easily identify from just
> > the subpass attachment what kind of an attachment it is.  This will make
> > iteration over anv_subpass::attachments a little easier in some case.
> > ---
> >  src/intel/vulkan/anv_pass.c        | 35 +++++++++++++++++++++++++++---
> -----
> >  src/intel/vulkan/anv_private.h     | 16 +++++++++++-----
> >  src/intel/vulkan/genX_cmd_buffer.c |  2 +-
> >  3 files changed, 39 insertions(+), 14 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_pass.c b/src/intel/vulkan/anv_pass.c
> > index a77e52b..5b8b138 100644
> > --- a/src/intel/vulkan/anv_pass.c
> > +++ b/src/intel/vulkan/anv_pass.c
> > @@ -65,7 +65,7 @@ VkResult anv_CreateRenderPass(
> >     anv_multialloc_add(&ma, &attachments, pCreateInfo->attachmentCount);
> >     anv_multialloc_add(&ma, &subpass_flushes, pCreateInfo->subpassCount
> + 1);
> >
> > -   VkAttachmentReference *subpass_attachments;
> > +   struct anv_subpass_attachment *subpass_attachments;
> >     uint32_t subpass_attachment_count = 0;
> >     for (uint32_t i = 0; i < pCreateInfo->subpassCount; i++) {
> >        subpass_attachment_count +=
> > @@ -117,7 +117,11 @@ VkResult anv_CreateRenderPass(
> >
> >           for (uint32_t j = 0; j < desc->inputAttachmentCount; j++) {
> >              uint32_t a = desc->pInputAttachments[j].attachment;
> > -            subpass->input_attachments[j] = desc->pInputAttachments[j];
> > +            subpass->input_attachments[j] = (struct
> anv_subpass_attachment) {
> > +               .usage =       VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT,
> > +               .attachment =  desc->pInputAttachments[j].attachment,
> > +               .layout =      desc->pInputAttachments[j].layout,
> > +            };
> >              if (a != VK_ATTACHMENT_UNUSED) {
> >                 has_input = true;
> >                 pass->attachments[a].usage |=
> VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT;
> > @@ -138,7 +142,11 @@ VkResult anv_CreateRenderPass(
> >
> >           for (uint32_t j = 0; j < desc->colorAttachmentCount; j++) {
> >              uint32_t a = desc->pColorAttachments[j].attachment;
> > -            subpass->color_attachments[j] = desc->pColorAttachments[j];
> > +            subpass->color_attachments[j] = (struct
> anv_subpass_attachment) {
> > +               .usage =       VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT,
> > +               .attachment =  desc->pColorAttachments[j].attachment,
> > +               .layout =      desc->pColorAttachments[j].layout,
> > +            };
>
> I kind of liked reusing the VkAttachmentReference struct instead of
> having to roll our own.


I used to think that way.  However, as we extend stuff, it tends to stop
being practical.  There are a number of cases where we started off with
Vulkan structures and then ended up no longer using them.  This is
especially prone to happen with extensions.


> What do you think about creating helper
> functions, like the following instead:
>
> static bool is_color_attachment(struct anv_subpass* subpass,
>                                 VkAttachmentReference* att_ref)
> {
>    return att_ref >= subpass->color_attachments &&
>           att_ref < subpass->color_attachments + subpass->color_count;
> }
>

I did exactly that in earlier versions of this series.  However, when I
found myself trying to write an is_depth_attachment function it got messy
and I decided it was easier to just stash the usage.  It's actually rather
convenient.

--Jason


> -Nanley
>
>
> >              if (a != VK_ATTACHMENT_UNUSED) {
> >                 has_color = true;
> >                 pass->attachments[a].usage |=
> VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT;
> > @@ -157,7 +165,11 @@ VkResult anv_CreateRenderPass(
> >
> >           for (uint32_t j = 0; j < desc->colorAttachmentCount; j++) {
> >              uint32_t a = desc->pResolveAttachments[j].attachment;
> > -            subpass->resolve_attachments[j] =
> desc->pResolveAttachments[j];
> > +            subpass->resolve_attachments[j] = (struct
> anv_subpass_attachment) {
> > +               .usage =       VK_IMAGE_USAGE_TRANSFER_DST_BIT,
> > +               .attachment =  desc->pResolveAttachments[j].attachment,
> > +               .layout =      desc->pResolveAttachments[j].layout,
> > +            };
> >              if (a != VK_ATTACHMENT_UNUSED) {
> >                 subpass->has_resolve = true;
> >                 uint32_t color_att = desc->pColorAttachments[j].att
> achment;
> > @@ -174,8 +186,12 @@ VkResult anv_CreateRenderPass(
> >
> >        if (desc->pDepthStencilAttachment) {
> >           uint32_t a = desc->pDepthStencilAttachment->attachment;
> > -         *subpass_attachments++ = subpass->depth_stencil_attachment =
> > -            *desc->pDepthStencilAttachment;
> > +         subpass->depth_stencil_attachment = (struct
> anv_subpass_attachment) {
> > +            .usage =       VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT,
> > +            .attachment =  desc->pDepthStencilAttachment->attachment,
> > +            .layout =      desc->pDepthStencilAttachment->layout,
> > +         };
> > +         *subpass_attachments++ = subpass->depth_stencil_attachment;
> >           if (a != VK_ATTACHMENT_UNUSED) {
> >              has_depth = true;
> >              pass->attachments[a].usage |=
> > @@ -186,8 +202,11 @@ VkResult anv_CreateRenderPass(
> >                                        *desc->pDepthStencilAttachment);
> >           }
> >        } else {
> > -         subpass->depth_stencil_attachment.attachment =
> VK_ATTACHMENT_UNUSED;
> > -         subpass->depth_stencil_attachment.layout =
> VK_IMAGE_LAYOUT_UNDEFINED;
> > +         subpass->depth_stencil_attachment = (struct
> anv_subpass_attachment) {
> > +            .usage =       VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT,
> > +            .attachment =  VK_ATTACHMENT_UNUSED,
> > +            .layout =   VK_IMAGE_LAYOUT_UNDEFINED,
> > +         };
> >        }
> >     }
> >
> > diff --git a/src/intel/vulkan/anv_private.h
> b/src/intel/vulkan/anv_private.h
> > index d424498..9a8da2b 100644
> > --- a/src/intel/vulkan/anv_private.h
> > +++ b/src/intel/vulkan/anv_private.h
> > @@ -2865,6 +2865,12 @@ struct anv_framebuffer {
> >     struct anv_image_view *                      attachments[0];
> >  };
> >
> > +struct anv_subpass_attachment {
> > +   VkImageUsageFlagBits usage;
> > +   uint32_t attachment;
> > +   VkImageLayout layout;
> > +};
> > +
> >  struct anv_subpass {
> >     uint32_t                                     attachment_count;
> >
> > @@ -2872,14 +2878,14 @@ struct anv_subpass {
> >      * A pointer to all attachment references used in this subpass.
> >      * Only valid if ::attachment_count > 0.
> >      */
> > -   VkAttachmentReference *                      attachments;
> > +   struct anv_subpass_attachment *              attachments;
> >     uint32_t                                     input_count;
> > -   VkAttachmentReference *                      input_attachments;
> > +   struct anv_subpass_attachment *              input_attachments;
> >     uint32_t                                     color_count;
> > -   VkAttachmentReference *                      color_attachments;
> > -   VkAttachmentReference *                      resolve_attachments;
> > +   struct anv_subpass_attachment *              color_attachments;
> > +   struct anv_subpass_attachment *              resolve_attachments;
> >
> > -   VkAttachmentReference
> depth_stencil_attachment;
> > +   struct anv_subpass_attachment
> depth_stencil_attachment;
> >
> >     uint32_t                                     view_mask;
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> > index 2590ea3..f92e86f 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -3310,7 +3310,7 @@ cmd_buffer_subpass_transition_layouts(struct
> anv_cmd_buffer * const cmd_buffer,
> >        assert(subpass->attachments);
> >
> >     /* Iterate over the array of attachment references. */
> > -   for (const VkAttachmentReference *att_ref = subpass->attachments;
> > +   for (const struct anv_subpass_attachment *att_ref =
> subpass->attachments;
> >          att_ref < subpass->attachments + subpass->attachment_count;
> att_ref++) {
> >
> >        /* If the attachment is unused, we can't perform a layout
> transition. */
> > --
> > 2.5.0.400.gff86faf
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180213/3d9bef47/attachment-0001.html>


More information about the mesa-dev mailing list