All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] drm/i915/pmu: Make more struct i915_pmu centric
@ 2019-08-01 14:17 Tvrtko Ursulin
  2019-08-01 14:17 ` [PATCH 2/5] drm/i915/pmu: Convert engine sampling to uncore mmio Tvrtko Ursulin
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2019-08-01 14:17 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Just tidy the code a bit by removing a sea of overly verbose i915->pmu.*.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_pmu.c | 194 +++++++++++++++++---------------
 1 file changed, 104 insertions(+), 90 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index eff86483bec0..12008966b00e 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -74,8 +74,9 @@ static unsigned int event_enabled_bit(struct perf_event *event)
 	return config_enabled_bit(event->attr.config);
 }
 
-static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active)
+static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
 {
+	struct drm_i915_private *i915 = container_of(pmu, typeof(*i915), pmu);
 	u64 enable;
 
 	/*
@@ -83,7 +84,7 @@ static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active)
 	 *
 	 * We start with a bitmask of all currently enabled events.
 	 */
-	enable = i915->pmu.enable;
+	enable = pmu->enable;
 
 	/*
 	 * Mask out all the ones which do not need the timer, or in
@@ -114,24 +115,26 @@ static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active)
 
 void i915_pmu_gt_parked(struct drm_i915_private *i915)
 {
-	if (!i915->pmu.base.event_init)
+	struct i915_pmu *pmu = &i915->pmu;
+
+	if (!pmu->base.event_init)
 		return;
 
-	spin_lock_irq(&i915->pmu.lock);
+	spin_lock_irq(&pmu->lock);
 	/*
 	 * Signal sampling timer to stop if only engine events are enabled and
 	 * GPU went idle.
 	 */
-	i915->pmu.timer_enabled = pmu_needs_timer(i915, false);
-	spin_unlock_irq(&i915->pmu.lock);
+	pmu->timer_enabled = pmu_needs_timer(pmu, false);
+	spin_unlock_irq(&pmu->lock);
 }
 
-static void __i915_pmu_maybe_start_timer(struct drm_i915_private *i915)
+static void __i915_pmu_maybe_start_timer(struct i915_pmu *pmu)
 {
-	if (!i915->pmu.timer_enabled && pmu_needs_timer(i915, true)) {
-		i915->pmu.timer_enabled = true;
-		i915->pmu.timer_last = ktime_get();
-		hrtimer_start_range_ns(&i915->pmu.timer,
+	if (!pmu->timer_enabled && pmu_needs_timer(pmu, true)) {
+		pmu->timer_enabled = true;
+		pmu->timer_last = ktime_get();
+		hrtimer_start_range_ns(&pmu->timer,
 				       ns_to_ktime(PERIOD), 0,
 				       HRTIMER_MODE_REL_PINNED);
 	}
@@ -139,15 +142,17 @@ static void __i915_pmu_maybe_start_timer(struct drm_i915_private *i915)
 
 void i915_pmu_gt_unparked(struct drm_i915_private *i915)
 {
-	if (!i915->pmu.base.event_init)
+	struct i915_pmu *pmu = &i915->pmu;
+
+	if (!pmu->base.event_init)
 		return;
 
-	spin_lock_irq(&i915->pmu.lock);
+	spin_lock_irq(&pmu->lock);
 	/*
 	 * Re-enable sampling timer when GPU goes active.
 	 */
-	__i915_pmu_maybe_start_timer(i915);
-	spin_unlock_irq(&i915->pmu.lock);
+	__i915_pmu_maybe_start_timer(pmu);
+	spin_unlock_irq(&pmu->lock);
 }
 
 static void
@@ -251,15 +256,16 @@ static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
 {
 	struct drm_i915_private *i915 =
 		container_of(hrtimer, struct drm_i915_private, pmu.timer);
+	struct i915_pmu *pmu = &i915->pmu;
 	unsigned int period_ns;
 	ktime_t now;
 
-	if (!READ_ONCE(i915->pmu.timer_enabled))
+	if (!READ_ONCE(pmu->timer_enabled))
 		return HRTIMER_NORESTART;
 
 	now = ktime_get();
-	period_ns = ktime_to_ns(ktime_sub(now, i915->pmu.timer_last));
-	i915->pmu.timer_last = now;
+	period_ns = ktime_to_ns(ktime_sub(now, pmu->timer_last));
+	pmu->timer_last = now;
 
 	/*
 	 * Strictly speaking the passed in period may not be 100% accurate for
@@ -443,6 +449,7 @@ static u64 get_rc6(struct drm_i915_private *i915)
 {
 #if IS_ENABLED(CONFIG_PM)
 	struct intel_runtime_pm *rpm = &i915->runtime_pm;
+	struct i915_pmu *pmu = &i915->pmu;
 	intel_wakeref_t wakeref;
 	unsigned long flags;
 	u64 val;
@@ -458,16 +465,16 @@ static u64 get_rc6(struct drm_i915_private *i915)
 		 * previously.
 		 */
 
-		spin_lock_irqsave(&i915->pmu.lock, flags);
+		spin_lock_irqsave(&pmu->lock, flags);
 
-		if (val >= i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur) {
-			i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur = 0;
-			i915->pmu.sample[__I915_SAMPLE_RC6].cur = val;
+		if (val >= pmu->sample[__I915_SAMPLE_RC6_ESTIMATED].cur) {
+			pmu->sample[__I915_SAMPLE_RC6_ESTIMATED].cur = 0;
+			pmu->sample[__I915_SAMPLE_RC6].cur = val;
 		} else {
-			val = i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur;
+			val = pmu->sample[__I915_SAMPLE_RC6_ESTIMATED].cur;
 		}
 
-		spin_unlock_irqrestore(&i915->pmu.lock, flags);
+		spin_unlock_irqrestore(&pmu->lock, flags);
 	} else {
 		struct device *kdev = rpm->kdev;
 
@@ -478,7 +485,7 @@ static u64 get_rc6(struct drm_i915_private *i915)
 		 * on top of the last known real value, as the approximated RC6
 		 * counter value.
 		 */
-		spin_lock_irqsave(&i915->pmu.lock, flags);
+		spin_lock_irqsave(&pmu->lock, flags);
 
 		/*
 		 * After the above branch intel_runtime_pm_get_if_in_use failed
@@ -494,20 +501,20 @@ static u64 get_rc6(struct drm_i915_private *i915)
 		if (pm_runtime_status_suspended(kdev)) {
 			val = pm_runtime_suspended_time(kdev);
 
-			if (!i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur)
-				i915->pmu.suspended_time_last = val;
+			if (!pmu->sample[__I915_SAMPLE_RC6_ESTIMATED].cur)
+				pmu->suspended_time_last = val;
 
-			val -= i915->pmu.suspended_time_last;
-			val += i915->pmu.sample[__I915_SAMPLE_RC6].cur;
+			val -= pmu->suspended_time_last;
+			val += pmu->sample[__I915_SAMPLE_RC6].cur;
 
-			i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur = val;
-		} else if (i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur) {
-			val = i915->pmu.sample[__I915_SAMPLE_RC6_ESTIMATED].cur;
+			pmu->sample[__I915_SAMPLE_RC6_ESTIMATED].cur = val;
+		} else if (pmu->sample[__I915_SAMPLE_RC6_ESTIMATED].cur) {
+			val = pmu->sample[__I915_SAMPLE_RC6_ESTIMATED].cur;
 		} else {
-			val = i915->pmu.sample[__I915_SAMPLE_RC6].cur;
+			val = pmu->sample[__I915_SAMPLE_RC6].cur;
 		}
 
-		spin_unlock_irqrestore(&i915->pmu.lock, flags);
+		spin_unlock_irqrestore(&pmu->lock, flags);
 	}
 
 	return val;
@@ -520,6 +527,7 @@ static u64 __i915_pmu_event_read(struct perf_event *event)
 {
 	struct drm_i915_private *i915 =
 		container_of(event->pmu, typeof(*i915), pmu.base);
+	struct i915_pmu *pmu = &i915->pmu;
 	u64 val = 0;
 
 	if (is_engine_event(event)) {
@@ -542,12 +550,12 @@ static u64 __i915_pmu_event_read(struct perf_event *event)
 		switch (event->attr.config) {
 		case I915_PMU_ACTUAL_FREQUENCY:
 			val =
-			   div_u64(i915->pmu.sample[__I915_SAMPLE_FREQ_ACT].cur,
+			   div_u64(pmu->sample[__I915_SAMPLE_FREQ_ACT].cur,
 				   USEC_PER_SEC /* to MHz */);
 			break;
 		case I915_PMU_REQUESTED_FREQUENCY:
 			val =
-			   div_u64(i915->pmu.sample[__I915_SAMPLE_FREQ_REQ].cur,
+			   div_u64(pmu->sample[__I915_SAMPLE_FREQ_REQ].cur,
 				   USEC_PER_SEC /* to MHz */);
 			break;
 		case I915_PMU_INTERRUPTS:
@@ -582,24 +590,25 @@ static void i915_pmu_enable(struct perf_event *event)
 	struct drm_i915_private *i915 =
 		container_of(event->pmu, typeof(*i915), pmu.base);
 	unsigned int bit = event_enabled_bit(event);
+	struct i915_pmu *pmu = &i915->pmu;
 	unsigned long flags;
 
-	spin_lock_irqsave(&i915->pmu.lock, flags);
+	spin_lock_irqsave(&pmu->lock, flags);
 
 	/*
 	 * Update the bitmask of enabled events and increment
 	 * the event reference counter.
 	 */
-	BUILD_BUG_ON(ARRAY_SIZE(i915->pmu.enable_count) != I915_PMU_MASK_BITS);
-	GEM_BUG_ON(bit >= ARRAY_SIZE(i915->pmu.enable_count));
-	GEM_BUG_ON(i915->pmu.enable_count[bit] == ~0);
-	i915->pmu.enable |= BIT_ULL(bit);
-	i915->pmu.enable_count[bit]++;
+	BUILD_BUG_ON(ARRAY_SIZE(pmu->enable_count) != I915_PMU_MASK_BITS);
+	GEM_BUG_ON(bit >= ARRAY_SIZE(pmu->enable_count));
+	GEM_BUG_ON(pmu->enable_count[bit] == ~0);
+	pmu->enable |= BIT_ULL(bit);
+	pmu->enable_count[bit]++;
 
 	/*
 	 * Start the sampling timer if needed and not already enabled.
 	 */
-	__i915_pmu_maybe_start_timer(i915);
+	__i915_pmu_maybe_start_timer(pmu);
 
 	/*
 	 * For per-engine events the bitmask and reference counting
@@ -625,7 +634,7 @@ static void i915_pmu_enable(struct perf_event *event)
 		engine->pmu.enable_count[sample]++;
 	}
 
-	spin_unlock_irqrestore(&i915->pmu.lock, flags);
+	spin_unlock_irqrestore(&pmu->lock, flags);
 
 	/*
 	 * Store the current counter value so we can report the correct delta
@@ -640,9 +649,10 @@ static void i915_pmu_disable(struct perf_event *event)
 	struct drm_i915_private *i915 =
 		container_of(event->pmu, typeof(*i915), pmu.base);
 	unsigned int bit = event_enabled_bit(event);
+	struct i915_pmu *pmu = &i915->pmu;
 	unsigned long flags;
 
-	spin_lock_irqsave(&i915->pmu.lock, flags);
+	spin_lock_irqsave(&pmu->lock, flags);
 
 	if (is_engine_event(event)) {
 		u8 sample = engine_event_sample(event);
@@ -664,18 +674,18 @@ static void i915_pmu_disable(struct perf_event *event)
 			engine->pmu.enable &= ~BIT(sample);
 	}
 
-	GEM_BUG_ON(bit >= ARRAY_SIZE(i915->pmu.enable_count));
-	GEM_BUG_ON(i915->pmu.enable_count[bit] == 0);
+	GEM_BUG_ON(bit >= ARRAY_SIZE(pmu->enable_count));
+	GEM_BUG_ON(pmu->enable_count[bit] == 0);
 	/*
 	 * Decrement the reference count and clear the enabled
 	 * bitmask when the last listener on an event goes away.
 	 */
-	if (--i915->pmu.enable_count[bit] == 0) {
-		i915->pmu.enable &= ~BIT_ULL(bit);
-		i915->pmu.timer_enabled &= pmu_needs_timer(i915, true);
+	if (--pmu->enable_count[bit] == 0) {
+		pmu->enable &= ~BIT_ULL(bit);
+		pmu->timer_enabled &= pmu_needs_timer(pmu, true);
 	}
 
-	spin_unlock_irqrestore(&i915->pmu.lock, flags);
+	spin_unlock_irqrestore(&pmu->lock, flags);
 }
 
 static void i915_pmu_event_start(struct perf_event *event, int flags)
@@ -824,8 +834,9 @@ add_pmu_attr(struct perf_pmu_events_attr *attr, const char *name,
 }
 
 static struct attribute **
-create_event_attributes(struct drm_i915_private *i915)
+create_event_attributes(struct i915_pmu *pmu)
 {
+	struct drm_i915_private *i915 = container_of(pmu, typeof(*i915), pmu);
 	static const struct {
 		u64 config;
 		const char *name;
@@ -939,8 +950,8 @@ create_event_attributes(struct drm_i915_private *i915)
 		}
 	}
 
-	i915->pmu.i915_attr = i915_attr;
-	i915->pmu.pmu_attr = pmu_attr;
+	pmu->i915_attr = i915_attr;
+	pmu->pmu_attr = pmu_attr;
 
 	return attr;
 
@@ -956,7 +967,7 @@ err:;
 	return NULL;
 }
 
-static void free_event_attributes(struct drm_i915_private *i915)
+static void free_event_attributes(struct i915_pmu *pmu)
 {
 	struct attribute **attr_iter = i915_pmu_events_attr_group.attrs;
 
@@ -964,12 +975,12 @@ static void free_event_attributes(struct drm_i915_private *i915)
 		kfree((*attr_iter)->name);
 
 	kfree(i915_pmu_events_attr_group.attrs);
-	kfree(i915->pmu.i915_attr);
-	kfree(i915->pmu.pmu_attr);
+	kfree(pmu->i915_attr);
+	kfree(pmu->pmu_attr);
 
 	i915_pmu_events_attr_group.attrs = NULL;
-	i915->pmu.i915_attr = NULL;
-	i915->pmu.pmu_attr = NULL;
+	pmu->i915_attr = NULL;
+	pmu->pmu_attr = NULL;
 }
 
 static int i915_pmu_cpu_online(unsigned int cpu, struct hlist_node *node)
@@ -1006,7 +1017,7 @@ static int i915_pmu_cpu_offline(unsigned int cpu, struct hlist_node *node)
 
 static enum cpuhp_state cpuhp_slot = CPUHP_INVALID;
 
-static int i915_pmu_register_cpuhp_state(struct drm_i915_private *i915)
+static int i915_pmu_register_cpuhp_state(struct i915_pmu *pmu)
 {
 	enum cpuhp_state slot;
 	int ret;
@@ -1019,7 +1030,7 @@ static int i915_pmu_register_cpuhp_state(struct drm_i915_private *i915)
 		return ret;
 
 	slot = ret;
-	ret = cpuhp_state_add_instance(slot, &i915->pmu.node);
+	ret = cpuhp_state_add_instance(slot, &pmu->node);
 	if (ret) {
 		cpuhp_remove_multi_state(slot);
 		return ret;
@@ -1029,15 +1040,16 @@ static int i915_pmu_register_cpuhp_state(struct drm_i915_private *i915)
 	return 0;
 }
 
-static void i915_pmu_unregister_cpuhp_state(struct drm_i915_private *i915)
+static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
 {
 	WARN_ON(cpuhp_slot == CPUHP_INVALID);
-	WARN_ON(cpuhp_state_remove_instance(cpuhp_slot, &i915->pmu.node));
+	WARN_ON(cpuhp_state_remove_instance(cpuhp_slot, &pmu->node));
 	cpuhp_remove_multi_state(cpuhp_slot);
 }
 
 void i915_pmu_register(struct drm_i915_private *i915)
 {
+	struct i915_pmu *pmu = &i915->pmu;
 	int ret;
 
 	if (INTEL_GEN(i915) <= 2) {
@@ -1045,56 +1057,58 @@ void i915_pmu_register(struct drm_i915_private *i915)
 		return;
 	}
 
-	i915_pmu_events_attr_group.attrs = create_event_attributes(i915);
+	i915_pmu_events_attr_group.attrs = create_event_attributes(pmu);
 	if (!i915_pmu_events_attr_group.attrs) {
 		ret = -ENOMEM;
 		goto err;
 	}
 
-	i915->pmu.base.attr_groups	= i915_pmu_attr_groups;
-	i915->pmu.base.task_ctx_nr	= perf_invalid_context;
-	i915->pmu.base.event_init	= i915_pmu_event_init;
-	i915->pmu.base.add		= i915_pmu_event_add;
-	i915->pmu.base.del		= i915_pmu_event_del;
-	i915->pmu.base.start		= i915_pmu_event_start;
-	i915->pmu.base.stop		= i915_pmu_event_stop;
-	i915->pmu.base.read		= i915_pmu_event_read;
-	i915->pmu.base.event_idx	= i915_pmu_event_event_idx;
-
-	spin_lock_init(&i915->pmu.lock);
-	hrtimer_init(&i915->pmu.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
-	i915->pmu.timer.function = i915_sample;
-
-	ret = perf_pmu_register(&i915->pmu.base, "i915", -1);
+	pmu->base.attr_groups	= i915_pmu_attr_groups;
+	pmu->base.task_ctx_nr	= perf_invalid_context;
+	pmu->base.event_init	= i915_pmu_event_init;
+	pmu->base.add		= i915_pmu_event_add;
+	pmu->base.del		= i915_pmu_event_del;
+	pmu->base.start		= i915_pmu_event_start;
+	pmu->base.stop		= i915_pmu_event_stop;
+	pmu->base.read		= i915_pmu_event_read;
+	pmu->base.event_idx	= i915_pmu_event_event_idx;
+
+	spin_lock_init(&pmu->lock);
+	hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	pmu->timer.function = i915_sample;
+
+	ret = perf_pmu_register(&pmu->base, "i915", -1);
 	if (ret)
 		goto err;
 
-	ret = i915_pmu_register_cpuhp_state(i915);
+	ret = i915_pmu_register_cpuhp_state(pmu);
 	if (ret)
 		goto err_unreg;
 
 	return;
 
 err_unreg:
-	perf_pmu_unregister(&i915->pmu.base);
+	perf_pmu_unregister(&pmu->base);
 err:
-	i915->pmu.base.event_init = NULL;
-	free_event_attributes(i915);
+	pmu->base.event_init = NULL;
+	free_event_attributes(pmu);
 	DRM_NOTE("Failed to register PMU! (err=%d)\n", ret);
 }
 
 void i915_pmu_unregister(struct drm_i915_private *i915)
 {
-	if (!i915->pmu.base.event_init)
+	struct i915_pmu *pmu = &i915->pmu;
+
+	if (!pmu->base.event_init)
 		return;
 
-	WARN_ON(i915->pmu.enable);
+	WARN_ON(pmu->enable);
 
-	hrtimer_cancel(&i915->pmu.timer);
+	hrtimer_cancel(&pmu->timer);
 
-	i915_pmu_unregister_cpuhp_state(i915);
+	i915_pmu_unregister_cpuhp_state(pmu);
 
-	perf_pmu_unregister(&i915->pmu.base);
-	i915->pmu.base.event_init = NULL;
-	free_event_attributes(i915);
+	perf_pmu_unregister(&pmu->base);
+	pmu->base.event_init = NULL;
+	free_event_attributes(pmu);
 }
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/5] drm/i915/pmu: Convert engine sampling to uncore mmio
  2019-08-01 14:17 [PATCH 1/5] drm/i915/pmu: Make more struct i915_pmu centric Tvrtko Ursulin
@ 2019-08-01 14:17 ` Tvrtko Ursulin
  2019-08-01 14:17 ` [PATCH 3/5] drm/i915/pmu: Convert sampling to gt Tvrtko Ursulin
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2019-08-01 14:17 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Drops one macro using implicit dev_priv.

v2:
 * Use ENGINE_READ_FW. (Chris)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_pmu.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 12008966b00e..09265b6b78b2 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -162,29 +162,30 @@ add_sample(struct i915_pmu_sample *sample, u32 val)
 }
 
 static void
-engines_sample(struct drm_i915_private *dev_priv, unsigned int period_ns)
+engines_sample(struct drm_i915_private *i915, unsigned int period_ns)
 {
+	struct intel_uncore *uncore = &i915->uncore;
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 	intel_wakeref_t wakeref;
 	unsigned long flags;
 
-	if ((dev_priv->pmu.enable & ENGINE_SAMPLE_MASK) == 0)
+	if ((i915->pmu.enable & ENGINE_SAMPLE_MASK) == 0)
 		return;
 
 	wakeref = 0;
-	if (READ_ONCE(dev_priv->gt.awake))
-		wakeref = intel_runtime_pm_get_if_in_use(&dev_priv->runtime_pm);
+	if (READ_ONCE(i915->gt.awake))
+		wakeref = intel_runtime_pm_get_if_in_use(&i915->runtime_pm);
 	if (!wakeref)
 		return;
 
-	spin_lock_irqsave(&dev_priv->uncore.lock, flags);
-	for_each_engine(engine, dev_priv, id) {
+	spin_lock_irqsave(&uncore->lock, flags);
+	for_each_engine(engine, i915, id) {
 		struct intel_engine_pmu *pmu = &engine->pmu;
 		bool busy;
 		u32 val;
 
-		val = I915_READ_FW(RING_CTL(engine->mmio_base));
+		val = ENGINE_READ_FW(engine, RING_CTL);
 		if (val == 0) /* powerwell off => engine idle */
 			continue;
 
@@ -202,15 +203,15 @@ engines_sample(struct drm_i915_private *dev_priv, unsigned int period_ns)
 		 */
 		busy = val & (RING_WAIT_SEMAPHORE | RING_WAIT);
 		if (!busy) {
-			val = I915_READ_FW(RING_MI_MODE(engine->mmio_base));
+			val = ENGINE_READ_FW(engine, RING_MI_MODE);
 			busy = !(val & MODE_IDLE);
 		}
 		if (busy)
 			add_sample(&pmu->sample[I915_SAMPLE_BUSY], period_ns);
 	}
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, flags);
+	spin_unlock_irqrestore(&uncore->lock, flags);
 
-	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
+	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
 }
 
 static void
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/5] drm/i915/pmu: Convert sampling to gt
  2019-08-01 14:17 [PATCH 1/5] drm/i915/pmu: Make more struct i915_pmu centric Tvrtko Ursulin
  2019-08-01 14:17 ` [PATCH 2/5] drm/i915/pmu: Convert engine sampling to uncore mmio Tvrtko Ursulin
