From 5700c87db65a7c6119f94948b11f808e9b68c112 Mon Sep 17 00:00:00 2001 From: Faith Ekstrand Date: Wed, 19 Nov 2025 14:45:55 -0500 Subject: [PATCH] pan/bi: Add some helpers an an info field for needing the extended FIFO The logic here is a bit scattered around and is about to get more complicated. This adds a helper which better documents the interactions as well as an info field to make the driver's life easier. Reviewed-by: Olivia Lee Part-of: --- src/panfrost/compiler/bifrost_compile.c | 12 ++++++--- src/panfrost/compiler/bifrost_compile.h | 27 +++++++++++++++++++++ src/panfrost/lib/pan_shader.c | 4 +++ src/panfrost/util/pan_ir.h | 5 ++++ src/panfrost/vulkan/csf/panvk_vX_cmd_draw.c | 9 +------ 5 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/panfrost/compiler/bifrost_compile.c b/src/panfrost/compiler/bifrost_compile.c index 7adcb388cc0..bd5f77c3e19 100644 --- a/src/panfrost/compiler/bifrost_compile.c +++ b/src/panfrost/compiler/bifrost_compile.c @@ -1410,8 +1410,12 @@ bi_emit_store_vary(bi_builder *b, nir_intrinsic_instr *instr) /* We don't patch these offsets in the no_psiz variant, so if * multiview is enabled we can't switch to the basic format by * using no_psiz */ - bool extended_position_fifo = b->shader->nir->info.outputs_written & - (VARYING_BIT_LAYER | VARYING_BIT_PSIZ); + const uint64_t outputs = b->shader->nir->info.outputs_written; + bool extended_position_fifo = + valhal_writes_extended_fifo(outputs, false, true); + /* Must be the same with and without no_psiz */ + assert(valhal_writes_extended_fifo(outputs, true, true) == + extended_position_fifo); unsigned position_fifo_stride = extended_position_fifo ? 8 : 4; index_offset += view_index * position_fifo_stride; } @@ -6860,10 +6864,10 @@ bi_compile_variant(nir_shader *nir, if (idvs == BI_IDVS_ALL) { /* Varying shader is only enabled if we can have any kind of varying - * written (that mean not position, layer or point size) */ + * written (that mean not position, or an extended FIFO attribute) */ info->vs.secondary_enable = (nir->info.outputs_written & - ~(VARYING_BIT_POS | VARYING_BIT_LAYER | VARYING_BIT_PSIZ)) != 0; + ~(VARYING_BIT_POS | VALHAL_EX_FIFO_VARYING_BITS)) != 0; } } diff --git a/src/panfrost/compiler/bifrost_compile.h b/src/panfrost/compiler/bifrost_compile.h index 626e87a274b..2ce589b0568 100644 --- a/src/panfrost/compiler/bifrost_compile.h +++ b/src/panfrost/compiler/bifrost_compile.h @@ -92,6 +92,33 @@ void bifrost_compile_shader_nir(nir_shader *nir, struct util_dynarray *binary, struct pan_shader_info *info); +#define VALHAL_EX_FIFO_VARYING_BITS \ + (VARYING_BIT_PSIZ | VARYING_BIT_LAYER) + +static inline bool +valhal_writes_extended_fifo(uint64_t outputs_written, + bool no_psiz, bool multiview) +{ + uint64_t ex_fifo_written = outputs_written & VALHAL_EX_FIFO_VARYING_BITS; + if (ex_fifo_written == 0) + return false; + + /* Multiview shaders depend on the FIFO format for indexing per-view + * output writes. We don't currently patch these offsets in the no_psiz + * variant, so we need the extended format, regardless of point size. + */ + if (multiview) + return true; + + /* If we're not rendering in points mode, the no_psiz variant has point + * size write patched out for us. + */ + if (no_psiz) + ex_fifo_written &= ~VARYING_BIT_PSIZ; + + return ex_fifo_written != 0; +} + #define DEFINE_OPTIONS(arch) \ static const nir_shader_compiler_options bifrost_nir_options_v##arch = { \ .lower_scmp = true, \ diff --git a/src/panfrost/lib/pan_shader.c b/src/panfrost/lib/pan_shader.c index b516a720d98..98c8110b446 100644 --- a/src/panfrost/lib/pan_shader.c +++ b/src/panfrost/lib/pan_shader.c @@ -95,6 +95,10 @@ pan_shader_compile(nir_shader *s, struct pan_compile_inputs *inputs, info->vs.writes_point_size = s->info.outputs_written & VARYING_BIT_PSIZ; + info->vs.needs_extended_fifo = arch >= 9 && + valhal_writes_extended_fifo(s->info.outputs_written, + true, inputs->view_mask != 0); + if (arch >= 9) { info->varyings.output_count = util_last_bit(s->info.outputs_written >> VARYING_SLOT_VAR0); diff --git a/src/panfrost/util/pan_ir.h b/src/panfrost/util/pan_ir.h index 5e1c43ae8fe..5c4e167dc24 100644 --- a/src/panfrost/util/pan_ir.h +++ b/src/panfrost/util/pan_ir.h @@ -246,6 +246,11 @@ struct pan_shader_info { struct { bool writes_point_size; + /* True if this shader needs the extended FIFO format for + * more than just point size. + */ + bool needs_extended_fifo; + /* If the primary shader writes point size, the Valhall * driver may need a variant that does not write point * size. Offset to such a shader in the program binary. diff --git a/src/panfrost/vulkan/csf/panvk_vX_cmd_draw.c b/src/panfrost/vulkan/csf/panvk_vX_cmd_draw.c index 9de45f79c25..ec435daf40e 100644 --- a/src/panfrost/vulkan/csf/panvk_vX_cmd_draw.c +++ b/src/panfrost/vulkan/csf/panvk_vX_cmd_draw.c @@ -2153,15 +2153,8 @@ set_tiler_idvs_flags(struct cs_builder *b, struct panvk_cmd_buffer *cmdbuf, bool writes_point_size = vs->info.vs.writes_point_size && ia->primitive_topology == VK_PRIMITIVE_TOPOLOGY_POINT_LIST; - bool multiview = cmdbuf->state.gfx.render.view_mask; bool writes_layer = vs->info.outputs_written & VARYING_BIT_LAYER; - - /* Multiview shaders depend on the FIFO format for indexing per-view - * output writes. We don't currently patch these offsets in the no_psiz - * variant, so we still need the extended format even though the shader - * does not write point size. */ - bool extended_fifo = writes_point_size || writes_layer || - (vs->info.vs.writes_point_size && multiview); + bool extended_fifo = writes_point_size || vs->info.vs.needs_extended_fifo; bool dirty = gfx_state_dirty(cmdbuf, VS) || fs_user_dirty(cmdbuf) || dyn_gfx_state_dirty(cmdbuf, IA_PRIMITIVE_RESTART_ENABLE) ||