From 81d22da6de4076d8418e94bcdd7a326f1cd584f6 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Sat, 13 Nov 2021 14:52:52 -0500 Subject: [PATCH] asahi: Fix BIND_PIPELINE sizing and alignment Fix a bug in BIND_PIPELINE XML reported by Dougall, which cleans up a bit of both decoder and driver. Instead of... * 17 bytes BIND_PIPELINE (17) * An unused 8 byte record (25) * A set of N 8 byte records (25 + 8 * N) * Oops, 1 byte too many! One just disappeared (24 + 8 * N) It seems to instead be * 24 bytes BIND_PIPELINE (24) * A set of N 8 byte records (24 + 8 * N) without the sentinel record. These means the 8 byte records themselves are shuffled, with the high byte of the pointers split from the low word, but that's less gross than an off-by-one. It's still not clear what the last 8 bytes of the BIND_VERTEX_PIPELINE structure mean, or the last 4 byte of the BIND_FRAGMENT_PIPELINE structure which seems to be a bit shorter. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/asahi/lib/cmdbuf.xml | 14 +++++++------ src/asahi/lib/decode.c | 29 +++++++++------------------ src/gallium/drivers/asahi/agx_state.c | 18 ++++++++--------- 3 files changed, 25 insertions(+), 36 deletions(-) diff --git a/src/asahi/lib/cmdbuf.xml b/src/asahi/lib/cmdbuf.xml index f7e05c52aa0..f975eaeeb1b 100644 --- a/src/asahi/lib/cmdbuf.xml +++ b/src/asahi/lib/cmdbuf.xml @@ -398,9 +398,8 @@ - + specified in 32-bit word units. Intepretation per-shader stage. --> + @@ -418,13 +417,16 @@ + + - - - + + + + diff --git a/src/asahi/lib/decode.c b/src/asahi/lib/decode.c index 29b57025b23..217aaab6b9c 100644 --- a/src/asahi/lib/decode.c +++ b/src/asahi/lib/decode.c @@ -354,7 +354,7 @@ agxdecode_record(uint64_t va, size_t size, bool verbose) assert(size == AGX_CULL_LENGTH); DUMP_CL(CULL, map, "Cull"); } else if (tag == 0x800000) { - assert(size == (AGX_BIND_PIPELINE_LENGTH + 4)); + assert(size == (AGX_BIND_PIPELINE_LENGTH - 4)); agx_unpack(agxdecode_dump_stream, map, BIND_PIPELINE, cmd); agxdecode_stateful(cmd.pipeline, "Pipeline", agxdecode_pipeline, verbose); @@ -394,35 +394,24 @@ agxdecode_cmd(const uint8_t *map, bool verbose) agx_unpack(agxdecode_dump_stream, map, BIND_PIPELINE, cmd); agxdecode_stateful(cmd.pipeline, "Pipeline", agxdecode_pipeline, verbose); DUMP_UNPACKED(BIND_PIPELINE, cmd, "Bind vertex pipeline\n"); - - /* Random unaligned null byte, it's pretty awful.. */ - if (map[AGX_BIND_PIPELINE_LENGTH]) { - fprintf(agxdecode_dump_stream, "Unk unaligned %X\n", - map[AGX_BIND_PIPELINE_LENGTH]); - } - - return AGX_BIND_PIPELINE_LENGTH + 1; - } else if (map[1] == 0xc0 && map[2] == 0x61) { - DUMP_CL(DRAW, map - 1, "Draw"); + return AGX_BIND_PIPELINE_LENGTH; + } else if (map[2] == 0xc0 && map[3] == 0x61) { + DUMP_CL(DRAW, map, "Draw"); return AGX_DRAW_LENGTH; - } else if (map[1] == 0x00 && map[2] == 0x00) { + } else if (map[2] == 0x00 && map[3] == 0x00) { /* No need to explicitly dump the record */ agx_unpack(agxdecode_dump_stream, map, RECORD, cmd); - /* XXX: Why? */ - if (pipeline_base && ((cmd.data >> 32) == 0)) { - cmd.data |= pipeline_base & 0xFF00000000ull; - } - - struct agx_bo *mem = agxdecode_find_mapped_gpu_mem_containing(cmd.data); + uint64_t address = (((uint64_t) cmd.pointer_hi) << 32) | cmd.pointer_lo; + struct agx_bo *mem = agxdecode_find_mapped_gpu_mem_containing(address); if (mem) - agxdecode_record(cmd.data, cmd.size_words * 4, verbose); + agxdecode_record(address, cmd.size_words * 4, verbose); else DUMP_UNPACKED(RECORD, cmd, "Non-existant record (XXX)\n"); return AGX_RECORD_LENGTH; - } else if (map[0] == 0 && map[1] == 0 && map[2] == 0xC0 && map[3] == 0x00) { + } else if (map[1] == 0 && map[2] == 0 && map[3] == 0xC0 && map[4] == 0x00) { ASSERTED unsigned zero[4] = { 0 }; assert(memcmp(map + 4, zero, sizeof(zero)) == 0); return STATE_DONE; diff --git a/src/gallium/drivers/asahi/agx_state.c b/src/gallium/drivers/asahi/agx_state.c index 6a9027a0d67..3180db83d1d 100644 --- a/src/gallium/drivers/asahi/agx_state.c +++ b/src/gallium/drivers/asahi/agx_state.c @@ -1432,9 +1432,13 @@ agx_push_record(uint8_t **out, unsigned size_words, uint64_t ptr) assert(ptr < (1ull << 40)); assert(size_words < (1ull << 24)); - uint64_t value = (size_words | (ptr << 24)); - memcpy(*out, &value, sizeof(value)); - *out += sizeof(value); + agx_pack(*out, RECORD, cfg) { + cfg.pointer_hi = (ptr >> 32); + cfg.pointer_lo = (uint32_t) ptr; + cfg.size_words = size_words; + }; + + *out += AGX_RECORD_LENGTH; } static uint8_t * @@ -1451,17 +1455,11 @@ agx_encode_state(struct agx_context *ctx, uint8_t *out, cfg.texture_count = ctx->stage[PIPE_SHADER_VERTEX].texture_count; } - /* yes, it's really 17 bytes */ out += AGX_BIND_PIPELINE_LENGTH; - *(out++) = 0x0; struct agx_pool *pool = &ctx->batch->pool; - struct agx_ptr zero = agx_pool_alloc_aligned(pool, 16, 256); - memset(zero.cpu, 0, 16); - bool reads_tib = ctx->fs->info.reads_tib; - agx_push_record(&out, 0, zero.gpu); agx_push_record(&out, 5, demo_interpolation(ctx->fs, pool)); agx_push_record(&out, 5, demo_launch_fragment(ctx, pool, pipeline_fragment, varyings, ctx->fs->info.varyings.nr_descs)); agx_push_record(&out, 4, demo_linkage(ctx->vs, pool)); @@ -1480,7 +1478,7 @@ agx_encode_state(struct agx_context *ctx, uint8_t *out, agx_push_record(&out, 3, demo_unk12(pool)); agx_push_record(&out, 2, agx_pool_upload(pool, ctx->rast->cull, sizeof(ctx->rast->cull))); - return (out - 1); // XXX: alignment fixup, or something + return out; } static enum agx_primitive