@ 2019-08-01 14:17 ` Tvrtko Ursulin
  2019-08-01 14:17 ` [PATCH 4/5] drm/i915/pmu: Make get_rc6 take intel_gt Tvrtko Ursulin
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2019-08-01 14:17 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Engines and frequencies are a GT thing so adjust sampling routines to
match.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_pmu.c | 43 ++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 09265b6b78b2..5cf9a47a0c43 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -162,9 +162,10 @@ add_sample(struct i915_pmu_sample *sample, u32 val)
 }
 
 static void
-engines_sample(struct drm_i915_private *i915, unsigned int period_ns)
+engines_sample(struct intel_gt *gt, unsigned int period_ns)
 {
-	struct intel_uncore *uncore = &i915->uncore;
+	struct drm_i915_private *i915 = gt->i915;
+	struct intel_uncore *uncore = gt->uncore;
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 	intel_wakeref_t wakeref;
@@ -174,7 +175,7 @@ engines_sample(struct drm_i915_private *i915, unsigned int period_ns)
 		return;
 
 	wakeref = 0;
-	if (READ_ONCE(i915->gt.awake))
+	if (READ_ONCE(gt->awake))
 		wakeref = intel_runtime_pm_get_if_in_use(&i915->runtime_pm);
 	if (!wakeref)
 		return;
@@ -221,34 +222,35 @@ add_sample_mult(struct i915_pmu_sample *sample, u32 val, u32 mul)
 }
 
 static void
