[1/7] radv: record if a render pass has depth/stencil resolve attachments

Submitted by Samuel Pitoiset on May 27, 2019, 3:41 p.m.

Details

Message ID 20190527154150.23144-2-samuel.pitoiset@gmail.com
State New
Headers show
Series "radv: implement VK_KHR_depth_stencil_resolve" ( rev: 1 ) in Mesa

Browsing this patch as part of:
"radv: implement VK_KHR_depth_stencil_resolve" rev 1 in Mesa
<< prev patch [1/7] next patch >>

Commit Message

Samuel Pitoiset May 27, 2019, 3:41 p.m.
Only supported with vkCreateRenderPass2().

Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
---
 src/amd/vulkan/radv_pass.c    | 30 +++++++++++++++++++++++++++++-
 src/amd/vulkan/radv_private.h |  3 +++
 2 files changed, 32 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/amd/vulkan/radv_pass.c b/src/amd/vulkan/radv_pass.c
index 4d1e38a780e..b21bf37e401 100644
--- a/src/amd/vulkan/radv_pass.c
+++ b/src/amd/vulkan/radv_pass.c
@@ -75,6 +75,10 @@  radv_render_pass_compile(struct radv_render_pass *pass)
 		    subpass->depth_stencil_attachment->attachment == VK_ATTACHMENT_UNUSED)
 			subpass->depth_stencil_attachment = NULL;
 
+		if (subpass->ds_resolve_attachment &&
+		    subpass->ds_resolve_attachment->attachment == VK_ATTACHMENT_UNUSED)
+			subpass->ds_resolve_attachment = NULL;
+
 		for (uint32_t j = 0; j < subpass->attachment_count; j++) {
 			struct radv_subpass_attachment *subpass_att =
 				&subpass->attachments[j];
@@ -126,6 +130,9 @@  radv_render_pass_compile(struct radv_render_pass *pass)
 				subpass->has_resolve = true;
 			}
 		}
+
+		if (subpass->ds_resolve_attachment)
+			subpass->has_resolve = true;
 	}
 }
 
