From 2d3c50d484bb25fe595959b00cfb92fe538d1d6f Mon Sep 17 00:00:00 2001 From: Lars-Ivar Hesselberg Simonsen Date: Wed, 22 Jan 2025 18:18:24 +0100 Subject: [PATCH] panvk: Fix barriers in secondary cmdbufs w/o rp's When encountering pipeline barriers in secondary command buffers that do not start their renderpasses, our barrier logic would not detect the need to flush existing draws, leading to race conditions in case of subpassLoad. This change ensures we flush existing draws when required in secondary command buffers. Reviewed-by: Erik Faye-Lund Reviewed-by: Boris Brezillon Part-of: --- src/panfrost/ci/panfrost-g610-fails.txt | 5 ----- src/panfrost/vulkan/csf/panvk_cmd_buffer.h | 9 +++++++++ src/panfrost/vulkan/csf/panvk_vX_cmd_buffer.c | 2 +- src/panfrost/vulkan/csf/panvk_vX_cmd_draw.c | 16 ++++++---------- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/panfrost/ci/panfrost-g610-fails.txt b/src/panfrost/ci/panfrost-g610-fails.txt index 2de17518f10..66dc54d09fd 100644 --- a/src/panfrost/ci/panfrost-g610-fails.txt +++ b/src/panfrost/ci/panfrost-g610-fails.txt @@ -259,12 +259,7 @@ spec@ext_image_dma_buf_import@ext_image_dma_buf_import-refcount-multithread,Cras # CTS bug, tries to use vkCmdSetPatchControlPointsEXT when we don't have that dEQP-VK.pipeline.fast_linked_library.misc.interpolate_at_sample_no_sample_shading,Crash -dEQP-VK.renderpass.dedicated_allocation.attachment_allocation.input_output.63,Fail -dEQP-VK.renderpass2.dedicated_allocation.attachment_allocation.input_output.63,Fail - -dEQP-VK.renderpass.suballocation.attachment_allocation.input_output.63,Fail dEQP-VK.renderpass.multiple_subpasses_multiple_command_buffers.test,Fail -dEQP-VK.renderpass2.suballocation.attachment_allocation.input_output.63,Fail dEQP-VK.glsl.loops.special.do_while_dynamic_iterations.dowhile_trap_vertex,Crash diff --git a/src/panfrost/vulkan/csf/panvk_cmd_buffer.h b/src/panfrost/vulkan/csf/panvk_cmd_buffer.h index 404181e5a0b..b26c041d23b 100644 --- a/src/panfrost/vulkan/csf/panvk_cmd_buffer.h +++ b/src/panfrost/vulkan/csf/panvk_cmd_buffer.h @@ -400,6 +400,15 @@ struct panvk_cmd_buffer { VK_DEFINE_HANDLE_CASTS(panvk_cmd_buffer, vk.base, VkCommandBuffer, VK_OBJECT_TYPE_COMMAND_BUFFER) +static bool +inherits_render_ctx(struct panvk_cmd_buffer *cmdbuf) +{ + return (cmdbuf->vk.level == VK_COMMAND_BUFFER_LEVEL_SECONDARY && + (cmdbuf->flags & + VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT)) || + (cmdbuf->state.gfx.render.flags & VK_RENDERING_RESUMING_BIT); +} + static inline struct cs_builder * panvk_get_cs_builder(struct panvk_cmd_buffer *cmdbuf, uint32_t subqueue) { diff --git a/src/panfrost/vulkan/csf/panvk_vX_cmd_buffer.c b/src/panfrost/vulkan/csf/panvk_vX_cmd_buffer.c index fd6a34dd561..14fae6c9b5a 100644 --- a/src/panfrost/vulkan/csf/panvk_vX_cmd_buffer.c +++ b/src/panfrost/vulkan/csf/panvk_vX_cmd_buffer.c @@ -404,7 +404,7 @@ collect_cs_deps(struct panvk_cmd_buffer *cmdbuf, add_execution_dependency(wait_masks, src_stages, dst_stages); /* within a render pass */ - if (cmdbuf->state.gfx.render.tiler) { + if (cmdbuf->state.gfx.render.tiler || inherits_render_ctx(cmdbuf)) { if (should_split_render_pass(wait_masks, src_access, dst_access)) { deps->needs_draw_flush = true; } else { diff --git a/src/panfrost/vulkan/csf/panvk_vX_cmd_draw.c b/src/panfrost/vulkan/csf/panvk_vX_cmd_draw.c index d83ef117df6..fd797013ac3 100644 --- a/src/panfrost/vulkan/csf/panvk_vX_cmd_draw.c +++ b/src/panfrost/vulkan/csf/panvk_vX_cmd_draw.c @@ -690,15 +690,6 @@ cs_render_desc_ringbuf_move_ptr(struct cs_builder *b, uint32_t size, cs_wait_slot(b, SB_ID(LS), false); } -static bool -inherits_render_ctx(struct panvk_cmd_buffer *cmdbuf) -{ - return (cmdbuf->vk.level == VK_COMMAND_BUFFER_LEVEL_SECONDARY && - (cmdbuf->flags & - VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT)) || - (cmdbuf->state.gfx.render.flags & VK_RENDERING_RESUMING_BIT); -} - static VkResult get_tiler_desc(struct panvk_cmd_buffer *cmdbuf) { @@ -1187,7 +1178,7 @@ set_provoking_vertex_mode(struct panvk_cmd_buffer *cmdbuf) /* If this is not the first draw, first_provoking_vertex should match * the one from the previous draws. Unfortunately, we can't check it * when the render pass is inherited. */ - assert(!cmdbuf->state.gfx.render.fbds.gpu || + assert(!cmdbuf->state.gfx.render.fbds.gpu || inherits_render_ctx(cmdbuf) || fbinfo->first_provoking_vertex == first_provoking_vertex); fbinfo->first_provoking_vertex = first_provoking_vertex; @@ -2614,6 +2605,11 @@ panvk_per_arch(CmdEndRendering)(VkCommandBuffer commandBuffer) memset(&cmdbuf->state.gfx.render.oq, 0, sizeof(cmdbuf->state.gfx.render.oq)); cmdbuf->state.gfx.render.tiler = 0; + /* If we're finished with this render pass, make sure we reset the flags + * so any barrier encountered after EndRendering() doesn't try to flush + * draws. */ + cmdbuf->state.gfx.render.flags = 0; + /* If we're not suspending, we need to resolve attachments. */ if (!suspending) panvk_per_arch(cmd_resolve_attachments)(cmdbuf);