From 5704d0ad275fea1e50526a3fdc268a63a731bf80 Mon Sep 17 00:00:00 2001 From: Connor Abbott Date: Mon, 12 Jun 2023 16:22:04 +0200 Subject: [PATCH] tu: Don't use A6XX_PC_PRIMITIVE_CNTL_0::TESS_UPPER_LEFT_DOMAIN_ORIGIN This field also affects triangle strip and triangle fan ordering, so we would get the incorrect (D3D) order with tessellation and geometry shaders both enabled. Instead flip clockwise/counterclockwise when the domain origin is upper-left, as radv does. Because the register is only emitted when tessellation is active which forces sysmem, it shouldn't regress performance to emit it directly instead of using a draw state. We're already very tight on draw states. Cc: mesa-stable Part-of: --- src/freedreno/vulkan/tu_cmd_buffer.cc | 31 ++++++++++-- src/freedreno/vulkan/tu_cmd_buffer.h | 11 ++++- src/freedreno/vulkan/tu_pipeline.cc | 71 ++++++++++++++------------- src/freedreno/vulkan/tu_pipeline.h | 3 ++ 4 files changed, 78 insertions(+), 38 deletions(-) diff --git a/src/freedreno/vulkan/tu_cmd_buffer.cc b/src/freedreno/vulkan/tu_cmd_buffer.cc index 55d43dc4a61..d7138361907 100644 --- a/src/freedreno/vulkan/tu_cmd_buffer.cc +++ b/src/freedreno/vulkan/tu_cmd_buffer.cc @@ -3069,6 +3069,23 @@ tu_CmdBindPipeline(VkCommandBuffer commandBuffer, if (pipeline->output.rb_depth_cntl_disable) cmd->state.dirty |= TU_CMD_DIRTY_DS; + + if (pipeline->active_stages & MESA_SHADER_TESS_CTRL) { + if (!cmd->state.tess_params.valid || + cmd->state.tess_params.output_upper_left != + pipeline->program.tess_output_upper_left || + cmd->state.tess_params.output_lower_left != + pipeline->program.tess_output_lower_left || + cmd->state.tess_params.spacing != pipeline->program.tess_spacing) { + cmd->state.tess_params.output_upper_left = + pipeline->program.tess_output_upper_left; + cmd->state.tess_params.output_lower_left = + pipeline->program.tess_output_lower_left; + cmd->state.tess_params.spacing = pipeline->program.tess_spacing; + cmd->state.tess_params.valid = true; + cmd->state.dirty |= TU_CMD_DIRTY_TESS_PARAMS; + } + } } VKAPI_ATTR void VKAPI_CALL @@ -5437,13 +5454,21 @@ tu6_draw_common(struct tu_cmd_buffer *cmd, tu_cs_emit_regs( cs, A6XX_PC_PRIMITIVE_CNTL_0(.primitive_restart = primitive_restart, - .provoking_vtx_last = provoking_vtx_last, - .tess_upper_left_domain_origin = - tess_upper_left_domain_origin)); + .provoking_vtx_last = provoking_vtx_last)); prim_params->valid = true; prim_params->primitive_restart = primitive_restart; prim_params->provoking_vtx_last = provoking_vtx_last; prim_params->tess_upper_left_domain_origin = tess_upper_left_domain_origin; + cmd->state.dirty |= TU_CMD_DIRTY_TESS_PARAMS; + } + + struct tu_tess_params *tess_params = &cmd->state.tess_params; + if (cmd->state.dirty & TU_CMD_DIRTY_TESS_PARAMS) { + tu_cs_emit_regs(cs, A6XX_PC_TESS_CNTL( + .spacing = tess_params->spacing, + .output = prim_params->tess_upper_left_domain_origin ? + tess_params->output_upper_left : + tess_params->output_lower_left)); } /* Early exit if there is nothing to emit, saves CPU cycles */ diff --git a/src/freedreno/vulkan/tu_cmd_buffer.h b/src/freedreno/vulkan/tu_cmd_buffer.h index 1925c085837..70aafc68afc 100644 --- a/src/freedreno/vulkan/tu_cmd_buffer.h +++ b/src/freedreno/vulkan/tu_cmd_buffer.h @@ -69,8 +69,9 @@ enum tu_cmd_dirty_bits TU_CMD_DIRTY_SCISSORS = BIT(12), TU_CMD_DIRTY_BLEND = BIT(13), TU_CMD_DIRTY_PATCH_CONTROL_POINTS = BIT(14), + TU_CMD_DIRTY_TESS_PARAMS = BIT(15), /* all draw states were disabled and need to be re-enabled: */ - TU_CMD_DIRTY_DRAW_STATE = BIT(15) + TU_CMD_DIRTY_DRAW_STATE = BIT(16) }; /* There are only three cache domains we have to care about: the CCU, or @@ -273,6 +274,12 @@ struct tu_primitive_params { bool tess_upper_left_domain_origin; }; +struct tu_tess_params { + bool valid; + enum a6xx_tess_output output_upper_left, output_lower_left; + enum a6xx_tess_spacing spacing; +}; + /* This should be for state that is set inside a renderpass and used at * renderpass end time, e.g. to decide whether to use sysmem. This needs * special handling for secondary cmdbufs and suspending/resuming render @@ -547,6 +554,8 @@ struct tu_cmd_state struct tu_primitive_params last_prim_params; + struct tu_tess_params tess_params; + uint64_t descriptor_buffer_iova[MAX_SETS]; }; diff --git a/src/freedreno/vulkan/tu_pipeline.cc b/src/freedreno/vulkan/tu_pipeline.cc index 3e15f669698..e8cdcb0acfe 100644 --- a/src/freedreno/vulkan/tu_pipeline.cc +++ b/src/freedreno/vulkan/tu_pipeline.cc @@ -1391,40 +1391,6 @@ tu6_emit_vpc(struct tu_cs *cs, tu_cs_emit_pkt4(cs, REG_A6XX_PC_TESS_NUM_VERTEX, 1); tu_cs_emit(cs, hs->tess.tcs_vertices_out); - /* In SPIR-V generated from GLSL, the tessellation primitive params are - * are specified in the tess eval shader, but in SPIR-V generated from - * HLSL, they are specified in the tess control shader. */ - const struct ir3_shader_variant *tess = - ds->tess.spacing == TESS_SPACING_UNSPECIFIED ? hs : ds; - tu_cs_emit_pkt4(cs, REG_A6XX_PC_TESS_CNTL, 1); - enum a6xx_tess_output output; - if (tess->tess.point_mode) - output = TESS_POINTS; - else if (tess->tess.primitive_mode == TESS_PRIMITIVE_ISOLINES) - output = TESS_LINES; - else if (tess->tess.ccw) - output = TESS_CCW_TRIS; - else - output = TESS_CW_TRIS; - - enum a6xx_tess_spacing spacing; - switch (tess->tess.spacing) { - case TESS_SPACING_EQUAL: - spacing = TESS_EQUAL; - break; - case TESS_SPACING_FRACTIONAL_ODD: - spacing = TESS_FRACTIONAL_ODD; - break; - case TESS_SPACING_FRACTIONAL_EVEN: - spacing = TESS_FRACTIONAL_EVEN; - break; - case TESS_SPACING_UNSPECIFIED: - default: - unreachable("invalid tess spacing"); - } - tu_cs_emit(cs, A6XX_PC_TESS_CNTL_SPACING(spacing) | - A6XX_PC_TESS_CNTL_OUTPUT(output)); - tu6_emit_link_map(cs, vs, hs, SB6_HS_SHADER); tu6_emit_link_map(cs, hs, ds, SB6_DS_SHADER); } @@ -4126,6 +4092,43 @@ tu_pipeline_builder_parse_shader_stages(struct tu_pipeline_builder *builder, tu6_emit_patch_control_points(&cs, pipeline, pipeline->tess.patch_control_points); } + + /* In SPIR-V generated from GLSL, the tessellation primitive params are + * are specified in the tess eval shader, but in SPIR-V generated from + * HLSL, they are specified in the tess control shader. */ + const struct ir3_shader_variant *tess = + ds->tess.spacing == TESS_SPACING_UNSPECIFIED ? hs : ds; + if (tess->tess.point_mode) { + pipeline->program.tess_output_lower_left = + pipeline->program.tess_output_upper_left = TESS_POINTS; + } else if (tess->tess.primitive_mode == TESS_PRIMITIVE_ISOLINES) { + pipeline->program.tess_output_lower_left = + pipeline->program.tess_output_upper_left = TESS_LINES; + } else if (tess->tess.ccw) { + /* Tessellation orientation in HW is specified with a lower-left + * origin, we need to swap them if the origin is upper-left. + */ + pipeline->program.tess_output_lower_left = TESS_CCW_TRIS; + pipeline->program.tess_output_upper_left = TESS_CW_TRIS; + } else { + pipeline->program.tess_output_lower_left = TESS_CW_TRIS; + pipeline->program.tess_output_upper_left = TESS_CCW_TRIS; + } + + switch (tess->tess.spacing) { + case TESS_SPACING_EQUAL: + pipeline->program.tess_spacing = TESS_EQUAL; + break; + case TESS_SPACING_FRACTIONAL_ODD: + pipeline->program.tess_spacing = TESS_FRACTIONAL_ODD; + break; + case TESS_SPACING_FRACTIONAL_EVEN: + pipeline->program.tess_spacing = TESS_FRACTIONAL_EVEN; + break; + case TESS_SPACING_UNSPECIFIED: + default: + unreachable("invalid tess spacing"); + } } struct ir3_shader_variant *last_shader; diff --git a/src/freedreno/vulkan/tu_pipeline.h b/src/freedreno/vulkan/tu_pipeline.h index 21c2b9dccd8..fd6f481d39c 100644 --- a/src/freedreno/vulkan/tu_pipeline.h +++ b/src/freedreno/vulkan/tu_pipeline.h @@ -236,6 +236,9 @@ struct tu_pipeline bool writes_viewport; bool per_samp; + + enum a6xx_tess_output tess_output_upper_left, tess_output_lower_left; + enum a6xx_tess_spacing tess_spacing; } program; struct