From 591a3c738d7a812e220d23967d358d1e4e6034d5 Mon Sep 17 00:00:00 2001 From: Emma Anholt Date: Mon, 14 Jun 2021 14:13:45 -0700 Subject: [PATCH] freedreno: Be more strict about QUERY_AVAILABLE to simplify the code. ARB_oq doesn't just say "polling in a loop will make it complete eventually", it says "querying will make it complete in finite time." Part-of: --- src/freedreno/.clang-format | 1 + .../drivers/freedreno/freedreno_query_acc.c | 48 +++++--------- .../drivers/freedreno/freedreno_query_acc.h | 2 - .../drivers/freedreno/freedreno_query_hw.c | 62 ++++++------------- .../drivers/freedreno/freedreno_query_hw.h | 2 - 5 files changed, 36 insertions(+), 79 deletions(-) diff --git a/src/freedreno/.clang-format b/src/freedreno/.clang-format index 3036ea5fe30..461df1c8675 100644 --- a/src/freedreno/.clang-format +++ b/src/freedreno/.clang-format @@ -22,6 +22,7 @@ Cpp11BracedListStyle: true ForEachMacros: - LIST_FOR_EACH_ENTRY - LIST_FOR_EACH_ENTRY_SAFE + - LIST_FOR_EACH_ENTRY_SAFE_REV - list_for_each_entry - list_for_each_entry_safe - foreach_list_typed diff --git a/src/gallium/drivers/freedreno/freedreno_query_acc.c b/src/gallium/drivers/freedreno/freedreno_query_acc.c index c8370f7733a..d579a54759f 100644 --- a/src/gallium/drivers/freedreno/freedreno_query_acc.c +++ b/src/gallium/drivers/freedreno/freedreno_query_acc.c @@ -147,38 +147,14 @@ fd_acc_get_query_result(struct fd_context *ctx, struct fd_query *q, bool wait, assert(list_is_empty(&aq->node)); - /* if !wait, then check the last sample (the one most likely to - * not be ready yet) and bail if it is not ready: + /* ARB_occlusion_query says: + * + * "Querying the state for a given occlusion query forces that + * occlusion query to complete within a finite amount of time." + * + * So, regardless of whether we are supposed to wait or not, we do need to + * flush now. */ - if (!wait) { - int ret; - - if (pending(rsc, false)) { - assert(!q->base.flushed); - tc_assert_driver_thread(ctx->tc); - - /* piglit spec@arb_occlusion_query@occlusion_query_conform - * test, and silly apps perhaps, get stuck in a loop trying - * to get query result forever with wait==false.. we don't - * wait to flush unnecessarily but we also don't want to - * spin forever: - */ - if (aq->no_wait_cnt++ > 5) { - fd_context_access_begin(ctx); - fd_batch_flush(rsc->track->write_batch); - fd_context_access_end(ctx); - } - return false; - } - - ret = fd_resource_wait( - ctx, rsc, FD_BO_PREP_READ | FD_BO_PREP_NOSYNC | FD_BO_PREP_FLUSH); - if (ret) - return false; - - fd_bo_cpu_fini(rsc->bo); - } - if (rsc->track->write_batch) { tc_assert_driver_thread(ctx->tc); fd_context_access_begin(ctx); @@ -186,8 +162,14 @@ fd_acc_get_query_result(struct fd_context *ctx, struct fd_query *q, bool wait, fd_context_access_end(ctx); } - /* get the result: */ - fd_resource_wait(ctx, rsc, FD_BO_PREP_READ); + if (!wait) { + int ret = fd_resource_wait( + ctx, rsc, FD_BO_PREP_READ | FD_BO_PREP_NOSYNC | FD_BO_PREP_FLUSH); + if (ret) + return false; + } else { + fd_resource_wait(ctx, rsc, FD_BO_PREP_READ); + } void *ptr = fd_bo_map(rsc->bo); p->result(aq, ptr, result); diff --git a/src/gallium/drivers/freedreno/freedreno_query_acc.h b/src/gallium/drivers/freedreno/freedreno_query_acc.h index 79dd399c4fc..84f86feb7ad 100644 --- a/src/gallium/drivers/freedreno/freedreno_query_acc.h +++ b/src/gallium/drivers/freedreno/freedreno_query_acc.h @@ -89,8 +89,6 @@ struct fd_acc_query { struct list_head node; /* list-node in ctx->active_acc_queries */ - int no_wait_cnt; /* see fd_acc_get_query_result() */ - void *query_data; /* query specific data */ }; diff --git a/src/gallium/drivers/freedreno/freedreno_query_hw.c b/src/gallium/drivers/freedreno/freedreno_query_hw.c index 2bfffdc8350..2dbf2b2a30e 100644 --- a/src/gallium/drivers/freedreno/freedreno_query_hw.c +++ b/src/gallium/drivers/freedreno/freedreno_query_hw.c @@ -187,7 +187,7 @@ fd_hw_get_query_result(struct fd_context *ctx, struct fd_query *q, bool wait, { struct fd_hw_query *hq = fd_hw_query(q); const struct fd_hw_sample_provider *p = hq->provider; - struct fd_hw_sample_period *period; + struct fd_hw_sample_period *period, *tmp; DBG("%p: wait=%d", q, wait); @@ -197,47 +197,10 @@ fd_hw_get_query_result(struct fd_context *ctx, struct fd_query *q, bool wait, assert(list_is_empty(&hq->list)); assert(!hq->period); - /* if !wait, then check the last sample (the one most likely to - * not be ready yet) and bail if it is not ready: + /* sum the result across all sample periods. Start with the last period + * so that no-wait will bail quickly. */ - if (!wait) { - int ret; - - period = LIST_ENTRY(struct fd_hw_sample_period, hq->periods.prev, list); - - struct fd_resource *rsc = fd_resource(period->end->prsc); - - if (pending(rsc, false)) { - assert(!q->base.flushed); - tc_assert_driver_thread(ctx->tc); - - /* piglit spec@arb_occlusion_query@occlusion_query_conform - * test, and silly apps perhaps, get stuck in a loop trying - * to get query result forever with wait==false.. we don't - * wait to flush unnecessarily but we also don't want to - * spin forever: - */ - if (hq->no_wait_cnt++ > 5) { - fd_context_access_begin(ctx); - fd_batch_flush(rsc->track->write_batch); - fd_context_access_end(ctx); - } - return false; - } - - if (!rsc->bo) - return false; - - ret = fd_resource_wait( - ctx, rsc, FD_BO_PREP_READ | FD_BO_PREP_NOSYNC | FD_BO_PREP_FLUSH); - if (ret) - return false; - - fd_bo_cpu_fini(rsc->bo); - } - - /* sum the result across all sample periods: */ - LIST_FOR_EACH_ENTRY (period, &hq->periods, list) { + LIST_FOR_EACH_ENTRY_SAFE_REV (period, tmp, &hq->periods, list) { struct fd_hw_sample *start = period->start; ASSERTED struct fd_hw_sample *end = period->end; unsigned i; @@ -248,6 +211,14 @@ fd_hw_get_query_result(struct fd_context *ctx, struct fd_query *q, bool wait, struct fd_resource *rsc = fd_resource(start->prsc); + /* ARB_occlusion_query says: + * + * "Querying the state for a given occlusion query forces that + * occlusion query to complete within a finite amount of time." + * + * So, regardless of whether we are supposed to wait or not, we do need to + * flush now. + */ if (rsc->track->write_batch) { tc_assert_driver_thread(ctx->tc); fd_context_access_begin(ctx); @@ -259,7 +230,14 @@ fd_hw_get_query_result(struct fd_context *ctx, struct fd_query *q, bool wait, if (!rsc->bo) continue; - fd_resource_wait(ctx, rsc, FD_BO_PREP_READ); + if (!wait) { + int ret = fd_resource_wait( + ctx, rsc, FD_BO_PREP_READ | FD_BO_PREP_NOSYNC | FD_BO_PREP_FLUSH); + if (ret) + return false; + } else { + fd_resource_wait(ctx, rsc, FD_BO_PREP_READ); + } void *ptr = fd_bo_map(rsc->bo); diff --git a/src/gallium/drivers/freedreno/freedreno_query_hw.h b/src/gallium/drivers/freedreno/freedreno_query_hw.h index 01fc9ce370f..bf8ee7820f0 100644 --- a/src/gallium/drivers/freedreno/freedreno_query_hw.h +++ b/src/gallium/drivers/freedreno/freedreno_query_hw.h @@ -124,8 +124,6 @@ struct fd_hw_query { struct fd_hw_sample_period *period; struct list_head list; /* list-node in batch->active_queries */ - - int no_wait_cnt; /* see fd_hw_get_query_result */ }; static inline struct fd_hw_query *