[Mesa-dev] [PATCH 1/6] radeonsi: extract IB and bo list saving into separate functions
Michael Schellenberger Costa
mschellenbergercosta at googlemail.com
Wed Jun 22 10:01:54 UTC 2016
Hi Nicolai
Am 22.06.2016 um 11:40 schrieb Nicolai Hähnle:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> ---
> src/gallium/drivers/radeon/r600_pipe_common.c | 53 +++++++++++++++++++++++++++
> src/gallium/drivers/radeon/r600_pipe_common.h | 12 ++++++
> src/gallium/drivers/radeonsi/si_debug.c | 39 +++++++++-----------
> src/gallium/drivers/radeonsi/si_hw_context.c | 25 +------------
> src/gallium/drivers/radeonsi/si_pipe.c | 8 +---
> src/gallium/drivers/radeonsi/si_pipe.h | 5 +--
> 6 files changed, 88 insertions(+), 54 deletions(-)
>
> diff --git a/src/gallium/drivers/radeon/r600_pipe_common.c b/src/gallium/drivers/radeon/r600_pipe_common.c
> index fa9f70d..ee70a1a 100644
> --- a/src/gallium/drivers/radeon/r600_pipe_common.c
> +++ b/src/gallium/drivers/radeon/r600_pipe_common.c
> @@ -302,6 +302,59 @@ static void r600_flush_dma_ring(void *ctx, unsigned flags,
> rctx->ws->fence_reference(fence, rctx->last_sdma_fence);
> }
>
> +/**
> + * Store a linearized copy of all chunks of \p cs together with the buffer
> + * list in \p saved.
> + */
> +void radeon_save_cs(struct radeon_winsys *ws, struct radeon_winsys_cs *cs,
> + struct radeon_saved_cs *saved)
> +{
> + void *buf;
> + unsigned i;
> +
> + /* Save the IB chunks. */
> + saved->num_dw = cs->prev_dw + cs->current.cdw;
> + saved->ib = MALLOC(4 * saved->num_dw);
> + if (!saved->ib)
> + goto oom;
> +
> + buf = saved->ib;
> + for (i = 0; i < cs->num_prev; ++i) {
> + memcpy(buf, cs->prev[i].buf, cs->prev[i].cdw * 4);
> + buf += cs->prev[i].cdw;
> + }
> + memcpy(buf, cs->current.buf, cs->current.cdw * 4);
> +
> + /* Save the buffer list. */
> + saved->bo_count = ws->cs_get_buffer_list(cs, NULL);
> + saved->bo_list = CALLOC(saved->bo_count,
> + sizeof(saved->bo_list[0]));
> + if (!saved->bo_list) {
> + FREE(saved->ib);
> + goto oom;
> + }
> + ws->cs_get_buffer_list(cs, saved->bo_list);
> +
> + return;
> +
> +oom:
> + fprintf(stderr, "%s: out of memory\n", __func__);
> + memset(saved, 0, sizeof(*saved));
Is that Goto really worth it? It costs you one extra line of code and
obfuscates things.
--Michael
> +}
> +
> +void radeon_clear_saved_cs(struct radeon_saved_cs *saved)
> +{
> + unsigned i;
> +
> + FREE(saved->ib);
> +
> + for (i = 0; i < saved->bo_count; i++)
> + pb_reference(&saved->bo_list[i].buf, NULL);
> + FREE(saved->bo_list);
> +
> + memset(saved, 0, sizeof(*saved));
> +}
> +
> static enum pipe_reset_status r600_get_reset_status(struct pipe_context *ctx)
> {
> struct r600_common_context *rctx = (struct r600_common_context *)ctx;
> diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h b/src/gallium/drivers/radeon/r600_pipe_common.h
> index fb6d1a5..a83908d 100644
> --- a/src/gallium/drivers/radeon/r600_pipe_common.h
> +++ b/src/gallium/drivers/radeon/r600_pipe_common.h
> @@ -458,6 +458,15 @@ struct r600_ring {
> struct pipe_fence_handle **fence);
> };
>
> +/* Saved CS data for debugging features. */
> +struct radeon_saved_cs {
> + uint32_t *ib;
> + unsigned num_dw;
> +
> + struct radeon_bo_list_item *bo_list;
> + unsigned bo_count;
> +};
> +
> struct r600_common_context {
> struct pipe_context b; /* base class */
>
> @@ -623,6 +632,9 @@ const char *r600_get_llvm_processor_name(enum radeon_family family);
> void r600_need_dma_space(struct r600_common_context *ctx, unsigned num_dw,
> struct r600_resource *dst, struct r600_resource *src);
> void r600_dma_emit_wait_idle(struct r600_common_context *rctx);
> +void radeon_save_cs(struct radeon_winsys *ws, struct radeon_winsys_cs *cs,
> + struct radeon_saved_cs *saved);
> +void radeon_clear_saved_cs(struct radeon_saved_cs *saved);
>
> /* r600_gpu_load.c */
> void r600_gpu_load_kill_thread(struct r600_common_screen *rscreen);
> diff --git a/src/gallium/drivers/radeonsi/si_debug.c b/src/gallium/drivers/radeonsi/si_debug.c
> index b551c72..176a195 100644
> --- a/src/gallium/drivers/radeonsi/si_debug.c
> +++ b/src/gallium/drivers/radeonsi/si_debug.c
> @@ -508,7 +508,7 @@ static void si_dump_last_ib(struct si_context *sctx, FILE *f)
> {
> int last_trace_id = -1;
>
> - if (!sctx->last_ib)
> + if (!sctx->last_gfx.ib)
> return;
>
> if (sctx->last_trace_buf) {
> @@ -533,11 +533,8 @@ static void si_dump_last_ib(struct si_context *sctx, FILE *f)
> sctx->init_config_gs_rings->ndw,
> -1, "IB2: Init GS rings");
>
> - si_parse_ib(f, sctx->last_ib, sctx->last_ib_dw_size,
> + si_parse_ib(f, sctx->last_gfx.ib, sctx->last_gfx.num_dw,
> last_trace_id, "IB");
> - free(sctx->last_ib); /* dump only once */
> - sctx->last_ib = NULL;
> - r600_resource_reference(&sctx->last_trace_buf, NULL);
> }
>
> static const char *priority_to_string(enum radeon_bo_priority priority)
> @@ -592,32 +589,33 @@ static int bo_list_compare_va(const struct radeon_bo_list_item *a,
> a->vm_address > b->vm_address ? 1 : 0;
> }
>
> -static void si_dump_last_bo_list(struct si_context *sctx, FILE *f)
> +static void si_dump_bo_list(struct si_context *sctx,
> + const struct radeon_saved_cs *saved, FILE *f)
> {
> unsigned i,j;
>
> - if (!sctx->last_bo_list)
> + if (!saved->bo_list)
> return;
>
> /* Sort the list according to VM adddresses first. */
> - qsort(sctx->last_bo_list, sctx->last_bo_count,
> - sizeof(sctx->last_bo_list[0]), (void*)bo_list_compare_va);
> + qsort(saved->bo_list, saved->bo_count,
> + sizeof(saved->bo_list[0]), (void*)bo_list_compare_va);
>
> fprintf(f, "Buffer list (in units of pages = 4kB):\n"
> COLOR_YELLOW " Size VM start page "
> "VM end page Usage" COLOR_RESET "\n");
>
> - for (i = 0; i < sctx->last_bo_count; i++) {
> + for (i = 0; i < saved->bo_count; i++) {
> /* Note: Buffer sizes are expected to be aligned to 4k by the winsys. */
> const unsigned page_size = sctx->b.screen->info.gart_page_size;
> - uint64_t va = sctx->last_bo_list[i].vm_address;
> - uint64_t size = sctx->last_bo_list[i].buf->size;
> + uint64_t va = saved->bo_list[i].vm_address;
> + uint64_t size = saved->bo_list[i].buf->size;
> bool hit = false;
>
> /* If there's unused virtual memory between 2 buffers, print it. */
> if (i) {
> - uint64_t previous_va_end = sctx->last_bo_list[i-1].vm_address +
> - sctx->last_bo_list[i-1].buf->size;
> + uint64_t previous_va_end = saved->bo_list[i-1].vm_address +
> + saved->bo_list[i-1].buf->size;
>
> if (va > previous_va_end) {
> fprintf(f, " %10"PRIu64" -- hole --\n",
> @@ -631,7 +629,7 @@ static void si_dump_last_bo_list(struct si_context *sctx, FILE *f)
>
> /* Print the usage. */
> for (j = 0; j < 64; j++) {
> - if (!(sctx->last_bo_list[i].priority_usage & (1llu << j)))
> + if (!(saved->bo_list[i].priority_usage & (1llu << j)))
> continue;
>
> fprintf(f, "%s%s", !hit ? "" : ", ", priority_to_string(j));
> @@ -641,11 +639,6 @@ static void si_dump_last_bo_list(struct si_context *sctx, FILE *f)
> }
> fprintf(f, "\nNote: The holes represent memory not used by the IB.\n"
> " Other buffers can still be allocated there.\n\n");
> -
> - for (i = 0; i < sctx->last_bo_count; i++)
> - pb_reference(&sctx->last_bo_list[i].buf, NULL);
> - free(sctx->last_bo_list);
> - sctx->last_bo_list = NULL;
> }
>
> static void si_dump_framebuffer(struct si_context *sctx, FILE *f)
> @@ -687,10 +680,14 @@ static void si_dump_debug_state(struct pipe_context *ctx, FILE *f,
> si_dump_shader(sctx->screen, &sctx->gs_shader, f);
> si_dump_shader(sctx->screen, &sctx->ps_shader, f);
>
> - si_dump_last_bo_list(sctx, f);
> + si_dump_bo_list(sctx, &sctx->last_gfx, f);
> si_dump_last_ib(sctx, f);
>
> fprintf(f, "Done.\n");
> +
> + /* dump only once */
> + radeon_clear_saved_cs(&sctx->last_gfx);
> + r600_resource_reference(&sctx->last_trace_buf, NULL);
> }
>
> static bool si_vm_fault_occured(struct si_context *sctx, uint32_t *out_addr)
> diff --git a/src/gallium/drivers/radeonsi/si_hw_context.c b/src/gallium/drivers/radeonsi/si_hw_context.c
> index d1b9851..696217d 100644
> --- a/src/gallium/drivers/radeonsi/si_hw_context.c
> +++ b/src/gallium/drivers/radeonsi/si_hw_context.c
> @@ -130,32 +130,11 @@ void si_context_gfx_flush(void *context, unsigned flags,
> si_trace_emit(ctx);
>
> if (ctx->is_debug) {
> - uint32_t *buf;
> - unsigned i;
> -
> /* Save the IB for debug contexts. */
> - free(ctx->last_ib);
> - ctx->last_ib_dw_size = cs->prev_dw + cs->current.cdw;
> - ctx->last_ib = malloc(ctx->last_ib_dw_size * 4);
> - buf = ctx->last_ib;
> - for (i = 0; i < cs->num_prev; ++i) {
> - memcpy(buf, cs->prev[i].buf, cs->prev[i].cdw * 4);
> - buf += cs->prev[i].cdw;
> - }
> - memcpy(buf, cs->current.buf, cs->current.cdw * 4);
> + radeon_clear_saved_cs(&ctx->last_gfx);
> + radeon_save_cs(ws, cs, &ctx->last_gfx);
> r600_resource_reference(&ctx->last_trace_buf, ctx->trace_buf);
> r600_resource_reference(&ctx->trace_buf, NULL);
> -
> - /* Save the buffer list. */
> - if (ctx->last_bo_list) {
> - for (i = 0; i < ctx->last_bo_count; i++)
> - pb_reference(&ctx->last_bo_list[i].buf, NULL);
> - free(ctx->last_bo_list);
> - }
> - ctx->last_bo_count = ws->cs_get_buffer_list(cs, NULL);
> - ctx->last_bo_list = calloc(ctx->last_bo_count,
> - sizeof(ctx->last_bo_list[0]));
> - ws->cs_get_buffer_list(cs, ctx->last_bo_list);
> }
>
> /* Flush the CS. */
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.c b/src/gallium/drivers/radeonsi/si_pipe.c
> index e65a30f..0c59a12 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.c
> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> @@ -87,12 +87,8 @@ static void si_destroy_context(struct pipe_context *context)
>
> r600_resource_reference(&sctx->trace_buf, NULL);
> r600_resource_reference(&sctx->last_trace_buf, NULL);
> - free(sctx->last_ib);
> - if (sctx->last_bo_list) {
> - for (i = 0; i < sctx->last_bo_count; i++)
> - pb_reference(&sctx->last_bo_list[i].buf, NULL);
> - free(sctx->last_bo_list);
> - }
> + radeon_clear_saved_cs(&sctx->last_gfx);
> +
> FREE(sctx);
> }
>
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.h b/src/gallium/drivers/radeonsi/si_pipe.h
> index 7e68bb7..81ad570 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.h
> +++ b/src/gallium/drivers/radeonsi/si_pipe.h
> @@ -321,14 +321,11 @@ struct si_context {
>
> /* Debug state. */
> bool is_debug;
> - uint32_t *last_ib;
> - unsigned last_ib_dw_size;
> + struct radeon_saved_cs last_gfx;
> struct r600_resource *last_trace_buf;
> struct r600_resource *trace_buf;
> unsigned trace_id;
> uint64_t dmesg_timestamp;
> - unsigned last_bo_count;
> - struct radeon_bo_list_item *last_bo_list;
>
> /* Other state */
> bool need_check_render_feedback;
>
More information about the mesa-dev
mailing list