-frequency_sample(struct drm_i915_private *dev_priv, unsigned int period_ns)
+frequency_sample(struct intel_gt *gt, unsigned int period_ns)
 {
-	if (dev_priv->pmu.enable &
-	    config_enabled_mask(I915_PMU_ACTUAL_FREQUENCY)) {
+	struct drm_i915_private *i915 = gt->i915;
+	struct intel_uncore *uncore = gt->uncore;
+	struct i915_pmu *pmu = &i915->pmu;
+
+	if (pmu->enable & config_enabled_mask(I915_PMU_ACTUAL_FREQUENCY)) {
 		u32 val;
 
-		val = dev_priv->gt_pm.rps.cur_freq;
-		if (dev_priv->gt.awake) {
+		val = i915->gt_pm.rps.cur_freq;
+		if (gt->awake) {
 			intel_wakeref_t wakeref;
 
-			with_intel_runtime_pm_if_in_use(&dev_priv->runtime_pm,
+			with_intel_runtime_pm_if_in_use(&i915->runtime_pm,
 							wakeref) {
-				val = intel_uncore_read_notrace(&dev_priv->uncore,
+				val = intel_uncore_read_notrace(uncore,
 								GEN6_RPSTAT1);
-				val = intel_get_cagf(dev_priv, val);
+				val = intel_get_cagf(i915, val);
 			}
 		}
 
-		add_sample_mult(&dev_priv->pmu.sample[__I915_SAMPLE_FREQ_ACT],
-				intel_gpu_freq(dev_priv, val),
+		add_sample_mult(&pmu->sample[__I915_SAMPLE_FREQ_ACT],
+				intel_gpu_freq(i915, val),
 				period_ns / 1000);
 	}
 
-	if (dev_priv->pmu.enable &
-	    config_enabled_mask(I915_PMU_REQUESTED_FREQUENCY)) {
-		add_sample_mult(&dev_priv->pmu.sample[__I915_SAMPLE_FREQ_REQ],
-				intel_gpu_freq(dev_priv,
-					       dev_priv->gt_pm.rps.cur_freq),
+	if (pmu->enable & config_enabled_mask(I915_PMU_REQUESTED_FREQUENCY)) {
+		add_sample_mult(&pmu->sample[__I915_SAMPLE_FREQ_REQ],
+				intel_gpu_freq(i915, i915->gt_pm.rps.cur_freq),
 				period_ns / 1000);
 	}
 }
@@ -258,6 +260,7 @@ static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
 	struct drm_i915_private *i915 =
 		container_of(hrtimer, struct drm_i915_private, pmu.timer);
 	struct i915_pmu *pmu = &i915->pmu;
+	struct intel_gt *gt = &i915->gt;
 	unsigned int period_ns;
 	ktime_t now;
 
@@ -274,8 +277,8 @@ static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
 	 * grabbing the forcewake. However the potential error from timer call-
 	 * back delay greatly dominates this so we keep it simple.
 	 */
-	engines_sample(i915, period_ns);
-	frequency_sample(i915, period_ns);
+	engines_sample(gt, period_ns);
+	frequency_sample(gt, period_ns);
 
 	hrtimer_forward(hrtimer, now, ns_to_ktime(PERIOD));
 
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/5] drm/i915/pmu: Make get_rc6 take intel_gt
  2019-08-01 14:17 [PATCH 1/5] drm/i915/pmu: Make more struct i915_pmu centric Tvrtko Ursulin
  2019-08-01 14:17 ` [PATCH 2/5] drm/i915/pmu: Convert engine sampling to uncore mmio Tvrtko Ursulin
  2019-08-01 14:17 ` [PATCH 3/5] drm/i915/pmu: Convert sampling to gt Tvrtko Ursulin
@ 2019-08-01 14:17 ` Tvrtko Ursulin
  2019-08-01 14:45   ` Chris Wilson
  2019-08-01 14:17 ` [PATCH 5/5] drm/i915/pmu: Support multiple GPUs Tvrtko Ursulin
  2019-08-01 21:22 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/pmu: Make more struct i915_pmu centric (rev3) Patchwork
  4 siblings, 1 reply; 17+ messages in thread
From: Tvrtko Ursulin @ 2019-08-01 14:17 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

RC6 is a GT state so make the function parameter reflect that.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_pmu.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 5cf9a47a0c43..e0e0180bca7c 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -431,8 +431,9 @@ static int i915_pmu_event_init(struct perf_event *event)
 	return 0;
 }
 
-static u64 __get_rc6(struct drm_i915_private *i915)
+static u64 __get_rc6(struct intel_gt *gt)
 {
+	struct drm_i915_private *i915 = gt->i915;
 	u64 val;
 
 	val = intel_rc6_residency_ns(i915,
@@ -449,9 +450,10 @@ static u64 __get_rc6(struct drm_i915_private *i915)
 	return val;
 }
 
-static u64 get_rc6(struct drm_i915_private *i915)
+static u64 get_rc6(struct intel_gt *gt)
 {
 #if IS_ENABLED(CONFIG_PM)
+	struct drm_i915_private *i915 = gt->i915;
 	struct intel_runtime_pm *rpm = &i915->runtime_pm;
 	struct i915_pmu *pmu = &i915->pmu;
 	intel_wakeref_t wakeref;
@@ -460,7 +462,7 @@ static u64 get_rc6(struct drm_i915_private *i915)
 
 	wakeref = intel_runtime_pm_get_if_in_use(rpm);
 	if (wakeref) {
-		val = __get_rc6(i915);
+		val = __get_rc6(gt);
 		intel_runtime_pm_put(rpm, wakeref);
 
 		/*
@@ -523,7 +525,7 @@ static u64 get_rc6(struct drm_i915_private *i915)
 
 	return val;
 #else
-	return __get_rc6(i915);
+	return __get_rc6(gt);
 #endif
 }
 
@@ -566,7 +568,7 @@ static u64 __i915_pmu_event_read(struct perf_event *event)
 			val = count_interrupts(i915);
 			break;
 		case I915_PMU_RC6_RESIDENCY:
-			val = get_rc6(i915);
+			val = get_rc6(&i915->gt);
 			break;
 		}
 	}
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/5] drm/i915/pmu: Support multiple GPUs
  2019-08-01 14:17 [PATCH 1/5] drm/i915/pmu: Make more struct i915_pmu centric Tvrtko Ursulin
                   ` (2 preceding siblings ...)
  2019-08-01 14:17 ` [PATCH 4/5] drm/i915/pmu: Make get_rc6 take intel_gt Tvrtko Ursulin
@ 2019-08-01 14:17 ` Tvrtko Ursulin
  2019-08-01 14:54   ` Chris Wilson
                     ` (2 more replies)
  2019-08-01 21:22 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/pmu: Make more struct i915_pmu centric (rev3) Patchwork
  4 siblings, 3 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2019-08-01 14:17 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

With discrete graphics system can have both integrated and discrete GPU
handled by i915.

Currently we use a fixed name ("i915") when registering as the uncore PMU
provider which stops working in this case.

To fix this we add the PCI device name string to non-integrated devices
handled by us. Integrated devices keep the legacy name preserving
backward compatibility.

v2:
 * Detect IGP and keep legacy name. (Michal)
 * Use PCI device name as suffix. (Michal, Chris)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
Is our GPU always "0000:00:02.0"? CI will tell me.
---
 drivers/gpu/drm/i915/i915_pmu.c | 27 +++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_pmu.h |  4 ++++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index e0e0180bca7c..9a404d85c4e9 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -1053,6 +1053,15 @@ static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
 	cpuhp_remove_multi_state(cpuhp_slot);
 }
 
+static bool is_igp(struct pci_dev *pdev)
+{
+	/* IGP is 0000:00:02.0 */
+	return pdev->bus->parent == NULL &&
+	       pdev->bus->number == 0 &&
+	       PCI_SLOT(pdev->devfn) == 2 &&
+	       PCI_FUNC(pdev->devfn) == 0;
+}
+
 void i915_pmu_register(struct drm_i915_private *i915)
 {
 	struct i915_pmu *pmu = &i915->pmu;
@@ -1083,10 +1092,19 @@ void i915_pmu_register(struct drm_i915_private *i915)
 	hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	pmu->timer.function = i915_sample;
 
-	ret = perf_pmu_register(&pmu->base, "i915", -1);
-	if (ret)
+	if (!is_igp(i915->drm.pdev))
+		pmu->name = kasprintf(GFP_KERNEL,
+				      "i915-%s",
+				      dev_name(i915->drm.dev));
+	else
+		pmu->name = "i915";
+	if (!pmu->name)
 		goto err;
 
+	ret = perf_pmu_register(&pmu->base, pmu->name, -1);
+	if (ret)
+		goto err_name;
+
 	ret = i915_pmu_register_cpuhp_state(pmu);
 	if (ret)
 		goto err_unreg;
@@ -1095,6 +1113,9 @@ void i915_pmu_register(struct drm_i915_private *i915)
 
 err_unreg:
 	perf_pmu_unregister(&pmu->base);
+err_name:
+	if (!is_igp(i915->drm.pdev))
+		kfree(pmu->name);
 err:
 	pmu->base.event_init = NULL;
 	free_event_attributes(pmu);
@@ -1116,5 +1137,7 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
 
 	perf_pmu_unregister(&pmu->base);
 	pmu->base.event_init = NULL;
+	if (!is_igp(i915->drm.pdev))
+		kfree(pmu->name);
 	free_event_attributes(pmu);
 }
diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
index 4fc4f2478301..8d7a388dcdc7 100644
--- a/drivers/gpu/drm/i915/i915_pmu.h
+++ b/drivers/gpu/drm/i915/i915_pmu.h
@@ -46,6 +46,10 @@ struct i915_pmu {
 	 * @base: PMU base.
 	 */
 	struct pmu base;
+	/**
+	 * @name: Name as registered with perf core.
+	 */
+	char *name;
 	/**
 	 * @lock: Lock protecting enable mask and ref count handling.
 	 */
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/5] drm/i915/pmu: Make get_rc6 take intel_gt
  2019-08-01 14:17 ` [PATCH 4/5] drm/i915/pmu: Make get_rc6 take intel_gt Tvrtko Ursulin
@ 2019-08-01 14:45   ` Chris Wilson
  0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-08-01 14:45 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2019-08-01 15:17:31)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> RC6 is a GT state so make the function parameter reflect that.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_pmu.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 5cf9a47a0c43..e0e0180bca7c 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -431,8 +431,9 @@ static int i915_pmu_event_init(struct perf_event *event)
>         return 0;
>  }
>  
> -static u64 __get_rc6(struct drm_i915_private *i915)
> +static u64 __get_rc6(struct intel_gt *gt)
>  {
> +       struct drm_i915_private *i915 = gt->i915;

This ties nicely into a patch that moved the rc6 state beneath
intel_gt...
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/5] drm/i915/pmu: Support multiple GPUs
  2019-08-01 14:17 ` [PATCH 5/5] drm/i915/pmu: Support multiple GPUs Tvrtko Ursulin
@ 2019-08-01 14:54   ` Chris Wilson
  2019-08-01 15:10     ` Tvrtko Ursulin
  2019-08-01 15:08   ` [PATCH v3 " Tvrtko Ursulin
  2019-08-01 15:54   ` [PATCH v4 " Tvrtko Ursulin
  2 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2019-08-01 14:54 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2019-08-01 15:17:32)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> With discrete graphics system can have both integrated and discrete GPU
> handled by i915.
> 
> Currently we use a fixed name ("i915") when registering as the uncore PMU
> provider which stops working in this case.
> 
> To fix this we add the PCI device name string to non-integrated devices
> handled by us. Integrated devices keep the legacy name preserving
> backward compatibility.
> 
> v2:
>  * Detect IGP and keep legacy name. (Michal)
>  * Use PCI device name as suffix. (Michal, Chris)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
> Is our GPU always "0000:00:02.0"? CI will tell me.

It always has been. (With a few additional 2.1 for Windows95 multihead
where each head had to be a unique device!)

One hopes that by now it is firmly ingrained that it will always be
kept to 00:02.0.

> ---
>  drivers/gpu/drm/i915/i915_pmu.c | 27 +++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/i915_pmu.h |  4 ++++
>  2 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index e0e0180bca7c..9a404d85c4e9 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -1053,6 +1053,15 @@ static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
>         cpuhp_remove_multi_state(cpuhp_slot);
>  }
>  
> +static bool is_igp(struct pci_dev *pdev)
> +{
> +       /* IGP is 0000:00:02.0 */
> +       return pdev->bus->parent == NULL &&

pci_domain_nr(pdev->bus) == 0 ?

> +              pdev->bus->number == 0 &&
> +              PCI_SLOT(pdev->devfn) == 2 &&
> +              PCI_FUNC(pdev->devfn) == 0;

I am surprised there isn't already a convenience function. None that I
could find.

> +}
> +
>  void i915_pmu_register(struct drm_i915_private *i915)
>  {
>         struct i915_pmu *pmu = &i915->pmu;
> @@ -1083,10 +1092,19 @@ void i915_pmu_register(struct drm_i915_private *i915)
>         hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>         pmu->timer.function = i915_sample;
>  
> -       ret = perf_pmu_register(&pmu->base, "i915", -1);
> -       if (ret)
> +       if (!is_igp(i915->drm.pdev))
> +               pmu->name = kasprintf(GFP_KERNEL,
> +                                     "i915-%s",
> +                                     dev_name(i915->drm.dev));
> +       else
> +               pmu->name = "i915";

Makes sense, and quite a neat solution.

> +       if (!pmu->name)
>                 goto err;
>  
> +       ret = perf_pmu_register(&pmu->base, pmu->name, -1);
> +       if (ret)
> +               goto err_name;
> +
>         ret = i915_pmu_register_cpuhp_state(pmu);
>         if (ret)
>                 goto err_unreg;
> @@ -1095,6 +1113,9 @@ void i915_pmu_register(struct drm_i915_private *i915)
>  
>  err_unreg:
>         perf_pmu_unregister(&pmu->base);
> +err_name:
> +       if (!is_igp(i915->drm.pdev))
> +               kfree(pmu->name);
kfree_const(pmu->name);

>  err:
>         pmu->base.event_init = NULL;
>         free_event_attributes(pmu);
> @@ -1116,5 +1137,7 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
>  
>         perf_pmu_unregister(&pmu->base);
>         pmu->base.event_init = NULL;
> +       if (!is_igp(i915->drm.pdev))
> +               kfree(pmu->name);

kfree_const(pmu->name);

Works for me, I wonder what PeterZ will say...
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3 5/5] drm/i915/pmu: Support multiple GPUs
  2019-08-01 14:17 ` [PATCH 5/5] drm/i915/pmu: Support multiple GPUs Tvrtko Ursulin
  2019-08-01 14:54   ` Chris Wilson
@ 2019-08-01 15:08   ` Tvrtko Ursulin
  2019-08-01 15:43     ` Chris Wilson
  2019-08-01 15:54   ` [PATCH v4 " Tvrtko Ursulin
  2 siblings, 1 reply; 17+ messages in thread
From: Tvrtko Ursulin @ 2019-08-01 15:08 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

With discrete graphics system can have both integrated and discrete GPU
handled by i915.

Currently we use a fixed name ("i915") when registering as the uncore PMU
provider which stops working in this case.

To fix this we add the PCI device name string to non-integrated devices
handled by us. Integrated devices keep the legacy name preserving
backward compatibility.

v2:
 * Detect IGP and keep legacy name. (Michal)
 * Use PCI device name as suffix. (Michal, Chris)

v3:
 * Constify the name. (Chris)
 * Use pci_domain_nr. (Chris)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_pmu.c | 27 +++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_pmu.h |  4 ++++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index e0e0180bca7c..c732adac3136 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -1053,6 +1053,15 @@ static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
 	cpuhp_remove_multi_state(cpuhp_slot);
 }
 
+static bool is_igp(struct pci_dev *pdev)
+{
+	/* IGP is 0000:00:02.0 */
+	return pci_domain_nr(pdev->bus) == 0 &&
+	       pdev->bus->number == 0 &&
+	       PCI_SLOT(pdev->devfn) == 2 &&
+	       PCI_FUNC(pdev->devfn) == 0;
+}
+
 void i915_pmu_register(struct drm_i915_private *i915)
 {
 	struct i915_pmu *pmu = &i915->pmu;
@@ -1083,10 +1092,19 @@ void i915_pmu_register(struct drm_i915_private *i915)
 	hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	pmu->timer.function = i915_sample;
 
-	ret = perf_pmu_register(&pmu->base, "i915", -1);
-	if (ret)
+	if (!is_igp(i915->drm.pdev))
+		pmu->name = kasprintf(GFP_KERNEL,
+				      "i915-%s",
+				      dev_name(i915->drm.dev));
+	else
+		pmu->name = "i915";
+	if (!pmu->name)
 		goto err;
 
+	ret = perf_pmu_register(&pmu->base, pmu->name, -1);
+	if (ret)
+		goto err_name;
+
 	ret = i915_pmu_register_cpuhp_state(pmu);
 	if (ret)
 		goto err_unreg;
@@ -1095,6 +1113,9 @@ void i915_pmu_register(struct drm_i915_private *i915)
 
 err_unreg:
 	perf_pmu_unregister(&pmu->base);
+err_name:
+	if (!is_igp(i915->drm.pdev))
+		kfree_const(pmu->name);
 err:
 	pmu->base.event_init = NULL;
 	free_event_attributes(pmu);
@@ -1116,5 +1137,7 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
 
 	perf_pmu_unregister(&pmu->base);
 	pmu->base.event_init = NULL;
+	if (!is_igp(i915->drm.pdev))
+		kfree_const(pmu->name);
 	free_event_attributes(pmu);
 }
diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
index 4fc4f2478301..ff24f3bb0102 100644
--- a/drivers/gpu/drm/i915/i915_pmu.h
+++ b/drivers/gpu/drm/i915/i915_pmu.h
@@ -46,6 +46,10 @@ struct i915_pmu {
 	 * @base: PMU base.
 	 */
 	struct pmu base;
+	/**
+	 * @name: Name as registered with perf core.
+	 */
+	const char *name;
 	/**
 	 * @lock: Lock protecting enable mask and ref count handling.
 	 */
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/5] drm/i915/pmu: Support multiple GPUs
  2019-08-01 14:54   ` Chris Wilson
@ 2019-08-01 15:10     ` Tvrtko Ursulin
  2019-08-01 15:20       ` Chris Wilson
  0 siblings, 1 reply; 17+ messages in thread
From: Tvrtko Ursulin @ 2019-08-01 15:10 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx


On 01/08/2019 15:54, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-08-01 15:17:32)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> With discrete graphics system can have both integrated and discrete GPU
>> handled by i915.
>>
>> Currently we use a fixed name ("i915") when registering as the uncore PMU
>> provider which stops working in this case.
>>
>> To fix this we add the PCI device name string to non-integrated devices
>> handled by us. Integrated devices keep the legacy name preserving
>> backward compatibility.
>>
>> v2:
>>   * Detect IGP and keep legacy name. (Michal)
>>   * Use PCI device name as suffix. (Michal, Chris)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> ---
>> Is our GPU always "0000:00:02.0"? CI will tell me.
> 
> It always has been. (With a few additional 2.1 for Windows95 multihead
> where each head had to be a unique device!)
> 
> One hopes that by now it is firmly ingrained that it will always be
> kept to 00:02.0.

Re-assuring, thanks! However still some tests to do before I am happy 
this is upstream worthy, not least CI.

> 
>> ---
>>   drivers/gpu/drm/i915/i915_pmu.c | 27 +++++++++++++++++++++++++--
>>   drivers/gpu/drm/i915/i915_pmu.h |  4 ++++
>>   2 files changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
>> index e0e0180bca7c..9a404d85c4e9 100644
>> --- a/drivers/gpu/drm/i915/i915_pmu.c
>> +++ b/drivers/gpu/drm/i915/i915_pmu.c
>> @@ -1053,6 +1053,15 @@ static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
>>          cpuhp_remove_multi_state(cpuhp_slot);
>>   }
>>   
>> +static bool is_igp(struct pci_dev *pdev)
>> +{
>> +       /* IGP is 0000:00:02.0 */
>> +       return pdev->bus->parent == NULL &&
> 
> pci_domain_nr(pdev->bus) == 0 ?

Aha, thanks!

> 
>> +              pdev->bus->number == 0 &&
>> +              PCI_SLOT(pdev->devfn) == 2 &&
>> +              PCI_FUNC(pdev->devfn) == 0;
> 
> I am surprised there isn't already a convenience function. None that I
> could find.
> 
>> +}
>> +
>>   void i915_pmu_register(struct drm_i915_private *i915)
>>   {
>>          struct i915_pmu *pmu = &i915->pmu;
>> @@ -1083,10 +1092,19 @@ void i915_pmu_register(struct drm_i915_private *i915)
>>          hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>>          pmu->timer.function = i915_sample;
>>   
>> -       ret = perf_pmu_register(&pmu->base, "i915", -1);
>> -       if (ret)
>> +       if (!is_igp(i915->drm.pdev))
>> +               pmu->name = kasprintf(GFP_KERNEL,
>> +                                     "i915-%s",
>> +                                     dev_name(i915->drm.dev));
>> +       else
>> +               pmu->name = "i915";
> 
> Makes sense, and quite a neat solution.
> 
>> +       if (!pmu->name)
>>                  goto err;
>>   
>> +       ret = perf_pmu_register(&pmu->base, pmu->name, -1);
>> +       if (ret)
>> +               goto err_name;
>> +
>>          ret = i915_pmu_register_cpuhp_state(pmu);
>>          if (ret)
>>                  goto err_unreg;
>> @@ -1095,6 +1113,9 @@ void i915_pmu_register(struct drm_i915_private *i915)
>>   
>>   err_unreg:
>>          perf_pmu_unregister(&pmu->base);
>> +err_name:
>> +       if (!is_igp(i915->drm.pdev))
>> +               kfree(pmu->name);
> kfree_const(pmu->name);
> 
>>   err:
>>          pmu->base.event_init = NULL;
>>          free_event_attributes(pmu);
>> @@ -1116,5 +1137,7 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
>>   
>>          perf_pmu_unregister(&pmu->base);
>>          pmu->base.event_init = NULL;
>> +       if (!is_igp(i915->drm.pdev))
>> +               kfree(pmu->name);
> 
> kfree_const(pmu->name);
> 
> Works for me, I wonder what PeterZ will say...

In what sense?

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks,

Tvrtko

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/5] drm/i915/pmu: Support multiple GPUs
  2019-08-01 15:10     ` Tvrtko Ursulin
@ 2019-08-01 15:20       ` Chris Wilson
  2019-08-01 15:31         ` Tvrtko Ursulin
  0 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2019-08-01 15:20 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2019-08-01 16:10:14)
> 
> On 01/08/2019 15:54, Chris Wilson wrote:
> > Works for me, I wonder what PeterZ will say...
> 
> In what sense?

Just wondering if he has a plan for hotpluggable pmu devices. I can
certainly imagine his surprise in the future when he finds an adhoc
scheme in a random driver.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/5] drm/i915/pmu: Support multiple GPUs
  2019-08-01 15:20       ` Chris Wilson
@ 2019-08-01 15:31         ` Tvrtko Ursulin
  0 siblings, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2019-08-01 15:31 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx


On 01/08/2019 16:20, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-08-01 16:10:14)
>>
>> On 01/08/2019 15:54, Chris Wilson wrote:
>>> Works for me, I wonder what PeterZ will say...
>>
>> In what sense?
> 
> Just wondering if he has a plan for hotpluggable pmu devices. I can
> certainly imagine his surprise in the future when he finds an adhoc
> scheme in a random driver.

There is time to run this past him. I'll send something out.

Regards,

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 5/5] drm/i915/pmu: Support multiple GPUs
  2019-08-01 15:08   ` [PATCH v3 " Tvrtko Ursulin
@ 2019-08-01 15:43     ` Chris Wilson
  0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-08-01 15:43 UTC (permalink / raw)
  To: Intel-gfx, Tvrtko Ursulin

