Re: [Intel-gfx] [PATCH v2 2/3] drm/i915/perf: prepare driver to receive multiple ctx handles

[Chris Wilson]

Quoting Lionel Landwerlin (2020-03-31 12:48:20)
> -static struct intel_context *oa_context(struct i915_perf_stream *stream)
> -{
> -   return stream->pinned_ctx ?: stream->engine->kernel_context;

Maybe better to use pinned_ctxs[0] if set?

There are certain risks* to using the kernel_context, so we should keep
its use minimised.

* Mostly the danger of hangs/corruption, inadvertent global barriers.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 2/3] drm/i915/perf: prepare driver to receive multiple ctx handles

[Lionel Landwerlin]

Make all the internal necessary changes before we flip the switch.

v2: Use an unlimited number of intel contexts (Chris)

Signed-off-by: Lionel Landwerlin 
---
 drivers/gpu/drm/i915/i915_perf.c   | 584 +++--
 drivers/gpu/drm/i915/i915_perf_types.h |  23 +-
 2 files changed, 362 insertions(+), 245 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 8b28afb0526e..112bb5bd6665 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -192,6 +192,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 
@@ -329,7 +330,8 @@ static const struct i915_oa_format 
gen12_oa_formats[I915_OA_FORMAT_MAX] = {
  * @single_context: Whether a single or all gpu contexts should be monitored
  * @hold_preemption: Whether the preemption is disabled for the filtered
  *   context
- * @ctx_handle: A gem ctx handle for use with @single_context
+ * @n_ctx_handles: Length of @ctx_handles
+ * @ctx_handles: An array of gem context handles
  * @metrics_set: An ID for an OA unit metric set advertised via sysfs
  * @oa_format: An OA unit HW report format
  * @oa_periodic: Whether to enable periodic OA unit sampling
@@ -349,9 +351,10 @@ static const struct i915_oa_format 
gen12_oa_formats[I915_OA_FORMAT_MAX] = {
 struct perf_open_properties {
u32 sample_flags;
 
-   u64 single_context:1;
u64 hold_preemption:1;
-   u64 ctx_handle;
+
+   u32 n_ctx_handles;
+   u32 *ctx_handles;
 
/* OA sampling state */
int metrics_set;
@@ -625,6 +628,21 @@ static int append_oa_sample(struct i915_perf_stream 
*stream,
return 0;
 }
 
+static int ctx_id_equal(const void *key, const void *elem)
+{
+   return *((int *) elem) - *((int *) key);
+}
+
+static inline bool ctx_id_match(struct i915_perf_stream *stream,
+   u32 masked_ctx_id)
+{
+   return bsearch(_ctx_id,
+  stream->ctx_ids,
+  stream->n_ctxs,
+  sizeof(*stream->ctx_ids),
+  ctx_id_equal) != NULL;
+}
+
 /**
  * Copies all buffered OA reports into userspace read() buffer.
  * @stream: An i915-perf stream opened for OA metrics
@@ -736,7 +754,7 @@ static int gen8_append_oa_reports(struct i915_perf_stream 
*stream,
continue;
}
 
-   ctx_id = report32[2] & stream->specific_ctx_id_mask;
+   ctx_id = report32[2] & stream->ctx_id_mask;
 
/*
 * Squash whatever is in the CTX_ID field if it's marked as
@@ -781,26 +799,33 @@ static int gen8_append_oa_reports(struct i915_perf_stream 
*stream,
 * switches since it's not-uncommon for periodic samples to
 * identify a switch before any 'context switch' report.
 */
-   if (!stream->perf->exclusive_stream->ctx ||
-   stream->specific_ctx_id == ctx_id ||
-   stream->oa_buffer.last_ctx_id == stream->specific_ctx_id ||
-   reason & OAREPORT_REASON_CTX_SWITCH) {
-
-   /*
-* While filtering for a single context we avoid
-* leaking the IDs of other contexts.
-*/
-   if (stream->perf->exclusive_stream->ctx &&
-   stream->specific_ctx_id != ctx_id) {
-   report32[2] = INVALID_CTX_ID;
-   }
-
+   if (!stream->perf->exclusive_stream->n_ctxs) {
ret = append_oa_sample(stream, buf, count, offset,
   report);
if (ret)
break;
+   } else {
+   bool ctx_match = ctx_id != INVALID_CTX_ID &&
+   ctx_id_match(stream, ctx_id);
+
+   if (ctx_match ||
+   stream->oa_buffer.last_ctx_match ||
+   reason & OAREPORT_REASON_CTX_SWITCH) {
+
+   /*
+* While filtering for a single context we avoid
+* leaking the IDs of other contexts.
+*/
+   if (!ctx_match)
+   report32[2] = INVALID_CTX_ID;
+
+   ret = append_oa_sample(stream, buf, count, 
offset,
+  report);
+   if (ret)
+   break;
+   }
 
-   stream->oa_buffer.last_ctx_id = ctx_id;
+   stream->oa_buffer.last_ctx_match = ctx_match;
}
 
/*
@@ -1191,138 +1216,163 @@ static int i915_oa_read(struct