From 8534c24593288e744b97879e9a5809bfac90d956 Mon Sep 17 00:00:00 2001 From: Mel Henning Date: Tue, 2 Sep 2025 19:30:00 -0400 Subject: [PATCH] nvk: Clear second SET_RENDER_ENABLE operand The hardware actually compares a pair of 64-bit values, rather than comparing a single value against zero like we previously assumed. This wasn't an issue in most cases before because if the buffer is zero-initialized the previous code happens to work. If we get a buffer with garbage in it though we would run into issues. Fixes: 80eac1337d ("nvk: Always copy conditional rendering value before compare") Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/13821 Reviewed-by: Faith Ekstrand Reviewed-by: Mary Guillemard Part-of: (cherry picked from commit 90ac7d13dce89d236b2273255fc291edf49bc204) --- .pick_status.json | 2 +- src/nouveau/vulkan/nvk_cmd_draw.c | 52 +++++++++++++++++++++++++------ 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index a0d132f716b..6acad212e21 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -9364,7 +9364,7 @@ "description": "nvk: Clear second SET_RENDER_ENABLE operand", "nominated": true, "nomination_type": 2, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "80eac1337d47dd7899781bdc74796bf167b33c90", "notes": null diff --git a/src/nouveau/vulkan/nvk_cmd_draw.c b/src/nouveau/vulkan/nvk_cmd_draw.c index eb7e3b7e33f..bcb242c5881 100644 --- a/src/nouveau/vulkan/nvk_cmd_draw.c +++ b/src/nouveau/vulkan/nvk_cmd_draw.c @@ -4854,7 +4854,8 @@ nvk_CmdBeginConditionalRenderingEXT(VkCommandBuffer commandBuffer, * then the rendering commands are discarded, * otherwise they are executed as normal." * - * The hardware compare a 64-bit value, as such we are required to copy it. + * The hardware compares a pair of 64-bit values, so we need to copy the + * input value into one operand and zero into the other operatnd. */ uint64_t tmp_addr; VkResult result = nvk_cmd_buffer_cond_render_alloc(cmd, &tmp_addr); @@ -4863,37 +4864,68 @@ nvk_CmdBeginConditionalRenderingEXT(VkCommandBuffer commandBuffer, return; } - struct nv_push *p = nvk_cmd_buffer_push(cmd, 26); + /* Frustratingly, the u64s are not packed together */ + const uint64_t operand_a_addr = tmp_addr + 0; + const uint64_t operand_b_addr = tmp_addr + 16; + struct nv_push *p = nvk_cmd_buffer_push(cmd, 29); + + P_MTHD(p, NV90B5, LINE_LENGTH_IN); + P_NV90B5_LINE_LENGTH_IN(p, 1); + P_NV90B5_LINE_COUNT(p, 1); + + /* Copy value into operand A */ P_MTHD(p, NV90B5, OFFSET_IN_UPPER); P_NV90B5_OFFSET_IN_UPPER(p, addr >> 32); P_NV90B5_OFFSET_IN_LOWER(p, addr & 0xffffffff); - P_NV90B5_OFFSET_OUT_UPPER(p, tmp_addr >> 32); - P_NV90B5_OFFSET_OUT_LOWER(p, tmp_addr & 0xffffffff); - P_NV90B5_PITCH_IN(p, 4); - P_NV90B5_PITCH_OUT(p, 4); - P_NV90B5_LINE_LENGTH_IN(p, 4); - P_NV90B5_LINE_COUNT(p, 1); + P_NV90B5_OFFSET_OUT_UPPER(p, operand_a_addr >> 32); + P_NV90B5_OFFSET_OUT_LOWER(p, operand_a_addr & 0xffffffff); P_IMMD(p, NV90B5, SET_REMAP_COMPONENTS, { .dst_x = DST_X_SRC_X, .dst_y = DST_Y_SRC_X, .dst_z = DST_Z_NO_WRITE, .dst_w = DST_W_NO_WRITE, - .component_size = COMPONENT_SIZE_ONE, + .component_size = COMPONENT_SIZE_FOUR, .num_src_components = NUM_SRC_COMPONENTS_ONE, .num_dst_components = NUM_DST_COMPONENTS_TWO, }); P_IMMD(p, NV90B5, LAUNCH_DMA, { .data_transfer_type = DATA_TRANSFER_TYPE_PIPELINED, - .multi_line_enable = MULTI_LINE_ENABLE_TRUE, + .multi_line_enable = MULTI_LINE_ENABLE_FALSE, .flush_enable = FLUSH_ENABLE_TRUE, .src_memory_layout = SRC_MEMORY_LAYOUT_PITCH, .dst_memory_layout = DST_MEMORY_LAYOUT_PITCH, .remap_enable = REMAP_ENABLE_TRUE, }); + /* Copy zero into operand B */ + P_MTHD(p, NV90B5, OFFSET_OUT_UPPER); + P_NV90B5_OFFSET_OUT_UPPER(p, operand_b_addr >> 32); + P_NV90B5_OFFSET_OUT_LOWER(p, operand_b_addr & 0xffffffff); + + P_IMMD(p, NV90B5, SET_REMAP_CONST_A, 0); + P_IMMD(p, NV90B5, SET_REMAP_COMPONENTS, { + .dst_x = DST_X_CONST_A, + .dst_y = DST_Y_CONST_A, + .dst_z = DST_Z_NO_WRITE, + .dst_w = DST_W_NO_WRITE, + .component_size = COMPONENT_SIZE_FOUR, + .num_src_components = NUM_SRC_COMPONENTS_ONE, + .num_dst_components = NUM_DST_COMPONENTS_TWO, + }); + + P_IMMD(p, NV90B5, LAUNCH_DMA, { + .data_transfer_type = DATA_TRANSFER_TYPE_PIPELINED, + .multi_line_enable = MULTI_LINE_ENABLE_FALSE, + .flush_enable = FLUSH_ENABLE_TRUE, + .src_memory_layout = SRC_MEMORY_LAYOUT_PITCH, + .dst_memory_layout = DST_MEMORY_LAYOUT_PITCH, + .remap_enable = REMAP_ENABLE_TRUE, + }); + + /* Compare the operands */ P_MTHD(p, NV9097, SET_RENDER_ENABLE_A); P_NV9097_SET_RENDER_ENABLE_A(p, tmp_addr >> 32); P_NV9097_SET_RENDER_ENABLE_B(p, tmp_addr & 0xfffffff0);