Re: [RFC 04/14] drm/i915/pmu: Decouple uAPI engine ids

From: Ben Widawsky <benjamin.widawsky@intel.com>
To: Tvrtko Ursulin <tursulin@ursulin.net>
Cc: Intel-gfx@lists.freedesktop.org
Subject: Re: [RFC 04/14] drm/i915/pmu: Decouple uAPI engine ids
Date: Mon, 24 Jul 2017 18:18:17 -0700	[thread overview]
Message-ID: <20170725011817.GA4922@intel.com> (raw)
In-Reply-To: <20170718143618.12254-5-tvrtko.ursulin@linux.intel.com>

On 17-07-18 15:36:08, Tvrtko Ursulin wrote:
>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
>As elsewhere in the code we have to decouple the binary
>engine identifiers for easier maintenance.
>
>Also the sampler mask was incorrect in the timer callback.
>

I don't quite understand the point of this patch... can you perhaps embellish
the commit message?

>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>---
> drivers/gpu/drm/i915/i915_pmu.c | 44 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 39 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
>index f03ddad44da6..9a8208dba7a9 100644
>--- a/drivers/gpu/drm/i915/i915_pmu.c
>+++ b/drivers/gpu/drm/i915/i915_pmu.c
>@@ -10,6 +10,25 @@
> #define RING_MASK 0xffffffff
> #define RING_MAX 32
>
>+#define ENGINE_SAMPLE_MASK (0xf)
>+#define ENGINE_SAMPLE_BITS (4)
>+
>+static const unsigned int engine_map[I915_NUM_ENGINES] = {
>+	[RCS] = I915_SAMPLE_RCS,
>+	[BCS] = I915_SAMPLE_BCS,
>+	[VCS] = I915_SAMPLE_VCS,
>+	[VCS2] = I915_SAMPLE_VCS2,
>+	[VECS] = I915_SAMPLE_VECS,
>+};
>+
>+static const unsigned int user_engine_map[I915_NUM_ENGINES] = {
>+	[I915_SAMPLE_RCS] = RCS,
>+	[I915_SAMPLE_BCS] = BCS,
>+	[I915_SAMPLE_VCS] = VCS,
>+	[I915_SAMPLE_VCS2] = VCS2,
>+	[I915_SAMPLE_VECS] = VECS,
>+};
>+
> static void engines_sample(struct drm_i915_private *dev_priv)
> {
> 	struct intel_engine_cs *engine;
>@@ -26,9 +45,16 @@ static void engines_sample(struct drm_i915_private *dev_priv)
> 		return;
>
> 	for_each_engine(engine, dev_priv, id) {
>+		unsigned int user_engine = engine_map[id];
> 		u32 val;
>
>-		if ((dev_priv->pmu.enable & (0x7 << (4*id))) == 0)
>+		if (WARN_ON_ONCE(id >= ARRAY_SIZE(engine_map)))
>+			continue;
>+		else
>+			user_engine = engine_map[id];
>+
>+		if (!(dev_priv->pmu.enable &
>+		    (ENGINE_SAMPLE_MASK << (ENGINE_SAMPLE_BITS * user_engine))))
> 			continue;
>
> 		if (i915_seqno_passed(intel_engine_get_seqno(engine),
>@@ -112,6 +138,11 @@ static int engine_event_init(struct perf_event *event)
> 	int engine = event->attr.config >> 2;
> 	int sample = event->attr.config & 3;
>
>+	if (WARN_ON_ONCE(engine >= ARRAY_SIZE(user_engine_map)))
>+		return -ENOENT;
>+	else
>+		engine = user_engine_map[engine];
>+
> 	switch (sample) {
> 	case I915_SAMPLE_QUEUED:
> 	case I915_SAMPLE_BUSY:
>@@ -125,9 +156,6 @@ static int engine_event_init(struct perf_event *event)
> 		return -ENOENT;
> 	}
>
>-	if (engine >= I915_NUM_ENGINES)
>-		return -ENOENT;
>-
> 	if (!i915->engine[engine])
> 		return -ENODEV;
>
>@@ -369,7 +397,13 @@ static void i915_pmu_event_read(struct perf_event *event)
> 	if (event->attr.config < 32) {
> 		int engine = event->attr.config >> 2;
> 		int sample = event->attr.config & 3;
>-		val = i915->engine[engine]->pmu_sample[sample];
>+
>+		if (WARN_ON_ONCE(engine >= ARRAY_SIZE(user_engine_map))) {
>+			/* Do nothing */
>+		} else {
>+			engine = user_engine_map[engine];
>+			val = i915->engine[engine]->pmu_sample[sample];
>+		}
> 	} else switch (event->attr.config) {
> 	case I915_PMU_ACTUAL_FREQUENCY:
> 		val = i915->pmu.sample[__I915_SAMPLE_FREQ_ACT];
>-- 
>2.9.4
>

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx