Re: [Mesa-dev] [PATCH] radv: allow secondary command buffers to inherit unknown framebuffers

[Samuel Pitoiset]

What's the status of this patch?

On 12/20/18 8:05 PM, Rhys Perry wrote:

Fixes: f4e499ec79 ('radv: add initial non-conformant radv vulkan driver')
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107986
Signed-off-by: Rhys Perry 
---
  src/amd/vulkan/radv_cmd_buffer.c | 59 ++--
  src/amd/vulkan/radv_meta_clear.c |  8 +
  src/amd/vulkan/radv_private.h|  2 ++
  3 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index c61310f3fc9..96fe5acb3bf 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -730,6 +730,9 @@ radv_emit_rbplus_state(struct radv_cmd_buffer *cmd_buffer)
struct radv_framebuffer *framebuffer = cmd_buffer->state.framebuffer;
const struct radv_subpass *subpass = cmd_buffer->state.subpass;
  
+	/* FIXME: handle when the framebuffer is unknown in secondary framebuffers */

+   assert(!cmd_buffer->inherit_unknown_fb);
+
unsigned sx_ps_downconvert = 0;
unsigned sx_blend_opt_epsilon = 0;
unsigned sx_blend_opt_control = 0;
@@ -1189,19 +1192,22 @@ radv_update_bound_fast_clear_ds(struct radv_cmd_buffer 
*cmd_buffer,
struct radv_framebuffer *framebuffer = cmd_buffer->state.framebuffer;
const struct radv_subpass *subpass = cmd_buffer->state.subpass;
struct radeon_cmdbuf *cs = cmd_buffer->cs;
-   struct radv_attachment_info *att;
-   uint32_t att_idx;
+   struct radv_attachment_info *att = NULL;
  
-	if (!framebuffer || !subpass)

+   if (!subpass)
return;
-
-   att_idx = subpass->depth_stencil_attachment.attachment;
-   if (att_idx == VK_ATTACHMENT_UNUSED)
+   if (!framebuffer && !cmd_buffer->inherit_unknown_fb)
return;
  
-	att = &framebuffer->attachments[att_idx];

-   if (att->attachment->image != image)
-   return;
+   if (framebuffer) {
+   uint32_t att_idx = subpass->depth_stencil_attachment.attachment;
+   if (att_idx == VK_ATTACHMENT_UNUSED)
+   return;
+
+   att = &framebuffer->attachments[att_idx];
+   if (att->attachment->image != image)
+   return;
+   }
  
  	radeon_set_context_reg_seq(cs, R_028028_DB_STENCIL_CLEAR, 2);

radeon_emit(cs, ds_clear_value.stencil);
@@ -1212,6 +1218,8 @@ radv_update_bound_fast_clear_ds(struct radv_cmd_buffer 
*cmd_buffer,
 */
if ((aspects & VK_IMAGE_ASPECT_DEPTH_BIT) &&
ds_clear_value.depth == 0.0) {
+   assert(att);
+
VkImageLayout layout = subpass->depth_stencil_attachment.layout;
  
  		radv_update_zrange_precision(cmd_buffer, &att->ds, image,

@@ -1426,19 +1434,22 @@ radv_update_bound_fast_clear_color(struct 
radv_cmd_buffer *cmd_buffer,
struct radv_framebuffer *framebuffer = cmd_buffer->state.framebuffer;
const struct radv_subpass *subpass = cmd_buffer->state.subpass;
struct radeon_cmdbuf *cs = cmd_buffer->cs;
-   struct radv_attachment_info *att;
-   uint32_t att_idx;
  
-	if (!framebuffer || !subpass)

+   if (!subpass)
return;
-
-   att_idx = subpass->color_attachments[cb_idx].attachment;
-   if (att_idx == VK_ATTACHMENT_UNUSED)
+   if (!framebuffer && !cmd_buffer->inherit_unknown_fb)
return;
  
-	att = &framebuffer->attachments[att_idx];

-   if (att->attachment->image != image)
-   return;
+   if (framebuffer) {
+   struct radv_attachment_info *att;
+   uint32_t att_idx = 
subpass->color_attachments[cb_idx].attachment;
+   if (att_idx == VK_ATTACHMENT_UNUSED)
+   return;
+
+   att = &framebuffer->attachments[att_idx];
+   if (att->attachment->image != image)
+   return;
+   }
  
  	radeon_set_context_reg_seq(cs, R_028C8C_CB_COLOR0_CLEAR_WORD0 + cb_idx * 0x3c, 2);

radeon_emit(cs, color_values[0]);
@@ -2528,6 +2539,7 @@ VkResult radv_BeginCommandBuffer(
cmd_buffer->state.last_first_instance = -1;
cmd_buffer->state.predication_type = -1;
cmd_buffer->usage_flags = pBeginInfo->flags;
+   cmd_buffer->inherit_unknown_fb = false;
  
  	if (cmd_buffer->level == VK_COMMAND_BUFFER_LEVEL_SECONDARY &&

(pBeginInfo->flags & 
VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT)) {
@@ -2535,6 +2547,9 @@ VkResult radv_BeginCommandBuffer(
cmd_buffer->state.framebuffer = 
radv_framebuffer_from_handle(pBeginInfo->pInheritanceInfo->framebuffer);
cmd_buffer->state.pass = 
radv_render_pass_from_handle(pBeginInfo->pInheritanceInfo->renderPass);
  
+		if (cmd_buffer->state.pass && !pBeginInfo->pInheritanceInfo->framebuffer)

+   cmd_buffer->inherit_unknown_fb = true;
+
struct radv_subpass *subpass =
 

Re: [Mesa-dev] [PATCH] radv: allow secondary command buffers to inherit unknown framebuffers

[Bas Nieuwenhuizen]

On Thu, Dec 20, 2018 at 8:05 PM Rhys Perry  wrote:
>
> Fixes: f4e499ec79 ('radv: add initial non-conformant radv vulkan driver')
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107986
> Signed-off-by: Rhys Perry 
> ---
>  src/amd/vulkan/radv_cmd_buffer.c | 59 ++--
>  src/amd/vulkan/radv_meta_clear.c |  8 +
>  src/amd/vulkan/radv_private.h|  2 ++
>  3 files changed, 50 insertions(+), 19 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c 
> b/src/amd/vulkan/radv_cmd_buffer.c
> index c61310f3fc9..96fe5acb3bf 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -730,6 +730,9 @@ radv_emit_rbplus_state(struct radv_cmd_buffer *cmd_buffer)
> struct radv_framebuffer *framebuffer = cmd_buffer->state.framebuffer;
> const struct radv_subpass *subpass = cmd_buffer->state.subpass;
>
> +   /* FIXME: handle when the framebuffer is unknown in secondary 
> framebuffers */
> +   assert(!cmd_buffer->inherit_unknown_fb);
> +
> unsigned sx_ps_downconvert = 0;
> unsigned sx_blend_opt_epsilon = 0;
> unsigned sx_blend_opt_control = 0;
> @@ -1189,19 +1192,22 @@ radv_update_bound_fast_clear_ds(struct 
> radv_cmd_buffer *cmd_buffer,
> struct radv_framebuffer *framebuffer = cmd_buffer->state.framebuffer;
> const struct radv_subpass *subpass = cmd_buffer->state.subpass;
> struct radeon_cmdbuf *cs = cmd_buffer->cs;
> -   struct radv_attachment_info *att;
> -   uint32_t att_idx;
> +   struct radv_attachment_info *att = NULL;
>
> -   if (!framebuffer || !subpass)
> +   if (!subpass)
> return;
> -
> -   att_idx = subpass->depth_stencil_attachment.attachment;
> -   if (att_idx == VK_ATTACHMENT_UNUSED)
> +   if (!framebuffer && !cmd_buffer->inherit_unknown_fb)
> return;
>
> -   att = &framebuffer->attachments[att_idx];
> -   if (att->attachment->image != image)
> -   return;
> +   if (framebuffer) {
> +   uint32_t att_idx = 
> subpass->depth_stencil_attachment.attachment;
> +   if (att_idx == VK_ATTACHMENT_UNUSED)
> +   return;
> +
> +   att = &framebuffer->attachments[att_idx];
> +   if (att->attachment->image != image)
> +   return;
> +   }
>
> radeon_set_context_reg_seq(cs, R_028028_DB_STENCIL_CLEAR, 2);
> radeon_emit(cs, ds_clear_value.stencil);
> @@ -1212,6 +1218,8 @@ radv_update_bound_fast_clear_ds(struct radv_cmd_buffer 
> *cmd_buffer,
>  */
> if ((aspects & VK_IMAGE_ASPECT_DEPTH_BIT) &&
> ds_clear_value.depth == 0.0) {
> +   assert(att);
> +
> VkImageLayout layout = 
> subpass->depth_stencil_attachment.layout;
>
> radv_update_zrange_precision(cmd_buffer, &att->ds, image,
> @@ -1426,19 +1434,22 @@ radv_update_bound_fast_clear_color(struct 
> radv_cmd_buffer *cmd_buffer,
> struct radv_framebuffer *framebuffer = cmd_buffer->state.framebuffer;
> const struct radv_subpass *subpass = cmd_buffer->state.subpass;
> struct radeon_cmdbuf *cs = cmd_buffer->cs;
> -   struct radv_attachment_info *att;
> -   uint32_t att_idx;
>
> -   if (!framebuffer || !subpass)
> +   if (!subpass)
> return;
> -
> -   att_idx = subpass->color_attachments[cb_idx].attachment;
> -   if (att_idx == VK_ATTACHMENT_UNUSED)
> +   if (!framebuffer && !cmd_buffer->inherit_unknown_fb)
> return;
>
> -   att = &framebuffer->attachments[att_idx];
> -   if (att->attachment->image != image)
> -   return;
> +   if (framebuffer) {
> +   struct radv_attachment_info *att;
> +   uint32_t att_idx = 
> subpass->color_attachments[cb_idx].attachment;
> +   if (att_idx == VK_ATTACHMENT_UNUSED)
> +   return;
> +
> +   att = &framebuffer->attachments[att_idx];
> +   if (att->attachment->image != image)
> +   return;
> +   }

If we don't do fast clears I don't think this can be called?

>
> radeon_set_context_reg_seq(cs, R_028C8C_CB_COLOR0_CLEAR_WORD0 + 
> cb_idx * 0x3c, 2);
> radeon_emit(cs, color_values[0]);
> @@ -2528,6 +2539,7 @@ VkResult radv_BeginCommandBuffer(
> cmd_buffer->state.last_first_instance = -1;
> cmd_buffer->state.predication_type = -1;
> cmd_buffer->usage_flags = pBeginInfo->flags;
> +   cmd_buffer->inherit_unknown_fb = false;
>
> if (cmd_buffer->level == VK_COMMAND_BUFFER_LEVEL_SECONDARY &&
> (pBeginInfo->flags & 
> VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT)) {
> @@ -2535,6 +2547,9 @@ VkResult radv_BeginCommandBuffer(
> cmd_buffer->state.framebuffer = 
> radv_framebuffer_from_handle(pBeginInfo->pInheritanceInfo->framebuffer);
> cmd_bu

[Mesa-dev] [PATCH] radv: allow secondary command buffers to inherit unknown framebuffers

[Rhys Perry]

Fixes: f4e499ec79 ('radv: add initial non-conformant radv vulkan driver')
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107986
Signed-off-by: Rhys Perry 
---
 src/amd/vulkan/radv_cmd_buffer.c | 59 ++--
 src/amd/vulkan/radv_meta_clear.c |  8 +
 src/amd/vulkan/radv_private.h|  2 ++
 3 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index c61310f3fc9..96fe5acb3bf 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -730,6 +730,9 @@ radv_emit_rbplus_state(struct radv_cmd_buffer *cmd_buffer)
struct radv_framebuffer *framebuffer = cmd_buffer->state.framebuffer;
const struct radv_subpass *subpass = cmd_buffer->state.subpass;
 
+   /* FIXME: handle when the framebuffer is unknown in secondary 
framebuffers */
+   assert(!cmd_buffer->inherit_unknown_fb);
+
unsigned sx_ps_downconvert = 0;
unsigned sx_blend_opt_epsilon = 0;
unsigned sx_blend_opt_control = 0;
@@ -1189,19 +1192,22 @@ radv_update_bound_fast_clear_ds(struct radv_cmd_buffer 
*cmd_buffer,
struct radv_framebuffer *framebuffer = cmd_buffer->state.framebuffer;
const struct radv_subpass *subpass = cmd_buffer->state.subpass;
struct radeon_cmdbuf *cs = cmd_buffer->cs;
-   struct radv_attachment_info *att;
-   uint32_t att_idx;
+   struct radv_attachment_info *att = NULL;
 
-   if (!framebuffer || !subpass)
+   if (!subpass)
return;
-
-   att_idx = subpass->depth_stencil_attachment.attachment;
-   if (att_idx == VK_ATTACHMENT_UNUSED)
+   if (!framebuffer && !cmd_buffer->inherit_unknown_fb)
return;
 
-   att = &framebuffer->attachments[att_idx];
-   if (att->attachment->image != image)
-   return;
+   if (framebuffer) {
+   uint32_t att_idx = subpass->depth_stencil_attachment.attachment;
+   if (att_idx == VK_ATTACHMENT_UNUSED)
+   return;
+
+   att = &framebuffer->attachments[att_idx];
+   if (att->attachment->image != image)
+   return;
+   }
 
radeon_set_context_reg_seq(cs, R_028028_DB_STENCIL_CLEAR, 2);
radeon_emit(cs, ds_clear_value.stencil);
@@ -1212,6 +1218,8 @@ radv_update_bound_fast_clear_ds(struct radv_cmd_buffer 
*cmd_buffer,
 */
if ((aspects & VK_IMAGE_ASPECT_DEPTH_BIT) &&
ds_clear_value.depth == 0.0) {
+   assert(att);
+
VkImageLayout layout = subpass->depth_stencil_attachment.layout;
 
radv_update_zrange_precision(cmd_buffer, &att->ds, image,
@@ -1426,19 +1434,22 @@ radv_update_bound_fast_clear_color(struct 
radv_cmd_buffer *cmd_buffer,
struct radv_framebuffer *framebuffer = cmd_buffer->state.framebuffer;
const struct radv_subpass *subpass = cmd_buffer->state.subpass;
struct radeon_cmdbuf *cs = cmd_buffer->cs;
-   struct radv_attachment_info *att;
-   uint32_t att_idx;
 
-   if (!framebuffer || !subpass)
+   if (!subpass)
return;
-
-   att_idx = subpass->color_attachments[cb_idx].attachment;
-   if (att_idx == VK_ATTACHMENT_UNUSED)
+   if (!framebuffer && !cmd_buffer->inherit_unknown_fb)
return;
 
-   att = &framebuffer->attachments[att_idx];
-   if (att->attachment->image != image)
-   return;
+   if (framebuffer) {
+   struct radv_attachment_info *att;
+   uint32_t att_idx = 
subpass->color_attachments[cb_idx].attachment;
+   if (att_idx == VK_ATTACHMENT_UNUSED)
+   return;
+
+   att = &framebuffer->attachments[att_idx];
+   if (att->attachment->image != image)
+   return;
+   }
 
radeon_set_context_reg_seq(cs, R_028C8C_CB_COLOR0_CLEAR_WORD0 + cb_idx 
* 0x3c, 2);
radeon_emit(cs, color_values[0]);
@@ -2528,6 +2539,7 @@ VkResult radv_BeginCommandBuffer(
cmd_buffer->state.last_first_instance = -1;
cmd_buffer->state.predication_type = -1;
cmd_buffer->usage_flags = pBeginInfo->flags;
+   cmd_buffer->inherit_unknown_fb = false;
 
if (cmd_buffer->level == VK_COMMAND_BUFFER_LEVEL_SECONDARY &&
(pBeginInfo->flags & 
VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT)) {
@@ -2535,6 +2547,9 @@ VkResult radv_BeginCommandBuffer(
cmd_buffer->state.framebuffer = 
radv_framebuffer_from_handle(pBeginInfo->pInheritanceInfo->framebuffer);
cmd_buffer->state.pass = 
radv_render_pass_from_handle(pBeginInfo->pInheritanceInfo->renderPass);
 
+   if (cmd_buffer->state.pass && 
!pBeginInfo->pInheritanceInfo->framebuffer)
+   cmd_buffer->inherit_unknown_fb = true;
+
struct radv_subpass *subpass =

&cmd_buffer->s