From 0f436e0fe157cc78a0a3b35a9ed68133c4e1516a Mon Sep 17 00:00:00 2001 From: Konstantin Seurer Date: Sun, 4 Feb 2024 14:38:32 +0100 Subject: [PATCH] ac/parse_ib: Replace the parameter list with ac_ib_parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's more code but it should be more readable. This also makes adding optional arguments easier. Reviewed-by: Marek Olšák Part-of: --- src/amd/common/ac_debug.h | 27 +++++++--- src/amd/common/ac_gpu_info.c | 12 ++++- src/amd/common/ac_ib_parser.c | 11 +++- src/amd/common/ac_parse_ib.c | 54 +++++-------------- src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c | 32 +++++++++-- src/gallium/drivers/radeonsi/si_debug.c | 42 ++++++++++++--- 6 files changed, 117 insertions(+), 61 deletions(-) diff --git a/src/amd/common/ac_debug.h b/src/amd/common/ac_debug.h index 2f0039f1fde..e2dd63017b4 100644 --- a/src/amd/common/ac_debug.h +++ b/src/amd/common/ac_debug.h @@ -68,15 +68,28 @@ void ac_gather_context_rolls(FILE *f, uint32_t **ibs, uint32_t *ib_dw_sizes, uns struct hash_table *annotations, const struct radeon_info *info); /* ac_parse_ib.c */ + +struct ac_ib_parser { + /* Arguments to ac_parse_ib.* */ + FILE *f; + uint32_t *ib; + unsigned num_dw; + const int *trace_ids; + unsigned trace_id_count; + enum amd_gfx_level gfx_level; + enum radeon_family family; + enum amd_ip_type ip_type; + ac_debug_addr_callback addr_callback; + void *addr_callback_data; + + /* Internal */ + unsigned cur_dw; +}; + void ac_dump_reg(FILE *file, enum amd_gfx_level gfx_level, enum radeon_family family, unsigned offset, uint32_t value, uint32_t field_mask); -void ac_parse_ib_chunk(FILE *f, uint32_t *ib, int num_dw, const int *trace_ids, - unsigned trace_id_count, enum amd_gfx_level gfx_level, - enum radeon_family family, enum amd_ip_type ip_type, - ac_debug_addr_callback addr_callback, void *addr_callback_data); -void ac_parse_ib(FILE *f, uint32_t *ib, int num_dw, const int *trace_ids, unsigned trace_id_count, - const char *name, enum amd_gfx_level gfx_level, enum radeon_family family, - enum amd_ip_type ip_type, ac_debug_addr_callback addr_callback, void *addr_callback_data); +void ac_parse_ib_chunk(struct ac_ib_parser *ib); +void ac_parse_ib(struct ac_ib_parser *ib, const char *name); #ifdef __cplusplus } diff --git a/src/amd/common/ac_gpu_info.c b/src/amd/common/ac_gpu_info.c index 16a7254a521..bc86a3ccb41 100644 --- a/src/amd/common/ac_gpu_info.c +++ b/src/amd/common/ac_gpu_info.c @@ -1636,8 +1636,16 @@ bool ac_query_gpu_info(int fd, void *dev_p, struct radeon_info *info, exit(1); } - ac_parse_ib(stdout, ib, size / 4, NULL, 0, "IB", info->gfx_level, info->family, - AMD_IP_GFX, NULL, NULL); + struct ac_ib_parser ib_parser = { + .f = stdout, + .ib = ib, + .num_dw = size / 4, + .gfx_level = info->gfx_level, + .family = info->family, + .ip_type = AMD_IP_GFX, + }; + + ac_parse_ib(&ib_parser, "IB"); free(ib); exit(0); } diff --git a/src/amd/common/ac_ib_parser.c b/src/amd/common/ac_ib_parser.c index 02e59018261..3a728ca2034 100644 --- a/src/amd/common/ac_ib_parser.c +++ b/src/amd/common/ac_ib_parser.c @@ -55,7 +55,16 @@ int main(int argc, char **argv) } fclose(f); - ac_parse_ib(stdout, ib, size / 4, NULL, 0, filename, gfx_level, family, AMD_IP_GFX, NULL, NULL); + struct ac_ib_parser ib_parser = { + .f = stdout, + .ib = ib, + .num_dw = size / 4, + .gfx_level = gfx_level, + .family = family, + .ip_type = AMD_IP_GFX, + }; + + ac_parse_ib(&ib_parser, filename); free(ib); } diff --git a/src/amd/common/ac_parse_ib.c b/src/amd/common/ac_parse_ib.c index 85aef4097cf..a896baf4ca7 100644 --- a/src/amd/common/ac_parse_ib.c +++ b/src/amd/common/ac_parse_ib.c @@ -42,20 +42,6 @@ DEBUG_GET_ONCE_BOOL_OPTION(color, "AMD_COLOR", true); #define INDENT_PKT 8 -struct ac_ib_parser { - FILE *f; - uint32_t *ib; - unsigned num_dw; - const int *trace_ids; - unsigned trace_id_count; - enum amd_gfx_level gfx_level; - enum radeon_family family; - ac_debug_addr_callback addr_callback; - void *addr_callback_data; - - unsigned cur_dw; -}; - static void parse_gfx_compute_ib(FILE *f, struct ac_ib_parser *ib); static void print_spaces(FILE *f, unsigned num) @@ -949,43 +935,32 @@ static void parse_sdma_ib(FILE *f, struct ac_ib_parser *ib) * be NULL. * \param addr_callback_data user data for addr_callback */ -void ac_parse_ib_chunk(FILE *f, uint32_t *ib_ptr, int num_dw, const int *trace_ids, - unsigned trace_id_count, enum amd_gfx_level gfx_level, - enum radeon_family family, enum amd_ip_type ip_type, - ac_debug_addr_callback addr_callback, void *addr_callback_data) +void ac_parse_ib_chunk(struct ac_ib_parser *ib) { - struct ac_ib_parser ib = {0}; - ib.ib = ib_ptr; - ib.num_dw = num_dw; - ib.trace_ids = trace_ids; - ib.trace_id_count = trace_id_count; - ib.gfx_level = gfx_level; - ib.family = family; - ib.addr_callback = addr_callback; - ib.addr_callback_data = addr_callback_data; + struct ac_ib_parser tmp_ib = *ib; char *out; size_t outsize; struct u_memstream mem; u_memstream_open(&mem, &out, &outsize); FILE *const memf = u_memstream_get(&mem); - ib.f = memf; + tmp_ib.f = memf; - if (ip_type == AMD_IP_GFX || ip_type == AMD_IP_COMPUTE) - parse_gfx_compute_ib(memf, &ib); - else if (ip_type == AMD_IP_SDMA) - parse_sdma_ib(memf, &ib); + if (ib->ip_type == AMD_IP_GFX || ib->ip_type == AMD_IP_COMPUTE) + parse_gfx_compute_ib(memf, &tmp_ib); + else if (ib->ip_type == AMD_IP_SDMA) + parse_sdma_ib(memf, &tmp_ib); else unreachable("unsupported IP type"); u_memstream_close(&mem); if (out) { - format_ib_output(f, out); + format_ib_output(ib->f, out); free(out); } - if (ib.cur_dw > ib.num_dw) { + if (tmp_ib.cur_dw > tmp_ib.num_dw) { printf("\nPacket ends after the end of IB.\n"); exit(1); } @@ -1021,14 +996,11 @@ static const char *ip_name(const enum amd_ip_type ip) * be NULL. * \param addr_callback_data user data for addr_callback */ -void ac_parse_ib(FILE *f, uint32_t *ib, int num_dw, const int *trace_ids, unsigned trace_id_count, - const char *name, enum amd_gfx_level gfx_level, enum radeon_family family, - enum amd_ip_type ip_type, ac_debug_addr_callback addr_callback, void *addr_callback_data) +void ac_parse_ib(struct ac_ib_parser *ib, const char *name) { - fprintf(f, "------------------ %s begin - %s ------------------\n", name, ip_name(ip_type)); + fprintf(ib->f, "------------------ %s begin - %s ------------------\n", name, ip_name(ib->ip_type)); - ac_parse_ib_chunk(f, ib, num_dw, trace_ids, trace_id_count, gfx_level, family, ip_type, - addr_callback, addr_callback_data); + ac_parse_ib_chunk(ib); - fprintf(f, "------------------- %s end - %s -------------------\n\n", name, ip_name(ip_type)); + fprintf(ib->f, "------------------- %s end - %s -------------------\n\n", name, ip_name(ib->ip_type)); } diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c index 93a447ee7fe..b9f8d479066 100644 --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c @@ -1407,8 +1407,20 @@ radv_amdgpu_winsys_cs_dump(struct radeon_cmdbuf *_cs, FILE *file, const int *tra assert(addr_info.cpu_addr); if (type == RADV_CS_DUMP_TYPE_IBS) { - ac_parse_ib(file, addr_info.cpu_addr, cs->ib_buffers[0].cdw, trace_ids, trace_id_count, "main IB", - ws->info.gfx_level, ws->info.family, cs->hw_ip, radv_amdgpu_winsys_get_cpu_addr, cs); + struct ac_ib_parser ib_parser = { + .f = file, + .ib = addr_info.cpu_addr, + .num_dw = cs->ib_buffers[0].cdw, + .trace_ids = trace_ids, + .trace_id_count = trace_id_count, + .gfx_level = ws->info.gfx_level, + .family = ws->info.family, + .ip_type = cs->hw_ip, + .addr_callback = radv_amdgpu_winsys_get_cpu_addr, + .addr_callback_data = cs, + }; + + ac_parse_ib(&ib_parser, "main IB"); } else { uint32_t *ib_dw = addr_info.cpu_addr; ac_gather_context_rolls(file, &ib_dw, &cs->ib_buffers[0].cdw, 1, NULL, &ws->info); @@ -1434,8 +1446,20 @@ radv_amdgpu_winsys_cs_dump(struct radeon_cmdbuf *_cs, FILE *file, const int *tra } if (type == RADV_CS_DUMP_TYPE_IBS) { - ac_parse_ib(file, mapped, ib->cdw, trace_ids, trace_id_count, name, ws->info.gfx_level, ws->info.family, - cs->hw_ip, NULL, NULL); + struct ac_ib_parser ib_parser = { + .f = file, + .ib = mapped, + .num_dw = ib->cdw, + .trace_ids = trace_ids, + .trace_id_count = trace_id_count, + .gfx_level = ws->info.gfx_level, + .family = ws->info.family, + .ip_type = cs->hw_ip, + .addr_callback = radv_amdgpu_winsys_get_cpu_addr, + .addr_callback_data = cs, + }; + + ac_parse_ib(&ib_parser, name); } else { ibs[i] = mapped; ib_dw_sizes[i] = ib->cdw; diff --git a/src/gallium/drivers/radeonsi/si_debug.c b/src/gallium/drivers/radeonsi/si_debug.c index 367549bdee2..1858980efea 100644 --- a/src/gallium/drivers/radeonsi/si_debug.c +++ b/src/gallium/drivers/radeonsi/si_debug.c @@ -347,8 +347,18 @@ static void si_parse_current_ib(FILE *f, struct radeon_cmdbuf *cs, unsigned begi struct radeon_cmdbuf_chunk *chunk = &cs->prev[prev_idx]; if (begin < chunk->cdw) { - ac_parse_ib_chunk(f, chunk->buf + begin, MIN2(end, chunk->cdw) - begin, last_trace_id, - trace_id_count, gfx_level, family, AMD_IP_GFX, NULL, NULL); + struct ac_ib_parser ib_parser = { + .f = f, + .ib = chunk->buf + begin, + .num_dw = MIN2(end, chunk->cdw) - begin, + .trace_ids = last_trace_id, + .trace_id_count = trace_id_count, + .gfx_level = gfx_level, + .family = family, + .ip_type = AMD_IP_GFX, + }; + + ac_parse_ib_chunk(&ib_parser); } if (end <= chunk->cdw) @@ -363,8 +373,18 @@ static void si_parse_current_ib(FILE *f, struct radeon_cmdbuf *cs, unsigned begi assert(end <= cs->current.cdw); - ac_parse_ib_chunk(f, cs->current.buf + begin, end - begin, last_trace_id, trace_id_count, - gfx_level, family, AMD_IP_GFX, NULL, NULL); + struct ac_ib_parser ib_parser = { + .f = f, + .ib = cs->current.buf + begin, + .num_dw = end - begin, + .trace_ids = last_trace_id, + .trace_id_count = trace_id_count, + .gfx_level = gfx_level, + .family = family, + .ip_type = AMD_IP_GFX, + }; + + ac_parse_ib_chunk(&ib_parser); fprintf(f, "------------------- %s end (dw = %u) -------------------\n\n", name, orig_end); } @@ -393,8 +413,18 @@ static void si_log_chunk_type_cs_print(void *data, FILE *f) if (chunk->gfx_end != chunk->gfx_begin) { if (scs->flushed) { - ac_parse_ib(f, scs->gfx.ib + chunk->gfx_begin, chunk->gfx_end - chunk->gfx_begin, - &last_trace_id, map ? 1 : 0, "IB", ctx->gfx_level, ctx->family, AMD_IP_GFX, NULL, NULL); + struct ac_ib_parser ib_parser = { + .f = f, + .ib = scs->gfx.ib + chunk->gfx_begin, + .num_dw = chunk->gfx_end - chunk->gfx_begin, + .trace_ids = &last_trace_id, + .trace_id_count = map ? 1 : 0, + .gfx_level = ctx->gfx_level, + .family = ctx->family, + .ip_type = AMD_IP_GFX, + }; + + ac_parse_ib(&ib_parser, "IB"); } else { si_parse_current_ib(f, &ctx->gfx_cs, chunk->gfx_begin, chunk->gfx_end, &last_trace_id, map ? 1 : 0, "IB", ctx->gfx_level, ctx->family);