[Mesa-dev] [PATCH 5/9] i965: Replace open-coded gen6 queryobj offsets with simple helpers
Kenneth Graunke
kenneth at whitecape.org
Wed Jun 14 22:10:38 UTC 2017
On Friday, June 9, 2017 6:01:36 AM PDT Chris Wilson wrote:
> Lots of places open-coded the assumed layout of the predicate/results
> within the query object, replace those with simple helpers.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> Cc: Matt Turner <mattst88 at gmail.com>
> ---
> src/mesa/drivers/dri/i965/brw_conditional_render.c | 4 ++--
> src/mesa/drivers/dri/i965/brw_context.h | 14 ++++++++++++++
> src/mesa/drivers/dri/i965/gen6_queryobj.c | 6 +++---
> src/mesa/drivers/dri/i965/hsw_queryobj.c | 18 +++++++++---------
> 4 files changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_conditional_render.c b/src/mesa/drivers/dri/i965/brw_conditional_render.c
> index 046a42b5f5..197c35efe2 100644
> --- a/src/mesa/drivers/dri/i965/brw_conditional_render.c
> +++ b/src/mesa/drivers/dri/i965/brw_conditional_render.c
> @@ -66,13 +66,13 @@ set_predicate_for_occlusion_query(struct brw_context *brw,
> query->bo,
> I915_GEM_DOMAIN_INSTRUCTION,
> 0, /* write domain */
> - 0 /* offset */);
> + gen6_query_results_offset(query, 0));
> brw_load_register_mem64(brw,
> MI_PREDICATE_SRC1,
> query->bo,
> I915_GEM_DOMAIN_INSTRUCTION,
> 0, /* write domain */
> - 8 /* offset */);
> + gen6_query_results_offset(query, 1));
> }
>
> static void
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index d1503312d4..c5acb83ad0 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -427,6 +427,20 @@ struct brw_query_object {
> bool flushed;
> };
>
> +#define GEN6_QUERY_PREDICATE (2)
> +#define GEN6_QUERY_RESULTS (0)
> +
> +static inline unsigned gen6_query_predicate_offset(const struct brw_query_object *query)
> +{
> + return GEN6_QUERY_PREDICATE * sizeof(uint64_t);
> +}
> +
> +static inline unsigned gen6_query_results_offset(const struct brw_query_object *query,
> + unsigned idx)
> +{
> + return (GEN6_QUERY_RESULTS + idx) * sizeof(uint64_t);
> +}
> +
> enum brw_gpu_ring {
> UNKNOWN_RING,
> RENDER_RING,
> diff --git a/src/mesa/drivers/dri/i965/gen6_queryobj.c b/src/mesa/drivers/dri/i965/gen6_queryobj.c
> index cc0f6f0b77..f913f986ae 100644
> --- a/src/mesa/drivers/dri/i965/gen6_queryobj.c
> +++ b/src/mesa/drivers/dri/i965/gen6_queryobj.c
> @@ -71,7 +71,7 @@ set_query_availability(struct brw_context *brw, struct brw_query_object *query,
> }
>
> brw_emit_pipe_control_write(brw, flags,
> - query->bo, 2 * sizeof(uint64_t),
> + query->bo, gen6_query_predicate_offset(query),
> available, 0);
> }
> }
> @@ -318,7 +318,7 @@ gen6_begin_query(struct gl_context *ctx, struct gl_query_object *q)
> {
> struct brw_context *brw = brw_context(ctx);
> struct brw_query_object *query = (struct brw_query_object *)q;
> - const int idx = 0;
> + const int idx = GEN6_QUERY_RESULTS;
>
> /* Since we're starting a new query, we need to throw away old results. */
> brw_bo_unreference(query->bo);
> @@ -407,7 +407,7 @@ gen6_end_query(struct gl_context *ctx, struct gl_query_object *q)
> {
> struct brw_context *brw = brw_context(ctx);
> struct brw_query_object *query = (struct brw_query_object *)q;
> - const int idx = 1;
> + const int idx = GEN6_QUERY_RESULTS + 1;
>
> switch (query->Base.Target) {
> case GL_TIME_ELAPSED:
> diff --git a/src/mesa/drivers/dri/i965/hsw_queryobj.c b/src/mesa/drivers/dri/i965/hsw_queryobj.c
> index b81ab3b6f8..cb1a2df52d 100644
> --- a/src/mesa/drivers/dri/i965/hsw_queryobj.c
> +++ b/src/mesa/drivers/dri/i965/hsw_queryobj.c
> @@ -191,7 +191,7 @@ load_overflow_data_to_cs_gprs(struct brw_context *brw,
> struct brw_query_object *query,
> int idx)
> {
> - int offset = idx * sizeof(uint64_t) * 4;
> + int offset = gen6_query_results_offset(query, 0) + idx * sizeof(uint64_t) * 4;
FWIW, I'm pretty sure 4 here is BRW_MAX_XFB_STREAMS.
I personally don't think that the code is more readable after patches
4-5, but I suppose that's a matter of taste. I'd be inclined to leave the
code with hardcoded offsets, but add a comment to the top of the file
describing the layout (I thought we had one already, but it looks like we
don't).
That said, the patches look correct to me, so if someone else wants to
chime in and say that they prefer this style, I'm okay with them.
>
> brw_load_register_mem64(brw,
> HSW_CS_GPR(1),
> @@ -304,7 +304,7 @@ hsw_result_to_gpr0(struct gl_context *ctx, struct brw_query_object *query,
> query->bo,
> I915_GEM_DOMAIN_INSTRUCTION,
> I915_GEM_DOMAIN_INSTRUCTION,
> - 2 * sizeof(uint64_t));
> + gen6_query_predicate_offset(query));
> return;
> }
>
> @@ -323,7 +323,7 @@ hsw_result_to_gpr0(struct gl_context *ctx, struct brw_query_object *query,
> query->bo,
> I915_GEM_DOMAIN_INSTRUCTION,
> I915_GEM_DOMAIN_INSTRUCTION,
> - 0 * sizeof(uint64_t));
> + gen6_query_results_offset(query, 0));
> } else if (query->Base.Target == GL_TRANSFORM_FEEDBACK_STREAM_OVERFLOW_ARB
> || query->Base.Target == GL_TRANSFORM_FEEDBACK_OVERFLOW_ARB) {
> /* Don't do anything in advance here, since the math for this is a little
> @@ -335,13 +335,13 @@ hsw_result_to_gpr0(struct gl_context *ctx, struct brw_query_object *query,
> query->bo,
> I915_GEM_DOMAIN_INSTRUCTION,
> I915_GEM_DOMAIN_INSTRUCTION,
> - 0 * sizeof(uint64_t));
> + gen6_query_results_offset(query, 0));
> brw_load_register_mem64(brw,
> HSW_CS_GPR(2),
> query->bo,
> I915_GEM_DOMAIN_INSTRUCTION,
> I915_GEM_DOMAIN_INSTRUCTION,
> - 1 * sizeof(uint64_t));
> + gen6_query_results_offset(query, 1));
>
> BEGIN_BATCH(5);
> OUT_BATCH(HSW_MI_MATH | (5 - 2));
> @@ -411,14 +411,14 @@ store_query_result_imm(struct brw_context *brw, struct brw_bo *bo,
> }
>
> static void
> -set_predicate(struct brw_context *brw, struct brw_bo *query_bo)
> +set_predicate(struct brw_context *brw, struct brw_query_object *query)
> {
> brw_load_register_imm64(brw, MI_PREDICATE_SRC1, 0ull);
>
> /* Load query availability into SRC0 */
> - brw_load_register_mem64(brw, MI_PREDICATE_SRC0, query_bo,
> + brw_load_register_mem64(brw, MI_PREDICATE_SRC0, query->bo,
> I915_GEM_DOMAIN_INSTRUCTION, 0,
> - 2 * sizeof(uint64_t));
> + gen6_query_predicate_offset(query));
>
> /* predicate = !(query_availability == 0); */
> BEGIN_BATCH(1);
> @@ -485,7 +485,7 @@ hsw_store_query_result(struct gl_context *ctx, struct gl_query_object *q,
> */
> hsw_result_to_gpr0(ctx, query, buf, offset, pname, ptype);
> if (pipelined)
> - set_predicate(brw, query->bo);
> + set_predicate(brw, query);
> store_query_result_reg(brw, bo->buffer, offset, ptype, HSW_CS_GPR(0),
> pipelined);
> } else {
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170614/e7dc8503/attachment.sig>
More information about the mesa-dev
mailing list