All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: chris@chris-wilson.co.uk
Subject: [Intel-gfx] [PATCH v9 2/4] drm/i915/perf: stop using the kernel context
Date: Wed, 29 Apr 2020 12:02:40 +0300	[thread overview]
Message-ID: <20200429090242.978170-3-lionel.g.landwerlin@intel.com> (raw)
In-Reply-To: <20200429090242.978170-1-lionel.g.landwerlin@intel.com>

Chris doesn't like that.

v2: Don't forget to configure the kernel so that periodic reports are
    written appropriately (Lionel)

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c       | 153 +++++++++++++++++--------
 drivers/gpu/drm/i915/i915_perf_types.h |  10 +-
 2 files changed, 113 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 6d8a1e624bc3..f71f45b68265 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1356,9 +1356,31 @@ free_noa_wait(struct i915_perf_stream *stream)
 	i915_vma_unpin_and_release(&stream->noa_wait, 0);
 }
 
+static int i915_perf_stream_sync(struct i915_perf_stream *stream,
+				 bool enable)
+{
+	struct i915_active *active;
+	int err = 0;
+
+	active = i915_active_create();
+	if (!active)
+		return -ENOMEM;
+
+	if (enable)
+		err = stream->perf->ops.enable_metric_set(stream, active);
+	else
+		stream->perf->ops.disable_metric_set(stream, active);
+	if (err == 0)
+		__i915_active_wait(active, TASK_UNINTERRUPTIBLE);
+
+	i915_active_put(active);
+	return err;
+}
+
 static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
 {
 	struct i915_perf *perf = stream->perf;
+	int err;
 
 	BUG_ON(stream != perf->exclusive_stream);
 
@@ -1369,7 +1391,14 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
 	 * See i915_oa_init_reg_state() and lrc_configure_all_contexts()
 	 */
 	WRITE_ONCE(perf->exclusive_stream, NULL);
-	perf->ops.disable_metric_set(stream);
+	err = i915_perf_stream_sync(stream, false /* enable */);
+	if (err) {
+		drm_err(&perf->i915->drm,
+			"Error while disabling OA stream\n");
+	}
+
+	intel_context_unpin(stream->config_context);
+	intel_context_put(stream->config_context);
 
 	free_oa_buffer(stream);
 
@@ -2013,11 +2042,6 @@ emit_oa_config(struct i915_perf_stream *stream,
 	return err;
 }
 
-static struct intel_context *oa_context(struct i915_perf_stream *stream)
-{
-	return stream->pinned_ctx ?: stream->engine->kernel_context;
-}
-
 static int
 hsw_enable_metric_set(struct i915_perf_stream *stream,
 		      struct i915_active *active)
@@ -2040,13 +2064,14 @@ hsw_enable_metric_set(struct i915_perf_stream *stream,
 			 0, GEN6_CSUNIT_CLOCK_GATE_DISABLE);
 
 	return emit_oa_config(stream, stream->oa_config,
-			      oa_context(stream),
+			      stream->config_context,
 			      active,
 			      BIT(I915_OA_CONFIG_PART_GLOBAL) |
 			      BIT(I915_OA_CONFIG_PART_PER_CONTEXT));
 }
 
