Module: Mesa
Branch: main
Commit: 2cab67a9f1825b21584b86fbec5d03cca671830d
URL:    
http://cgit.freedesktop.org/mesa/mesa/commit/?id=2cab67a9f1825b21584b86fbec5d03cca671830d

Author: Faith Ekstrand <faith.ekstr...@collabora.com>
Date:   Mon Jan  8 09:33:32 2024 -0600

nvk: Invalidate state after secondary command buffers

Today, the only thing that this really affects is descriptor sets and
dynamic state as everything else is re-emitted almost every time.
However, as we add more dirtying, we'll need to be more and more careful
about stale state leaking across secondary command buffer executions.

Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27049>

---

 src/nouveau/vulkan/nvk_cmd_buffer.c   | 24 ++++++++++++++++++++++++
 src/nouveau/vulkan/nvk_cmd_buffer.h   |  3 +++
 src/nouveau/vulkan/nvk_cmd_dispatch.c |  6 ++++++
 src/nouveau/vulkan/nvk_cmd_draw.c     | 19 +++++++++++++++++++
 4 files changed, 52 insertions(+)

diff --git a/src/nouveau/vulkan/nvk_cmd_buffer.c 
b/src/nouveau/vulkan/nvk_cmd_buffer.c
index 24615afb5b5..c15ce19659b 100644
--- a/src/nouveau/vulkan/nvk_cmd_buffer.c
+++ b/src/nouveau/vulkan/nvk_cmd_buffer.c
@@ -307,6 +307,9 @@ nvk_CmdExecuteCommands(VkCommandBuffer commandBuffer,
 {
    VK_FROM_HANDLE(nvk_cmd_buffer, cmd, commandBuffer);
 
+   if (commandBufferCount == 0)
+      return;
+
    nvk_cmd_buffer_flush_push(cmd);
 
    for (uint32_t i = 0; i < commandBufferCount; i++) {
@@ -327,6 +330,27 @@ nvk_CmdExecuteCommands(VkCommandBuffer commandBuffer,
        */
       util_dynarray_append_dynarray(&cmd->pushes, &other->pushes);
    }
+
+   /* From the Vulkan 1.3.275 spec:
+    *
+    *    "When secondary command buffer(s) are recorded to execute on a
+    *    primary command buffer, the secondary command buffer inherits no
+    *    state from the primary command buffer, and all state of the primary
+    *    command buffer is undefined after an execute secondary command buffer
+    *    command is recorded. There is one exception to this rule - if the
+    *    primary command buffer is inside a render pass instance, then the
+    *    render pass and subpass state is not disturbed by executing secondary
+    *    command buffers. For state dependent commands (such as draws and
+    *    dispatches), any state consumed by those commands must not be
+    *    undefined."
+    *
+    * Therefore, it's the client's job to reset all the state in the primary
+    * after the secondary executes.  However, if we're doing any internal
+    * dirty tracking, we may miss the fact that a secondary has messed with
+    * GPU state if we don't invalidate all our internal tracking.
+    */
+   nvk_cmd_invalidate_graphics_state(cmd);
+   nvk_cmd_invalidate_compute_state(cmd);
 }
 
 enum nvk_barrier {
diff --git a/src/nouveau/vulkan/nvk_cmd_buffer.h 
b/src/nouveau/vulkan/nvk_cmd_buffer.h
index 32a2065dea3..86ec031f21e 100644
--- a/src/nouveau/vulkan/nvk_cmd_buffer.h
+++ b/src/nouveau/vulkan/nvk_cmd_buffer.h
@@ -204,6 +204,9 @@ void nvk_cmd_buffer_begin_graphics(struct nvk_cmd_buffer 
*cmd,
 void nvk_cmd_buffer_begin_compute(struct nvk_cmd_buffer *cmd,
                                   const VkCommandBufferBeginInfo *pBeginInfo);
 
+void nvk_cmd_invalidate_graphics_state(struct nvk_cmd_buffer *cmd);
+void nvk_cmd_invalidate_compute_state(struct nvk_cmd_buffer *cmd);
+
 void nvk_cmd_bind_graphics_pipeline(struct nvk_cmd_buffer *cmd,
                                     struct nvk_graphics_pipeline *pipeline);
 void nvk_cmd_bind_compute_pipeline(struct nvk_cmd_buffer *cmd,
diff --git a/src/nouveau/vulkan/nvk_cmd_dispatch.c 
b/src/nouveau/vulkan/nvk_cmd_dispatch.c
index c43bf377360..bbd886fb2e7 100644
--- a/src/nouveau/vulkan/nvk_cmd_dispatch.c
+++ b/src/nouveau/vulkan/nvk_cmd_dispatch.c
@@ -63,6 +63,12 @@ nvk_cmd_buffer_begin_compute(struct nvk_cmd_buffer *cmd,
    }
 }
 
+void
+nvk_cmd_invalidate_compute_state(struct nvk_cmd_buffer *cmd)
+{
+   memset(&cmd->state.cs, 0, sizeof(cmd->state.cs));
+}
+
 static void
 nva0c0_qmd_set_dispatch_size(UNUSED struct nvk_device *dev, uint32_t *qmd,
                              uint32_t x, uint32_t y, uint32_t z)
diff --git a/src/nouveau/vulkan/nvk_cmd_draw.c 
b/src/nouveau/vulkan/nvk_cmd_draw.c
index 49b6e9e1d9e..4ef277c982f 100644
--- a/src/nouveau/vulkan/nvk_cmd_draw.c
+++ b/src/nouveau/vulkan/nvk_cmd_draw.c
@@ -512,6 +512,25 @@ nvk_cmd_buffer_begin_graphics(struct nvk_cmd_buffer *cmd,
    }
 }
 
+void
+nvk_cmd_invalidate_graphics_state(struct nvk_cmd_buffer *cmd)
+{
+   vk_dynamic_graphics_state_dirty_all(&cmd->vk.dynamic_graphics_state);
+
+   /* From the Vulkan 1.3.275 spec:
+    *
+    *    "...There is one exception to this rule - if the primary command
+    *    buffer is inside a render pass instance, then the render pass and
+    *    subpass state is not disturbed by executing secondary command
+    *    buffers."
+    *
+    * We need to reset everything EXCEPT the render pass state.
+    */
+   struct nvk_rendering_state render_save = cmd->state.gfx.render;
+   memset(&cmd->state.gfx, 0, sizeof(cmd->state.gfx));
+   cmd->state.gfx.render = render_save;
+}
+
 static void
 nvk_attachment_init(struct nvk_attachment *att,
                     const VkRenderingAttachmentInfo *info)

Reply via email to