Quoting Tvrtko Ursulin (2019-08-01 16:08:04)
> @@ -1095,6 +1113,9 @@ void i915_pmu_register(struct drm_i915_private *i915)
>  
>  err_unreg:
>         perf_pmu_unregister(&pmu->base);
> +err_name:
> +       if (!is_igp(i915->drm.pdev))
> +               kfree_const(pmu->name);

With the kfree_const() magic you don't need to test again for !is_igp(),
it applies that for you by only freeing if it is not a static string.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4 5/5] drm/i915/pmu: Support multiple GPUs
  2019-08-01 14:17 ` [PATCH 5/5] drm/i915/pmu: Support multiple GPUs Tvrtko Ursulin
  2019-08-01 14:54   ` Chris Wilson
  2019-08-01 15:08   ` [PATCH v3 " Tvrtko Ursulin
@ 2019-08-01 15:54   ` Tvrtko Ursulin
  2019-09-06 15:47     ` [Intel-gfx] " Tvrtko Ursulin
  2 siblings, 1 reply; 17+ messages in thread
From: Tvrtko Ursulin @ 2019-08-01 15:54 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

With discrete graphics system can have both integrated and discrete GPU
handled by i915.

Currently we use a fixed name ("i915") when registering as the uncore PMU
provider which stops working in this case.

To fix this we add the PCI device name string to non-integrated devices
handled by us. Integrated devices keep the legacy name preserving
backward compatibility.

v2:
 * Detect IGP and keep legacy name. (Michal)
 * Use PCI device name as suffix. (Michal, Chris)

v3:
 * Constify the name. (Chris)
 * Use pci_domain_nr. (Chris)

v4:
 * Fix kfree_const usage. (Chris)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_pmu.c | 25 +++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_pmu.h |  4 ++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index e0e0180bca7c..e0fea227077e 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -1053,6 +1053,15 @@ static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
 	cpuhp_remove_multi_state(cpuhp_slot);
 }
 
+static bool is_igp(struct pci_dev *pdev)
+{
+	/* IGP is 0000:00:02.0 */
+	return pci_domain_nr(pdev->bus) == 0 &&
+	       pdev->bus->number == 0 &&
+	       PCI_SLOT(pdev->devfn) == 2 &&
+	       PCI_FUNC(pdev->devfn) == 0;
+}
+
 void i915_pmu_register(struct drm_i915_private *i915)
 {
 	struct i915_pmu *pmu = &i915->pmu;
@@ -1083,10 +1092,19 @@ void i915_pmu_register(struct drm_i915_private *i915)
 	hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	pmu->timer.function = i915_sample;
 
-	ret = perf_pmu_register(&pmu->base, "i915", -1);
-	if (ret)
+	if (!is_igp(i915->drm.pdev))
+		pmu->name = kasprintf(GFP_KERNEL,
+				      "i915-%s",
+				      dev_name(i915->drm.dev));
+	else
+		pmu->name = "i915";
+	if (!pmu->name)
 		goto err;
 
+	ret = perf_pmu_register(&pmu->base, pmu->name, -1);
+	if (ret)
+		goto err_name;
+
 	ret = i915_pmu_register_cpuhp_state(pmu);
 	if (ret)
 		goto err_unreg;
@@ -1095,6 +1113,8 @@ void i915_pmu_register(struct drm_i915_private *i915)
 
 err_unreg:
 	perf_pmu_unregister(&pmu->base);
+err_name:
+	kfree_const(pmu->name);
 err:
 	pmu->base.event_init = NULL;
 	free_event_attributes(pmu);
@@ -1116,5 +1136,6 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
 
 	perf_pmu_unregister(&pmu->base);
 	pmu->base.event_init = NULL;
+	kfree_const(pmu->name);
 	free_event_attributes(pmu);
 }
diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
index 4fc4f2478301..ff24f3bb0102 100644
--- a/drivers/gpu/drm/i915/i915_pmu.h
+++ b/drivers/gpu/drm/i915/i915_pmu.h
@@ -46,6 +46,10 @@ struct i915_pmu {
 	 * @base: PMU base.
 	 */
 	struct pmu base;
+	/**
+	 * @name: Name as registered with perf core.
+	 */
+	const char *name;
 	/**
 	 * @lock: Lock protecting enable mask and ref count handling.
 	 */
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/pmu: Make more struct i915_pmu centric (rev3)
  2019-08-01 14:17 [PATCH 1/5] drm/i915/pmu: Make more struct i915_pmu centric Tvrtko Ursulin
                   ` (3 preceding siblings ...)
  2019-08-01 14:17 ` [PATCH 5/5] drm/i915/pmu: Support multiple GPUs Tvrtko Ursulin
@ 2019-08-01 21:22 ` Patchwork
  2019-08-01 21:37   ` Chris Wilson
  4 siblings, 1 reply; 17+ messages in thread
From: Patchwork @ 2019-08-01 21:22 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915/pmu: Make more struct i915_pmu centric (rev3)
URL   : https://patchwork.freedesktop.org/series/64550/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6605 -> Patchwork_13839
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_13839 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13839, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_13839:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@reload:
    - fi-kbl-r:           [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-kbl-r/igt@i915_module_load@reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-kbl-r/igt@i915_module_load@reload.html
    - fi-whl-u:           [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-whl-u/igt@i915_module_load@reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-whl-u/igt@i915_module_load@reload.html
    - fi-skl-iommu:       [PASS][5] -> [INCOMPLETE][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-skl-iommu/igt@i915_module_load@reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-skl-iommu/igt@i915_module_load@reload.html
    - fi-hsw-4770r:       [PASS][7] -> [INCOMPLETE][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-hsw-4770r/igt@i915_module_load@reload.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-hsw-4770r/igt@i915_module_load@reload.html
    - fi-skl-6700k2:      [PASS][9] -> [INCOMPLETE][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-skl-6700k2/igt@i915_module_load@reload.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-skl-6700k2/igt@i915_module_load@reload.html
    - fi-bsw-kefka:       [PASS][11] -> [INCOMPLETE][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-bsw-kefka/igt@i915_module_load@reload.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-bsw-kefka/igt@i915_module_load@reload.html
    - fi-bdw-5557u:       [PASS][13] -> [INCOMPLETE][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-bdw-5557u/igt@i915_module_load@reload.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-bdw-5557u/igt@i915_module_load@reload.html
    - fi-skl-guc:         [PASS][15] -> [INCOMPLETE][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-skl-guc/igt@i915_module_load@reload.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-skl-guc/igt@i915_module_load@reload.html
    - fi-kbl-guc:         [PASS][17] -> [INCOMPLETE][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-kbl-guc/igt@i915_module_load@reload.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-kbl-guc/igt@i915_module_load@reload.html
    - fi-cfl-8109u:       [PASS][19] -> [INCOMPLETE][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-cfl-8109u/igt@i915_module_load@reload.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-cfl-8109u/igt@i915_module_load@reload.html
    - fi-kbl-7500u:       [PASS][21] -> [INCOMPLETE][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-kbl-7500u/igt@i915_module_load@reload.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-kbl-7500u/igt@i915_module_load@reload.html
    - fi-cfl-8700k:       [PASS][23] -> [INCOMPLETE][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-cfl-8700k/igt@i915_module_load@reload.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-cfl-8700k/igt@i915_module_load@reload.html
    - fi-snb-2520m:       [PASS][25] -> [INCOMPLETE][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-snb-2520m/igt@i915_module_load@reload.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-snb-2520m/igt@i915_module_load@reload.html
    - fi-cfl-guc:         [PASS][27] -> [INCOMPLETE][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-cfl-guc/igt@i915_module_load@reload.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-cfl-guc/igt@i915_module_load@reload.html
    - fi-skl-6770hq:      [PASS][29] -> [INCOMPLETE][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-skl-6770hq/igt@i915_module_load@reload.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-skl-6770hq/igt@i915_module_load@reload.html
    - fi-ilk-650:         [PASS][31] -> [INCOMPLETE][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-ilk-650/igt@i915_module_load@reload.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-ilk-650/igt@i915_module_load@reload.html
    - fi-ivb-3770:        [PASS][33] -> [DMESG-WARN][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-ivb-3770/igt@i915_module_load@reload.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-ivb-3770/igt@i915_module_load@reload.html
    - fi-skl-lmem:        [PASS][35] -> [INCOMPLETE][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-skl-lmem/igt@i915_module_load@reload.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-skl-lmem/igt@i915_module_load@reload.html
    - fi-hsw-peppy:       [PASS][37] -> [INCOMPLETE][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-hsw-peppy/igt@i915_module_load@reload.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-hsw-peppy/igt@i915_module_load@reload.html
    - fi-skl-6260u:       [PASS][39] -> [INCOMPLETE][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-skl-6260u/igt@i915_module_load@reload.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-skl-6260u/igt@i915_module_load@reload.html
    - fi-kbl-7567u:       [PASS][41] -> [INCOMPLETE][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-kbl-7567u/igt@i915_module_load@reload.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-kbl-7567u/igt@i915_module_load@reload.html
    - fi-kbl-x1275:       [PASS][43] -> [INCOMPLETE][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-kbl-x1275/igt@i915_module_load@reload.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-kbl-x1275/igt@i915_module_load@reload.html
    - fi-bwr-2160:        [PASS][45] -> [INCOMPLETE][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-bwr-2160/igt@i915_module_load@reload.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-bwr-2160/igt@i915_module_load@reload.html

  * igt@runner@aborted:
    - fi-ilk-650:         NOTRUN -> [FAIL][47]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-ilk-650/igt@runner@aborted.html
    - fi-pnv-d510:        NOTRUN -> [FAIL][48]
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-pnv-d510/igt@runner@aborted.html
    - fi-cfl-8109u:       NOTRUN -> [FAIL][49]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-cfl-8109u/igt@runner@aborted.html
    - fi-gdg-551:         NOTRUN -> [FAIL][50]
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-gdg-551/igt@runner@aborted.html
    - fi-kbl-7500u:       NOTRUN -> [FAIL][51]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-kbl-7500u/igt@runner@aborted.html
    - fi-bxt-j4205:       NOTRUN -> [FAIL][52]
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-bxt-j4205/igt@runner@aborted.html
    - fi-whl-u:           NOTRUN -> [FAIL][53]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-whl-u/igt@runner@aborted.html
    - fi-cml-u2:          NOTRUN -> [FAIL][54]
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-cml-u2/igt@runner@aborted.html
    - fi-bxt-dsi:         NOTRUN -> [FAIL][55]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-bxt-dsi/igt@runner@aborted.html
    - fi-kbl-7567u:       NOTRUN -> [FAIL][56]
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-kbl-7567u/igt@runner@aborted.html
    - fi-kbl-x1275:       NOTRUN -> [FAIL][57]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-kbl-x1275/igt@runner@aborted.html
    - fi-bsw-kefka:       NOTRUN -> [FAIL][58]
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-bsw-kefka/igt@runner@aborted.html
    - fi-cfl-8700k:       NOTRUN -> [FAIL][59]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-cfl-8700k/igt@runner@aborted.html
    - fi-kbl-8809g:       NOTRUN -> [FAIL][60]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-kbl-8809g/igt@runner@aborted.html
    - fi-kbl-r:           NOTRUN -> [FAIL][61]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-kbl-r/igt@runner@aborted.html
    - fi-bdw-5557u:       NOTRUN -> [FAIL][62]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-bdw-5557u/igt@runner@aborted.html
    - fi-elk-e7500:       NOTRUN -> [FAIL][63]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-elk-e7500/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in Patchwork_13839 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [PASS][64] -> [INCOMPLETE][65] ([fdo#107718])
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@i915_module_load@reload:
    - fi-kbl-8809g:       [PASS][66] -> [INCOMPLETE][67] ([fdo#103665])
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-kbl-8809g/igt@i915_module_load@reload.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-kbl-8809g/igt@i915_module_load@reload.html
    - fi-byt-j1900:       [PASS][68] -> [INCOMPLETE][69] ([fdo#102657])
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-byt-j1900/igt@i915_module_load@reload.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-byt-j1900/igt@i915_module_load@reload.html
    - fi-pnv-d510:        [PASS][70] -> [INCOMPLETE][71] ([fdo#110740])
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-pnv-d510/igt@i915_module_load@reload.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-pnv-d510/igt@i915_module_load@reload.html
    - fi-skl-6600u:       [PASS][72] -> [INCOMPLETE][73] ([fdo#104108])
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-skl-6600u/igt@i915_module_load@reload.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-skl-6600u/igt@i915_module_load@reload.html
    - fi-elk-e7500:       [PASS][74] -> [INCOMPLETE][75] ([fdo#103989])
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-elk-e7500/igt@i915_module_load@reload.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-elk-e7500/igt@i915_module_load@reload.html
    - fi-glk-dsi:         [PASS][76] -> [INCOMPLETE][77] ([fdo#103359] / [k.org#198133])
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-glk-dsi/igt@i915_module_load@reload.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-glk-dsi/igt@i915_module_load@reload.html
    - fi-cml-u2:          [PASS][78] -> [INCOMPLETE][79] ([fdo#110566])
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-cml-u2/igt@i915_module_load@reload.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-cml-u2/igt@i915_module_load@reload.html
    - fi-bxt-dsi:         [PASS][80] -> [INCOMPLETE][81] ([fdo#103927])
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-bxt-dsi/igt@i915_module_load@reload.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-bxt-dsi/igt@i915_module_load@reload.html
    - fi-bxt-j4205:       [PASS][82] -> [INCOMPLETE][83] ([fdo#103927])
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-bxt-j4205/igt@i915_module_load@reload.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-bxt-j4205/igt@i915_module_load@reload.html
    - fi-byt-n2820:       [PASS][84] -> [INCOMPLETE][85] ([fdo#102657])
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-byt-n2820/igt@i915_module_load@reload.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-byt-n2820/igt@i915_module_load@reload.html
    - fi-snb-2600:        [PASS][86] -> [INCOMPLETE][87] ([fdo#105411])
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-snb-2600/igt@i915_module_load@reload.html
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-snb-2600/igt@i915_module_load@reload.html
    - fi-gdg-551:         [PASS][88] -> [INCOMPLETE][89] ([fdo#108316])
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-gdg-551/igt@i915_module_load@reload.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-gdg-551/igt@i915_module_load@reload.html

  * igt@prime_vgem@basic-busy-default:
    - fi-bxt-dsi:         [PASS][90] -> [FAIL][91] ([fdo#111277])
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-bxt-dsi/igt@prime_vgem@basic-busy-default.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-bxt-dsi/igt@prime_vgem@basic-busy-default.html

  * igt@prime_vgem@basic-fence-mmap:
    - fi-byt-j1900:       [PASS][92] -> [INCOMPLETE][93] ([fdo#102657] / [fdo#111276])
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-byt-j1900/igt@prime_vgem@basic-fence-mmap.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-byt-j1900/igt@prime_vgem@basic-fence-mmap.html
    - fi-gdg-551:         [PASS][94] -> [INCOMPLETE][95] ([fdo#108316] / [fdo#111276])
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-gdg-551/igt@prime_vgem@basic-fence-mmap.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-gdg-551/igt@prime_vgem@basic-fence-mmap.html

  * igt@prime_vgem@basic-fence-read:
    - fi-bsw-kefka:       [PASS][96] -> [INCOMPLETE][97] ([fdo#111278])
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-bsw-kefka/igt@prime_vgem@basic-fence-read.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-bsw-kefka/igt@prime_vgem@basic-fence-read.html

  * igt@prime_vgem@basic-wait-default:
    - fi-bxt-j4205:       [PASS][98] -> [FAIL][99] ([fdo#111277])
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-bxt-j4205/igt@prime_vgem@basic-wait-default.html
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-bxt-j4205/igt@prime_vgem@basic-wait-default.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_execlists:
    - fi-skl-gvtdvm:      [DMESG-FAIL][100] ([fdo#111108]) -> [PASS][101]
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html

  * igt@kms_busy@basic-flip-a:
    - fi-kbl-7567u:       [SKIP][102] ([fdo#109271] / [fdo#109278]) -> [PASS][103] +2 similar issues
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-kbl-7567u/igt@kms_busy@basic-flip-a.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-kbl-7567u/igt@kms_busy@basic-flip-a.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          [FAIL][104] ([fdo#103167]) -> [PASS][105]
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

  * igt@prime_vgem@basic-fence-read:
    - fi-pnv-d510:        [INCOMPLETE][106] ([fdo#110740]) -> [PASS][107]
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-pnv-d510/igt@prime_vgem@basic-fence-read.html
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-pnv-d510/igt@prime_vgem@basic-fence-read.html

  * igt@prime_vgem@basic-wait-default:
    - fi-bxt-dsi:         [FAIL][108] ([fdo#111277]) -> [PASS][109]
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-bxt-dsi/igt@prime_vgem@basic-wait-default.html
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-bxt-dsi/igt@prime_vgem@basic-wait-default.html

  
#### Warnings ####

  * igt@i915_module_load@reload:
    - fi-icl-u2:          [DMESG-WARN][110] ([fdo#110595]) -> [INCOMPLETE][111] ([fdo#107713])
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-icl-u2/igt@i915_module_load@reload.html
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-icl-u2/igt@i915_module_load@reload.html
    - fi-icl-u3:          [DMESG-WARN][112] ([fdo#107724]) -> [INCOMPLETE][113] ([fdo#107713])
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-icl-u3/igt@i915_module_load@reload.html
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-icl-u3/igt@i915_module_load@reload.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102657]: https://bugs.freedesktop.org/show_bug.cgi?id=102657
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#103989]: https://bugs.freedesktop.org/show_bug.cgi?id=103989
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108316]: https://bugs.freedesktop.org/show_bug.cgi?id=108316
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566
  [fdo#110595]: https://bugs.freedesktop.org/show_bug.cgi?id=110595
  [fdo#110740]: https://bugs.freedesktop.org/show_bug.cgi?id=110740
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111108]: https://bugs.freedesktop.org/show_bug.cgi?id=111108
  [fdo#111276]: https://bugs.freedesktop.org/show_bug.cgi?id=111276
  [fdo#111277]: https://bugs.freedesktop.org/show_bug.cgi?id=111277
  [fdo#111278]: https://bugs.freedesktop.org/show_bug.cgi?id=111278
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (47 -> 44)
------------------------------

  Additional (2): fi-icl-dsi fi-apl-guc 
  Missing    (5): fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6605 -> Patchwork_13839

  CI-20190529: 20190529
  CI_DRM_6605: 09970f7b8f1336416254cfac87f196578e3c1d13 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5120: b3138fbea79d5d7935e53530b90efe3e816236f4 @

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/pmu: Make more struct i915_pmu centric (rev3)
  2019-08-01 21:22 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/pmu: Make more struct i915_pmu centric (rev3) Patchwork
@ 2019-08-01 21:37   ` Chris Wilson
  0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-08-01 21:37 UTC (permalink / raw)
  To: Patchwork, Tvrtko Ursulin; +Cc: intel-gfx