@@ -291,10 +298,15 @@  VkResult radv_CreateRenderPass(
 static unsigned
 radv_num_subpass_attachments2(const VkSubpassDescription2KHR *desc)
 {
+	const VkSubpassDescriptionDepthStencilResolveKHR *ds_resolve =
+		vk_find_struct_const(desc->pNext,
+				     SUBPASS_DESCRIPTION_DEPTH_STENCIL_RESOLVE_KHR);
+
 	return desc->inputAttachmentCount +
 	       desc->colorAttachmentCount +
 	       (desc->pResolveAttachments ? desc->colorAttachmentCount : 0) +
-	       (desc->pDepthStencilAttachment != NULL);
+	       (desc->pDepthStencilAttachment != NULL) +
+	       (ds_resolve && ds_resolve->pDepthStencilResolveAttachment);
 }
 
 VkResult radv_CreateRenderPass2KHR(
@@ -411,6 +423,22 @@  VkResult radv_CreateRenderPass2KHR(
 				.layout = desc->pDepthStencilAttachment->layout,
 			};
 		}
+
+		const VkSubpassDescriptionDepthStencilResolveKHR *ds_resolve =
+			vk_find_struct_const(desc->pNext,
+					     SUBPASS_DESCRIPTION_DEPTH_STENCIL_RESOLVE_KHR);
+
+		if (ds_resolve && ds_resolve->pDepthStencilResolveAttachment) {
+			subpass->ds_resolve_attachment = p++;
+
+			*subpass->ds_resolve_attachment = (struct radv_subpass_attachment) {
+				.attachment =  ds_resolve->pDepthStencilResolveAttachment->attachment,
+				.layout =      ds_resolve->pDepthStencilResolveAttachment->layout,
+			};
+
+			subpass->depth_resolve_mode = ds_resolve->depthResolveMode;
+			subpass->stencil_resolve_mode = ds_resolve->stencilResolveMode;
+		}
 	}
 
 	for (unsigned i = 0; i < pCreateInfo->dependencyCount; ++i) {
diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
index 7834a505562..e826740bc9f 100644
--- a/src/amd/vulkan/radv_private.h
+++ b/src/amd/vulkan/radv_private.h
@@ -1882,6 +1882,9 @@  struct radv_subpass {
 	struct radv_subpass_attachment *             color_attachments;
 	struct radv_subpass_attachment *             resolve_attachments;
 	struct radv_subpass_attachment *             depth_stencil_attachment;
+	struct radv_subpass_attachment *             ds_resolve_attachment;
+	VkResolveModeFlagBitsKHR                     depth_resolve_mode;
+	VkResolveModeFlagBitsKHR                     stencil_resolve_mode;
 
 	/** Subpass has at least one resolve attachment */
 	bool                                         has_resolve;

Comments

On Mon, May 27, 2019 at 5:38 PM Samuel Pitoiset
<samuel.pitoiset@gmail.com> wrote:
>
> Only supported with vkCreateRenderPass2().
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> ---
>  src/amd/vulkan/radv_pass.c    | 30 +++++++++++++++++++++++++++++-
>  src/amd/vulkan/radv_private.h |  3 +++
>  2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/src/amd/vulkan/radv_pass.c b/src/amd/vulkan/radv_pass.c
> index 4d1e38a780e..b21bf37e401 100644
> --- a/src/amd/vulkan/radv_pass.c
> +++ b/src/amd/vulkan/radv_pass.c
> @@ -75,6 +75,10 @@ radv_render_pass_compile(struct radv_render_pass *pass)
>                     subpass->depth_stencil_attachment->attachment == VK_ATTACHMENT_UNUSED)
>                         subpass->depth_stencil_attachment = NULL;
>
> +               if (subpass->ds_resolve_attachment &&
> +                   subpass->ds_resolve_attachment->attachment == VK_ATTACHMENT_UNUSED)
> +                       subpass->ds_resolve_attachment = NULL;
> +
>                 for (uint32_t j = 0; j < subpass->attachment_count; j++) {
>                         struct radv_subpass_attachment *subpass_att =
>                                 &subpass->attachments[j];
> @@ -126,6 +130,9 @@ radv_render_pass_compile(struct radv_render_pass *pass)
>                                 subpass->has_resolve = true;
>                         }
>                 }
> +
> +               if (subpass->ds_resolve_attachment)
> +                       subpass->has_resolve = true;

I think this makes the code assume that there are also color resolves
to be done, which might not be the case?

