From 3497069a046a7dc448d4edac133a2be27fabd4fc Mon Sep 17 00:00:00 2001 From: Benjamin Lee Date: Sat, 1 Feb 2025 01:42:52 -0800 Subject: [PATCH] panfrost: support 16-bit varyings This is complicated by two things: mediump varyings, and the lack of u16 regfmt support in LD_VAR. With mediump, a load(_interpolated)_input with a 16-bit dest size may either be an explicit 16-bit type or a mediump type lowered by nir_lower_mediump_io. With explicit 16-bit types, we write 16-bit values in the VS, but with mediump we write 32-bit in the VS (for messy reasons). bi_emit_load_vary needs to distinguish these cases by checking for a mediump type, and set the appropriate source_format to convert the type on the LD_VAR_BUF path. Types like 'mediump uint16' are luckily not allowed. The missing u16 regfmt for LD_VAR means that we take the obvious approach for 16-bit int varyings of emitting 16-bit int formats in the attribute descriptor and loading them to u16. Instead, we just write/read all 16-bit varyings as f16 regardless of type. Unlike with mediump, we don't need to do any 32bit->16bit conversion when loading in the FS, so as long as we use the same type between the attribute descriptor and LD_VAR, the conversion is a no-op and the mismatch doesn't matter. Signed-off-by: Benjamin Lee Reviewed-by: Lars-Ivar Hesselberg Simonsen Part-of: --- src/panfrost/compiler/bifrost_compile.c | 44 ++++++++++++++------- src/panfrost/util/pan_collect_varyings.c | 2 +- src/panfrost/util/pan_lower_noperspective.c | 7 +++- 3 files changed, 37 insertions(+), 16 deletions(-) diff --git a/src/panfrost/compiler/bifrost_compile.c b/src/panfrost/compiler/bifrost_compile.c index 3ce9ad645a5..8c2650cfe7e 100644 --- a/src/panfrost/compiler/bifrost_compile.c +++ b/src/panfrost/compiler/bifrost_compile.c @@ -583,7 +583,13 @@ bi_emit_load_vary(bi_builder *b, nir_intrinsic_instr *instr) bi_index dest = (component == 0) ? bi_def_index(&instr->def) : bi_temp(b->shader); + nir_io_semantics sem = nir_intrinsic_io_semantics(instr); + unsigned sz = instr->def.bit_size; + assert(sz == 16 || sz == 32); + /* mediump varyings are always written as 32-bits in the VS, but may be read + * to 16 in the FS. */ + unsigned src_sz = sem.medium_precision ? 32 : sz; if (smooth) { nir_intrinsic_instr *parent = nir_src_as_intrinsic(instr->src[0]); @@ -592,13 +598,17 @@ bi_emit_load_vary(bi_builder *b, nir_intrinsic_instr *instr) sample = bi_interp_for_intrinsic(parent->intrinsic); src0 = bi_varying_src0_for_barycentric(b, parent); - assert(sz == 16 || sz == 32); regfmt = (sz == 16) ? BI_REGISTER_FORMAT_F16 : BI_REGISTER_FORMAT_F32; - source_format = BI_SOURCE_FORMAT_F32; + source_format = + (src_sz == 16) ? BI_SOURCE_FORMAT_F16 : BI_SOURCE_FORMAT_F32; } else { - assert(sz == 32); - regfmt = BI_REGISTER_FORMAT_U32; - source_format = BI_SOURCE_FORMAT_FLAT32; + /* u16 regfmt is not supported by LD_VAR_BUF, but using f16 for integers + * is okay because we use a f16 attribute descriptor for all 16-bit + * varyings regardless of whether they are floats or ints. The + * conversion is a no-op. */ + regfmt = (sz == 16) ? BI_REGISTER_FORMAT_F16 : BI_REGISTER_FORMAT_AUTO; + source_format = (src_sz == 16) ? + BI_SOURCE_FORMAT_FLAT16 : BI_SOURCE_FORMAT_FLAT32; /* Valhall can't have bi_null() here, although the source is * logically unused for flat varyings @@ -1098,14 +1108,19 @@ static void bi_emit_store_vary(bi_builder *b, nir_intrinsic_instr *instr) { /* In principle we can do better for 16-bit. At the moment we require - * 32-bit to permit the use of .auto, in order to force .u32 for flat - * varyings, to handle internal TGSI shaders that set flat in the VS - * but smooth in the FS */ + * mediump varyings to be 32-bit to permit the use of .auto, in order to + * force .u32 for flat varyings, to handle internal TGSI shaders that set + * flat in the VS but smooth in the FS. + * + * Explicit 16-bit types are unaffected, and written as 16-bit. */ ASSERTED nir_alu_type T = nir_intrinsic_src_type(instr); ASSERTED unsigned T_size = nir_alu_type_get_type_size(T); - assert(T_size == 32 || (b->shader->arch >= 9 && T_size == 16)); - enum bi_register_format regfmt = BI_REGISTER_FORMAT_AUTO; + assert(T_size == 32 || T_size == 16); + /* 16-bit varyings are always written and loaded as F16, regardless of + * whether they are float or int */ + enum bi_register_format regfmt = + T_size == 16 ? BI_REGISTER_FORMAT_F16 : BI_REGISTER_FORMAT_AUTO; unsigned imm_index = 0; bool immediate = bi_is_intr_immediate(instr, &imm_index, 16); @@ -1128,15 +1143,16 @@ bi_emit_store_vary(bi_builder *b, nir_intrinsic_instr *instr) * introduce a TRIM.i32 pseudoinstruction? */ if (nr < nir_intrinsic_src_components(instr, 0)) { - assert(T_size == 32 && "todo: 16-bit trim"); - bi_index chans[4] = {bi_null(), bi_null(), bi_null(), bi_null()}; - unsigned src_comps = nir_intrinsic_src_components(instr, 0); + unsigned comps_per_reg = instr->def.bit_size == 16 ? 2 : 1; + unsigned src_comps = + DIV_ROUND_UP(nir_intrinsic_src_components(instr, 0), comps_per_reg); + unsigned dst_comps = DIV_ROUND_UP(nr, comps_per_reg); bi_emit_split_i32(b, chans, data, src_comps); bi_index tmp = bi_temp(b->shader); - bi_instr *collect = bi_collect_i32_to(b, tmp, nr); + bi_instr *collect = bi_collect_i32_to(b, tmp, dst_comps); bi_foreach_src(collect, w) collect->src[w] = chans[w]; diff --git a/src/panfrost/util/pan_collect_varyings.c b/src/panfrost/util/pan_collect_varyings.c index 1987d85d713..e76f953959b 100644 --- a/src/panfrost/util/pan_collect_varyings.c +++ b/src/panfrost/util/pan_collect_varyings.c @@ -126,7 +126,7 @@ walk_varyings(UNUSED nir_builder *b, nir_instr *instr, void *data) * fragment shader instead. */ bool flat = intr->intrinsic != nir_intrinsic_load_interpolated_input; - bool auto32 = !info->quirk_no_auto32; + bool auto32 = !info->quirk_no_auto32 && size == 32; nir_alu_type type = (flat && auto32) ? nir_type_uint : nir_type_float; if (sem.medium_precision) { diff --git a/src/panfrost/util/pan_lower_noperspective.c b/src/panfrost/util/pan_lower_noperspective.c index 6cc0b4996a5..ee5ac7838a6 100644 --- a/src/panfrost/util/pan_lower_noperspective.c +++ b/src/panfrost/util/pan_lower_noperspective.c @@ -182,7 +182,10 @@ lower_noperspective_vs(nir_builder *b, nir_intrinsic_instr *intrin, is_noperspective_output(b, sem.location, state->noperspective_outputs); nir_def *old_value = intrin->src[0].ssa; - nir_def *noperspective_value = nir_fmul(b, old_value, state->pos_w); + nir_def *pos_w = state->pos_w; + if (old_value->bit_size == 16) + pos_w = nir_f2f16(b, pos_w); + nir_def *noperspective_value = nir_fmul(b, old_value, pos_w); nir_def *new_value = nir_bcsel(b, is_noperspective, noperspective_value, old_value); @@ -205,6 +208,8 @@ lower_noperspective_fs(nir_builder *b, nir_intrinsic_instr *intrin, nir_def *bary = intrin->src[0].ssa; nir_def *fragcoord_w = nir_load_frag_coord_zw_pan(b, bary, .component = 3); + if (intrin->def.bit_size == 16) + fragcoord_w = nir_f2f16(b, fragcoord_w); nir_def *new_value = nir_fmul(b, &intrin->def, fragcoord_w); nir_def_rewrite_uses_after(&intrin->def, new_value, new_value->parent_instr);