From dee7455281db618162e35689dbea1f3c415a4eab Mon Sep 17 00:00:00 2001 From: Natalie Vock Date: Mon, 23 Feb 2026 11:51:13 +0100 Subject: [PATCH] radv/rt: Fix cases in which the bound BVH build pipeline gets clobbered The most egregious case was AS updates, in which case radv_copy_memory would decide to use compute, which overwrites the bound pipeline with a copy shader. Subsequent dispatches assumed the update pipeline to be bound, but dispatched another copy shader instead. There is also a chance of this happening for geometry info copying for RRA. Instead of adding another pass (which would make us have to bump MAX_ENCODE_PASSES again), just rebind the pipeline - RRA captures aren't the most performance-sensitive thing in the world. --- src/amd/vulkan/radv_acceleration_structure.c | 88 ++++++++++++-------- 1 file changed, 55 insertions(+), 33 deletions(-) diff --git a/src/amd/vulkan/radv_acceleration_structure.c b/src/amd/vulkan/radv_acceleration_structure.c index 89e03872769..a93af54c41c 100644 --- a/src/amd/vulkan/radv_acceleration_structure.c +++ b/src/amd/vulkan/radv_acceleration_structure.c @@ -5,6 +5,7 @@ */ #include "meta/radv_meta.h" +#include "radv_buffer.h" #include "radv_cs.h" #include "radv_entrypoints.h" @@ -328,7 +329,7 @@ radv_get_build_config(VkDevice _device, struct vk_acceleration_structure_build_s if (device->meta_state.accel_struct_build.build_args.propagate_cull_flags) update_key |= VK_BUILD_FLAG_PROPAGATE_CULL_FLAGS; - state->config.update_key[0] = update_key; + state->config.update_key[1] = update_key; } static void @@ -702,6 +703,7 @@ radv_init_header(VkCommandBuffer commandBuffer, const struct vk_acceleration_str sizeof(header) - base); if (layout.geometry_info_offset != RADV_OFFSET_UNUSED) { + VK_FROM_HANDLE(radv_buffer, dst_buffer, vk_buffer_to_handle(dst->buffer)); uint64_t geometry_infos_size = state->build_info->geometryCount * sizeof(struct radv_accel_struct_geometry_info); struct radv_accel_struct_geometry_info *geometry_infos = malloc(geometry_infos_size); @@ -716,8 +718,14 @@ radv_init_header(VkCommandBuffer commandBuffer, const struct vk_acceleration_str geometry_infos[i].primitive_count = state->build_range_infos[i].primitiveCount; } - radv_CmdUpdateBuffer(commandBuffer, vk_buffer_to_handle(dst->buffer), dst->offset + layout.geometry_info_offset, - geometry_infos_size, geometry_infos); + radv_update_memory(cmd_buffer, vk_acceleration_structure_get_va(dst) + layout.geometry_info_offset, + geometry_infos_size, geometry_infos, radv_get_copy_flags_from_bo(dst_buffer->bo)); + + /* CmdUpdateBuffer might use compute, which clobbers the pipeline bind point. If that happens, rebind the header + * pipeline to restore the state expected at entry of radv_init_header. + */ + radv_bvh_build_bind_pipeline(commandBuffer, RADV_META_OBJECT_KEY_BVH_HEADER, header_spv, sizeof(header_spv), + sizeof(struct header_args), 0); free(geometry_infos); } @@ -740,7 +748,7 @@ radv_init_update_scratch(VkCommandBuffer commandBuffer, const struct vk_accelera layout.size - layout.internal_ready_count_offset, 0x0, RADV_COPY_FLAGS_DEVICE_LOCAL); /* geometryCount == 1 passes the data as push constant. */ - if (radv_use_bvh8(pdev) && !(state->config.update_key[0] & RADV_BUILD_FLAG_UPDATE_SINGLE_GEOMETRY)) { + if (radv_use_bvh8(pdev) && !(state->config.update_key[1] & RADV_BUILD_FLAG_UPDATE_SINGLE_GEOMETRY)) { uint32_t data_size = sizeof(struct vk_bvh_geometry_data) * state->build_info->geometryCount; struct vk_bvh_geometry_data *data = malloc(data_size); if (!data) { @@ -767,11 +775,40 @@ radv_init_update_scratch(VkCommandBuffer commandBuffer, const struct vk_accelera } } +static void +radv_update_copy_prepare(VkCommandBuffer commandBuffer, const struct vk_acceleration_structure_build_state *state, + bool flushed_cp_after_init_update_scratch, bool flushed_compute_after_init_update_scratch) +{ +} + +static void +radv_update_copy(VkCommandBuffer commandBuffer, const struct vk_acceleration_structure_build_state *state) +{ + VK_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer); + VK_FROM_HANDLE(vk_acceleration_structure, src, state->build_info->srcAccelerationStructure); + VK_FROM_HANDLE(vk_acceleration_structure, dst, state->build_info->dstAccelerationStructure); + struct radv_device *device = radv_cmd_buffer_device(cmd_buffer); + + if (src != dst) { + struct acceleration_structure_layout layout; + radv_get_acceleration_structure_layout(device, state, &layout); + + /* Copy header/metadata */ + const uint64_t src_va = vk_acceleration_structure_get_va(src); + const uint64_t dst_va = vk_acceleration_structure_get_va(dst); + + radv_copy_memory(cmd_buffer, src_va, dst_va, layout.bvh_offset, RADV_COPY_FLAGS_DEVICE_LOCAL, + RADV_COPY_FLAGS_DEVICE_LOCAL); + } +} + static void radv_update_prepare(VkCommandBuffer commandBuffer, const struct vk_acceleration_structure_build_state *state, bool flushed_cp_after_init_update_scratch, bool flushed_compute_after_init_update_scratch) { VK_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer); + VK_FROM_HANDLE(vk_acceleration_structure, src, state->build_info->srcAccelerationStructure); + VK_FROM_HANDLE(vk_acceleration_structure, dst, state->build_info->dstAccelerationStructure); struct radv_device *device = radv_cmd_buffer_device(cmd_buffer); const struct radv_physical_device *pdev = radv_device_physical(device); @@ -784,7 +821,14 @@ radv_update_prepare(VkCommandBuffer commandBuffer, const struct vk_acceleration_ cmd_buffer->state.flush_bits |= RADV_CMD_FLAG_INV_L2; } - uint32_t flags = state->config.update_key[0]; + uint32_t flags = state->config.update_key[1]; + + /* If we copied anything, we need to wait for that copy. */ + if (!(flags & RADV_BUILD_FLAG_UPDATE_IN_PLACE)) { + cmd_buffer->state.flush_bits |= RADV_CMD_FLAG_CS_PARTIAL_FLUSH | RADV_CMD_FLAG_INV_VCACHE | + radv_src_access_flush(cmd_buffer, VK_PIPELINE_STAGE_2_COMPUTE_SHADER_BIT, + VK_ACCESS_2_SHADER_WRITE_BIT, 0, NULL, NULL); + } if (radv_use_bvh8(pdev)) { radv_bvh_build_bind_pipeline(commandBuffer, RADV_META_OBJECT_KEY_BVH_UPDATE, update_gfx12_spv, @@ -813,18 +857,6 @@ radv_update_as(VkCommandBuffer commandBuffer, const struct vk_acceleration_struc VK_FROM_HANDLE(vk_acceleration_structure, dst, state->build_info->dstAccelerationStructure); struct radv_device *device = radv_cmd_buffer_device(cmd_buffer); - if (src != dst) { - struct acceleration_structure_layout layout; - radv_get_acceleration_structure_layout(device, state, &layout); - - /* Copy header/metadata */ - const uint64_t src_va = vk_acceleration_structure_get_va(src); - const uint64_t dst_va = vk_acceleration_structure_get_va(dst); - - radv_copy_memory(cmd_buffer, src_va, dst_va, layout.bvh_offset, RADV_COPY_FLAGS_DEVICE_LOCAL, - RADV_COPY_FLAGS_DEVICE_LOCAL); - } - struct update_scratch_layout layout; radv_get_update_scratch_layout(device, state, &layout); @@ -860,18 +892,6 @@ radv_update_as_gfx12(VkCommandBuffer commandBuffer, const struct vk_acceleration VK_FROM_HANDLE(vk_acceleration_structure, dst, state->build_info->dstAccelerationStructure); struct radv_device *device = radv_cmd_buffer_device(cmd_buffer); - if (src != dst) { - struct acceleration_structure_layout layout; - radv_get_acceleration_structure_layout(device, state, &layout); - - /* Copy header/metadata */ - const uint64_t src_va = vk_acceleration_structure_get_va(src); - const uint64_t dst_va = vk_acceleration_structure_get_va(dst); - - radv_copy_memory(cmd_buffer, src_va, dst_va, layout.bvh_offset, RADV_COPY_FLAGS_DEVICE_LOCAL, - RADV_COPY_FLAGS_DEVICE_LOCAL); - } - struct update_scratch_layout layout; radv_get_update_scratch_layout(device, state, &layout); @@ -884,7 +904,7 @@ radv_update_as_gfx12(VkCommandBuffer commandBuffer, const struct vk_acceleration .leaf_node_count = state->leaf_node_count, }; - if (state->config.update_key[0] & RADV_BUILD_FLAG_UPDATE_SINGLE_GEOMETRY) { + if (state->config.update_key[1] & RADV_BUILD_FLAG_UPDATE_SINGLE_GEOMETRY) { const VkAccelerationStructureGeometryKHR *geom = state->build_info->pGeometries ? &state->build_info->pGeometries[0] : state->build_info->ppGeometries[0]; update_consts.geom_data0 = vk_fill_geometry_data(state->build_info->type, 0, 0, geom, state->build_range_infos); @@ -968,11 +988,13 @@ radv_device_init_accel_struct_build_state(struct radv_device *device) .get_as_size = radv_get_as_size, .get_update_scratch_size = radv_get_update_scratch_size, .init_update_scratch = radv_init_update_scratch, - .update_prepare[0] = radv_update_prepare, + .update_prepare[0] = radv_update_copy_prepare, + .update_as[0] = radv_update_copy, + .update_prepare[1] = radv_update_prepare, }; if (radv_use_bvh8(pdev)) { - device->meta_state.accel_struct_build.build_ops.update_as[0] = radv_update_as_gfx12; + device->meta_state.accel_struct_build.build_ops.update_as[1] = radv_update_as_gfx12; device->meta_state.accel_struct_build.build_ops.get_encode_scratch_size = radv_get_encode_scratch_size; device->meta_state.accel_struct_build.build_ops.encode_prepare[0] = radv_encode_prepare_gfx12; device->meta_state.accel_struct_build.build_ops.encode_as[0] = radv_encode_as_gfx12; @@ -983,7 +1005,7 @@ radv_device_init_accel_struct_build_state(struct radv_device *device) device->meta_state.accel_struct_build.build_ops.encode_prepare[3] = radv_init_header_prepare; device->meta_state.accel_struct_build.build_ops.encode_as[3] = radv_init_header; } else { - device->meta_state.accel_struct_build.build_ops.update_as[0] = radv_update_as; + device->meta_state.accel_struct_build.build_ops.update_as[1] = radv_update_as; device->meta_state.accel_struct_build.build_ops.encode_prepare[0] = radv_encode_prepare; device->meta_state.accel_struct_build.build_ops.encode_as[0] = radv_encode_as; device->meta_state.accel_struct_build.build_ops.encode_prepare[1] = radv_init_header_prepare;