-static void hsw_disable_metric_set(struct i915_perf_stream *stream)
+static void hsw_disable_metric_set(struct i915_perf_stream *stream,
+				   struct i915_active *active)
 {
 	struct intel_uncore *uncore = stream->uncore;
 
@@ -2171,13 +2196,14 @@ gen8_load_flex(struct i915_request *rq,
 	return 0;
 }
 
-static int gen8_modify_context(struct intel_context *ce,
+static int gen8_modify_context(struct i915_perf_stream *stream,
+			       struct intel_context *ce,
 			       const struct flex *flex, unsigned int count)
 {
 	struct i915_request *rq;
 	int err;
 
-	rq = intel_engine_create_kernel_request(ce->engine);
+	rq = intel_context_create_request(stream->config_context);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
@@ -2219,7 +2245,8 @@ gen8_modify_self(struct intel_context *ce,
 	return err;
 }
 
-static int gen8_configure_context(struct i915_gem_context *ctx,
+static int gen8_configure_context(struct i915_perf_stream *stream,
+				  struct i915_gem_context *ctx,
 				  struct flex *flex, unsigned int count)
 {
 	struct i915_gem_engines_iter it;
@@ -2237,7 +2264,7 @@ static int gen8_configure_context(struct i915_gem_context *ctx,
 			continue;
 
 		flex->value = intel_sseu_make_rpcs(ctx->i915, &ce->sseu);
-		err = gen8_modify_context(ce, flex, count);
+		err = gen8_modify_context(stream, ce, flex, count);
 
 		intel_context_unpin(ce);
 		if (err)
@@ -2287,7 +2314,7 @@ static int gen12_configure_oar_context(struct i915_perf_stream *stream,
 	if (err)
 		return err;
 
-	err = gen8_modify_context(ce, regs_context, ARRAY_SIZE(regs_context));
+	err = gen8_modify_context(stream, ce, regs_context, ARRAY_SIZE(regs_context));
 	intel_context_unlock_pinned(ce);
 	if (err)
 		return err;
@@ -2330,6 +2357,7 @@ oa_configure_all_contexts(struct i915_perf_stream *stream,
 	struct drm_i915_private *i915 = stream->perf->i915;
 	struct intel_engine_cs *engine;
 	struct i915_gem_context *ctx, *cn;
+	struct intel_context *kernel_context;
 	int err;
 
 	lockdep_assert_held(&stream->perf->lock);
@@ -2357,7 +2385,7 @@ oa_configure_all_contexts(struct i915_perf_stream *stream,
 
 		spin_unlock(&i915->gem.contexts.lock);
 
-		err = gen8_configure_context(ctx, regs, num_regs);
+		err = gen8_configure_context(stream, ctx, regs, num_regs);
 		if (err) {
 			i915_gem_context_put(ctx);
 			return err;
@@ -2370,12 +2398,23 @@ oa_configure_all_contexts(struct i915_perf_stream *stream,
 	spin_unlock(&i915->gem.contexts.lock);
 
 	/*
-	 * After updating all other contexts, we need to modify ourselves.
-	 * If we don't modify the kernel_context, we do not get events while
-	 * idle.
+	 * Modify the kernel context has this is where we're parked, we want
+	 * the periodic ticking on idle to be consistent with what the perf
+	 * stream was configured with.
+	 */
+	kernel_context = stream->engine->kernel_context;
+	regs[0].value = intel_sseu_make_rpcs(i915, &kernel_context->sseu);
+	err = gen8_modify_context(stream, kernel_context, regs, num_regs);
+	if (err)
+		return err;
+
+	/*
+	 * After updating all other contexts, we need to modify ourselves. If
+	 * we don't modify the stream->perf_context, we do not get events
+	 * while idle.
 	 */
 	for_each_uabi_engine(engine, i915) {
-		struct intel_context *ce = engine->kernel_context;
+		struct intel_context *ce = stream->config_context;
 
 		if (engine->class != RENDER_CLASS)
 			continue;
@@ -2494,8 +2533,9 @@ gen8_enable_metric_set(struct i915_perf_stream *stream,
 	if (err)
 		return err;
 
-	return emit_oa_config(stream, oa_config,
-			      oa_context(stream),
+
+	return emit_oa_config(stream, stream->oa_config,
+			      stream->config_context,
 			      active,
 			      BIT(I915_OA_CONFIG_PART_GLOBAL) |
 			      BIT(I915_OA_CONFIG_PART_PER_CONTEXT));
@@ -2554,44 +2594,47 @@ gen12_enable_metric_set(struct i915_perf_stream *stream,
 			return ret;
 	}
 
-	return emit_oa_config(stream, oa_config,
-			      oa_context(stream),
+	return emit_oa_config(stream, stream->oa_config,
+			      stream->config_context,
 			      active,
 			      BIT(I915_OA_CONFIG_PART_GLOBAL) |
 			      BIT(I915_OA_CONFIG_PART_PER_CONTEXT));
 }
 
-static void gen8_disable_metric_set(struct i915_perf_stream *stream)
+static void gen8_disable_metric_set(struct i915_perf_stream *stream,
+				    struct i915_active *active)
 {
 	struct intel_uncore *uncore = stream->uncore;
 
 	/* Reset all contexts' slices/subslices configurations. */
-	lrc_configure_all_contexts(stream, NULL, NULL);
+	lrc_configure_all_contexts(stream, NULL, active);
 
 	intel_uncore_rmw(uncore, GDT_CHICKEN_BITS, GT_NOA_ENABLE, 0);
 }
 
-static void gen10_disable_metric_set(struct i915_perf_stream *stream)
+static void gen10_disable_metric_set(struct i915_perf_stream *stream,
+				     struct i915_active *active)
 {
 	struct intel_uncore *uncore = stream->uncore;
 
 	/* Reset all contexts' slices/subslices configurations. */
-	lrc_configure_all_contexts(stream, NULL, NULL);
+	lrc_configure_all_contexts(stream, NULL, active);
 
 	/* Make sure we disable noa to save power. */
 	intel_uncore_rmw(uncore, RPM_CONFIG1, GEN10_GT_NOA_ENABLE, 0);
 }
 
-static void gen12_disable_metric_set(struct i915_perf_stream *stream)
+static void gen12_disable_metric_set(struct i915_perf_stream *stream,
+				     struct i915_active *active)
 {
 	struct intel_uncore *uncore = stream->uncore;
 
 	/* Reset all contexts' slices/subslices configurations. */
-	gen12_configure_all_contexts(stream, NULL, NULL);
+	gen12_configure_all_contexts(stream, NULL, active);
 
 	/* disable the context save/restore or OAR counters */
 	if (stream->ctx)
-		gen12_configure_oar_context(stream, NULL);
+		gen12_configure_oar_context(stream, active);
 
 	/* Make sure we disable noa to save power. */
 	intel_uncore_rmw(uncore, RPM_CONFIG1, GEN10_GT_NOA_ENABLE, 0);
@@ -2763,23 +2806,6 @@ static const struct i915_perf_stream_ops i915_oa_stream_ops = {
 	.read = i915_oa_read,
 };
 
-static int i915_perf_stream_enable_sync(struct i915_perf_stream *stream)
-{
-	struct i915_active *active;
-	int err;
-
-	active = i915_active_create();
-	if (!active)
-		return -ENOMEM;
-
-	err = stream->perf->ops.enable_metric_set(stream, active);
-	if (err == 0)
-		__i915_active_wait(active, TASK_UNINTERRUPTIBLE);
-
-	i915_active_put(active);
-	return err;
-}
-
 static void
 get_default_sseu_config(struct intel_sseu *out_sseu,
 			struct intel_engine_cs *engine)
@@ -2837,6 +2863,7 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
 {
 	struct drm_i915_private *i915 = stream->perf->i915;
 	struct i915_perf *perf = stream->perf;
+	struct intel_timeline *timeline;
 	int format_size;
 	int ret;
 
@@ -2946,10 +2973,30 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
 
 	stream->ops = &i915_oa_stream_ops;
 
+	timeline = intel_timeline_create(stream->engine->gt, NULL);
+	if (IS_ERR(timeline)) {
+		ret = PTR_ERR(timeline);
+		goto err_timeline;
+	}
+
+	stream->config_context = intel_context_create(stream->engine);
+	if (IS_ERR(stream->config_context)) {
+		intel_timeline_put(timeline);
+		ret = PTR_ERR(stream->config_context);
+		goto err_timeline;
+	}
+
+	stream->config_context->sseu = props->sseu;
+	stream->config_context->timeline = timeline;
+
+	ret = intel_context_pin(stream->config_context);
+	if (ret)
+		goto err_context_pin;
+
 	perf->sseu = props->sseu;
 	WRITE_ONCE(perf->exclusive_stream, stream);
 
-	ret = i915_perf_stream_enable_sync(stream);
+	ret = i915_perf_stream_sync(stream, true /* enable */);
 	if (ret) {
 		DRM_DEBUG("Unable to enable metric set\n");
 		goto err_enable;
@@ -2968,8 +3015,14 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
 
 err_enable:
 	WRITE_ONCE(perf->exclusive_stream, NULL);
-	perf->ops.disable_metric_set(stream);
+	i915_perf_stream_sync(stream, false /* enable */);
 
+	intel_context_unpin(stream->config_context);
+
+err_context_pin:
+	intel_context_put(stream->config_context);
+
+err_timeline:
 	free_oa_buffer(stream);
 
 err_oa_buf_alloc:
@@ -3221,6 +3274,8 @@ static long i915_perf_config_locked(struct i915_perf_stream *stream,
 		return -EINVAL;
 
 	if (config != stream->oa_config) {
+		struct intel_context *ce = stream->pinned_ctx ?: stream->config_context;
+
 		active = i915_active_create();
 		if (!active) {
 			ret = -ENOMEM;
@@ -3237,7 +3292,7 @@ static long i915_perf_config_locked(struct i915_perf_stream *stream,
 		 * so it will effectively take effect when idle.
 		 */
 		ret = emit_oa_config(stream, config,
-				     oa_context(stream),
+				     ce,
 				     active,
 				     BIT(I915_OA_CONFIG_PART_GLOBAL) |
 				     BIT(I915_OA_CONFIG_PART_PER_CONTEXT));
diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
index a36a455ae336..a8b903592a39 100644
--- a/drivers/gpu/drm/i915/i915_perf_types.h
+++ b/drivers/gpu/drm/i915/i915_perf_types.h
@@ -16,6 +16,7 @@
 #include <linux/uuid.h>
 #include <linux/wait.h>
 
+#include "gt/intel_context_types.h"
 #include "gt/intel_sseu.h"
 #include "i915_reg.h"
 #include "intel_wakeref.h"
@@ -311,6 +312,12 @@ struct i915_perf_stream {
 	 * buffer should be checked for available data.
 	 */
 	u64 poll_oa_period;
+
+	/**
+	 * @config_context: A logical context for use by the perf stream for
+	 * configuring the HW.
+	 */
+	struct intel_context *config_context;
 };
 
 /**
@@ -348,7 +355,8 @@ struct i915_oa_ops {
 	 * @disable_metric_set: Remove system constraints associated with using
 	 * the OA unit.
 	 */
-	void (*disable_metric_set)(struct i915_perf_stream *stream);
+	void (*disable_metric_set)(struct i915_perf_stream *stream,
+				   struct i915_active *active);
 
 	/**
 	 * @oa_enable: Enable periodic sampling
-- 
2.26.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2020-04-29  9:02 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-29  9:02 [Intel-gfx] [PATCH v9 0/4] drm/i915/perf: Add support for multi context perf queries Lionel Landwerlin
2020-04-29  9:02 ` [Intel-gfx] [PATCH v9 1/4] drm/i915/perf: break OA config buffer object in 2 Lionel Landwerlin
2020-04-29  9:02 ` Lionel Landwerlin [this message]
2020-04-29  9:02 ` [Intel-gfx] [PATCH v9 3/4] drm/i915/perf: prepare driver to receive multiple ctx handles Lionel Landwerlin
2020-04-29  9:02 ` [Intel-gfx] [PATCH v9 4/4] drm/i915/perf: enable filtering on multiple contexts Lionel Landwerlin
2020-04-29 10:33 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/perf: Add support for multi context perf queries (rev3) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200429090242.978170-3-lionel.g.landwerlin@intel.com \
    --to=lionel.g.landwerlin@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.