[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 <pendingchaos02 at gmail.com>
> ---
>   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->state.pass->subpasses[pBeginInfo->pInheritanceInfo->subpass];
>   
> @@ -2542,7 +2557,9 @@ VkResult radv_BeginCommandBuffer(
>   		if (result != VK_SUCCESS)
>   			return result;
>   
> -		radv_cmd_buffer_set_subpass(cmd_buffer, subpass, false);
> +		cmd_buffer->state.subpass = subpass;
> +		if (!cmd_buffer->inherit_unknown_fb && cmd_buffer->state.framebuffer)
> +			cmd_buffer->state.dirty |= RADV_CMD_DIRTY_FRAMEBUFFER;
>   	}
>   
>   	if (unlikely(cmd_buffer->device->trace_bo)) {
> @@ -3118,6 +3135,10 @@ void radv_CmdExecuteCommands(
>   		if (secondary->sample_positions_needed)
>   			primary->sample_positions_needed = true;
>   
> +		if (secondary->inherit_unknown_fb &&
> +		    primary->state.dirty & RADV_CMD_DIRTY_FRAMEBUFFER)
> +			radv_emit_framebuffer_state(primary);
> +
>   		primary->device->ws->cs_execute_secondary(primary->cs, secondary->cs);
>   
>   
> diff --git a/src/amd/vulkan/radv_meta_clear.c b/src/amd/vulkan/radv_meta_clear.c
> index 7a364ec684a..d6a64906196 100644
> --- a/src/amd/vulkan/radv_meta_clear.c
> +++ b/src/amd/vulkan/radv_meta_clear.c
> @@ -950,6 +950,14 @@ radv_can_fast_clear_depth(struct radv_cmd_buffer *cmd_buffer,
>   			  const VkClearDepthStencilValue clear_value,
>   			  uint32_t view_mask)
>   {
> +	/* Framebuffer information is needed for the TC-compat bug workaround
> +	 * in radv_update_bound_fast_clear_ds(). This isn't available when
> +	 * cmd_buffer->inherit_unknown_fb is true.
> +	 */
> +	if ((aspects & VK_IMAGE_ASPECT_DEPTH_BIT) &&
> +	    cmd_buffer->inherit_unknown_fb && clear_value.depth == 0.0)
> +		return false;
> +
>   	if (!radv_image_view_can_fast_clear(cmd_buffer->device, iview))
>   		return false;
>   
> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
> index 9a9cd5ff935..56dfd9882d3 100644
> --- a/src/amd/vulkan/radv_private.h
> +++ b/src/amd/vulkan/radv_private.h
> @@ -1118,6 +1118,8 @@ struct radv_cmd_buffer {
>   	 * Whether a query pool has been resetted and we have to flush caches.
>   	 */
>   	bool pending_reset_query;
> +
> +	bool inherit_unknown_fb;
>   };
>   
>   struct radv_image;