>         }
>  }
>
> @@ -291,10 +298,15 @@ VkResult radv_CreateRenderPass(
>  static unsigned
>  radv_num_subpass_attachments2(const VkSubpassDescription2KHR *desc)
>  {
> +       const VkSubpassDescriptionDepthStencilResolveKHR *ds_resolve =
> +               vk_find_struct_const(desc->pNext,
> +                                    SUBPASS_DESCRIPTION_DEPTH_STENCIL_RESOLVE_KHR);
> +
>         return desc->inputAttachmentCount +
>                desc->colorAttachmentCount +
>                (desc->pResolveAttachments ? desc->colorAttachmentCount : 0) +
> -              (desc->pDepthStencilAttachment != NULL);
> +              (desc->pDepthStencilAttachment != NULL) +
> +              (ds_resolve && ds_resolve->pDepthStencilResolveAttachment);
>  }
>
>  VkResult radv_CreateRenderPass2KHR(
> @@ -411,6 +423,22 @@ VkResult radv_CreateRenderPass2KHR(
>                                 .layout = desc->pDepthStencilAttachment->layout,
>                         };
>                 }
> +
> +               const VkSubpassDescriptionDepthStencilResolveKHR *ds_resolve =
> +                       vk_find_struct_const(desc->pNext,
> +                                            SUBPASS_DESCRIPTION_DEPTH_STENCIL_RESOLVE_KHR);
> +
> +               if (ds_resolve && ds_resolve->pDepthStencilResolveAttachment) {
> +                       subpass->ds_resolve_attachment = p++;
> +
> +                       *subpass->ds_resolve_attachment = (struct radv_subpass_attachment) {
> +                               .attachment =  ds_resolve->pDepthStencilResolveAttachment->attachment,
> +                               .layout =      ds_resolve->pDepthStencilResolveAttachment->layout,
> +                       };
> +
> +                       subpass->depth_resolve_mode = ds_resolve->depthResolveMode;
> +                       subpass->stencil_resolve_mode = ds_resolve->stencilResolveMode;
> +               }
>         }
>
>         for (unsigned i = 0; i < pCreateInfo->dependencyCount; ++i) {
> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
> index 7834a505562..e826740bc9f 100644
> --- a/src/amd/vulkan/radv_private.h
> +++ b/src/amd/vulkan/radv_private.h
> @@ -1882,6 +1882,9 @@ struct radv_subpass {
>         struct radv_subpass_attachment *             color_attachments;
>         struct radv_subpass_attachment *             resolve_attachments;
>         struct radv_subpass_attachment *             depth_stencil_attachment;
> +       struct radv_subpass_attachment *             ds_resolve_attachment;
> +       VkResolveModeFlagBitsKHR                     depth_resolve_mode;
> +       VkResolveModeFlagBitsKHR                     stencil_resolve_mode;
>
>         /** Subpass has at least one resolve attachment */
>         bool                                         has_resolve;
> --
> 2.21.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On 5/27/19 6:48 PM, Bas Nieuwenhuizen wrote:
> On Mon, May 27, 2019 at 5:38 PM Samuel Pitoiset
> <samuel.pitoiset@gmail.com> wrote:
>> Only supported with vkCreateRenderPass2().
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
>> ---
>>   src/amd/vulkan/radv_pass.c    | 30 +++++++++++++++++++++++++++++-
>>   src/amd/vulkan/radv_private.h |  3 +++
>>   2 files changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/amd/vulkan/radv_pass.c b/src/amd/vulkan/radv_pass.c
>> index 4d1e38a780e..b21bf37e401 100644
>> --- a/src/amd/vulkan/radv_pass.c
>> +++ b/src/amd/vulkan/radv_pass.c
>> @@ -75,6 +75,10 @@ radv_render_pass_compile(struct radv_render_pass *pass)
>>                      subpass->depth_stencil_attachment->attachment == VK_ATTACHMENT_UNUSED)
>>                          subpass->depth_stencil_attachment = NULL;
>>
>> +               if (subpass->ds_resolve_attachment &&
>> +                   subpass->ds_resolve_attachment->attachment == VK_ATTACHMENT_UNUSED)
>> +                       subpass->ds_resolve_attachment = NULL;
>> +
>>                  for (uint32_t j = 0; j < subpass->attachment_count; j++) {
>>                          struct radv_subpass_attachment *subpass_att =
>>                                  &subpass->attachments[j];
>> @@ -126,6 +130,9 @@ radv_render_pass_compile(struct radv_render_pass *pass)
>>                                  subpass->has_resolve = true;
>>                          }
>>                  }
>> +
>> +               if (subpass->ds_resolve_attachment)
>> +                       subpass->has_resolve = true;
> I think this makes the code assume that there are also color resolves
> to be done, which might not be the case?
Right, how about renaming has_resolve to has_color_resolves, remove this 
if statement and update the checks in radv_cmd_buffer_resolve_subpass() ?
>
>>          }
>>   }
>>
>> @@ -291,10 +298,15 @@ VkResult radv_CreateRenderPass(
>>   static unsigned
>>   radv_num_subpass_attachments2(const VkSubpassDescription2KHR *desc)
>>   {
>> +       const VkSubpassDescriptionDepthStencilResolveKHR *ds_resolve =
>> +               vk_find_struct_const(desc->pNext,
>> +                                    SUBPASS_DESCRIPTION_DEPTH_STENCIL_RESOLVE_KHR);
>> +
>>          return desc->inputAttachmentCount +
>>                 desc->colorAttachmentCount +
>>                 (desc->pResolveAttachments ? desc->colorAttachmentCount : 0) +
>> -              (desc->pDepthStencilAttachment != NULL);
>> +              (desc->pDepthStencilAttachment != NULL) +
>> +              (ds_resolve && ds_resolve->pDepthStencilResolveAttachment);
>>   }
>>
>>   VkResult radv_CreateRenderPass2KHR(
>> @@ -411,6 +423,22 @@ VkResult radv_CreateRenderPass2KHR(
>>                                  .layout = desc->pDepthStencilAttachment->layout,
>>                          };
>>                  }
>> +
>> +               const VkSubpassDescriptionDepthStencilResolveKHR *ds_resolve =
>> +                       vk_find_struct_const(desc->pNext,
>> +                                            SUBPASS_DESCRIPTION_DEPTH_STENCIL_RESOLVE_KHR);
>> +
>> +               if (ds_resolve && ds_resolve->pDepthStencilResolveAttachment) {
>> +                       subpass->ds_resolve_attachment = p++;
>> +
>> +                       *subpass->ds_resolve_attachment = (struct radv_subpass_attachment) {
>> +                               .attachment =  ds_resolve->pDepthStencilResolveAttachment->attachment,
>> +                               .layout =      ds_resolve->pDepthStencilResolveAttachment->layout,
>> +                       };
>> +
>> +                       subpass->depth_resolve_mode = ds_resolve->depthResolveMode;
>> +                       subpass->stencil_resolve_mode = ds_resolve->stencilResolveMode;
>> +               }
>>          }
>>
>>          for (unsigned i = 0; i < pCreateInfo->dependencyCount; ++i) {
>> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
>> index 7834a505562..e826740bc9f 100644
>> --- a/src/amd/vulkan/radv_private.h
>> +++ b/src/amd/vulkan/radv_private.h
>> @@ -1882,6 +1882,9 @@ struct radv_subpass {
>>          struct radv_subpass_attachment *             color_attachments;
>>          struct radv_subpass_attachment *             resolve_attachments;
>>          struct radv_subpass_attachment *             depth_stencil_attachment;
>> +       struct radv_subpass_attachment *             ds_resolve_attachment;
>> +       VkResolveModeFlagBitsKHR                     depth_resolve_mode;
>> +       VkResolveModeFlagBitsKHR                     stencil_resolve_mode;
>>
>>          /** Subpass has at least one resolve attachment */
>>          bool                                         has_resolve;
>> --
>> 2.21.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
That seems OK.

On Tue, May 28, 2019 at 8:25 AM Samuel Pitoiset
<samuel.pitoiset@gmail.com> wrote:
>
>
> On 5/27/19 6:48 PM, Bas Nieuwenhuizen wrote:
> > On Mon, May 27, 2019 at 5:38 PM Samuel Pitoiset
> > <samuel.pitoiset@gmail.com> wrote:
> >> Only supported with vkCreateRenderPass2().
> >>
> >> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> >> ---
> >>   src/amd/vulkan/radv_pass.c    | 30 +++++++++++++++++++++++++++++-
> >>   src/amd/vulkan/radv_private.h |  3 +++
> >>   2 files changed, 32 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/amd/vulkan/radv_pass.c b/src/amd/vulkan/radv_pass.c
> >> index 4d1e38a780e..b21bf37e401 100644
> >> --- a/src/amd/vulkan/radv_pass.c
> >> +++ b/src/amd/vulkan/radv_pass.c
> >> @@ -75,6 +75,10 @@ radv_render_pass_compile(struct radv_render_pass *pass)
> >>                      subpass->depth_stencil_attachment->attachment == VK_ATTACHMENT_UNUSED)
> >>                          subpass->depth_stencil_attachment = NULL;
> >>
> >> +               if (subpass->ds_resolve_attachment &&
> >> +                   subpass->ds_resolve_attachment->attachment == VK_ATTACHMENT_UNUSED)
> >> +                       subpass->ds_resolve_attachment = NULL;
> >> +
> >>                  for (uint32_t j = 0; j < subpass->attachment_count; j++) {
> >>                          struct radv_subpass_attachment *subpass_att =
> >>                                  &subpass->attachments[j];
> >> @@ -126,6 +130,9 @@ radv_render_pass_compile(struct radv_render_pass *pass)
> >>                                  subpass->has_resolve = true;
> >>                          }
> >>                  }
> >> +
> >> +               if (subpass->ds_resolve_attachment)
> >> +                       subpass->has_resolve = true;
> > I think this makes the code assume that there are also color resolves
> > to be done, which might not be the case?
> Right, how about renaming has_resolve to has_color_resolves, remove this
> if statement and update the checks in radv_cmd_buffer_resolve_subpass() ?
> >
> >>          }
> >>   }
> >>
> >> @@ -291,10 +298,15 @@ VkResult radv_CreateRenderPass(
> >>   static unsigned
> >>   radv_num_subpass_attachments2(const VkSubpassDescription2KHR *desc)
> >>   {
> >> +       const VkSubpassDescriptionDepthStencilResolveKHR *ds_resolve =
> >> +               vk_find_struct_const(desc->pNext,
> >> +                                    SUBPASS_DESCRIPTION_DEPTH_STENCIL_RESOLVE_KHR);
> >> +
> >>          return desc->inputAttachmentCount +
> >>                 desc->colorAttachmentCount +
> >>                 (desc->pResolveAttachments ? desc->colorAttachmentCount : 0) +
> >> -              (desc->pDepthStencilAttachment != NULL);
> >> +              (desc->pDepthStencilAttachment != NULL) +
> >> +              (ds_resolve && ds_resolve->pDepthStencilResolveAttachment);
> >>   }
> >>
> >>   VkResult radv_CreateRenderPass2KHR(
> >> @@ -411,6 +423,22 @@ VkResult radv_CreateRenderPass2KHR(
> >>                                  .layout = desc->pDepthStencilAttachment->layout,
> >>                          };
> >>                  }
> >> +
> >> +               const VkSubpassDescriptionDepthStencilResolveKHR *ds_resolve =
> >> +                       vk_find_struct_const(desc->pNext,
> >> +                                            SUBPASS_DESCRIPTION_DEPTH_STENCIL_RESOLVE_KHR);
> >> +
> >> +               if (ds_resolve && ds_resolve->pDepthStencilResolveAttachment) {
> >> +                       subpass->ds_resolve_attachment = p++;
> >> +
> >> +                       *subpass->ds_resolve_attachment = (struct radv_subpass_attachment) {
> >> +                               .attachment =  ds_resolve->pDepthStencilResolveAttachment->attachment,
> >> +                               .layout =      ds_resolve->pDepthStencilResolveAttachment->layout,
> >> +                       };
> >> +
> >> +                       subpass->depth_resolve_mode = ds_resolve->depthResolveMode;
> >> +                       subpass->stencil_resolve_mode = ds_resolve->stencilResolveMode;
> >> +               }
> >>          }
> >>
> >>          for (unsigned i = 0; i < pCreateInfo->dependencyCount; ++i) {
> >> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
> >> index 7834a505562..e826740bc9f 100644
> >> --- a/src/amd/vulkan/radv_private.h
> >> +++ b/src/amd/vulkan/radv_private.h
> >> @@ -1882,6 +1882,9 @@ struct radv_subpass {
> >>          struct radv_subpass_attachment *             color_attachments;
> >>          struct radv_subpass_attachment *             resolve_attachments;
> >>          struct radv_subpass_attachment *             depth_stencil_attachment;
> >> +       struct radv_subpass_attachment *             ds_resolve_attachment;
> >> +       VkResolveModeFlagBitsKHR                     depth_resolve_mode;
> >> +       VkResolveModeFlagBitsKHR                     stencil_resolve_mode;
> >>
> >>          /** Subpass has at least one resolve attachment */
> >>          bool                                         has_resolve;
> >> --
> >> 2.21.0
> >>
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev