[Mesa-dev] [PATCH 07/10] i965: ensure isolated timer reports while idle don't confuse filtering

[Kenneth Graunke]

On Wednesday, June 14, 2017 5:38:32 PM PDT Lionel Landwerlin wrote:
> From: Robert Bragg <robert at sixbynine.org>
> 
> From experimentation in IGT, we found that the OA unit might label
> some report as "idle" (using an invalid context ID), right after a
> report for a given context. Deltas generated by those reports actually
> belong to the previous context, even though they're not labelled as
> such.
> 
> This change makes ensure that while reading OA reports, we only
> consider the GPU actually idle after 2 reports with an invalid context
> ID.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_performance_query.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_performance_query.c b/src/mesa/drivers/dri/i965/brw_performance_query.c
> index d11784c0352..c6574df302a 100644
> --- a/src/mesa/drivers/dri/i965/brw_performance_query.c
> +++ b/src/mesa/drivers/dri/i965/brw_performance_query.c
> @@ -863,6 +863,7 @@ accumulate_oa_reports(struct brw_context *brw,
>     struct exec_node *first_samples_node;
>     bool in_ctx = true;
>     uint32_t ctx_id;
> +   int out_duration = 0;
>  
>     assert(o->Ready);
>     assert(obj->oa.map != NULL);
> @@ -937,10 +938,16 @@ accumulate_oa_reports(struct brw_context *brw,
>                 if (in_ctx && report[2] != ctx_id) {
>                    DBG("i915 perf: Switch AWAY (observed by ID change)\n");
>                    in_ctx = false;
> +                  out_duration = 0;
>                 } else if (in_ctx == false && report[2] == ctx_id) {
>                    DBG("i915 perf: Switch TO\n");
>                    in_ctx = true;
> -                  add = false;
> +
> +                  /* We didn't *really* Switch AWAY in the case that we
> +                   * e.g. saw a single periodic report while idle...
> +                   */
> +                  if (out_duration >= 1)
> +                     add = false;
>                 } else if (in_ctx) {
>                    assert(report[2] == ctx_id);
>                    DBG("i915 perf: Continuation IN\n");
> @@ -948,6 +955,7 @@ accumulate_oa_reports(struct brw_context *brw,
>                    assert(report[2] != ctx_id);
>                    DBG("i915 perf: Continuation OUT\n");
>                    add = false;
> +                  out_duration++;
>                 }
>              }
>  
> 

Yeesh.  Hardware bugs.  Great find.

Would it make sense to record the commit message as a comment?
I guess there's always tig blame...

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- 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/20170615/8924cda7/attachment.sig>