From eae86f455ccfe1a1ef5bb217aa18735ad8a9b976 Mon Sep 17 00:00:00 2001 From: Emma Anholt Date: Wed, 18 Jun 2025 14:59:07 -0700 Subject: [PATCH] v3d: Stop advertising support for HW clip planes. The GL frontend is perfectly good at lowering it like we do. Cuts out a bunch of duplicate code. We still have ucp_enables for the FS due to lowering of CLIPDIST to discards in the FS. Part-of: --- src/broadcom/compiler/nir_to_vir.c | 9 ------- src/broadcom/compiler/v3d_compiler.h | 4 +-- src/broadcom/compiler/vir.c | 32 ++++++++--------------- src/broadcom/vulkan/v3dv_pipeline.c | 35 +++++++++++++------------- src/gallium/drivers/v3d/v3d_context.h | 3 +-- src/gallium/drivers/v3d/v3d_program.c | 4 +-- src/gallium/drivers/v3d/v3d_screen.c | 1 + src/gallium/drivers/v3d/v3d_uniforms.c | 9 ------- src/gallium/drivers/v3d/v3dx_state.c | 10 -------- 9 files changed, 31 insertions(+), 76 deletions(-) diff --git a/src/broadcom/compiler/nir_to_vir.c b/src/broadcom/compiler/nir_to_vir.c index e621ea729c1..8efcc80bf09 100644 --- a/src/broadcom/compiler/nir_to_vir.c +++ b/src/broadcom/compiler/nir_to_vir.c @@ -3536,15 +3536,6 @@ ntq_emit_intrinsic(struct v3d_compile *c, nir_intrinsic_instr *instr) nir_src_comp_as_uint(instr->src[0], 0))); break; - case nir_intrinsic_load_user_clip_plane: - for (int i = 0; i < nir_intrinsic_dest_components(instr); i++) { - ntq_store_def(c, &instr->def, i, - vir_uniform(c, QUNIFORM_USER_CLIP_PLANE, - nir_intrinsic_ucp_id(instr) * - 4 + i)); - } - break; - case nir_intrinsic_load_viewport_x_scale: ntq_store_def(c, &instr->def, 0, vir_uniform(c, QUNIFORM_VIEWPORT_X_SCALE, 0)); diff --git a/src/broadcom/compiler/v3d_compiler.h b/src/broadcom/compiler/v3d_compiler.h index 5685b793545..4c49de7b00e 100644 --- a/src/broadcom/compiler/v3d_compiler.h +++ b/src/broadcom/compiler/v3d_compiler.h @@ -205,8 +205,6 @@ enum quniform_contents { QUNIFORM_VIEWPORT_Z_OFFSET, QUNIFORM_VIEWPORT_Z_SCALE, - QUNIFORM_USER_CLIP_PLANE, - /** * A reference to a V3D 3.x texture config parameter 0 uniform. * @@ -414,7 +412,6 @@ struct v3d_key { */ uint32_t sampler_is_32b; - uint8_t ucp_enables; bool is_last_geometry_stage; bool robust_uniform_access; bool robust_storage_access; @@ -437,6 +434,7 @@ struct v3d_fs_key { uint8_t swap_color_rb; /* Mask of which render targets need to be written as 32-bit floats */ uint8_t f32_color_rb; + uint8_t ucp_enables; /* Color format information per render target. Only set when logic * operations are enabled, when fbfetch is in use or when falling back diff --git a/src/broadcom/compiler/vir.c b/src/broadcom/compiler/vir.c index a747be0bd7c..f32238616d2 100644 --- a/src/broadcom/compiler/vir.c +++ b/src/broadcom/compiler/vir.c @@ -1119,41 +1119,29 @@ v3d_nir_lower_fs_early(struct v3d_compile *c) static void v3d_nir_lower_gs_late(struct v3d_compile *c) { - if (c->key->ucp_enables) { - NIR_PASS(_, c->s, nir_lower_clip_gs, c->key->ucp_enables, - true, NULL); - } - - /* Note: GS output scalarizing must happen after nir_lower_clip_gs. */ NIR_PASS(_, c->s, nir_lower_io_to_scalar, nir_var_shader_out, NULL, NULL); } static void v3d_nir_lower_vs_late(struct v3d_compile *c) { - if (c->key->ucp_enables) { - NIR_PASS(_, c->s, nir_lower_clip_vs, c->key->ucp_enables, - false, true, NULL); - NIR_PASS(_, c->s, nir_lower_io_to_scalar, - nir_var_shader_out, NULL, NULL); - } - - /* Note: VS output scalarizing must happen after nir_lower_clip_vs. */ NIR_PASS(_, c->s, nir_lower_io_to_scalar, nir_var_shader_out, NULL, NULL); } static void v3d_nir_lower_fs_late(struct v3d_compile *c) { - /* In OpenGL the fragment shader can't read gl_ClipDistance[], but - * Vulkan allows it, in which case the SPIR-V compiler will declare - * VARING_SLOT_CLIP_DIST0 as compact array variable. Pass true as - * the last parameter to always operate with a compact array in both - * OpenGL and Vulkan so we do't have to care about the API we - * are using. + /* If there are clip distance writes (either GL/Vulkan + * gl_ClipDistance[], or lowered user clip planes for desktop GL), + * then we need to emit the discards for them at the top of the fragment + * shader. + * + * The SPIR-V compiler will declare VARING_SLOT_CLIP_DIST0 as compact + * array variable, so we have GL's clip lowering follow suit + * (PIPE_CAP_NIR_COMPACT_ARRAYS). */ - if (c->key->ucp_enables) - NIR_PASS(_, c->s, nir_lower_clip_fs, c->key->ucp_enables, true, false); + if (c->fs_key->ucp_enables) + NIR_PASS(_, c->s, nir_lower_clip_fs, c->fs_key->ucp_enables, true, false); NIR_PASS(_, c->s, nir_lower_io_to_scalar, nir_var_shader_in, NULL, NULL); } diff --git a/src/broadcom/vulkan/v3dv_pipeline.c b/src/broadcom/vulkan/v3dv_pipeline.c index 1f35f7ad079..8f2d65b0d99 100644 --- a/src/broadcom/vulkan/v3dv_pipeline.c +++ b/src/broadcom/vulkan/v3dv_pipeline.c @@ -987,8 +987,7 @@ shader_debug_output(const char *message, void *data) static void pipeline_populate_v3d_key(struct v3d_key *key, - const struct v3dv_pipeline_stage *p_stage, - uint32_t ucp_enables) + const struct v3dv_pipeline_stage *p_stage) { assert(p_stage->pipeline->shared_data && p_stage->pipeline->shared_data->maps[p_stage->stage]); @@ -1024,18 +1023,6 @@ pipeline_populate_v3d_key(struct v3d_key *key, unreachable("unsupported shader stage"); } - /* Vulkan doesn't have fixed function state for user clip planes. Instead, - * shaders can write to gl_ClipDistance[], in which case the SPIR-V compiler - * takes care of adding a single compact array variable at - * VARYING_SLOT_CLIP_DIST0, so we don't need any user clip plane lowering. - * - * The only lowering we are interested is specific to the fragment shader, - * where we want to emit discards to honor writes to gl_ClipDistance[] in - * previous stages. This is done via nir_lower_clip_fs() so we only set up - * the ucp enable mask for that stage. - */ - key->ucp_enables = ucp_enables; - const VkPipelineRobustnessBufferBehaviorEXT robust_buffer_enabled = VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_EXT; @@ -1204,7 +1191,19 @@ pipeline_populate_v3d_fs_key(struct v3d_fs_key *key, struct v3dv_device *device = p_stage->pipeline->device; assert(device); - pipeline_populate_v3d_key(&key->base, p_stage, ucp_enables); + pipeline_populate_v3d_key(&key->base, p_stage); + + /* Vulkan doesn't have fixed function state for user clip planes. Instead, + * shaders can write to gl_ClipDistance[], in which case the SPIR-V compiler + * takes care of adding a single compact array variable at + * VARYING_SLOT_CLIP_DIST0, so we don't need any user clip plane lowering. + * + * The only lowering we are interested is specific to the fragment shader, + * where we want to emit discards to honor writes to gl_ClipDistance[] in + * previous stages. This is done via nir_lower_clip_fs() so we only set up + * the ucp enable mask for that stage. + */ + key->ucp_enables = ucp_enables; const VkPipelineInputAssemblyStateCreateInfo *ia_info = pCreateInfo->pInputAssemblyState; @@ -1293,7 +1292,7 @@ pipeline_populate_v3d_gs_key(struct v3d_gs_key *key, memset(key, 0, sizeof(*key)); - pipeline_populate_v3d_key(&key->base, p_stage, 0); + pipeline_populate_v3d_key(&key->base, p_stage); struct v3dv_pipeline *pipeline = p_stage->pipeline; @@ -1336,7 +1335,7 @@ pipeline_populate_v3d_vs_key(struct v3d_vs_key *key, assert(device); memset(key, 0, sizeof(*key)); - pipeline_populate_v3d_key(&key->base, p_stage, 0); + pipeline_populate_v3d_key(&key->base, p_stage); struct v3dv_pipeline *pipeline = p_stage->pipeline; @@ -3272,7 +3271,7 @@ pipeline_compile_compute(struct v3dv_pipeline *pipeline, struct v3d_key key; memset(&key, 0, sizeof(key)); - pipeline_populate_v3d_key(&key, p_stage, 0); + pipeline_populate_v3d_key(&key, p_stage); pipeline->shared_data->variants[BROADCOM_SHADER_COMPUTE] = pipeline_compile_shader_variant(p_stage, &key, sizeof(key), alloc, &result); diff --git a/src/gallium/drivers/v3d/v3d_context.h b/src/gallium/drivers/v3d/v3d_context.h index 2017fb7d8d7..1951014afbe 100644 --- a/src/gallium/drivers/v3d/v3d_context.h +++ b/src/gallium/drivers/v3d/v3d_context.h @@ -75,7 +75,7 @@ void v3d_job_add_bo(struct v3d_job *job, struct v3d_bo *bo); #define V3D_DIRTY_SCISSOR (1ull << 19) #define V3D_DIRTY_FLAT_SHADE_FLAGS (1ull << 20) #define V3D_DIRTY_PRIM_MODE (1ull << 21) -#define V3D_DIRTY_CLIP (1ull << 22) + #define V3D_DIRTY_UNCOMPILED_CS (1ull << 23) #define V3D_DIRTY_UNCOMPILED_VS (1ull << 24) #define V3D_DIRTY_UNCOMPILED_GS (1ull << 25) @@ -675,7 +675,6 @@ struct v3d_context { uint32_t n_primitives_generated_queries_in_flight; struct pipe_poly_stipple stipple; - struct pipe_clip_state clip; struct pipe_viewport_state viewport; struct v3d_ssbo_stateobj ssbo[PIPE_SHADER_TYPES]; struct v3d_shaderimg_stateobj shaderimg[PIPE_SHADER_TYPES]; diff --git a/src/gallium/drivers/v3d/v3d_program.c b/src/gallium/drivers/v3d/v3d_program.c index fdf77eb9098..c99c2ad5799 100644 --- a/src/gallium/drivers/v3d/v3d_program.c +++ b/src/gallium/drivers/v3d/v3d_program.c @@ -603,7 +603,7 @@ v3d_update_compiled_fs(struct v3d_context *v3d, uint8_t prim_mode) memset(key, 0, sizeof(*key)); v3d_setup_shared_key(v3d, &key->base, &v3d->tex[PIPE_SHADER_FRAGMENT]); - key->base.ucp_enables = v3d->rasterizer->base.clip_plane_enable; + key->ucp_enables = v3d->rasterizer->base.clip_plane_enable; key->is_points = (prim_mode == MESA_PRIM_POINTS); key->is_lines = (prim_mode >= MESA_PRIM_LINES && prim_mode <= MESA_PRIM_LINE_STRIP); @@ -751,7 +751,6 @@ v3d_update_compiled_gs(struct v3d_context *v3d, uint8_t prim_mode) memset(key, 0, sizeof(*key)); v3d_setup_shared_key(v3d, &key->base, &v3d->tex[PIPE_SHADER_GEOMETRY]); - key->base.ucp_enables = v3d->rasterizer->base.clip_plane_enable; key->base.is_last_geometry_stage = true; key->num_used_outputs = v3d->prog.fs->prog_data.fs->num_inputs; STATIC_ASSERT(sizeof(key->used_outputs) == @@ -824,7 +823,6 @@ v3d_update_compiled_vs(struct v3d_context *v3d, uint8_t prim_mode) memset(key, 0, sizeof(*key)); v3d_setup_shared_key(v3d, &key->base, &v3d->tex[PIPE_SHADER_VERTEX]); - key->base.ucp_enables = v3d->rasterizer->base.clip_plane_enable; key->base.is_last_geometry_stage = !v3d->prog.bind_gs; if (!v3d->prog.bind_gs) { diff --git a/src/gallium/drivers/v3d/v3d_screen.c b/src/gallium/drivers/v3d/v3d_screen.c index 00d532753bb..51aa25f85f6 100644 --- a/src/gallium/drivers/v3d/v3d_screen.c +++ b/src/gallium/drivers/v3d/v3d_screen.c @@ -369,6 +369,7 @@ v3d_init_screen_caps(struct v3d_screen *screen) caps->native_fence_fd = true; + caps->clip_planes = 0; caps->depth_clip_disable = screen->devinfo.ver >= 71; caps->min_line_width = diff --git a/src/gallium/drivers/v3d/v3d_uniforms.c b/src/gallium/drivers/v3d/v3d_uniforms.c index fe06237ba6a..8fb78dfcc68 100644 --- a/src/gallium/drivers/v3d/v3d_uniforms.c +++ b/src/gallium/drivers/v3d/v3d_uniforms.c @@ -263,11 +263,6 @@ v3d_write_uniforms(struct v3d_context *v3d, struct v3d_job *job, cl_aligned_f(&uniforms, v3d->viewport.scale[2]); break; - case QUNIFORM_USER_CLIP_PLANE: - cl_aligned_f(&uniforms, - v3d->clip.ucp[data / 4][data % 4]); - break; - case QUNIFORM_TMU_CONFIG_P0: write_tmu_p0(job, &uniforms, texstate, data); break; @@ -446,10 +441,6 @@ v3d_set_shader_uniform_dirty_flags(struct v3d_compiled_shader *shader) dirty |= V3D_DIRTY_VIEWPORT; break; - case QUNIFORM_USER_CLIP_PLANE: - dirty |= V3D_DIRTY_CLIP; - break; - case QUNIFORM_TMU_CONFIG_P0: case QUNIFORM_TMU_CONFIG_P1: case QUNIFORM_TEXTURE_CONFIG_P1: diff --git a/src/gallium/drivers/v3d/v3dx_state.c b/src/gallium/drivers/v3d/v3dx_state.c index f0b17231f8a..1252d0c3c8a 100644 --- a/src/gallium/drivers/v3d/v3dx_state.c +++ b/src/gallium/drivers/v3d/v3dx_state.c @@ -68,15 +68,6 @@ v3d_set_stencil_ref(struct pipe_context *pctx, v3d->dirty |= V3D_DIRTY_STENCIL_REF; } -static void -v3d_set_clip_state(struct pipe_context *pctx, - const struct pipe_clip_state *clip) -{ - struct v3d_context *v3d = v3d_context(pctx); - v3d->clip = *clip; - v3d->dirty |= V3D_DIRTY_CLIP; -} - static void v3d_set_sample_mask(struct pipe_context *pctx, unsigned sample_mask) { @@ -1442,7 +1433,6 @@ v3dX(state_init)(struct pipe_context *pctx) { pctx->set_blend_color = v3d_set_blend_color; pctx->set_stencil_ref = v3d_set_stencil_ref; - pctx->set_clip_state = v3d_set_clip_state; pctx->set_sample_mask = v3d_set_sample_mask; pctx->set_constant_buffer = v3d_set_constant_buffer; pctx->set_framebuffer_state = v3d_set_framebuffer_state;