[Mesa-dev] [PATCH 1/2] radv: introduce radv_subpass_attachment data structure

Samuel Pitoiset samuel.pitoiset at gmail.com
Mon Jul 9 08:19:57 UTC 2018



On 07/08/2018 09:50 PM, Bas Nieuwenhuizen wrote:
> On Sun, Jul 8, 2018 at 5:47 PM, Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>> Needed for VK_KHR_create_renderpass2.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   src/amd/vulkan/radv_cmd_buffer.c      |  4 ++--
>>   src/amd/vulkan/radv_meta_clear.c      |  4 ++--
>>   src/amd/vulkan/radv_meta_resolve.c    | 14 +++++++-------
>>   src/amd/vulkan/radv_meta_resolve_cs.c |  4 ++--
>>   src/amd/vulkan/radv_meta_resolve_fs.c |  6 +++---
>>   src/amd/vulkan/radv_pass.c            | 28 +++++++++++++++++----------
>>   src/amd/vulkan/radv_private.h         | 15 +++++++++-----
>>   7 files changed, 44 insertions(+), 31 deletions(-)
>>
>> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
>> index 1ea023a811..0ddddf50f6 100644
>> --- a/src/amd/vulkan/radv_cmd_buffer.c
>> +++ b/src/amd/vulkan/radv_cmd_buffer.c
>> @@ -2054,7 +2054,7 @@ static void radv_subpass_barrier(struct radv_cmd_buffer *cmd_buffer, const struc
>>   }
>>
>>   static void radv_handle_subpass_image_transition(struct radv_cmd_buffer *cmd_buffer,
>> -                                                VkAttachmentReference att)
>> +                                                struct radv_subpass_attachment att)
>>   {
>>          unsigned idx = att.attachment;
>>          struct radv_image_view *view = cmd_buffer->state.framebuffer->attachments[idx].attachment;
>> @@ -3944,7 +3944,7 @@ void radv_CmdEndRenderPass(
>>          for (unsigned i = 0; i < cmd_buffer->state.framebuffer->attachment_count; ++i) {
>>                  VkImageLayout layout = cmd_buffer->state.pass->attachments[i].final_layout;
>>                  radv_handle_subpass_image_transition(cmd_buffer,
>> -                                     (VkAttachmentReference){i, layout});
>> +                                     (struct radv_subpass_attachment){i, layout});
>>          }
>>
>>          vk_free(&cmd_buffer->pool->alloc, cmd_buffer->state.attachments);
>> diff --git a/src/amd/vulkan/radv_meta_clear.c b/src/amd/vulkan/radv_meta_clear.c
>> index 2c0bb37387..d7c9849734 100644
>> --- a/src/amd/vulkan/radv_meta_clear.c
>> +++ b/src/amd/vulkan/radv_meta_clear.c
>> @@ -366,10 +366,10 @@ emit_color_clear(struct radv_cmd_buffer *cmd_buffer,
>>
>>          struct radv_subpass clear_subpass = {
>>                  .color_count = 1,
>> -               .color_attachments = (VkAttachmentReference[]) {
>> +               .color_attachments = (struct radv_subpass_attachment[]) {
>>                          subpass->color_attachments[clear_att->colorAttachment]
>>                  },
>> -               .depth_stencil_attachment = (VkAttachmentReference) { VK_ATTACHMENT_UNUSED, VK_IMAGE_LAYOUT_UNDEFINED }
>> +               .depth_stencil_attachment = (struct radv_subpass_attachment) { VK_ATTACHMENT_UNUSED, VK_IMAGE_LAYOUT_UNDEFINED }
>>          };
>>
>>          radv_cmd_buffer_set_subpass(cmd_buffer, &clear_subpass, false);
>> diff --git a/src/amd/vulkan/radv_meta_resolve.c b/src/amd/vulkan/radv_meta_resolve.c
>> index d4d3552f31..b049237ba6 100644
>> --- a/src/amd/vulkan/radv_meta_resolve.c
>> +++ b/src/amd/vulkan/radv_meta_resolve.c
>> @@ -613,8 +613,8 @@ radv_cmd_buffer_resolve_subpass(struct radv_cmd_buffer *cmd_buffer)
>>                  return;
>>
>>          for (uint32_t i = 0; i < subpass->color_count; ++i) {
>> -               VkAttachmentReference src_att = subpass->color_attachments[i];
>> -               VkAttachmentReference dest_att = subpass->resolve_attachments[i];
>> +               struct radv_subpass_attachment src_att = subpass->color_attachments[i];
>> +               struct radv_subpass_attachment dest_att = subpass->resolve_attachments[i];
>>
>>                  if (src_att.attachment == VK_ATTACHMENT_UNUSED ||
>>                      dest_att.attachment == VK_ATTACHMENT_UNUSED)
>> @@ -641,8 +641,8 @@ radv_cmd_buffer_resolve_subpass(struct radv_cmd_buffer *cmd_buffer)
>>                         RADV_META_SAVE_GRAPHICS_PIPELINE);
>>
>>          for (uint32_t i = 0; i < subpass->color_count; ++i) {
>> -               VkAttachmentReference src_att = subpass->color_attachments[i];
>> -               VkAttachmentReference dest_att = subpass->resolve_attachments[i];
>> +               struct radv_subpass_attachment src_att = subpass->color_attachments[i];
>> +               struct radv_subpass_attachment dest_att = subpass->resolve_attachments[i];
>>
>>                  if (src_att.attachment == VK_ATTACHMENT_UNUSED ||
>>                      dest_att.attachment == VK_ATTACHMENT_UNUSED)
>> @@ -657,7 +657,7 @@ radv_cmd_buffer_resolve_subpass(struct radv_cmd_buffer *cmd_buffer)
>>
>>                  struct radv_subpass resolve_subpass = {
>>                          .color_count = 2,
>> -                       .color_attachments = (VkAttachmentReference[]) { src_att, dest_att },
>> +                       .color_attachments = (struct radv_subpass_attachment[]) { src_att, dest_att },
>>                          .depth_stencil_attachment = { .attachment = VK_ATTACHMENT_UNUSED },
>>                  };
>>
>> @@ -684,8 +684,8 @@ radv_decompress_resolve_subpass_src(struct radv_cmd_buffer *cmd_buffer)
>>          struct radv_framebuffer *fb = cmd_buffer->state.framebuffer;
>>
>>          for (uint32_t i = 0; i < subpass->color_count; ++i) {
>> -               VkAttachmentReference src_att = subpass->color_attachments[i];
>> -               VkAttachmentReference dest_att = subpass->resolve_attachments[i];
>> +               struct radv_subpass_attachment src_att = subpass->color_attachments[i];
>> +               struct radv_subpass_attachment dest_att = subpass->resolve_attachments[i];
>>
>>                  if (src_att.attachment == VK_ATTACHMENT_UNUSED ||
>>                      dest_att.attachment == VK_ATTACHMENT_UNUSED)
>> diff --git a/src/amd/vulkan/radv_meta_resolve_cs.c b/src/amd/vulkan/radv_meta_resolve_cs.c
>> index 4a37892b86..daf11e0576 100644
>> --- a/src/amd/vulkan/radv_meta_resolve_cs.c
>> +++ b/src/amd/vulkan/radv_meta_resolve_cs.c
>> @@ -501,8 +501,8 @@ radv_cmd_buffer_resolve_subpass_cs(struct radv_cmd_buffer *cmd_buffer)
>>                         RADV_META_SAVE_DESCRIPTORS);
>>
>>          for (uint32_t i = 0; i < subpass->color_count; ++i) {
>> -               VkAttachmentReference src_att = subpass->color_attachments[i];
>> -               VkAttachmentReference dest_att = subpass->resolve_attachments[i];
>> +               struct radv_subpass_attachment src_att = subpass->color_attachments[i];
>> +               struct radv_subpass_attachment dest_att = subpass->resolve_attachments[i];
>>                  struct radv_image_view *src_iview = cmd_buffer->state.framebuffer->attachments[src_att.attachment].attachment;
>>                  struct radv_image_view *dst_iview = cmd_buffer->state.framebuffer->attachments[dest_att.attachment].attachment;
>>                  if (dest_att.attachment == VK_ATTACHMENT_UNUSED)
>> diff --git a/src/amd/vulkan/radv_meta_resolve_fs.c b/src/amd/vulkan/radv_meta_resolve_fs.c
>> index ef8c1d8b1d..5f4f241893 100644
>> --- a/src/amd/vulkan/radv_meta_resolve_fs.c
>> +++ b/src/amd/vulkan/radv_meta_resolve_fs.c
>> @@ -611,8 +611,8 @@ radv_cmd_buffer_resolve_subpass_fs(struct radv_cmd_buffer *cmd_buffer)
>>          radv_decompress_resolve_subpass_src(cmd_buffer);
>>
>>          for (uint32_t i = 0; i < subpass->color_count; ++i) {
>> -               VkAttachmentReference src_att = subpass->color_attachments[i];
>> -               VkAttachmentReference dest_att = subpass->resolve_attachments[i];
>> +               struct radv_subpass_attachment src_att = subpass->color_attachments[i];
>> +               struct radv_subpass_attachment dest_att = subpass->resolve_attachments[i];
>>
>>                  if (src_att.attachment == VK_ATTACHMENT_UNUSED ||
>>                      dest_att.attachment == VK_ATTACHMENT_UNUSED)
>> @@ -623,7 +623,7 @@ radv_cmd_buffer_resolve_subpass_fs(struct radv_cmd_buffer *cmd_buffer)
>>
>>                  struct radv_subpass resolve_subpass = {
>>                          .color_count = 1,
>> -                       .color_attachments = (VkAttachmentReference[]) { dest_att },
>> +                       .color_attachments = (struct radv_subpass_attachment[]) { dest_att },
>>                          .depth_stencil_attachment = { .attachment = VK_ATTACHMENT_UNUSED },
>>                  };
>>
>> diff --git a/src/amd/vulkan/radv_pass.c b/src/amd/vulkan/radv_pass.c
>> index 15fee444cd..0e0f767751 100644
>> --- a/src/amd/vulkan/radv_pass.c
>> +++ b/src/amd/vulkan/radv_pass.c
>> @@ -80,7 +80,7 @@ VkResult radv_CreateRenderPass(
>>                  // att->stencil_store_op = pCreateInfo->pAttachments[i].stencilStoreOp;
>>          }
>>          uint32_t subpass_attachment_count = 0;
>> -       VkAttachmentReference *p;
>> +       struct radv_subpass_attachment *p;
>>          for (uint32_t i = 0; i < pCreateInfo->subpassCount; i++) {
>>                  const VkSubpassDescription *desc = &pCreateInfo->pSubpasses[i];
>>
>> @@ -94,7 +94,7 @@ VkResult radv_CreateRenderPass(
>>          if (subpass_attachment_count) {
>>                  pass->subpass_attachments =
>>                          vk_alloc2(&device->alloc, pAllocator,
>> -                                   subpass_attachment_count * sizeof(VkAttachmentReference), 8,
>> +                                   subpass_attachment_count * sizeof(struct radv_subpass_attachment), 8,
>>                                      VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
>>                  if (pass->subpass_attachments == NULL) {
>>                          vk_free2(&device->alloc, pAllocator, pass);
>> @@ -119,8 +119,10 @@ VkResult radv_CreateRenderPass(
>>                          p += desc->inputAttachmentCount;
>>
>>                          for (uint32_t j = 0; j < desc->inputAttachmentCount; j++) {
>> -                               subpass->input_attachments[j]
>> -                                       = desc->pInputAttachments[j];
>> +                               subpass->input_attachments[j] = (struct radv_subpass_attachment) {
>> +                                       .attachment = desc->pInputAttachments[j].attachment,
>> +                                       .layout = desc->pInputAttachments[j].layout,
>> +                               };
>>                                  if (desc->pInputAttachments[j].attachment != VK_ATTACHMENT_UNUSED)
>>                                          pass->attachments[desc->pInputAttachments[j].attachment].view_mask |= subpass->view_mask;
>>                          }
>> @@ -131,8 +133,10 @@ VkResult radv_CreateRenderPass(
>>                          p += desc->colorAttachmentCount;
>>
>>                          for (uint32_t j = 0; j < desc->colorAttachmentCount; j++) {
>> -                               subpass->color_attachments[j]
>> -                                       = desc->pColorAttachments[j];
>> +                               subpass->color_attachments[j] = (struct radv_subpass_attachment) {
>> +                                       .attachment = desc->pColorAttachments[j].attachment,
>> +                                       .layout = desc->pColorAttachments[j].layout,
>> +                               };
>>                                  if (desc->pColorAttachments[j].attachment != VK_ATTACHMENT_UNUSED) {
>>                                          pass->attachments[desc->pColorAttachments[j].attachment].view_mask |= subpass->view_mask;
>>                                          color_sample_count = pCreateInfo->pAttachments[desc->pColorAttachments[j].attachment].samples;
>> @@ -147,8 +151,10 @@ VkResult radv_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 radv_subpass_attachment) {
>> +                                       .attachment = desc->pResolveAttachments[j].attachment,
>> +                                       .layout = desc->pResolveAttachments[j].layout,
>> +                               };
>>                                  if (a != VK_ATTACHMENT_UNUSED) {
>>                                          subpass->has_resolve = true;
>>                                          pass->attachments[desc->pResolveAttachments[j].attachment].view_mask |= subpass->view_mask;
>> @@ -157,8 +163,10 @@ VkResult radv_CreateRenderPass(
>>                  }
>>
>>                  if (desc->pDepthStencilAttachment) {
>> -                       subpass->depth_stencil_attachment =
>> -                               *desc->pDepthStencilAttachment;
>> +                       subpass->depth_stencil_attachment = (struct radv_subpass_attachment) {
>> +                               .attachment = desc->pDepthStencilAttachment->attachment,
>> +                               .layout = desc->pDepthStencilAttachment->layout,
>> +                       };
>>                          if (desc->pDepthStencilAttachment->attachment != VK_ATTACHMENT_UNUSED) {
>>                                  pass->attachments[desc->pDepthStencilAttachment->attachment].view_mask |= subpass->view_mask;
>>                                  depth_sample_count = pCreateInfo->pAttachments[desc->pDepthStencilAttachment->attachment].samples;
>> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
>> index 4e4b3a6037..106e38a755 100644
>> --- a/src/amd/vulkan/radv_private.h
>> +++ b/src/amd/vulkan/radv_private.h
>> @@ -1706,13 +1706,18 @@ struct radv_subpass_barrier {
>>          VkAccessFlags        dst_access_mask;
>>   };
>>
>> +struct radv_subpass_attachment {
>> +       uint32_t         attachment;
>> +       VkImageLayout    layout;
>> +};
> 
> This is exactly the same as VkAttachmentReference, right? I also don't
> see you extending the struct in the next patch. What is the goal of
> this refactoring?

I think I wanted to refactor that part initially but it was not so easy 
to do ... Would you prefer a refactoring or are you okay if we duplicate 
the functions somehow?

> 
>> +
>>   struct radv_subpass {
>>          uint32_t                                     input_count;
>>          uint32_t                                     color_count;
>> -       VkAttachmentReference *                      input_attachments;
>> -       VkAttachmentReference *                      color_attachments;
>> -       VkAttachmentReference *                      resolve_attachments;
>> -       VkAttachmentReference                        depth_stencil_attachment;
>> +       struct radv_subpass_attachment *             input_attachments;
>> +       struct radv_subpass_attachment *             color_attachments;
>> +       struct radv_subpass_attachment *             resolve_attachments;
>> +       struct radv_subpass_attachment               depth_stencil_attachment;
>>
>>          /** Subpass has at least one resolve attachment */
>>          bool                                         has_resolve;
>> @@ -1736,7 +1741,7 @@ struct radv_render_pass_attachment {
>>   struct radv_render_pass {
>>          uint32_t                                     attachment_count;
>>          uint32_t                                     subpass_count;
>> -       VkAttachmentReference *                      subpass_attachments;
>> +       struct radv_subpass_attachment *             subpass_attachments;
>>          struct radv_render_pass_attachment *         attachments;
>>          struct radv_subpass_barrier                  end_barrier;
>>          struct radv_subpass                          subpasses[0];
>> --
>> 2.18.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list