Mesa (staging/19.3): intel/perf: simplify the processing of OA reports

[GitLab Mirror]

Module: Mesa
Branch: staging/19.3
Commit: f7597aeefed4d500bf368f16066b7d7293ad0e9a
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=f7597aeefed4d500bf368f16066b7d7293ad0e9a

Author: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
Date:   Tue Dec  3 16:33:25 2019 +0200

intel/perf: simplify the processing of OA reports

This is a more accurate description of what happens in processing the
OA reports.

Previously we only had a somewhat difficult to parse state machine
tracking the context ID.

What we really only need to do to decide if the delta between 2
reports (r0 & r1) should be accumulated in the query result is :

   * whether the r0 is tagged with the context ID relevant to us

   * if r0 is not tagged with our context ID and r1 is: does r0 have a
     invalid context id? If not then we're in a case where i915 has
     resubmitted the same context for execution through the execlist
     submission port

v2: Update comment (Ken)

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
Cc: <mesa-stable at lists.freedesktop.org>
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
(cherry picked from commit 8c0b05826304370ef9e5f1e607d0f0305a0eb759)

---

 src/intel/perf/gen_perf.c | 64 ++++++++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 28 deletions(-)

diff --git a/src/intel/perf/gen_perf.c b/src/intel/perf/gen_perf.c
index 2df25487aad..20d84437e43 100644
--- a/src/intel/perf/gen_perf.c
+++ b/src/intel/perf/gen_perf.c
@@ -2216,6 +2216,17 @@ discard_all_queries(struct gen_perf_context *perf_ctx)
    }
 }
 
+/* Looks for the validity bit of context ID (dword 2) of an OA report. */
+static bool
+oa_report_ctx_id_valid(const struct gen_device_info *devinfo,
+                       const uint32_t *report)
+{
+   assert(devinfo->gen >= 8);
+   if (devinfo->gen == 8)
+      return (report[0] & (1 << 25)) != 0;
+   return (report[0] & (1 << 16)) != 0;
+}
+
 /**
  * Accumulate raw OA counter values based on deltas between pairs of
  * OA reports.
@@ -2243,7 +2254,7 @@ accumulate_oa_reports(struct gen_perf_context *perf_ctx,
    uint32_t *last;
    uint32_t *end;
    struct exec_node *first_samples_node;
-   bool in_ctx = true;
+   bool last_report_ctx_match = true;
    int out_duration = 0;
 
    assert(query->oa.map != NULL);
@@ -2289,6 +2300,7 @@ accumulate_oa_reports(struct gen_perf_context *perf_ctx,
          switch (header->type) {
          case DRM_I915_PERF_RECORD_SAMPLE: {
             uint32_t *report = (uint32_t *)(header + 1);
+            bool report_ctx_match = true;
             bool add = true;
 
             /* Ignore reports that come before the start marker.
@@ -2317,35 +2329,30 @@ accumulate_oa_reports(struct gen_perf_context *perf_ctx,
              * of OA counters while any other context is acctive.
              */
             if (devinfo->gen >= 8) {
-               if (in_ctx && report[2] != query->oa.result.hw_id) {
-                  DBG("i915 perf: Switch AWAY (observed by ID change)\n");
-                  in_ctx = false;
+               /* Consider that the current report matches our context only if
+                * the report says the report ID is valid.
+                */
+               report_ctx_match = oa_report_ctx_id_valid(devinfo, report) &&
+                  report[2] == start[2];
+               if (report_ctx_match)
                   out_duration = 0;
-               } else if (in_ctx == false && report[2] == query->oa.result.hw_id) {
-                  DBG("i915 perf: Switch TO\n");
-                  in_ctx = true;
-
-                  /* 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.
-                   *
-                   * 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] == query->oa.result.hw_id);
-                  DBG("i915 perf: Continuation IN\n");
-               } else {
-                  assert(report[2] != query->oa.result.hw_id);
-                  DBG("i915 perf: Continuation OUT\n");
-                  add = false;
+               else
                   out_duration++;
-               }
+
+               /* Only add the delta between <last, report> if the last report
+                * was clearly identified as our context, or if we have at most
+                * 1 report without a matching ID.
+                *
+                * The OA unit will sometimes label reports with an invalid
+                * context ID when i915 rewrites the execlist submit register
+                * with the same context as the one currently running. This
+                * happens when i915 wants to notify the HW of ringbuffer tail
+                * register update. We have to consider this report as part of
+                * our context as the 3d pipeline behind the OACS unit is still
+                * processing the operations started at the previous execlist
+                * submission.
+                */
+               add = last_report_ctx_match && out_duration < 2;
             }
 
             if (add) {
@@ -2355,6 +2362,7 @@ accumulate_oa_reports(struct gen_perf_context *perf_ctx,
             }
 
             last = report;
+            last_report_ctx_match = report_ctx_match;
 
             break;
          }