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 <ch...@chris-wilson.co.uk>
> Cc: Kenneth Graunke <kenn...@whitecape.org>
> Cc: Matt Turner <matts...@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 {
>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
