[Mesa-dev] [PATCH 1/2] radv: introduce radv_subpass_attachment data structure
Samuel Pitoiset
samuel.pitoiset at gmail.com
Mon Jul 9 12:55:59 UTC 2018
On 07/09/2018 10:36 AM, Bas Nieuwenhuizen wrote:
> On Mon, Jul 9, 2018 at 10:19 AM, Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>>
>>
>> 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?
>
> I read the extension now and saw that VkAttachmentReference2KHR has
> more fields, we just don't use them yet. Happy to take this in case we
> need to use those extra fields eventually.
So, Rb?
>>
>>
>>>
>>>> +
>>>> 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