From 84381034a775346f839fbdf1f046057ef340b3dd Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Tue, 8 Nov 2022 10:43:37 -0600 Subject: [PATCH] vulkan: Unconditionally add barriers for missing external subpass deps This is a very scorched-earth approach which doesn't take into account whether or not there are any explicitly provided dependencies. We could take a finer-grained approach in theory but it's unlikely to matter in practice since you usually stall in Begin/EndRenderPass anyway. Fixes: 1d726940d288 ("vulkan: Add a common CmdBegin/EndRederPass implementation") Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/6203 Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/7650 Reviewed-by: Lionel Landwerlin Part-of: (cherry picked from commit 11b2a063bf1f18b3be9542be8c229427a33c92f0) --- .pick_status.json | 2 +- src/vulkan/runtime/vk_render_pass.c | 76 +++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/.pick_status.json b/.pick_status.json index d46bd76..44bb0d2 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -2038,7 +2038,7 @@ "description": "vulkan: Unconditionally add barriers for missing external subpass deps", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "1d726940d2881395994751078dad3bda0cabbdfb" }, diff --git a/src/vulkan/runtime/vk_render_pass.c b/src/vulkan/runtime/vk_render_pass.c index 188b710..848936e 100644 --- a/src/vulkan/runtime/vk_render_pass.c +++ b/src/vulkan/runtime/vk_render_pass.c @@ -1920,6 +1920,47 @@ begin_subpass(struct vk_command_buffer *cmd_buffer, mem_barrier.dstAccessMask |= dep->dst_access_mask; } + if (subpass_idx == 0) { + /* From the Vulkan 1.3.232 spec: + * + * "If there is no subpass dependency from VK_SUBPASS_EXTERNAL to the + * first subpass that uses an attachment, then an implicit subpass + * dependency exists from VK_SUBPASS_EXTERNAL to the first subpass it + * is used in. The implicit subpass dependency only exists if there + * exists an automatic layout transition away from initialLayout. The + * subpass dependency operates as if defined with the following + * parameters: + * + * VkSubpassDependency implicitDependency = { + * .srcSubpass = VK_SUBPASS_EXTERNAL; + * .dstSubpass = firstSubpass; // First subpass attachment is used in + * .srcStageMask = VK_PIPELINE_STAGE_NONE; + * .dstStageMask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT; + * .srcAccessMask = 0; + * .dstAccessMask = VK_ACCESS_INPUT_ATTACHMENT_READ_BIT | + * VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | + * VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | + * VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT | + * VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; + * .dependencyFlags = 0; + * };" + * + * We could track individual subpasses and attachments and views to make + * sure we only insert this barrier when it's absolutely necessary. + * However, this is only going to happen for the first subpass and + * you're probably going to take a stall in BeginRenderPass() anyway. + * If this is ever a perf problem, we can re-evaluate and do something + * more intellegent at that time. + */ + needs_mem_barrier = true; + mem_barrier.dstStageMask |= VK_PIPELINE_STAGE_ALL_COMMANDS_BIT; + mem_barrier.dstAccessMask |= VK_ACCESS_INPUT_ATTACHMENT_READ_BIT | + VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | + VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | + VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT | + VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; + } + uint32_t max_image_barrier_count = 0; for (uint32_t a = 0; a < subpass->attachment_count; a++) { const struct vk_subpass_attachment *sp_att = &subpass->attachments[a]; @@ -2073,6 +2114,41 @@ end_subpass(struct vk_command_buffer *cmd_buffer, mem_barrier.dstAccessMask |= dep->dst_access_mask; } + if (subpass_idx == pass->subpass_count - 1) { + /* From the Vulkan 1.3.232 spec: + * + * "Similarly, if there is no subpass dependency from the last + * subpass that uses an attachment to VK_SUBPASS_EXTERNAL, then an + * implicit subpass dependency exists from the last subpass it is + * used in to VK_SUBPASS_EXTERNAL. The implicit subpass dependency + * only exists if there exists an automatic layout transition into + * finalLayout. The subpass dependency operates as if defined with + * the following parameters: + * + * VkSubpassDependency implicitDependency = { + * .srcSubpass = lastSubpass; // Last subpass attachment is used in + * .dstSubpass = VK_SUBPASS_EXTERNAL; + * .srcStageMask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT; + * .dstStageMask = VK_PIPELINE_STAGE_NONE; + * .srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | + * VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; + * .dstAccessMask = 0; + * .dependencyFlags = 0; + * };" + * + * We could track individual subpasses and attachments and views to make + * sure we only insert this barrier when it's absolutely necessary. + * However, this is only going to happen for the last subpass and + * you're probably going to take a stall in EndRenderPass() anyway. + * If this is ever a perf problem, we can re-evaluate and do something + * more intellegent at that time. + */ + needs_mem_barrier = true; + mem_barrier.srcStageMask |= VK_PIPELINE_STAGE_ALL_COMMANDS_BIT; + mem_barrier.srcAccessMask |= VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | + VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; + } + if (needs_mem_barrier) { const VkDependencyInfo dependency_info = { .sType = VK_STRUCTURE_TYPE_DEPENDENCY_INFO, -- 2.7.4