From 098295f1a00dc27379779662b136725de0bec690 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Sat, 18 Mar 2023 16:58:22 -0400 Subject: [PATCH] asahi: Implement null textures Use the same silly workaround that Metal does, to fill in texture descriptors when there's nothing bound in the interest of robust behaviour. Fixes null pointer dereference in arb_shading_language_420pack-active-sampler-conflict. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/asahi/compiler/agx_compile.c | 2 ++ src/asahi/compiler/agx_compile.h | 3 +++ src/gallium/drivers/asahi/agx_pipe.c | 1 + src/gallium/drivers/asahi/agx_state.c | 38 +++++++++++++++++++++++++++++------ 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/asahi/compiler/agx_compile.c b/src/asahi/compiler/agx_compile.c index f6550da..50683af 100644 --- a/src/asahi/compiler/agx_compile.c +++ b/src/asahi/compiler/agx_compile.c @@ -2385,6 +2385,8 @@ agx_compile_shader_nir(nir_shader *nir, struct agx_shader_key *key, out->depth_layout = layout; } + out->nr_bindful_textures = nir->info.num_textures; + /* Late clip plane lowering created discards */ if (nir->info.stage == MESA_SHADER_FRAGMENT) { NIR_PASS_V(nir, agx_nir_lower_zs_emit); diff --git a/src/asahi/compiler/agx_compile.h b/src/asahi/compiler/agx_compile.h index 7d977f7..b870d5f 100644 --- a/src/asahi/compiler/agx_compile.h +++ b/src/asahi/compiler/agx_compile.h @@ -109,6 +109,9 @@ struct agx_shader_info { /* Shader needs a dummy sampler (for txf reads) */ bool needs_dummy_sampler; + /* Number of bindful textures used */ + unsigned nr_bindful_textures; + /* Number of 16-bit registers used by the main shader and preamble * respectively. */ diff --git a/src/gallium/drivers/asahi/agx_pipe.c b/src/gallium/drivers/asahi/agx_pipe.c index 2291ebe..5635be6 100644 --- a/src/gallium/drivers/asahi/agx_pipe.c +++ b/src/gallium/drivers/asahi/agx_pipe.c @@ -1386,6 +1386,7 @@ agx_get_param(struct pipe_screen *pscreen, enum pipe_cap param) case PIPE_CAP_LOAD_CONSTBUF: case PIPE_CAP_SEAMLESS_CUBE_MAP_PER_TEXTURE: case PIPE_CAP_TEXTURE_BUFFER_OBJECTS: + case PIPE_CAP_NULL_TEXTURES: return 1; case PIPE_CAP_TEXTURE_MULTISAMPLE: diff --git a/src/gallium/drivers/asahi/agx_state.c b/src/gallium/drivers/asahi/agx_state.c index a62adb0..3df99d5 100644 --- a/src/gallium/drivers/asahi/agx_state.c +++ b/src/gallium/drivers/asahi/agx_state.c @@ -1774,12 +1774,36 @@ translate_sampler_state_count(struct agx_context *ctx, ctx->stage[stage].custom_borders); } +/* + * Despite having both a layout *and* a flag that I only see Metal use with null + * textures, AGX doesn't seem to have "real" null textures. Instead we need to + * bind an arbitrary address and throw away the results to read all 0's. + * Accordingly, the caller must pass some address that lives at least as long as + * the texture descriptor itself. + */ +static void +agx_set_null_texture(struct agx_texture_packed *tex, uint64_t valid_address) +{ + agx_pack(tex, TEXTURE, cfg) { + cfg.layout = AGX_LAYOUT_NULL; + cfg.channels = AGX_CHANNELS_R8; + cfg.type = AGX_TEXTURE_TYPE_UNORM /* don't care */; + cfg.swizzle_r = AGX_CHANNEL_0; + cfg.swizzle_g = AGX_CHANNEL_0; + cfg.swizzle_b = AGX_CHANNEL_0; + cfg.swizzle_a = AGX_CHANNEL_0; + cfg.address = valid_address; + cfg.null = true; + } +} + static uint32_t agx_build_pipeline(struct agx_batch *batch, struct agx_compiled_shader *cs, enum pipe_shader_type stage, unsigned variable_shared_mem) { struct agx_context *ctx = batch->ctx; - unsigned nr_textures = ctx->stage[stage].texture_count; + unsigned nr_textures = cs->info.nr_bindful_textures; + unsigned nr_active_textures = ctx->stage[stage].texture_count; unsigned nr_samplers = sampler_count(ctx, cs, stage); bool custom_borders = ctx->stage[stage].custom_borders; @@ -1795,12 +1819,11 @@ agx_build_pipeline(struct agx_batch *batch, struct agx_compiled_shader *cs, struct agx_texture_packed *textures = T_tex.cpu; /* TODO: Dirty track me to save some CPU cycles and maybe improve caching */ - for (unsigned i = 0; i < nr_textures; ++i) { + for (unsigned i = 0; i < MIN2(nr_textures, nr_active_textures); ++i) { struct agx_sampler_view *tex = ctx->stage[stage].textures[i]; - /* TODO: Use an actual null texture for robustness */ if (tex == NULL) { - memset(&textures[i], 0, sizeof(textures[i])); + agx_set_null_texture(&textures[i], T_tex.gpu); continue; } @@ -1832,6 +1855,9 @@ agx_build_pipeline(struct agx_batch *batch, struct agx_compiled_shader *cs, textures[i] = texture; } + for (unsigned i = nr_active_textures; i < nr_textures; ++i) + agx_set_null_texture(&textures[i], T_tex.gpu); + /* TODO: Dirty track me to save some CPU cycles and maybe improve caching */ uint8_t *out_sampler = T_samp.cpu; if (nr_samplers && ctx->stage[stage].sampler_count == 0) { @@ -2185,7 +2211,7 @@ agx_encode_state(struct agx_batch *batch, uint8_t *out, bool is_lines, } out += AGX_VDM_STATE_LENGTH; - unsigned tex_count = ctx->stage[PIPE_SHADER_VERTEX].texture_count; + unsigned tex_count = ctx->vs->info.nr_bindful_textures; agx_pack(out, VDM_STATE_VERTEX_SHADER_WORD_0, cfg) { cfg.uniform_register_count = ctx->vs->info.push_count; cfg.preshader_register_count = ctx->vs->info.nr_preamble_gprs; @@ -2757,7 +2783,7 @@ agx_launch_grid(struct pipe_context *pipe, const struct pipe_grid_info *info) /* TODO: Ensure space if we allow multiple kernels in a batch */ uint8_t *out = batch->encoder_current; - unsigned nr_textures = ctx->stage[PIPE_SHADER_COMPUTE].texture_count; + unsigned nr_textures = cs->info.nr_bindful_textures; agx_pack(out, CDM_HEADER, cfg) { if (info->indirect) cfg.mode = AGX_CDM_MODE_INDIRECT_GLOBAL; -- 2.7.4