From 36612c96bd2a354b4c31eeb331d2f4bbad2f210e Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 27 Mar 2020 15:55:14 -0700 Subject: [PATCH] freedreno: Fix acc query handling in the presence of batch reordering. When we switch batches and start a new draw, we need to cap the queries in the previous batch and start queries again in the new one. FD_STAGE_NULL got renamed to 0 so that it would naturally return !is_active and end the queries at the end of the batch. Part-of: --- .../drivers/freedreno/freedreno_batch.h | 8 +-- .../drivers/freedreno/freedreno_context.h | 10 ++- .../drivers/freedreno/freedreno_query_acc.c | 64 +++++++++++++------ .../drivers/freedreno/freedreno_query_acc.h | 5 ++ 4 files changed, 62 insertions(+), 25 deletions(-) diff --git a/src/gallium/drivers/freedreno/freedreno_batch.h b/src/gallium/drivers/freedreno/freedreno_batch.h index a3bdfda1077..477c3e49f6a 100644 --- a/src/gallium/drivers/freedreno/freedreno_batch.h +++ b/src/gallium/drivers/freedreno/freedreno_batch.h @@ -47,11 +47,11 @@ enum fd_resource_status; * is active across IB's (or between tile IB and draw IB) */ enum fd_render_stage { - FD_STAGE_NULL = 0x01, - FD_STAGE_DRAW = 0x02, - FD_STAGE_CLEAR = 0x04, + FD_STAGE_NULL = 0x00, + FD_STAGE_DRAW = 0x01, + FD_STAGE_CLEAR = 0x02, /* used for driver internal draws (ie. util_blitter_blit()): */ - FD_STAGE_BLIT = 0x08, + FD_STAGE_BLIT = 0x04, FD_STAGE_ALL = 0xff, }; diff --git a/src/gallium/drivers/freedreno/freedreno_context.h b/src/gallium/drivers/freedreno/freedreno_context.h index 30deddd21b3..41f67a87d3c 100644 --- a/src/gallium/drivers/freedreno/freedreno_context.h +++ b/src/gallium/drivers/freedreno/freedreno_context.h @@ -202,6 +202,11 @@ struct fd_context { struct list_head acc_active_queries; /*@}*/ + /* Whether we need to walk the acc_active_queries next fd_set_stage() to + * update active queries (even if stage doesn't change). + */ + bool update_active_queries; + /* table with PIPE_PRIM_MAX entries mapping PIPE_PRIM_x to * DI_PT_x value to use for draw initiator. There are some * slight differences between generation: @@ -469,9 +474,8 @@ fd_batch_set_stage(struct fd_batch *batch, enum fd_render_stage stage) * don't enable queries which should be paused during internal * blits: */ - if ((batch->stage == FD_STAGE_BLIT) && - (stage != FD_STAGE_NULL)) - return; + if (batch->stage == FD_STAGE_BLIT && stage != FD_STAGE_NULL) + stage = FD_STAGE_BLIT; if (ctx->query_set_stage) ctx->query_set_stage(batch, stage); diff --git a/src/gallium/drivers/freedreno/freedreno_query_acc.c b/src/gallium/drivers/freedreno/freedreno_query_acc.c index fd7d61cf371..5e4354bd0e7 100644 --- a/src/gallium/drivers/freedreno/freedreno_query_acc.c +++ b/src/gallium/drivers/freedreno/freedreno_query_acc.c @@ -74,38 +74,59 @@ realloc_query_bo(struct fd_context *ctx, struct fd_acc_query *aq) fd_bo_cpu_fini(rsc->bo); } +static void +fd_acc_query_pause(struct fd_acc_query *aq) +{ + const struct fd_acc_sample_provider *p = aq->provider; + + if (!aq->batch) + return; + + p->pause(aq, aq->batch); + aq->batch = NULL; +} + +static void +fd_acc_query_resume(struct fd_acc_query *aq, struct fd_batch *batch) +{ + const struct fd_acc_sample_provider *p = aq->provider; + + aq->batch = batch; + p->resume(aq, aq->batch); +} + static void fd_acc_begin_query(struct fd_context *ctx, struct fd_query *q) { - struct fd_batch *batch = fd_context_batch(ctx); struct fd_acc_query *aq = fd_acc_query(q); - const struct fd_acc_sample_provider *p = aq->provider; DBG("%p", q); /* ->begin_query() discards previous results, so realloc bo: */ realloc_query_bo(ctx, aq); - /* then resume query if needed to collect first sample: */ - if (batch && is_active(aq, batch->stage)) - p->resume(aq, batch); + /* Signal that we need to update the active queries on the next draw */ + ctx->update_active_queries = true; /* add to active list: */ assert(list_is_empty(&aq->node)); list_addtail(&aq->node, &ctx->acc_active_queries); + + /* TIMESTAMP/GPU_FINISHED and don't do normal bracketing at draw time, we + * need to just emit the capture at this moment. + */ + if (skip_begin_query(q->type)) + fd_acc_query_resume(aq, fd_context_batch(ctx)); } static void fd_acc_end_query(struct fd_context *ctx, struct fd_query *q) { - struct fd_batch *batch = fd_context_batch(ctx); struct fd_acc_query *aq = fd_acc_query(q); - const struct fd_acc_sample_provider *p = aq->provider; DBG("%p", q); - if (batch && is_active(aq, batch->stage)) - p->pause(aq, batch); + fd_acc_query_pause(aq); /* remove from active list: */ list_delinit(&aq->node); @@ -208,23 +229,30 @@ fd_acc_create_query(struct fd_context *ctx, unsigned query_type, ctx->acc_sample_providers[idx]); } +/* Called at clear/draw/blit time to enable/disable the appropriate queries in + * the batch (and transfer active querying between batches in the case of + * batch reordering). + */ void fd_acc_query_set_stage(struct fd_batch *batch, enum fd_render_stage stage) { - if (stage != batch->stage) { - struct fd_acc_query *aq; - LIST_FOR_EACH_ENTRY(aq, &batch->ctx->acc_active_queries, node) { - const struct fd_acc_sample_provider *p = aq->provider; + struct fd_context *ctx = batch->ctx; - bool was_active = is_active(aq, batch->stage); + if (stage != batch->stage || ctx->update_active_queries) { + struct fd_acc_query *aq; + LIST_FOR_EACH_ENTRY(aq, &ctx->acc_active_queries, node) { + bool batch_change = aq->batch != batch; + bool was_active = aq->batch != NULL; bool now_active = is_active(aq, stage); - if (now_active && !was_active) - p->resume(aq, batch); - else if (was_active && !now_active) - p->pause(aq, batch); + if (was_active && (!now_active || batch_change)) + fd_acc_query_pause(aq); + if (now_active && (!was_active || batch_change)) + fd_acc_query_resume(aq, batch); } } + + ctx->update_active_queries = false; } void diff --git a/src/gallium/drivers/freedreno/freedreno_query_acc.h b/src/gallium/drivers/freedreno/freedreno_query_acc.h index fdde07fcc4f..15d5b880eaf 100644 --- a/src/gallium/drivers/freedreno/freedreno_query_acc.h +++ b/src/gallium/drivers/freedreno/freedreno_query_acc.h @@ -78,6 +78,11 @@ struct fd_acc_query { struct pipe_resource *prsc; + /* Pointer to the batch that our query has had resume() called on (if + * any). + */ + struct fd_batch *batch; + /* usually the same as provider->size but for batch queries we * need to calculate the size dynamically when the query is * allocated: -- 2.30.2