Quoting Patchwork (2019-08-01 22:22:58)
> == Series Details ==
> 
> Series: series starting with [1/5] drm/i915/pmu: Make more struct i915_pmu centric (rev3)
> URL   : https://patchwork.freedesktop.org/series/64550/
> State : failure
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_6605 -> Patchwork_13839
> ====================================================
> 
> Summary
> -------
> 
>   **FAILURE**
> 
>   Serious unknown changes coming with Patchwork_13839 absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_13839, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/
> 
> Possible new issues
> -------------------
> 
>   Here are the unknown changes that may have been introduced in Patchwork_13839:
> 
> ### IGT changes ###
> 
> #### Possible regressions ####
> 
>   * igt@i915_module_load@reload:
>     - fi-kbl-r:           [PASS][1] -> [INCOMPLETE][2]
>    [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6605/fi-kbl-r/igt@i915_module_load@reload.html
>    [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13839/fi-kbl-r/igt@i915_module_load@reload.html

Hmm, we need more trickery for kfree_const.

Oh, it only applies to .rodata of the vmlinux and not modules :(

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Intel-gfx] [PATCH v4 5/5] drm/i915/pmu: Support multiple GPUs
  2019-08-01 15:54   ` [PATCH v4 " Tvrtko Ursulin
@ 2019-09-06 15:47     ` Tvrtko Ursulin
  2019-10-10 12:37       ` Tvrtko Ursulin
  0 siblings, 1 reply; 17+ messages in thread
From: Tvrtko Ursulin @ 2019-09-06 15:47 UTC (permalink / raw)
  To: Peter Zijlstra, Thomas Gleixner
  Cc: Intel-gfx, linux-kernel, Chris Wilson, Michal Wajdeczko


Peter, Thomas,

If you could spare a moment for some brainstorming on the topic of 
uncore PMU and multiple providers it would be appreciated.

So from i915 we export some metrics as uncore PMU, which shows up under 
/sys/devices/i915. Shortsightedness or what, we did not realize that one 
day we could have more than one i915 device in a system which now 
creates a problem, or at least raises a question on naming.

The patch below works around this by appending the PCI device name to 
additional instances of i915 when it registers with perf_pmu_register.

Question is if there is a better solution, or if not, whether you are 
aware of any plans to extend the perf core to better support this? Are 
there any other uncore PMU providers in an identical situation?

Regards,

Tvrtko

On 01/08/2019 16:54, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> With discrete graphics system can have both integrated and discrete GPU
> handled by i915.
> 
> Currently we use a fixed name ("i915") when registering as the uncore PMU
> provider which stops working in this case.
> 
> To fix this we add the PCI device name string to non-integrated devices
> handled by us. Integrated devices keep the legacy name preserving
> backward compatibility.
> 
> v2:
>   * Detect IGP and keep legacy name. (Michal)
>   * Use PCI device name as suffix. (Michal, Chris)
> 
> v3:
>   * Constify the name. (Chris)
>   * Use pci_domain_nr. (Chris)
> 
> v4:
>   * Fix kfree_const usage. (Chris)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_pmu.c | 25 +++++++++++++++++++++++--
>   drivers/gpu/drm/i915/i915_pmu.h |  4 ++++
>   2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index e0e0180bca7c..e0fea227077e 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -1053,6 +1053,15 @@ static void i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
>   	cpuhp_remove_multi_state(cpuhp_slot);
>   }
>   
> +static bool is_igp(struct pci_dev *pdev)
> +{
> +	/* IGP is 0000:00:02.0 */
> +	return pci_domain_nr(pdev->bus) == 0 &&
> +	       pdev->bus->number == 0 &&
> +	       PCI_SLOT(pdev->devfn) == 2 &&
> +	       PCI_FUNC(pdev->devfn) == 0;
> +}
> +
>   void i915_pmu_register(struct drm_i915_private *i915)
>   {
>   	struct i915_pmu *pmu = &i915->pmu;
> @@ -1083,10 +1092,19 @@ void i915_pmu_register(struct drm_i915_private *i915)
>   	hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>   	pmu->timer.function = i915_sample;
>   
> -	ret = perf_pmu_register(&pmu->base, "i915", -1);
> -	if (ret)
> +	if (!is_igp(i915->drm.pdev))
> +		pmu->name = kasprintf(GFP_KERNEL,
> +				      "i915-%s",
> +				      dev_name(i915->drm.dev));
> +	else
> +		pmu->name = "i915";
> +	if (!pmu->name)
>   		goto err;
>   
> +	ret = perf_pmu_register(&pmu->base, pmu->name, -1);
> +	if (ret)
> +		goto err_name;
> +
>   	ret = i915_pmu_register_cpuhp_state(pmu);
>   	if (ret)
>   		goto err_unreg;
> @@ -1095,6 +1113,8 @@ void i915_pmu_register(struct drm_i915_private *i915)
>   
>   err_unreg:
>   	perf_pmu_unregister(&pmu->base);
> +err_name:
> +	kfree_const(pmu->name);
>   err:
>   	pmu->base.event_init = NULL;
>   	free_event_attributes(pmu);
> @@ -1116,5 +1136,6 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
>   
>   	perf_pmu_unregister(&pmu->base);
>   	pmu->base.event_init = NULL;
> +	kfree_const(pmu->name);
>   	free_event_attributes(pmu);
>   }
> diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
> index 4fc4f2478301..ff24f3bb0102 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.h
> +++ b/drivers/gpu/drm/i915/i915_pmu.h
> @@ -46,6 +46,10 @@ struct i915_pmu {
>   	 * @base: PMU base.
>   	 */
>   	struct pmu base;
> +	/**
> +	 * @name: Name as registered with perf core.
> +	 */
> +	const char *name;
>   	/**
>   	 * @lock: Lock protecting enable mask and ref count handling.
>   	 */
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Intel-gfx] [PATCH v4 5/5] drm/i915/pmu: Support multiple GPUs
  2019-09-06 15:47     ` [Intel-gfx] " Tvrtko Ursulin
@ 2019-10-10 12:37       ` Tvrtko Ursulin
  0 siblings, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2019-10-10 12:37 UTC (permalink / raw)
  To: Peter Zijlstra, Thomas Gleixner; +Cc: Intel-gfx, Michal Wajdeczko, linux-kernel


Hi guys,

Any feedback on the below?

On 06/09/2019 16:47, Tvrtko Ursulin wrote:
> 
> Peter, Thomas,
> 
> If you could spare a moment for some brainstorming on the topic of 
> uncore PMU and multiple providers it would be appreciated.
> 
> So from i915 we export some metrics as uncore PMU, which shows up under 
> /sys/devices/i915. Shortsightedness or what, we did not realize that one 
> day we could have more than one i915 device in a system which now 
> creates a problem, or at least raises a question on naming.
> 
> The patch below works around this by appending the PCI device name to 
> additional instances of i915 when it registers with perf_pmu_register.
> 
> Question is if there is a better solution, or if not, whether you are 
> aware of any plans to extend the perf core to better support this? Are 
> there any other uncore PMU providers in an identical situation?
> 
> Regards,
> 
> Tvrtko
> 
> On 01/08/2019 16:54, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> With discrete graphics system can have both integrated and discrete GPU
>> handled by i915.
>>
>> Currently we use a fixed name ("i915") when registering as the uncore PMU
>> provider which stops working in this case.
>>
>> To fix this we add the PCI device name string to non-integrated devices
>> handled by us. Integrated devices keep the legacy name preserving
>> backward compatibility.
>>
>> v2:
>>   * Detect IGP and keep legacy name. (Michal)
>>   * Use PCI device name as suffix. (Michal, Chris)
>>
>> v3:
>>   * Constify the name. (Chris)
>>   * Use pci_domain_nr. (Chris)
>>
>> v4:
>>   * Fix kfree_const usage. (Chris)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/i915_pmu.c | 25 +++++++++++++++++++++++--
>>   drivers/gpu/drm/i915/i915_pmu.h |  4 ++++
>>   2 files changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_pmu.c 
>> b/drivers/gpu/drm/i915/i915_pmu.c
>> index e0e0180bca7c..e0fea227077e 100644
>> --- a/drivers/gpu/drm/i915/i915_pmu.c
>> +++ b/drivers/gpu/drm/i915/i915_pmu.c
>> @@ -1053,6 +1053,15 @@ static void 
>> i915_pmu_unregister_cpuhp_state(struct i915_pmu *pmu)
>>       cpuhp_remove_multi_state(cpuhp_slot);
>>   }
>> +static bool is_igp(struct pci_dev *pdev)
>> +{
>> +    /* IGP is 0000:00:02.0 */
>> +    return pci_domain_nr(pdev->bus) == 0 &&
>> +           pdev->bus->number == 0 &&
>> +           PCI_SLOT(pdev->devfn) == 2 &&
>> +           PCI_FUNC(pdev->devfn) == 0;
>> +}
>> +
>>   void i915_pmu_register(struct drm_i915_private *i915)
>>   {
>>       struct i915_pmu *pmu = &i915->pmu;
>> @@ -1083,10 +1092,19 @@ void i915_pmu_register(struct drm_i915_private 
>> *i915)
>>       hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>>       pmu->timer.function = i915_sample;
>> -    ret = perf_pmu_register(&pmu->base, "i915", -1);
>> -    if (ret)
>> +    if (!is_igp(i915->drm.pdev))
>> +        pmu->name = kasprintf(GFP_KERNEL,
>> +                      "i915-%s",
>> +                      dev_name(i915->drm.dev));
>> +    else
>> +        pmu->name = "i915";
>> +    if (!pmu->name)
>>           goto err;
>> +    ret = perf_pmu_register(&pmu->base, pmu->name, -1);
>> +    if (ret)
>> +        goto err_name;
>> +
>>       ret = i915_pmu_register_cpuhp_state(pmu);
>>       if (ret)
>>           goto err_unreg;
>> @@ -1095,6 +1113,8 @@ void i915_pmu_register(struct drm_i915_private 
>> *i915)
>>   err_unreg:
>>       perf_pmu_unregister(&pmu->base);
>> +err_name:
>> +    kfree_const(pmu->name);
>>   err:
>>       pmu->base.event_init = NULL;
>>       free_event_attributes(pmu);
>> @@ -1116,5 +1136,6 @@ void i915_pmu_unregister(struct drm_i915_private 
>> *i915)
>>       perf_pmu_unregister(&pmu->base);
>>       pmu->base.event_init = NULL;
>> +    kfree_const(pmu->name);
>>       free_event_attributes(pmu);
>>   }
>> diff --git a/drivers/gpu/drm/i915/i915_pmu.h 
>> b/drivers/gpu/drm/i915/i915_pmu.h
>> index 4fc4f2478301..ff24f3bb0102 100644
>> --- a/drivers/gpu/drm/i915/i915_pmu.h
>> +++ b/drivers/gpu/drm/i915/i915_pmu.h
>> @@ -46,6 +46,10 @@ struct i915_pmu {
>>        * @base: PMU base.
>>        */
>>       struct pmu base;
>> +    /**
>> +     * @name: Name as registered with perf core.
>> +     */
>> +    const char *name;
>>       /**
>>        * @lock: Lock protecting enable mask and ref count handling.
>>        */
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2019-10-10 12:37 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-01 14:17 [PATCH 1/5] drm/i915/pmu: Make more struct i915_pmu centric Tvrtko Ursulin
2019-08-01 14:17 ` [PATCH 2/5] drm/i915/pmu: Convert engine sampling to uncore mmio Tvrtko Ursulin
2019-08-01 14:17 ` [PATCH 3/5] drm/i915/pmu: Convert sampling to gt Tvrtko Ursulin
2019-08-01 14:17 ` [PATCH 4/5] drm/i915/pmu: Make get_rc6 take intel_gt Tvrtko Ursulin
2019-08-01 14:45   ` Chris Wilson
2019-08-01 14:17 ` [PATCH 5/5] drm/i915/pmu: Support multiple GPUs Tvrtko Ursulin
2019-08-01 14:54   ` Chris Wilson
2019-08-01 15:10     ` Tvrtko Ursulin
2019-08-01 15:20       ` Chris Wilson
2019-08-01 15:31         ` Tvrtko Ursulin
2019-08-01 15:08   ` [PATCH v3 " Tvrtko Ursulin
2019-08-01 15:43     ` Chris Wilson
2019-08-01 15:54   ` [PATCH v4 " Tvrtko Ursulin
2019-09-06 15:47     ` [Intel-gfx] " Tvrtko Ursulin
2019-10-10 12:37       ` Tvrtko Ursulin
2019-08-01 21:22 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/pmu: Make more struct i915_pmu centric (rev3) Patchwork
2019-08-01 21:37   ` Chris Wilson

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.