[RFC 00/14] i915 PMU and engine busy stats

[RFC 00/14] i915 PMU and engine busy stats

[Tvrtko Ursulin]

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

Rough sketch of the idea I mentioned a few times to various people - merging
the engine busyness tracking with Chris i915 PMU RFC.

First patch is the actual PMU RFC by Chris. It is followed by some cleanup
patches, then come a few improvements, cheap execlists engine busyness tracking,
debugfs view for the same, and finally the i915 PMU is extended to use this
instead of timer based mmio sampling.

This makes it cheaper and also more accurate since engine busyness is not
derived via sampling.

But I haven't figure out the perf API yet. For example is it possible to access
our events in an usable fashion via perf top/stat or something? Do we want to
make the events discoverable as I did (patch 8).

I could not find much (any?) kernel API level documentation for perf.

Btw patch series actually works since intel-gpu-overlay can use these events
when they are available.

Chris Wilson (1):
  RFC drm/i915: Expose a PMU interface for perf queries

Tvrtko Ursulin (13):
  drm/i915/pmu: Add VCS2 engine to the PMU uAPI
  drm/i915/pmu: Add queued samplers to the PMU uAPI
  drm/i915/pmu: Decouple uAPI engine ids
  drm/i915/pmu: Helper to extract engine and sampler from PMU config
  drm/i915/pmu: Only sample enabled samplers
  drm/i915/pmu: Add fake regs
  drm/i915/pmu: Expose events in sysfs
  drm/i915/pmu: Suspend sampling when GPU is idle
  drm/i915: Wrap context schedule notification
  drm/i915: Engine busy time tracking
  drm/i915: Interface for controling engine stats collection
  drm/i915: Export engine busy stats in debugfs
  drm/i915/pmu: Wire up engine busy stats to PMU

 drivers/gpu/drm/i915/Makefile           |   1 +
 drivers/gpu/drm/i915/i915_debugfs.c     |  89 ++++
 drivers/gpu/drm/i915/i915_drv.c         |   2 +
 drivers/gpu/drm/i915/i915_drv.h         |  32 ++
 drivers/gpu/drm/i915/i915_gem.c         |   1 +
 drivers/gpu/drm/i915/i915_gem_request.c |   1 +
 drivers/gpu/drm/i915/i915_pmu.c         | 697 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_engine_cs.c  |  59 +++
 drivers/gpu/drm/i915/intel_lrc.c        |  19 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h |  46 +++
 include/uapi/drm/i915_drm.h             |  51 +++
 kernel/events/core.c                    |   1 +
 12 files changed, 996 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_pmu.c

-- 
2.9.4

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

[RFC 01/14] RFC drm/i915: Expose a PMU interface for perf queries

[Tvrtko Ursulin]

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

The first goal is to be able to measure GPU (and invidual ring) busyness
without having to poll registers from userspace. (Which not only incurs
holding the forcewake lock indefinitely, perturbing the system, but also
runs the risk of hanging the machine.) As an alternative we can use the
perf event counter interface to sample the ring registers periodically
and send those results to userspace.

To be able to do so, we need to export the two symbols from
kernel/events/core.c to register and unregister a PMU device.

v2: Use a common timer for the ring sampling.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/Makefile           |   1 +
 drivers/gpu/drm/i915/i915_drv.c         |   2 +
 drivers/gpu/drm/i915/i915_drv.h         |  23 ++
 drivers/gpu/drm/i915/i915_pmu.c         | 452 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.h |   2 +
 include/uapi/drm/i915_drm.h             |  41 +++
 kernel/events/core.c                    |   1 +
 7 files changed, 522 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/i915_pmu.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index f8227318dcaf..1c720013dc42 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -26,6 +26,7 @@ i915-y := i915_drv.o \
 
 i915-$(CONFIG_COMPAT)   += i915_ioc32.o
 i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o
+i915-$(CONFIG_PERF_EVENTS) += i915_pmu.o
 
 # GEM code
 i915-y += i915_cmd_parser.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d310d8245dca..f18ce519f6a2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1194,6 +1194,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
 	struct drm_device *dev = &dev_priv->drm;
 
 	i915_gem_shrinker_init(dev_priv);
+	i915_pmu_register(dev_priv);
 
 	/*
 	 * Notify a valid surface after modesetting,
@@ -1247,6 +1248,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 	intel_opregion_unregister(dev_priv);
 
 	i915_perf_unregister(dev_priv);
+	i915_pmu_unregister(dev_priv);
 
 	i915_teardown_sysfs(dev_priv);
 	i915_guc_log_unregister(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7c6fab08a2e6..de518503e033 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -40,6 +40,7 @@
 #include <linux/hash.h>
 #include <linux/intel-iommu.h>
 #include <linux/kref.h>
+#include <linux/perf_event.h>
 #include <linux/pm_qos.h>
 #include <linux/reservation.h>
 #include <linux/shmem_fs.h>
@@ -2093,6 +2094,12 @@ struct intel_cdclk_state {
 	unsigned int cdclk, vco, ref;
 };
 
+enum {
+	__I915_SAMPLE_FREQ_ACT = 0,
+	__I915_SAMPLE_FREQ_REQ,
+	__I915_NUM_PMU_SAMPLERS
+};
+
 struct drm_i915_private {
 	struct drm_device drm;
 
@@ -2591,6 +2598,13 @@ struct drm_i915_private {
 		int	irq;
 	} lpe_audio;
 
+	struct {
+		struct pmu base;
+		struct hrtimer timer;
+		u64 enable;
+		u64 sample[__I915_NUM_PMU_SAMPLERS];
+	} pmu;
+
 	/*
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
@@ -3760,6 +3774,15 @@ extern void i915_perf_fini(struct drm_i915_private *dev_priv);
 extern void i915_perf_register(struct drm_i915_private *dev_priv);
 extern void i915_perf_unregister(struct drm_i915_private *dev_priv);
 
+/* i915_pmu.c */
+#ifdef CONFIG_PERF_EVENTS
+extern void i915_pmu_register(struct drm_i915_private *i915);
+extern void i915_pmu_unregister(struct drm_i915_private *i915);
+#else
+static inline void i915_pmu_register(struct drm_i915_private *i915) {}
+static inline void i915_pmu_unregister(struct drm_i915_private *i915) {}
+#endif
+
 /* i915_suspend.c */
 extern int i915_save_state(struct drm_i915_private *dev_priv);
 extern int i915_restore_state(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
new file mode 100644
index 000000000000..f03ddad44da6
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -0,0 +1,452 @@
+#include <linux/perf_event.h>
+#include <linux/pm_runtime.h>
+
+#include "i915_drv.h"
+#include "intel_ringbuffer.h"
+
+#define FREQUENCY 200
+#define PERIOD max_t(u64, 10000, NSEC_PER_SEC / FREQUENCY)
+
+#define RING_MASK 0xffffffff
+#define RING_MAX 32
+
+static void engines_sample(struct drm_i915_private *dev_priv)
+{
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
+	bool fw = false;
+
+	if ((dev_priv->pmu.enable & RING_MASK) == 0)
+		return;
+
+	if (!dev_priv->gt.awake)
+		return;
+
+	if (!intel_runtime_pm_get_if_in_use(dev_priv))
+		return;
+
+	for_each_engine(engine, dev_priv, id) {
+		u32 val;
+
+		if ((dev_priv->pmu.enable & (0x7 << (4*id))) == 0)
+			continue;
+
+		if (i915_seqno_passed(intel_engine_get_seqno(engine),
+				      intel_engine_last_submit(engine)))
+			continue;
+
+		if (!fw) {
+			intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+			fw = true;
+		}
+
+		engine->pmu_sample[I915_SAMPLE_QUEUED] += PERIOD;
+
+		val = I915_READ_FW(RING_MI_MODE(engine->mmio_base));
+		if (!(val & MODE_IDLE))
+			engine->pmu_sample[I915_SAMPLE_BUSY] += PERIOD;
+
+		val = I915_READ_FW(RING_CTL(engine->mmio_base));
+		if (val & RING_WAIT)
+			engine->pmu_sample[I915_SAMPLE_WAIT] += PERIOD;
+		if (val & RING_WAIT_SEMAPHORE)
+			engine->pmu_sample[I915_SAMPLE_SEMA] += PERIOD;
+	}
+
+	if (fw)
+		intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+	intel_runtime_pm_put(dev_priv);
+}
+
+static void frequency_sample(struct drm_i915_private *dev_priv)
+{
+	if (dev_priv->pmu.enable & BIT_ULL(I915_PMU_ACTUAL_FREQUENCY)) {
+		u64 val;
+
+		val = dev_priv->rps.cur_freq;
+		if (dev_priv->gt.awake &&
+		    intel_runtime_pm_get_if_in_use(dev_priv)) {
+			val = I915_READ_NOTRACE(GEN6_RPSTAT1);
+			if (INTEL_GEN(dev_priv) >= 9)
+				val = (val & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
+			else if (IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8)
+				val = (val & HSW_CAGF_MASK) >> HSW_CAGF_SHIFT;
+			else
+				val = (val & GEN6_CAGF_MASK) >> GEN6_CAGF_SHIFT;
+			intel_runtime_pm_put(dev_priv);
+		}
+		val = intel_gpu_freq(dev_priv, val);
+		dev_priv->pmu.sample[__I915_SAMPLE_FREQ_ACT] += val * PERIOD;
+	}
+
+	if (dev_priv->pmu.enable & BIT_ULL(I915_PMU_REQUESTED_FREQUENCY)) {
+		u64 val = intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq);
+		dev_priv->pmu.sample[__I915_SAMPLE_FREQ_REQ] += val * PERIOD;
+	}
+}
+
+static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
+{
+	struct drm_i915_private *i915 =
+		container_of(hrtimer, struct drm_i915_private, pmu.timer);
+
+	if (i915->pmu.enable == 0)
+		return HRTIMER_NORESTART;
+
+	engines_sample(i915);
+	frequency_sample(i915);
+
+	hrtimer_forward_now(hrtimer, ns_to_ktime(PERIOD));
+	return HRTIMER_RESTART;
+}
+
+static void i915_pmu_event_destroy(struct perf_event *event)
+{
+	WARN_ON(event->parent);
+}
+
+static int engine_event_init(struct perf_event *event)
+{
+	struct drm_i915_private *i915 =
+		container_of(event->pmu, typeof(*i915), pmu.base);
+	int engine = event->attr.config >> 2;
+	int sample = event->attr.config & 3;
+
+	switch (sample) {
+	case I915_SAMPLE_QUEUED:
+	case I915_SAMPLE_BUSY:
+	case I915_SAMPLE_WAIT:
+		break;
+	case I915_SAMPLE_SEMA:
+		if (INTEL_GEN(i915) < 6)
+			return -ENODEV;
+		break;
+	default:
+		return -ENOENT;
+	}
+
+	if (engine >= I915_NUM_ENGINES)
+		return -ENOENT;
+
+	if (!i915->engine[engine])
+		return -ENODEV;
+
+	return 0;
+}
+
+static enum hrtimer_restart hrtimer_sample(struct hrtimer *hrtimer)
+{
+	struct perf_sample_data data;
+	struct perf_event *event;
+	u64 period;
+
+	event = container_of(hrtimer, struct perf_event, hw.hrtimer);
+	if (event->state != PERF_EVENT_STATE_ACTIVE)
+		return HRTIMER_NORESTART;
+
+	event->pmu->read(event);
+
+	perf_sample_data_init(&data, 0, event->hw.last_period);
+	perf_event_overflow(event, &data, NULL);
+
+	period = max_t(u64, 10000, event->hw.sample_period);
+	hrtimer_forward_now(hrtimer, ns_to_ktime(period));
+	return HRTIMER_RESTART;
+}
+
+static void init_hrtimer(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+
+	if (!is_sampling_event(event))
+		return;
+
+	hrtimer_init(&hwc->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	hwc->hrtimer.function = hrtimer_sample;
+
+	if (event->attr.freq) {
+		long freq = event->attr.sample_freq;
+
+		event->attr.sample_period = NSEC_PER_SEC / freq;
+		hwc->sample_period = event->attr.sample_period;
+		local64_set(&hwc->period_left, hwc->sample_period);
+		hwc->last_period = hwc->sample_period;
+		event->attr.freq = 0;
+	}
+}
+
+static int i915_pmu_event_init(struct perf_event *event)
+{
+	struct drm_i915_private *i915 =
+		container_of(event->pmu, typeof(*i915), pmu.base);
+	int ret;
+
+	/* XXX ideally only want pid == -1 && cpu == -1 */
+
+	if (event->attr.type != event->pmu->type)
+		return -ENOENT;
+
+	if (has_branch_stack(event))
+		return -EOPNOTSUPP;
+
+	ret = 0;
+	if (event->attr.config < RING_MAX) {
+		ret = engine_event_init(event);
+	} else switch (event->attr.config) {
+	case I915_PMU_ACTUAL_FREQUENCY:
+		if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
+			ret = -ENODEV; /* requires a mutex for sampling! */
+	case I915_PMU_REQUESTED_FREQUENCY:
+	case I915_PMU_ENERGY:
+	case I915_PMU_RC6_RESIDENCY:
+	case I915_PMU_RC6p_RESIDENCY:
+	case I915_PMU_RC6pp_RESIDENCY:
+		if (INTEL_GEN(i915) < 6)
+			ret = -ENODEV;
+		break;
+	}
+	if (ret)
+		return ret;
+
+	if (!event->parent)
+		event->destroy = i915_pmu_event_destroy;
+
+	init_hrtimer(event);
+
+	return 0;
+}
+
+static void i915_pmu_timer_start(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	s64 period;
+
+	if (!is_sampling_event(event))
+		return;
+
+	period = local64_read(&hwc->period_left);
+	if (period) {
+		if (period < 0)
+			period = 10000;
+
+		local64_set(&hwc->period_left, 0);
+	} else {
+		period = max_t(u64, 10000, hwc->sample_period);
+	}
+
+	hrtimer_start_range_ns(&hwc->hrtimer,
+			       ns_to_ktime(period), 0,
+			       HRTIMER_MODE_REL_PINNED);
+}
+
+static void i915_pmu_timer_cancel(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+
+	if (!is_sampling_event(event))
+		return;
+
+	local64_set(&hwc->period_left,
+		    ktime_to_ns(hrtimer_get_remaining(&hwc->hrtimer)));
+	hrtimer_cancel(&hwc->hrtimer);
+}
+
+static void i915_pmu_enable(struct perf_event *event)
+{
+	struct drm_i915_private *i915 =
+		container_of(event->pmu, typeof(*i915), pmu.base);
+
+	if (i915->pmu.enable == 0)
+		hrtimer_start_range_ns(&i915->pmu.timer,
+				       ns_to_ktime(PERIOD), 0,
+				       HRTIMER_MODE_REL_PINNED);
+
+	i915->pmu.enable |= BIT_ULL(event->attr.config);
+
+	i915_pmu_timer_start(event);
+}
+
+static void i915_pmu_disable(struct perf_event *event)
+{
+	struct drm_i915_private *i915 =
+		container_of(event->pmu, typeof(*i915), pmu.base);
+
+	i915->pmu.enable &= ~BIT_ULL(event->attr.config);
+	i915_pmu_timer_cancel(event);
+}
+
+static int i915_pmu_event_add(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+
+	if (flags & PERF_EF_START)
+		i915_pmu_enable(event);
+
+	hwc->state = !(flags & PERF_EF_START);
+
+	return 0;
+}
+
+static void i915_pmu_event_del(struct perf_event *event, int flags)
+{
+	i915_pmu_disable(event);
+}
+
+static void i915_pmu_event_start(struct perf_event *event, int flags)
+{
+	i915_pmu_enable(event);
+}
+
+static void i915_pmu_event_stop(struct perf_event *event, int flags)
+{
+	i915_pmu_disable(event);
+}
+
+static u64 read_energy_uJ(struct drm_i915_private *dev_priv)
+{
+	u64 power;
+
+	GEM_BUG_ON(INTEL_GEN(dev_priv) < 6);
+
+	intel_runtime_pm_get(dev_priv);
+
+	rdmsrl(MSR_RAPL_POWER_UNIT, power);
+	power = (power & 0x1f00) >> 8;
+	power = 1000000 >> power; /* convert to uJ */
+	power *= I915_READ_NOTRACE(MCH_SECP_NRG_STTS);
+
+	intel_runtime_pm_put(dev_priv);
+
+	return power;
+}
+
+static inline u64 calc_residency(struct drm_i915_private *dev_priv,
+				 const i915_reg_t reg)
+{
+	u64 val, units = 128, div = 100000;
+
+	GEM_BUG_ON(INTEL_GEN(dev_priv) < 6);
+
+	intel_runtime_pm_get(dev_priv);
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+		div = dev_priv->czclk_freq;
+		units = 1;
+		if (I915_READ_NOTRACE(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH)
+			units <<= 8;
+	} else if (IS_GEN9_LP(dev_priv)) {
+		div = 1200;
+		units = 1;
+	}
+	val = I915_READ_NOTRACE(reg);
+	intel_runtime_pm_put(dev_priv);
+
+	val *= units;
+	return DIV_ROUND_UP_ULL(val, div);
+}
+
+static u64 count_interrupts(struct drm_i915_private *i915)
+{
+	/* open-coded kstat_irqs() */
+	struct irq_desc *desc = irq_to_desc(i915->drm.pdev->irq);
+	u64 sum = 0;
+	int cpu;
+
+	if (!desc || !desc->kstat_irqs)
+		return 0;
+
+	for_each_possible_cpu(cpu)
+		sum += *per_cpu_ptr(desc->kstat_irqs, cpu);
+
+	return sum;
+}
+
+static void i915_pmu_event_read(struct perf_event *event)
+{
+	struct drm_i915_private *i915 =
+		container_of(event->pmu, typeof(*i915), pmu.base);
+	u64 val = 0;
+
+	if (event->attr.config < 32) {
+		int engine = event->attr.config >> 2;
+		int sample = event->attr.config & 3;
+		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];
+		break;
+	case I915_PMU_REQUESTED_FREQUENCY:
+		val = i915->pmu.sample[__I915_SAMPLE_FREQ_REQ];
+		break;
+	case I915_PMU_ENERGY:
+		val = read_energy_uJ(i915);
+		break;
+	case I915_PMU_INTERRUPTS:
+		val = count_interrupts(i915);
+		break;
+
+	case I915_PMU_RC6_RESIDENCY:
+		if (!i915->gt.awake)
+			return;
+
+		val = calc_residency(i915, IS_VALLEYVIEW(i915) ? VLV_GT_RENDER_RC6 : GEN6_GT_GFX_RC6);
+		break;
+
+	case I915_PMU_RC6p_RESIDENCY:
+		if (!i915->gt.awake)
+			return;
+
+		if (!IS_VALLEYVIEW(i915))
+			val = calc_residency(i915, GEN6_GT_GFX_RC6p);
+		break;
+
+	case I915_PMU_RC6pp_RESIDENCY:
+		if (!i915->gt.awake)
+			return;
+
+		if (!IS_VALLEYVIEW(i915))
+			val = calc_residency(i915, GEN6_GT_GFX_RC6pp);
+		break;
+	}
+
+	local64_set(&event->count, val);
+}
+
+static int i915_pmu_event_event_idx(struct perf_event *event)
+{
+	return 0;
+}
+
+void i915_pmu_register(struct drm_i915_private *i915)
+{
+	if (INTEL_GEN(i915) <= 2)
+		return;
+
+	i915->pmu.base.task_ctx_nr	= perf_sw_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;
+
+	hrtimer_init(&i915->pmu.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	i915->pmu.timer.function = i915_sample;
+	i915->pmu.enable = 0;
+
+	if (perf_pmu_register(&i915->pmu.base, "i915", -1))
+		i915->pmu.base.event_init = NULL;
+}
+
+void i915_pmu_unregister(struct drm_i915_private *i915)
+{
+	if (!i915->pmu.base.event_init)
+		return;
+
+	i915->pmu.enable = 0;
+
+	perf_pmu_unregister(&i915->pmu.base);
+	i915->pmu.base.event_init = NULL;
+
+	hrtimer_cancel(&i915->pmu.timer);
+}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index d33c93444c0d..0877b151239d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -245,6 +245,8 @@ struct intel_engine_cs {
 		I915_SELFTEST_DECLARE(bool mock : 1);
 	} breadcrumbs;
 
+	u64 pmu_sample[4];
+
 	/*
 	 * A pool of objects to use as shadow copies of client batch buffers
 	 * when the command parser is enabled. Prevents the client from
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 7ccbd6a2bbe0..733774f19a0b 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -86,6 +86,47 @@ enum i915_mocs_table_index {
 	I915_MOCS_CACHED,
 };
 
+/**
+ * DOC: perf_events exposed by i915 through /sys/bus/event_sources/drivers/i915
+ *
+ */
+#define I915_SAMPLE_QUEUED	0
+#define I915_SAMPLE_BUSY	1
+#define I915_SAMPLE_WAIT	2
+#define I915_SAMPLE_SEMA	3
+
+#define I915_SAMPLE_RCS		0
+#define I915_SAMPLE_VCS		1
+#define I915_SAMPLE_BCS		2
+#define I915_SAMPLE_VECS	3
+
+#define __I915_PMU_COUNT(ring, id) ((ring) << 2 | (id))
+
+#define I915_PMU_COUNT_RCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_RCS, I915_SAMPLE_BUSY)
+#define I915_PMU_COUNT_RCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_RCS, I915_SAMPLE_WAIT)
+#define I915_PMU_COUNT_RCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_RCS, I915_SAMPLE_SEMA)
+
+#define I915_PMU_COUNT_VCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_BUSY)
+#define I915_PMU_COUNT_VCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_WAIT)
+#define I915_PMU_COUNT_VCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_SEMA)
+
+#define I915_PMU_COUNT_BCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_BUSY)
+#define I915_PMU_COUNT_BCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_WAIT)
+#define I915_PMU_COUNT_BCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_SEMA)
+
+#define I915_PMU_COUNT_VECS_BUSY __I915_PMU_COUNT(I915_SAMPLE_VECS, I915_SAMPLE_BUSY)
+#define I915_PMU_COUNT_VECS_WAIT __I915_PMU_COUNT(I915_SAMPLE_VECS, I915_SAMPLE_WAIT)
+#define I915_PMU_COUNT_VECS_SEMA __I915_PMU_COUNT(I915_SAMPLE_VECS, I915_SAMPLE_SEMA)
+
+#define I915_PMU_ACTUAL_FREQUENCY 32
+#define I915_PMU_REQUESTED_FREQUENCY 33
+#define I915_PMU_ENERGY 34
+#define I915_PMU_INTERRUPTS 35
+
+#define I915_PMU_RC6_RESIDENCY		40
+#define I915_PMU_RC6p_RESIDENCY	41
+#define I915_PMU_RC6pp_RESIDENCY	42
+
 /* Each region is a minimum of 16k, and there are at most 255 of them.
  */
 #define I915_NR_TEX_REGIONS 255	/* table size 2k - maximum due to use
diff --git a/kernel/events/core.c b/kernel/events/core.c
index e46eba8cd1b7..7b8c6dce1078 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7386,6 +7386,7 @@ int perf_event_overflow(struct perf_event *event,
 {
 	return __perf_event_overflow(event, 1, data, regs);
 }
+EXPORT_SYMBOL_GPL(perf_event_overflow);
 
 /*
  * Generic software event infrastructure
-- 
2.9.4

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

[RFC 02/14] drm/i915/pmu: Add VCS2 engine to the PMU uAPI

[Tvrtko Ursulin]

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

It is missing from the original patch.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 include/uapi/drm/i915_drm.h | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 733774f19a0b..67e63757b35d 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -96,9 +96,10 @@ enum i915_mocs_table_index {
 #define I915_SAMPLE_SEMA	3
 
 #define I915_SAMPLE_RCS		0
-#define I915_SAMPLE_VCS		1
-#define I915_SAMPLE_BCS		2
-#define I915_SAMPLE_VECS	3
+#define I915_SAMPLE_BCS		1
+#define I915_SAMPLE_VCS		2
+#define I915_SAMPLE_VCS2	3
+#define I915_SAMPLE_VECS	4
 
 #define __I915_PMU_COUNT(ring, id) ((ring) << 2 | (id))
 
@@ -110,6 +111,10 @@ enum i915_mocs_table_index {
 #define I915_PMU_COUNT_VCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_WAIT)
 #define I915_PMU_COUNT_VCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_SEMA)
 
+#define I915_PMU_COUNT_VCS2_BUSY __I915_PMU_COUNT(I915_SAMPLE_VCS2, I915_SAMPLE_BUSY)
+#define I915_PMU_COUNT_VCS2_WAIT __I915_PMU_COUNT(I915_SAMPLE_VCS2, I915_SAMPLE_WAIT)
+#define I915_PMU_COUNT_VCS2_SEMA __I915_PMU_COUNT(I915_SAMPLE_VCS2, I915_SAMPLE_SEMA)
+
 #define I915_PMU_COUNT_BCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_BUSY)
 #define I915_PMU_COUNT_BCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_WAIT)
 #define I915_PMU_COUNT_BCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_SEMA)
-- 
2.9.4

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

[RFC 03/14] drm/i915/pmu: Add queued samplers to the PMU uAPI

[Tvrtko Ursulin]

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

This is also partially missing in the original patch.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 include/uapi/drm/i915_drm.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 67e63757b35d..7003599a460e 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -103,22 +103,27 @@ enum i915_mocs_table_index {
 
 #define __I915_PMU_COUNT(ring, id) ((ring) << 2 | (id))
 
+#define I915_PMU_COUNT_RCS_QUEUED __I915_PMU_COUNT(I915_SAMPLE_RCS, I915_SAMPLE_QUEUED)
 #define I915_PMU_COUNT_RCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_RCS, I915_SAMPLE_BUSY)
 #define I915_PMU_COUNT_RCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_RCS, I915_SAMPLE_WAIT)
 #define I915_PMU_COUNT_RCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_RCS, I915_SAMPLE_SEMA)
 
+#define I915_PMU_COUNT_VCS_QUEUED __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_QUEUED)
 #define I915_PMU_COUNT_VCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_BUSY)
 #define I915_PMU_COUNT_VCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_WAIT)
 #define I915_PMU_COUNT_VCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_SEMA)
 
+#define I915_PMU_COUNT_VCS2_QUEUED __I915_PMU_COUNT(I915_SAMPLE_VCS2, I915_SAMPLE_QUEUED)
 #define I915_PMU_COUNT_VCS2_BUSY __I915_PMU_COUNT(I915_SAMPLE_VCS2, I915_SAMPLE_BUSY)
 #define I915_PMU_COUNT_VCS2_WAIT __I915_PMU_COUNT(I915_SAMPLE_VCS2, I915_SAMPLE_WAIT)
 #define I915_PMU_COUNT_VCS2_SEMA __I915_PMU_COUNT(I915_SAMPLE_VCS2, I915_SAMPLE_SEMA)
 
+#define I915_PMU_COUNT_BCS_QUEUED __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_QUEUED)
 #define I915_PMU_COUNT_BCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_BUSY)
 #define I915_PMU_COUNT_BCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_WAIT)
 #define I915_PMU_COUNT_BCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_SEMA)
 
+#define I915_PMU_COUNT_VECS_QUEUED __I915_PMU_COUNT(I915_SAMPLE_VECS, I915_SAMPLE_QUEUED)
 #define I915_PMU_COUNT_VECS_BUSY __I915_PMU_COUNT(I915_SAMPLE_VECS, I915_SAMPLE_BUSY)
 #define I915_PMU_COUNT_VECS_WAIT __I915_PMU_COUNT(I915_SAMPLE_VECS, I915_SAMPLE_WAIT)
 #define I915_PMU_COUNT_VECS_SEMA __I915_PMU_COUNT(I915_SAMPLE_VECS, I915_SAMPLE_SEMA)
-- 
2.9.4

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

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

[Tvrtko Ursulin]

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.

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

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

[RFC 05/14] drm/i915/pmu: Helper to extract engine and sampler from PMU config

[Tvrtko Ursulin]

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

Less hardcoded shifts and ands in the code is better.

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

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 9a8208dba7a9..d80e673477b5 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -131,17 +131,21 @@ static void i915_pmu_event_destroy(struct perf_event *event)
 	WARN_ON(event->parent);
 }
 
+#define pmu_config_engine(config) ((config) >> 2)
+#define pmu_config_sampler(config) ((config) & 3)
+
 static int engine_event_init(struct perf_event *event)
 {
 	struct drm_i915_private *i915 =
 		container_of(event->pmu, typeof(*i915), pmu.base);
-	int engine = event->attr.config >> 2;
-	int sample = event->attr.config & 3;
+	unsigned int user_engine = pmu_config_engine(event->attr.config);
+	unsigned int sample = pmu_config_sampler(event->attr.config);
+	enum intel_engine_id engine_id;
 
-	if (WARN_ON_ONCE(engine >= ARRAY_SIZE(user_engine_map)))
+	if (WARN_ON_ONCE(user_engine >= ARRAY_SIZE(user_engine_map)))
 		return -ENOENT;
 	else
-		engine = user_engine_map[engine];
+		engine_id = user_engine_map[user_engine];
 
 	switch (sample) {
 	case I915_SAMPLE_QUEUED:
@@ -156,7 +160,7 @@ static int engine_event_init(struct perf_event *event)
 		return -ENOENT;
 	}
 
-	if (!i915->engine[engine])
+	if (!i915->engine[engine_id])
 		return -ENODEV;
 
 	return 0;
@@ -395,14 +399,14 @@ static void i915_pmu_event_read(struct perf_event *event)
 	u64 val = 0;
 
 	if (event->attr.config < 32) {
-		int engine = event->attr.config >> 2;
-		int sample = event->attr.config & 3;
+		unsigned int user_engine = pmu_config_engine(event->attr.config);
+		unsigned int sample = pmu_config_sampler(event->attr.config);
 
-		if (WARN_ON_ONCE(engine >= ARRAY_SIZE(user_engine_map))) {
+		if (WARN_ON_ONCE(user_engine >= ARRAY_SIZE(user_engine_map))) {
 			/* Do nothing */
 		} else {
-			engine = user_engine_map[engine];
-			val = i915->engine[engine]->pmu_sample[sample];
+			enum intel_engine_id id = user_engine_map[user_engine];
+			val = i915->engine[id]->pmu_sample[sample];
 		}
 	} else switch (event->attr.config) {
 	case I915_PMU_ACTUAL_FREQUENCY:
-- 
2.9.4

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

[RFC 06/14] drm/i915/pmu: Only sample enabled samplers

[Tvrtko Ursulin]

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

Save some execution time by not reading the MMIO registers
for the samplers which are not enabled.

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

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index d80e673477b5..4d61a1e72ee6 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -29,6 +29,14 @@ static const unsigned int user_engine_map[I915_NUM_ENGINES] = {
 	[I915_SAMPLE_VECS] = VECS,
 };
 
+static bool grab_forcewake(struct drm_i915_private *i915, bool fw)
+{
+	if (!fw)
+		intel_uncore_forcewake_get(i915, FORCEWAKE_ALL);
+
+	return true;
+}
+
 static void engines_sample(struct drm_i915_private *dev_priv)
 {
 	struct intel_engine_cs *engine;
@@ -46,6 +54,7 @@ static void engines_sample(struct drm_i915_private *dev_priv)
 
 	for_each_engine(engine, dev_priv, id) {
 		unsigned int user_engine = engine_map[id];
+		u8 sample_mask;
 		u32 val;
 
 		if (WARN_ON_ONCE(id >= ARRAY_SIZE(engine_map)))
@@ -53,30 +62,38 @@ static void engines_sample(struct drm_i915_private *dev_priv)
 		else
 			user_engine = engine_map[id];
 
-		if (!(dev_priv->pmu.enable &
-		    (ENGINE_SAMPLE_MASK << (ENGINE_SAMPLE_BITS * user_engine))))
+		sample_mask = (dev_priv->pmu.enable >>
+			      (ENGINE_SAMPLE_BITS * user_engine)) &
+			      ENGINE_SAMPLE_MASK;
+
+		if (!sample_mask)
 			continue;
 
 		if (i915_seqno_passed(intel_engine_get_seqno(engine),
 				      intel_engine_last_submit(engine)))
 			continue;
 
-		if (!fw) {
-			intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
-			fw = true;
-		}
-
-		engine->pmu_sample[I915_SAMPLE_QUEUED] += PERIOD;
+		if (sample_mask & BIT(I915_SAMPLE_QUEUED))
+			engine->pmu_sample[I915_SAMPLE_QUEUED] += PERIOD;
 
-		val = I915_READ_FW(RING_MI_MODE(engine->mmio_base));
-		if (!(val & MODE_IDLE))
-			engine->pmu_sample[I915_SAMPLE_BUSY] += PERIOD;
+		if (sample_mask & BIT(I915_SAMPLE_BUSY)) {
+			fw = grab_forcewake(dev_priv, fw);
+			val = I915_READ_FW(RING_MI_MODE(engine->mmio_base));
+			if (!(val & MODE_IDLE))
+				engine->pmu_sample[I915_SAMPLE_BUSY] += PERIOD;
+		}
 
-		val = I915_READ_FW(RING_CTL(engine->mmio_base));
-		if (val & RING_WAIT)
-			engine->pmu_sample[I915_SAMPLE_WAIT] += PERIOD;
-		if (val & RING_WAIT_SEMAPHORE)
-			engine->pmu_sample[I915_SAMPLE_SEMA] += PERIOD;
+		if (sample_mask &
+		    (BIT(I915_SAMPLE_WAIT) | BIT(I915_SAMPLE_SEMA))) {
+			fw = grab_forcewake(dev_priv, fw);
+			val = I915_READ_FW(RING_CTL(engine->mmio_base));
+			if ((sample_mask & BIT(I915_SAMPLE_WAIT)) &&
+			    (val & RING_WAIT))
+				engine->pmu_sample[I915_SAMPLE_WAIT] += PERIOD;
+			if ((sample_mask & BIT(I915_SAMPLE_SEMA)) &&
+			    (val & RING_WAIT_SEMAPHORE))
+				engine->pmu_sample[I915_SAMPLE_SEMA] += PERIOD;
+		}
 	}
 
 	if (fw)
-- 
2.9.4

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

[RFC 07/14] drm/i915/pmu: Add fake regs

[Tvrtko Ursulin]

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

Without this I can get a null ptr deref when trying to access
our events with perf.

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

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 4d61a1e72ee6..4195d89b1c82 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -183,8 +183,11 @@ static int engine_event_init(struct perf_event *event)
 	return 0;
 }
 
+static DEFINE_PER_CPU(struct pt_regs, i915_pmu_pt_regs);
+
 static enum hrtimer_restart hrtimer_sample(struct hrtimer *hrtimer)
 {
+	struct pt_regs *regs = this_cpu_ptr(&i915_pmu_pt_regs);
 	struct perf_sample_data data;
 	struct perf_event *event;
 	u64 period;
@@ -196,7 +199,7 @@ static enum hrtimer_restart hrtimer_sample(struct hrtimer *hrtimer)
 	event->pmu->read(event);
 
 	perf_sample_data_init(&data, 0, event->hw.last_period);
-	perf_event_overflow(event, &data, NULL);
+	perf_event_overflow(event, &data, regs);
 
 	period = max_t(u64, 10000, event->hw.sample_period);
 	hrtimer_forward_now(hrtimer, ns_to_ktime(period));
-- 
2.9.4

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

[RFC 08/14] drm/i915/pmu: Expose events in sysfs

[Tvrtko Ursulin]

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

This makes then visible to "perf list".

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

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 4195d89b1c82..7ea84a191876 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -474,11 +474,100 @@ static int i915_pmu_event_event_idx(struct perf_event *event)
 	return 0;
 }
 
+static ssize_t i915_pmu_format_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+        struct dev_ext_attribute *eattr;
+
+        eattr = container_of(attr, struct dev_ext_attribute, attr);
+        return sprintf(buf, "%s\n", (char *) eattr->var);
+}
+
+#define I915_PMU_FORMAT_ATTR(_name, _config)           \
+        (&((struct dev_ext_attribute[]) {               \
+                { .attr = __ATTR(_name, S_IRUGO, i915_pmu_format_show, NULL), \
+                  .var = (void *) _config, }            \
+        })[0].attr.attr)
+
+static struct attribute *i915_pmu_format_attrs[] = {
+        I915_PMU_FORMAT_ATTR(i915_eventid, "config:0-42"),
+        NULL,
+};
+
+static const struct attribute_group i915_pmu_format_attr_group = {
+        .name = "format",
+        .attrs = i915_pmu_format_attrs,
+};
+
+static ssize_t i915_pmu_event_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+        struct dev_ext_attribute *eattr;
+
+        eattr = container_of(attr, struct dev_ext_attribute, attr);
+        return sprintf(buf, "config=0x%lx\n", (unsigned long) eattr->var);
+}
+
+#define I915_PMU_EVENT_ATTR(_name, _config)            \
+        (&((struct dev_ext_attribute[]) {               \
+                { .attr = __ATTR(_name, S_IRUGO, i915_pmu_event_show, NULL), \
+                  .var = (void *) _config, }            \
+         })[0].attr.attr)
+
+static struct attribute *i915_pmu_events_attrs[] = {
+        I915_PMU_EVENT_ATTR(rcs-queued,	I915_PMU_COUNT_RCS_QUEUED),
+        I915_PMU_EVENT_ATTR(rcs-busy,	I915_PMU_COUNT_RCS_BUSY),
+        I915_PMU_EVENT_ATTR(rcs-wait,	I915_PMU_COUNT_RCS_WAIT),
+        I915_PMU_EVENT_ATTR(rcs-sema,	I915_PMU_COUNT_RCS_SEMA),
+
+        I915_PMU_EVENT_ATTR(bcs-queued,	I915_PMU_COUNT_BCS_QUEUED),
+        I915_PMU_EVENT_ATTR(bcs-busy,	I915_PMU_COUNT_BCS_BUSY),
+        I915_PMU_EVENT_ATTR(bcs-wait,	I915_PMU_COUNT_BCS_WAIT),
+        I915_PMU_EVENT_ATTR(bcs-sema,	I915_PMU_COUNT_BCS_SEMA),
+
+        I915_PMU_EVENT_ATTR(vcs-queued,	I915_PMU_COUNT_VCS_QUEUED),
+        I915_PMU_EVENT_ATTR(vcs-busy,	I915_PMU_COUNT_VCS_BUSY),
+        I915_PMU_EVENT_ATTR(vcs-wait,	I915_PMU_COUNT_VCS_WAIT),
+        I915_PMU_EVENT_ATTR(vcs-sema,	I915_PMU_COUNT_VCS_SEMA),
+
+        I915_PMU_EVENT_ATTR(vcs2-queued, I915_PMU_COUNT_VCS2_QUEUED),
+        I915_PMU_EVENT_ATTR(vcs2-busy,	 I915_PMU_COUNT_VCS2_BUSY),
+        I915_PMU_EVENT_ATTR(vcs2-wait,	 I915_PMU_COUNT_VCS2_WAIT),
+        I915_PMU_EVENT_ATTR(vcs2-sema,	 I915_PMU_COUNT_VCS2_SEMA),
+
+        I915_PMU_EVENT_ATTR(vecs-queued, I915_PMU_COUNT_VECS_QUEUED),
+        I915_PMU_EVENT_ATTR(vecs-busy,	 I915_PMU_COUNT_VECS_BUSY),
+        I915_PMU_EVENT_ATTR(vecs-wait,	 I915_PMU_COUNT_VECS_WAIT),
+        I915_PMU_EVENT_ATTR(vecs-sema,	 I915_PMU_COUNT_VECS_SEMA),
+
+        I915_PMU_EVENT_ATTR(actual-frequency,	 I915_PMU_ACTUAL_FREQUENCY),
+        I915_PMU_EVENT_ATTR(requested-frequency, I915_PMU_REQUESTED_FREQUENCY),
+        I915_PMU_EVENT_ATTR(energy,		 I915_PMU_ENERGY),
+        I915_PMU_EVENT_ATTR(interrupts,		 I915_PMU_INTERRUPTS),
+        I915_PMU_EVENT_ATTR(rc6-residency,	 I915_PMU_RC6_RESIDENCY),
+        I915_PMU_EVENT_ATTR(rc6p-residency,	 I915_PMU_RC6p_RESIDENCY),
+        I915_PMU_EVENT_ATTR(rc6pp-residency,	 I915_PMU_RC6pp_RESIDENCY),
+
+        NULL,
+};
+
+static const struct attribute_group i915_pmu_events_attr_group = {
+        .name = "events",
+        .attrs = i915_pmu_events_attrs,
+};
+
+static const struct attribute_group *i915_pmu_attr_groups[] = {
+        &i915_pmu_format_attr_group,
+        &i915_pmu_events_attr_group,
+        NULL
+};
+
 void i915_pmu_register(struct drm_i915_private *i915)
 {
 	if (INTEL_GEN(i915) <= 2)
 		return;
 
+	i915->pmu.base.attr_groups	= i915_pmu_attr_groups;
 	i915->pmu.base.task_ctx_nr	= perf_sw_context;
 	i915->pmu.base.event_init	= i915_pmu_event_init;
 	i915->pmu.base.add		= i915_pmu_event_add;
-- 
2.9.4

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

[RFC 09/14] drm/i915/pmu: Suspend sampling when GPU is idle

[Tvrtko Ursulin]

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

If only a subset of events is enabled we can afford to suspend
the sampling timer when the GPU is idle and so save some cycles
and power.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  6 ++++
 drivers/gpu/drm/i915/i915_gem.c         |  1 +
 drivers/gpu/drm/i915/i915_gem_request.c |  1 +
 drivers/gpu/drm/i915/i915_pmu.c         | 52 +++++++++++++++++++++++++++++++--
 4 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index de518503e033..f1fded6dd9cf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2600,7 +2600,9 @@ struct drm_i915_private {
 
 	struct {
 		struct pmu base;
+		spinlock_t lock;
 		struct hrtimer timer;
+		bool timer_enabled;
 		u64 enable;
 		u64 sample[__I915_NUM_PMU_SAMPLERS];
 	} pmu;
@@ -3778,9 +3780,13 @@ extern void i915_perf_unregister(struct drm_i915_private *dev_priv);
 #ifdef CONFIG_PERF_EVENTS
 extern void i915_pmu_register(struct drm_i915_private *i915);
 extern void i915_pmu_unregister(struct drm_i915_private *i915);
+extern void i915_pmu_gt_idle(struct drm_i915_private *i915);
+extern void i915_pmu_gt_active(struct drm_i915_private *i915);
 #else
 static inline void i915_pmu_register(struct drm_i915_private *i915) {}
 static inline void i915_pmu_unregister(struct drm_i915_private *i915) {}
+static inline void i915_pmu_gt_idle(struct drm_i915_private *i915) {}
+static inline void i915_pmu_gt_active(struct drm_i915_private *i915) {}
 #endif
 
 /* i915_suspend.c */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1b2dfa8bdeef..bb81c1fcbc40 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3235,6 +3235,7 @@ i915_gem_idle_work_handler(struct work_struct *work)
 
 	intel_engines_mark_idle(dev_priv);
 	i915_gem_timelines_mark_idle(dev_priv);
+	i915_pmu_gt_idle(dev_priv);
 
 	GEM_BUG_ON(!dev_priv->gt.awake);
 	dev_priv->gt.awake = false;
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 483af8921060..569c44a6ba2a 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -876,6 +876,7 @@ static void i915_gem_mark_busy(const struct intel_engine_cs *engine)
 	i915_update_gfx_val(dev_priv);
 	if (INTEL_GEN(dev_priv) >= 6)
 		gen6_rps_busy(dev_priv);
+	i915_pmu_gt_active(dev_priv);
 
 	queue_delayed_work(dev_priv->wq,
 			   &dev_priv->gt.retire_work,
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 7ea84a191876..4b113cad40d1 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -29,6 +29,40 @@ static const unsigned int user_engine_map[I915_NUM_ENGINES] = {
 	[I915_SAMPLE_VECS] = VECS,
 };
 
+static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active)
+{
+	if (gpu_active)
+		return i915->pmu.enable;
+	else
+		return i915->pmu.enable >> 32;
+}
+
+void i915_pmu_gt_idle(struct drm_i915_private *i915)
+{
+	spin_lock_irq(&i915->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);
+}
+
+void i915_pmu_gt_active(struct drm_i915_private *i915)
+{
+	spin_lock_irq(&i915->pmu.lock);
+	/*
+	 * Re-enable sampling timer when GPU goes active.
+	 */
+	if (!i915->pmu.timer_enabled && pmu_needs_timer(i915, true)) {
+		hrtimer_start_range_ns(&i915->pmu.timer,
+				       ns_to_ktime(PERIOD), 0,
+				       HRTIMER_MODE_REL_PINNED);
+		i915->pmu.timer_enabled = true;
+	}
+	spin_unlock_irq(&i915->pmu.lock);
+}
+
 static bool grab_forcewake(struct drm_i915_private *i915, bool fw)
 {
 	if (!fw)
@@ -133,7 +167,7 @@ static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
 	struct drm_i915_private *i915 =
 		container_of(hrtimer, struct drm_i915_private, pmu.timer);
 
-	if (i915->pmu.enable == 0)
+	if (!READ_ONCE(i915->pmu.timer_enabled))
 		return HRTIMER_NORESTART;
 
 	engines_sample(i915);
@@ -307,13 +341,19 @@ static void i915_pmu_enable(struct perf_event *event)
 {
 	struct drm_i915_private *i915 =
 		container_of(event->pmu, typeof(*i915), pmu.base);
+	unsigned long flags;
+
+	spin_lock_irqsave(&i915->pmu.lock, flags);
 
-	if (i915->pmu.enable == 0)
+	i915->pmu.enable |= BIT_ULL(event->attr.config);
+	if (pmu_needs_timer(i915, true) && !i915->pmu.timer_enabled) {
 		hrtimer_start_range_ns(&i915->pmu.timer,
 				       ns_to_ktime(PERIOD), 0,
 				       HRTIMER_MODE_REL_PINNED);
+		i915->pmu.timer_enabled = true;
+	}
 
-	i915->pmu.enable |= BIT_ULL(event->attr.config);
+	spin_unlock_irqrestore(&i915->pmu.lock, flags);
 
 	i915_pmu_timer_start(event);
 }
@@ -322,8 +362,13 @@ static void i915_pmu_disable(struct perf_event *event)
 {
 	struct drm_i915_private *i915 =
 		container_of(event->pmu, typeof(*i915), pmu.base);
+	unsigned long flags;
 
+	spin_lock_irqsave(&i915->pmu.lock, flags);
 	i915->pmu.enable &= ~BIT_ULL(event->attr.config);
+	i915->pmu.timer_enabled &= pmu_needs_timer(i915, true);
+	spin_unlock_irqrestore(&i915->pmu.lock, flags);
+
 	i915_pmu_timer_cancel(event);
 }
 
@@ -577,6 +622,7 @@ void i915_pmu_register(struct drm_i915_private *i915)
 	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;
 	i915->pmu.enable = 0;
-- 
2.9.4

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

[RFC 10/14] drm/i915: Wrap context schedule notification

[Tvrtko Ursulin]

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

No functional change just something which will be handy in the
following patch.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 699868d81de8..89aa29f23a92 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -307,6 +307,18 @@ execlists_context_status_change(struct drm_i915_gem_request *rq,
 				   status, rq);
 }
 
+static inline void
+execlists_context_schedule_in(struct drm_i915_gem_request *rq)
+{
+	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
+}
+
+static inline void
+execlists_context_schedule_out(struct drm_i915_gem_request *rq)
+{
+	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
+}
+
 static void
 execlists_update_context_pdps(struct i915_hw_ppgtt *ppgtt, u32 *reg_state)
 {
@@ -352,7 +364,7 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
 		if (rq) {
 			GEM_BUG_ON(count > !n);
 			if (!count++)
-				execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
+				execlists_context_schedule_in(rq);
 			port_set(&port[n], port_pack(rq, count));
 			desc = execlists_update_context(rq);
 			GEM_DEBUG_EXEC(port[n].context_id = upper_32_bits(desc));
@@ -603,8 +615,7 @@ static void intel_lrc_irq_handler(unsigned long data)
 			if (--count == 0) {
 				GEM_BUG_ON(status & GEN8_CTX_STATUS_PREEMPTED);
 				GEM_BUG_ON(!i915_gem_request_completed(rq));
-				execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
-
+				execlists_context_schedule_out(rq);
 				trace_i915_gem_request_out(rq);
 				i915_gem_request_put(rq);
 
-- 
2.9.4

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

[RFC 11/14] drm/i915: Engine busy time tracking

[Tvrtko Ursulin]

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

Track total time requests have been executing on the hardware.

To make this cheap it is hidden behind a static branch with the
intention that it is only enabled when there is a consumer
listening. This means that in the default off case the total
cost of the tracking is just a few no-op instructions on the
fast paths.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c  |  6 +++++
 drivers/gpu/drm/i915/intel_lrc.c        |  2 ++
 drivers/gpu/drm/i915/intel_ringbuffer.h | 39 +++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 24db316e0fd1..3e5e08c6b5ef 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -22,10 +22,14 @@
  *
  */
 
+#include <linux/static_key.h>
+
 #include "i915_drv.h"
 #include "intel_ringbuffer.h"
 #include "intel_lrc.h"
 
+DEFINE_STATIC_KEY_FALSE(i915_engine_stats_key);
+
 /* Haswell does have the CXT_SIZE register however it does not appear to be
  * valid. Now, docs explain in dwords what is in the context object. The full
  * size is 70720 bytes, however, the power context and execlist context will
@@ -223,6 +227,8 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 	/* Nothing to do here, execute in order of dependencies */
 	engine->schedule = NULL;
 
+	spin_lock_init(&engine->stats.lock);
+
 	ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier);
 
 	dev_priv->engine[id] = engine;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 89aa29f23a92..b72a2d7cd44c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -310,12 +310,14 @@ execlists_context_status_change(struct drm_i915_gem_request *rq,
 static inline void
 execlists_context_schedule_in(struct drm_i915_gem_request *rq)
 {
+	intel_engine_context_in(rq->engine);
 	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN);
 }
 
 static inline void
 execlists_context_schedule_out(struct drm_i915_gem_request *rq)
 {
+	intel_engine_context_out(rq->engine);
 	execlists_context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 0877b151239d..2eb1e970ad06 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -443,6 +443,13 @@ struct intel_engine_cs {
 	 * certain bits to encode the command length in the header).
 	 */
 	u32 (*get_cmd_length_mask)(u32 cmd_header);
+
+       struct {
+               spinlock_t lock;
+               unsigned int ref;
+               u64 start; /* Timestamp of the last idle to active transition. */
+               u64 total; /* Total time engined was busy. */
+       } stats;
 };
 
 static inline unsigned int
@@ -737,4 +744,36 @@ bool intel_engines_are_idle(struct drm_i915_private *dev_priv);
 void intel_engines_mark_idle(struct drm_i915_private *i915);
 void intel_engines_reset_default_submission(struct drm_i915_private *i915);
 
+DECLARE_STATIC_KEY_FALSE(i915_engine_stats_key);
+
+static inline void intel_engine_context_in(struct intel_engine_cs *engine)
+{
+	if (static_branch_unlikely(&i915_engine_stats_key)) {
+		unsigned long flags;
+
+		spin_lock_irqsave(&engine->stats.lock, flags);
+		if (engine->stats.ref++ == 0)
+			engine->stats.start = ktime_get_real_ns();
+		GEM_BUG_ON(engine->stats.ref == 0);
+		spin_unlock_irqrestore(&engine->stats.lock, flags);
+	}
+}
+
+static inline void intel_engine_context_out(struct intel_engine_cs *engine)
+{
+	if (static_branch_unlikely(&i915_engine_stats_key)) {
+		unsigned long flags;
+
+		spin_lock_irqsave(&engine->stats.lock, flags);
+		/*
+		 * After turning on the static key context out might be the
+		 * first event which then needs to be ignored (ref == 0).
+		 */
+		if (engine->stats.ref && --engine->stats.ref == 0)
+			engine->stats.total += ktime_get_real_ns() -
+					       engine->stats.start;
+		spin_unlock_irqrestore(&engine->stats.lock, flags);
+	}
+}
+
 #endif /* _INTEL_RINGBUFFER_H_ */
-- 
2.9.4

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

[RFC 12/14] drm/i915: Interface for controling engine stats collection

[Tvrtko Ursulin]

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

Enables other i915 components to enable and disable
the facility as needed.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c  | 53 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.h |  5 ++++
 2 files changed, 58 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 3e5e08c6b5ef..03e7459bad06 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -29,6 +29,8 @@
 #include "intel_lrc.h"
 
 DEFINE_STATIC_KEY_FALSE(i915_engine_stats_key);
+static DEFINE_MUTEX(i915_engine_stats_mutex);
+static int i915_engine_stats_ref;
 
 /* Haswell does have the CXT_SIZE register however it does not appear to be
  * valid. Now, docs explain in dwords what is in the context object. The full
@@ -1340,6 +1342,57 @@ void intel_engines_mark_idle(struct drm_i915_private *i915)
 	}
 }
 
+int intel_enable_engine_stats(struct drm_i915_private *dev_priv)
+{
+	if (!i915.enable_execlists)
+		return -ENODEV;
+
+	mutex_lock(&i915_engine_stats_mutex);
+	if (i915_engine_stats_ref++ == 0) {
+		struct intel_engine_cs *engine;
+		enum intel_engine_id id;
+
+		for_each_engine(engine, dev_priv, id) {
+			memset(&engine->stats, 0, sizeof(engine->stats));
+			spin_lock_init(&engine->stats.lock);
+		}
+
+		static_branch_enable(&i915_engine_stats_key);
+	}
+	mutex_unlock(&i915_engine_stats_mutex);
+
+	return 0;
+}
+
+void intel_disable_engine_stats(void)
+{
+	mutex_lock(&i915_engine_stats_mutex);
+	if (--i915_engine_stats_ref == 0)
+		static_branch_disable(&i915_engine_stats_key);
+	mutex_unlock(&i915_engine_stats_mutex);
+}
+
+u64 intel_engine_get_current_busy_ns(struct intel_engine_cs *engine)
+{
+	unsigned long flags;
+	u64 total;
+
+	spin_lock_irqsave(&engine->stats.lock, flags);
+
+	total = engine->stats.total;
+
+	/*
+	 * If the engine is executing something at the moment
+	 * add it to the total.
+	 */
+	if (engine->stats.ref)
+		total += ktime_get_real_ns() - engine->stats.start;
+
+	spin_unlock_irqrestore(&engine->stats.lock, flags);
+
+	return total;
+}
+
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftests/mock_engine.c"
 #endif
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 2eb1e970ad06..e0f495a6d0d9 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -776,4 +776,9 @@ static inline void intel_engine_context_out(struct intel_engine_cs *engine)
 	}
 }
 
+int intel_enable_engine_stats(struct drm_i915_private *i915);
+void intel_disable_engine_stats(void);
+
+u64 intel_engine_get_current_busy_ns(struct intel_engine_cs *engine);
+
 #endif /* _INTEL_RINGBUFFER_H_ */
-- 
2.9.4

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

[RFC 13/14] drm/i915: Export engine busy stats in debugfs

[Tvrtko Ursulin]

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

Export the stats added in the previous patch in debugfs.

Number of active clients reading this data is tracked and the
static key is only enabled whilst there are some.

Userspace is intended to keep the file descriptor open, seeking
to the beginning of the file periodically, and re-reading the
stats.

This is because the underlying implementation is costly on every
first open/last close transition, and also, because the stats
exported mostly make sense when they are considered relative to
the previous sample.

File lists nanoseconds each engine was active since the tracking
has started.

v2: Rebase.

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

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2ef75c1a6119..5a425cc225df 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4848,6 +4848,89 @@ static const struct file_operations i915_hpd_storm_ctl_fops = {
 	.write = i915_hpd_storm_ctl_write
 };
 
+struct i915_engine_stats_buf {
+	unsigned int len;
+	size_t available;
+	char buf[0];
+};
+
+static int i915_engine_stats_open(struct inode *inode, struct file *file)
+{
+	struct drm_i915_private *i915 = file->f_inode->i_private;
+	const unsigned int engine_name_len =
+		sizeof(((struct intel_engine_cs *)0)->name);
+	struct i915_engine_stats_buf *buf;
+	const unsigned int buf_size =
+		(engine_name_len + 2 + 19 + 1) * I915_NUM_ENGINES + 1 +
+		sizeof(*buf);
+	int ret;
+
+	buf = kzalloc(buf_size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = intel_enable_engine_stats(i915);
+	if (ret) {
+		kfree(buf);
+		return ret;
+	}
+
+	buf->len = buf_size;
+	file->private_data = buf;
+
+	return 0;
+}
+
+static int i915_engine_stats_release(struct inode *inode, struct file *file)
+{
+	intel_disable_engine_stats();
+	kfree(file->private_data);
+
+	return 0;
+}
+
+static ssize_t i915_engine_stats_read(struct file *file, char __user *ubuf,
+				      size_t count, loff_t *pos)
+{
+	struct i915_engine_stats_buf *buf =
+		(struct i915_engine_stats_buf *)file->private_data;
+
+	if (*pos == 0) {
+		struct drm_i915_private *dev_priv = file->f_inode->i_private;
+		char *ptr = &buf->buf[0];
+		int left = buf->len;
+		struct intel_engine_cs *engine;
+		enum intel_engine_id id;
+
+		buf->available = 0;
+
+		for_each_engine(engine, dev_priv, id) {
+			u64 total = intel_engine_get_current_busy_ns(engine);
+			int len;
+
+			len = snprintf(ptr, left, "%s: %llu\n",
+				       engine->name, total);
+			buf->available += len;
+			left -= len;
+			ptr += len;
+
+			if (len == 0)
+				return -EFBIG;
+		}
+	}
+
+	return simple_read_from_buffer(ubuf, count, pos, &buf->buf[0],
+				       buf->available);
+}
+
+static const struct file_operations i915_engine_stats_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_engine_stats_open,
+	.release = i915_engine_stats_release,
+	.read = i915_engine_stats_read,
+	.llseek = default_llseek,
+};
+
 static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_capabilities", i915_capabilities, 0},
 	{"i915_gem_objects", i915_gem_object_info, 0},
@@ -4938,6 +5021,12 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
 	struct dentry *ent;
 	int ret, i;
 
+	ent = debugfs_create_file("i915_engine_stats", S_IRUGO,
+				  minor->debugfs_root, to_i915(minor->dev),
+				  &i915_engine_stats_fops);
+	if (!ent)
+		return -ENOMEM;
+
 	ent = debugfs_create_file("i915_forcewake_user", S_IRUSR,
 				  minor->debugfs_root, to_i915(minor->dev),
 				  &i915_forcewake_fops);
-- 
2.9.4

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

[RFC 14/14] drm/i915/pmu: Wire up engine busy stats to PMU

[Tvrtko Ursulin]

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

We can use engine busy stats instead of the MMIO sampling timer
for better efficiency.

As minimum this saves period * num_engines / sec mmio reads,
and in a better case, when only engine busy samplers are active,
it enables us to not kick off the sampling timer at all.

It is also more accurate since it doesn't rely on sampling.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  3 ++
 drivers/gpu/drm/i915/i915_pmu.c | 64 +++++++++++++++++++++++++++++++++++++----
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f1fded6dd9cf..2986a01660d8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2603,8 +2603,11 @@ struct drm_i915_private {
 		spinlock_t lock;
 		struct hrtimer timer;
 		bool timer_enabled;
+		bool busy_stats;
 		u64 enable;
 		u64 sample[__I915_NUM_PMU_SAMPLERS];
+		struct work_struct enable_busy_stats;
+		struct delayed_work disable_busy_stats;
 	} pmu;
 
 	/*
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 4b113cad40d1..996612843594 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -29,12 +29,23 @@ static const unsigned int user_engine_map[I915_NUM_ENGINES] = {
 	[I915_SAMPLE_VECS] = VECS,
 };
 
+#define ENGINE_BUSY_BITS \
+	(BIT_ULL(I915_PMU_COUNT_RCS_BUSY) | \
+	 BIT_ULL(I915_PMU_COUNT_BCS_BUSY) | \
+	 BIT_ULL(I915_PMU_COUNT_VCS_BUSY) | \
+	 BIT_ULL(I915_PMU_COUNT_VCS2_BUSY) | \
+	 BIT_ULL(I915_PMU_COUNT_VECS_BUSY))
+
 static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active)
 {
-	if (gpu_active)
-		return i915->pmu.enable;
-	else
-		return i915->pmu.enable >> 32;
+	u64 mask = i915->pmu.enable;
+
+	if (!gpu_active)
+		mask >>= 32;
+	else if (i915->pmu.busy_stats)
+		mask &= ~ENGINE_BUSY_BITS;
+
+	return mask;
 }
 
 void i915_pmu_gt_idle(struct drm_i915_private *i915)
@@ -110,7 +121,8 @@ static void engines_sample(struct drm_i915_private *dev_priv)
 		if (sample_mask & BIT(I915_SAMPLE_QUEUED))
 			engine->pmu_sample[I915_SAMPLE_QUEUED] += PERIOD;
 
-		if (sample_mask & BIT(I915_SAMPLE_BUSY)) {
+		if ((sample_mask & BIT(I915_SAMPLE_BUSY)) &&
+		    !dev_priv->pmu.busy_stats) {
 			fw = grab_forcewake(dev_priv, fw);
 			val = I915_READ_FW(RING_MI_MODE(engine->mmio_base));
 			if (!(val & MODE_IDLE))
@@ -337,6 +349,11 @@ static void i915_pmu_timer_cancel(struct perf_event *event)
 	hrtimer_cancel(&hwc->hrtimer);
 }
 
+static bool supports_busy_stats(void)
+{
+	return i915.enable_execlists;
+}
+
 static void i915_pmu_enable(struct perf_event *event)
 {
 	struct drm_i915_private *i915 =
@@ -345,6 +362,13 @@ static void i915_pmu_enable(struct perf_event *event)
 
 	spin_lock_irqsave(&i915->pmu.lock, flags);
 
+	if (pmu_config_sampler(event->attr.config) == I915_SAMPLE_BUSY &&
+	    supports_busy_stats() && !i915->pmu.busy_stats) {
+		i915->pmu.busy_stats = true;
+		if (!cancel_delayed_work(&i915->pmu.disable_busy_stats))
+			queue_work(i915->wq, &i915->pmu.enable_busy_stats);
+	}
+
 	i915->pmu.enable |= BIT_ULL(event->attr.config);
 	if (pmu_needs_timer(i915, true) && !i915->pmu.timer_enabled) {
 		hrtimer_start_range_ns(&i915->pmu.timer,
@@ -367,6 +391,11 @@ static void i915_pmu_disable(struct perf_event *event)
 	spin_lock_irqsave(&i915->pmu.lock, flags);
 	i915->pmu.enable &= ~BIT_ULL(event->attr.config);
 	i915->pmu.timer_enabled &= pmu_needs_timer(i915, true);
+	if (!(i915->pmu.enable & ENGINE_BUSY_BITS) && i915->pmu.busy_stats) {
+		i915->pmu.busy_stats = false;
+		queue_delayed_work(i915->wq, &i915->pmu.disable_busy_stats,
+				   round_jiffies_up_relative(2 * HZ));
+	}
 	spin_unlock_irqrestore(&i915->pmu.lock, flags);
 
 	i915_pmu_timer_cancel(event);
@@ -471,7 +500,12 @@ static void i915_pmu_event_read(struct perf_event *event)
 			/* Do nothing */
 		} else {
 			enum intel_engine_id id = user_engine_map[user_engine];
-			val = i915->engine[id]->pmu_sample[sample];
+			struct intel_engine_cs *engine = i915->engine[id];
+
+			if (i915->pmu.busy_stats && sample == I915_SAMPLE_BUSY)
+				val = intel_engine_get_current_busy_ns(engine);
+			else
+				val = engine->pmu_sample[sample];
 		}
 	} else switch (event->attr.config) {
 	case I915_PMU_ACTUAL_FREQUENCY:
@@ -607,6 +641,19 @@ static const struct attribute_group *i915_pmu_attr_groups[] = {
         NULL
 };
 
+static void __enable_busy_stats(struct work_struct *work)
+{
+	struct drm_i915_private *i915 =
+		container_of(work, typeof(*i915), pmu.enable_busy_stats);
+
+	WARN_ON_ONCE(intel_enable_engine_stats(i915));
+}
+
+static void __disable_busy_stats(struct work_struct *work)
+{
+	intel_disable_engine_stats();
+}
+
 void i915_pmu_register(struct drm_i915_private *i915)
 {
 	if (INTEL_GEN(i915) <= 2)
@@ -624,6 +671,8 @@ void i915_pmu_register(struct drm_i915_private *i915)
 
 	spin_lock_init(&i915->pmu.lock);
 	hrtimer_init(&i915->pmu.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	INIT_WORK(&i915->pmu.enable_busy_stats, __enable_busy_stats);
+	INIT_DELAYED_WORK(&i915->pmu.disable_busy_stats, __disable_busy_stats);
 	i915->pmu.timer.function = i915_sample;
 	i915->pmu.enable = 0;
 
@@ -642,4 +691,7 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
 	i915->pmu.base.event_init = NULL;
 
 	hrtimer_cancel(&i915->pmu.timer);
+
+	flush_work(&i915->pmu.enable_busy_stats);
+	flush_delayed_work(&i915->pmu.disable_busy_stats);
 }
-- 
2.9.4

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

✓ Fi.CI.BAT: success for i915 PMU and engine busy stats

[Patchwork]

== Series Details ==

Series: i915 PMU and engine busy stats
URL   : https://patchwork.freedesktop.org/series/27488/
State : success

== Summary ==

Series 27488v1 i915 PMU and engine busy stats
https://patchwork.freedesktop.org/api/1.0/series/27488/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-n2820) fdo#101705

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:443s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:431s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:357s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:525s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:516s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:494s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:481s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:595s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:432s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:411s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:411s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:500s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:473s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:463s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:570s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:578s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:562s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:456s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:585s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:472s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:473s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:439s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:475s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:537s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:405s

10de1e17faaab452782e5a1baffd1b30a639a261 drm-tip: 2017y-07m-18d-10h-08m-42s UTC integration manifest
6601a6c drm/i915/pmu: Wire up engine busy stats to PMU
7d7d26e drm/i915: Export engine busy stats in debugfs
f4fddc8 drm/i915: Interface for controling engine stats collection
559f3b2 drm/i915: Engine busy time tracking
d02886e drm/i915: Wrap context schedule notification
cdcdd9f drm/i915/pmu: Suspend sampling when GPU is idle
c436b20 drm/i915/pmu: Expose events in sysfs
c0efa43 drm/i915/pmu: Add fake regs
da03ba4 drm/i915/pmu: Only sample enabled samplers
afea896 drm/i915/pmu: Helper to extract engine and sampler from PMU config
2ff9a43 drm/i915/pmu: Decouple uAPI engine ids
8a3ac8e drm/i915/pmu: Add queued samplers to the PMU uAPI
278c42b drm/i915/pmu: Add VCS2 engine to the PMU uAPI
dc1f338 RFC drm/i915: Expose a PMU interface for perf queries

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5221/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 11/14] drm/i915: Engine busy time tracking

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-07-18 15:36:15)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Track total time requests have been executing on the hardware.
> 
> To make this cheap it is hidden behind a static branch with the
> intention that it is only enabled when there is a consumer
> listening. This means that in the default off case the total
> cost of the tracking is just a few no-op instructions on the
> fast paths.

> +static inline void intel_engine_context_in(struct intel_engine_cs *engine)
> +{
> +       if (static_branch_unlikely(&i915_engine_stats_key)) {
> +               unsigned long flags;
> +
> +               spin_lock_irqsave(&engine->stats.lock, flags);

What's the purpose of this lock? RMW is ordered by virtue of the tasklet
(only one cpu can be doing the rmw at any time). Did I miss another
user?

> +               if (engine->stats.ref++ == 0)
> +                       engine->stats.start = ktime_get_real_ns();

Use ktime_get_raw() and leave the conversion to ns to the caller.
What is the cost of a ktime nowadays?

How do you handle the currently active case when reporting? Imagine a
continuous stream of requests, hoping between contexts without a
break.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 12/14] drm/i915: Interface for controling engine stats collection

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-07-18 15:36:16)
> +u64 intel_engine_get_current_busy_ns(struct intel_engine_cs *engine)
> +{
> +       unsigned long flags;
> +       u64 total;
> +
> +       spin_lock_irqsave(&engine->stats.lock, flags);
> +
> +       total = engine->stats.total;
> +
> +       /*
> +        * If the engine is executing something at the moment
> +        * add it to the total.
> +        */
> +       if (engine->stats.ref)
> +               total += ktime_get_real_ns() - engine->stats.start;
> +
> +       spin_unlock_irqrestore(&engine->stats.lock, flags);

Answers to another patch found here. I would say this is the other half
of the interface and should be kept together.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 12/14] drm/i915: Interface for controling engine stats collection

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-07-18 15:36:16)
> +int intel_enable_engine_stats(struct drm_i915_private *dev_priv)
> +{
> +       if (!i915.enable_execlists)
> +               return -ENODEV;
> +
> +       mutex_lock(&i915_engine_stats_mutex);
> +       if (i915_engine_stats_ref++ == 0) {
> +               struct intel_engine_cs *engine;
> +               enum intel_engine_id id;
> +
> +               for_each_engine(engine, dev_priv, id) {
> +                       memset(&engine->stats, 0, sizeof(engine->stats));
> +                       spin_lock_init(&engine->stats.lock);
> +               }
> +
> +               static_branch_enable(&i915_engine_stats_key);
> +       }
> +       mutex_unlock(&i915_engine_stats_mutex);

I don't think static_branch_enable() is a might_sleep, so it looks like
you can rewrite this avoiding the mutex and thus not requiring the
worker and then can use the error code here to decide if you need to
use the timer instead.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 12/14] drm/i915: Interface for controling engine stats collection

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-07-18 15:36:16)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Enables other i915 components to enable and disable
> the facility as needed.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c  | 53 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  5 ++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 3e5e08c6b5ef..03e7459bad06 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -29,6 +29,8 @@
>  #include "intel_lrc.h"
>  
>  DEFINE_STATIC_KEY_FALSE(i915_engine_stats_key);
> +static DEFINE_MUTEX(i915_engine_stats_mutex);
> +static int i915_engine_stats_ref;
>  
>  /* Haswell does have the CXT_SIZE register however it does not appear to be
>   * valid. Now, docs explain in dwords what is in the context object. The full
> @@ -1340,6 +1342,57 @@ void intel_engines_mark_idle(struct drm_i915_private *i915)
>         }
>  }
>  
> +int intel_enable_engine_stats(struct drm_i915_private *dev_priv)

The pattern I've been trying to use here is

	intel_engine_* - operate on the named engine

	intel_engines_* - operate on all engines

Long term though having a global static key is going to be a nasty wart.
Joonas will definitely ask the question how much will it cost us to use
an engine->bool and what we can do to minimise that cost.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 11/14] drm/i915: Engine busy time tracking

[Tvrtko Ursulin]

On 18/07/2017 16:19, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-07-18 15:36:15)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Track total time requests have been executing on the hardware.
>>
>> To make this cheap it is hidden behind a static branch with the
>> intention that it is only enabled when there is a consumer
>> listening. This means that in the default off case the total
>> cost of the tracking is just a few no-op instructions on the
>> fast paths.
> 
>> +static inline void intel_engine_context_in(struct intel_engine_cs *engine)
>> +{
>> +       if (static_branch_unlikely(&i915_engine_stats_key)) {
>> +               unsigned long flags;
>> +
>> +               spin_lock_irqsave(&engine->stats.lock, flags);
> 
> What's the purpose of this lock? RMW is ordered by virtue of the tasklet
> (only one cpu can be doing the rmw at any time). Did I miss another
> user?

Hm it should be a plain spin_lock and a _bh variant on the external API.

But the purpose is to allow atomic sampling of accumulated busy time 
plus the current engine status.

>> +               if (engine->stats.ref++ == 0)
>> +                       engine->stats.start = ktime_get_real_ns();
> 
> Use ktime_get_raw() and leave the conversion to ns to the caller.

You mean both store in ktime_t and don't fetch the wall time but 
monotonic? Do you say this because perf pmu perhaps needs monotonic or 
for some other reason?

> What is the cost of a ktime nowadays?

I don't see it in the profiles, so not much I think.

Regards,

Tvrtko

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

Re: [RFC 12/14] drm/i915: Interface for controling engine stats collection

[Tvrtko Ursulin]

On 18/07/2017 16:22, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-07-18 15:36:16)
>> +u64 intel_engine_get_current_busy_ns(struct intel_engine_cs *engine)
>> +{
>> +       unsigned long flags;
>> +       u64 total;
>> +
>> +       spin_lock_irqsave(&engine->stats.lock, flags);
>> +
>> +       total = engine->stats.total;
>> +
>> +       /*
>> +        * If the engine is executing something at the moment
>> +        * add it to the total.
>> +        */
>> +       if (engine->stats.ref)
>> +               total += ktime_get_real_ns() - engine->stats.start;
>> +
>> +       spin_unlock_irqrestore(&engine->stats.lock, flags);
> 
> Answers to another patch found here. I would say this is the other half
> of the interface and should be kept together.

Yes, it was an ugly split.

On 18/07/2017 16:43, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-07-18 15:36:16)
>> +int intel_enable_engine_stats(struct drm_i915_private *dev_priv)
>> +{
>> +       if (!i915.enable_execlists)
>> +               return -ENODEV;
>> +
>> +       mutex_lock(&i915_engine_stats_mutex);
>> +       if (i915_engine_stats_ref++ == 0) {
>> +               struct intel_engine_cs *engine;
>> +               enum intel_engine_id id;
>> +
>> +               for_each_engine(engine, dev_priv, id) {
>> +                       memset(&engine->stats, 0, sizeof(engine->stats));
>> +                       spin_lock_init(&engine->stats.lock);
>> +               }
>> +
>> +               static_branch_enable(&i915_engine_stats_key);
>> +       }
>> +       mutex_unlock(&i915_engine_stats_mutex);
> 
> I don't think static_branch_enable() is a might_sleep, so it looks like
> you can rewrite this avoiding the mutex and thus not requiring the
> worker and then can use the error code here to decide if you need to
> use the timer instead.

Perhaps I could get rid of the mutex though by using atomic_inc/dec_return.

But there is a mutex in jump label handling, so I think the workers have 
to stay - and it is also beneficial to have delayed static branch disable,
since the perf core seems to like calling start/stop on the events a lot.
But it is recommended practice for static branches in any way.

So from that angle I could perhaps even move the delayed disable to this
patch so it is automatically shared by all callers.

>> +static DEFINE_MUTEX(i915_engine_stats_mutex);
>> +static int i915_engine_stats_ref;
>>   
>>   /* Haswell does have the CXT_SIZE register however it does not appear to be
>>    * valid. Now, docs explain in dwords what is in the context object. The full
>> @@ -1340,6 +1342,57 @@ void intel_engines_mark_idle(struct drm_i915_private *i915)
>>          }
>>   }
>>   
>> +int intel_enable_engine_stats(struct drm_i915_private *dev_priv)
> 
> The pattern I've been trying to use here is
> 
> 	intel_engine_* - operate on the named engine
> 
> 	intel_engines_* - operate on all engines

Ok.
 
> Long term though having a global static key is going to be a nasty wart.
> Joonas will definitely ask the question how much will it cost us to use
> an engine->bool and what we can do to minimise that cost.

Why you think it is nasty? Sounds pretty cool to me.

But I think can re-organize the series to start with a normal branch and
then add the static one on top if so is desired.

Regards,

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

Re: [RFC 12/14] drm/i915: Interface for controling engine stats collection

[Tvrtko Ursulin]

Hi Ben,

On 18/07/2017 15:36, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Enables other i915 components to enable and disable
> the facility as needed.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_engine_cs.c  | 53 +++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_ringbuffer.h |  5 ++++
>   2 files changed, 58 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 3e5e08c6b5ef..03e7459bad06 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -29,6 +29,8 @@
>   #include "intel_lrc.h"
>   
>   DEFINE_STATIC_KEY_FALSE(i915_engine_stats_key);
> +static DEFINE_MUTEX(i915_engine_stats_mutex);
> +static int i915_engine_stats_ref;
>   
>   /* Haswell does have the CXT_SIZE register however it does not appear to be
>    * valid. Now, docs explain in dwords what is in the context object. The full
> @@ -1340,6 +1342,57 @@ void intel_engines_mark_idle(struct drm_i915_private *i915)
>   	}
>   }
>   
> +int intel_enable_engine_stats(struct drm_i915_private *dev_priv)
> +{
> +	if (!i915.enable_execlists)
> +		return -ENODEV;
> +
> +	mutex_lock(&i915_engine_stats_mutex);
> +	if (i915_engine_stats_ref++ == 0) {
> +		struct intel_engine_cs *engine;
> +		enum intel_engine_id id;
> +
> +		for_each_engine(engine, dev_priv, id) {
> +			memset(&engine->stats, 0, sizeof(engine->stats));
> +			spin_lock_init(&engine->stats.lock);
> +		}
> +
> +		static_branch_enable(&i915_engine_stats_key);
> +	}
> +	mutex_unlock(&i915_engine_stats_mutex);
> +
> +	return 0;
> +}
> +
> +void intel_disable_engine_stats(void)
> +{
> +	mutex_lock(&i915_engine_stats_mutex);
> +	if (--i915_engine_stats_ref == 0)
> +		static_branch_disable(&i915_engine_stats_key);
> +	mutex_unlock(&i915_engine_stats_mutex);
> +}
> +
> +u64 intel_engine_get_current_busy_ns(struct intel_engine_cs *engine)
> +{
> +	unsigned long flags;
> +	u64 total;
> +
> +	spin_lock_irqsave(&engine->stats.lock, flags);
> +
> +	total = engine->stats.total;
> +
> +	/*
> +	 * If the engine is executing something at the moment
> +	 * add it to the total.
> +	 */
> +	if (engine->stats.ref)
> +		total += ktime_get_real_ns() - engine->stats.start;
> +
> +	spin_unlock_irqrestore(&engine->stats.lock, flags);
> +
> +	return total;
> +}
> +
>   #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
>   #include "selftests/mock_engine.c"
>   #endif
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 2eb1e970ad06..e0f495a6d0d9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -776,4 +776,9 @@ static inline void intel_engine_context_out(struct intel_engine_cs *engine)
>   	}
>   }
>   
> +int intel_enable_engine_stats(struct drm_i915_private *i915);
> +void intel_disable_engine_stats(void);
> +
> +u64 intel_engine_get_current_busy_ns(struct intel_engine_cs *engine);

If we exported these symbols for other modules to use, what kind of API 
would they need? Presumably not per-engine but something to give the 
aggregated busyness of all engines? Or I have misunderstood you that 
there is this requirement?

Regards,

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

Re: [RFC 01/14] RFC drm/i915: Expose a PMU interface for perf queries

[Kamble, Sagar A]

Can we reuse calc_residency defined in i915_sysfs.c


On 7/18/2017 8:06 PM, Tvrtko Ursulin wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> The first goal is to be able to measure GPU (and invidual ring) busyness
> without having to poll registers from userspace. (Which not only incurs
> holding the forcewake lock indefinitely, perturbing the system, but also
> runs the risk of hanging the machine.) As an alternative we can use the
> perf event counter interface to sample the ring registers periodically
> and send those results to userspace.
>
> To be able to do so, we need to export the two symbols from
> kernel/events/core.c to register and unregister a PMU device.
>
> v2: Use a common timer for the ring sampling.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/Makefile           |   1 +
>   drivers/gpu/drm/i915/i915_drv.c         |   2 +
>   drivers/gpu/drm/i915/i915_drv.h         |  23 ++
>   drivers/gpu/drm/i915/i915_pmu.c         | 452 ++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_ringbuffer.h |   2 +
>   include/uapi/drm/i915_drm.h             |  41 +++
>   kernel/events/core.c                    |   1 +
>   7 files changed, 522 insertions(+)
>   create mode 100644 drivers/gpu/drm/i915/i915_pmu.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index f8227318dcaf..1c720013dc42 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -26,6 +26,7 @@ i915-y := i915_drv.o \
>   
>   i915-$(CONFIG_COMPAT)   += i915_ioc32.o
>   i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o
> +i915-$(CONFIG_PERF_EVENTS) += i915_pmu.o
>   
>   # GEM code
>   i915-y += i915_cmd_parser.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index d310d8245dca..f18ce519f6a2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1194,6 +1194,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
>   	struct drm_device *dev = &dev_priv->drm;
>   
>   	i915_gem_shrinker_init(dev_priv);
> +	i915_pmu_register(dev_priv);
>   
>   	/*
>   	 * Notify a valid surface after modesetting,
> @@ -1247,6 +1248,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>   	intel_opregion_unregister(dev_priv);
>   
>   	i915_perf_unregister(dev_priv);
> +	i915_pmu_unregister(dev_priv);
>   
>   	i915_teardown_sysfs(dev_priv);
>   	i915_guc_log_unregister(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7c6fab08a2e6..de518503e033 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -40,6 +40,7 @@
>   #include <linux/hash.h>
>   #include <linux/intel-iommu.h>
>   #include <linux/kref.h>
> +#include <linux/perf_event.h>
>   #include <linux/pm_qos.h>
>   #include <linux/reservation.h>
>   #include <linux/shmem_fs.h>
> @@ -2093,6 +2094,12 @@ struct intel_cdclk_state {
>   	unsigned int cdclk, vco, ref;
>   };
>   
> +enum {
> +	__I915_SAMPLE_FREQ_ACT = 0,
> +	__I915_SAMPLE_FREQ_REQ,
> +	__I915_NUM_PMU_SAMPLERS
> +};
> +
>   struct drm_i915_private {
>   	struct drm_device drm;
>   
> @@ -2591,6 +2598,13 @@ struct drm_i915_private {
>   		int	irq;
>   	} lpe_audio;
>   
> +	struct {
> +		struct pmu base;
> +		struct hrtimer timer;
> +		u64 enable;
> +		u64 sample[__I915_NUM_PMU_SAMPLERS];
> +	} pmu;
> +
>   	/*
>   	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>   	 * will be rejected. Instead look for a better place.
> @@ -3760,6 +3774,15 @@ extern void i915_perf_fini(struct drm_i915_private *dev_priv);
>   extern void i915_perf_register(struct drm_i915_private *dev_priv);
>   extern void i915_perf_unregister(struct drm_i915_private *dev_priv);
>   
> +/* i915_pmu.c */
> +#ifdef CONFIG_PERF_EVENTS
> +extern void i915_pmu_register(struct drm_i915_private *i915);
> +extern void i915_pmu_unregister(struct drm_i915_private *i915);
> +#else
> +static inline void i915_pmu_register(struct drm_i915_private *i915) {}
> +static inline void i915_pmu_unregister(struct drm_i915_private *i915) {}
> +#endif
> +
>   /* i915_suspend.c */
>   extern int i915_save_state(struct drm_i915_private *dev_priv);
>   extern int i915_restore_state(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> new file mode 100644
> index 000000000000..f03ddad44da6
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -0,0 +1,452 @@
> +#include <linux/perf_event.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "i915_drv.h"
> +#include "intel_ringbuffer.h"
> +
> +#define FREQUENCY 200
> +#define PERIOD max_t(u64, 10000, NSEC_PER_SEC / FREQUENCY)
> +
> +#define RING_MASK 0xffffffff
> +#define RING_MAX 32
> +
> +static void engines_sample(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_engine_cs *engine;
> +	enum intel_engine_id id;
> +	bool fw = false;
> +
> +	if ((dev_priv->pmu.enable & RING_MASK) == 0)
> +		return;
> +
> +	if (!dev_priv->gt.awake)
> +		return;
> +
> +	if (!intel_runtime_pm_get_if_in_use(dev_priv))
> +		return;
> +
> +	for_each_engine(engine, dev_priv, id) {
> +		u32 val;
> +
> +		if ((dev_priv->pmu.enable & (0x7 << (4*id))) == 0)
> +			continue;
> +
> +		if (i915_seqno_passed(intel_engine_get_seqno(engine),
> +				      intel_engine_last_submit(engine)))
> +			continue;
> +
> +		if (!fw) {
> +			intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> +			fw = true;
> +		}
> +
> +		engine->pmu_sample[I915_SAMPLE_QUEUED] += PERIOD;
> +
> +		val = I915_READ_FW(RING_MI_MODE(engine->mmio_base));
> +		if (!(val & MODE_IDLE))
> +			engine->pmu_sample[I915_SAMPLE_BUSY] += PERIOD;
> +
> +		val = I915_READ_FW(RING_CTL(engine->mmio_base));
> +		if (val & RING_WAIT)
> +			engine->pmu_sample[I915_SAMPLE_WAIT] += PERIOD;
> +		if (val & RING_WAIT_SEMAPHORE)
> +			engine->pmu_sample[I915_SAMPLE_SEMA] += PERIOD;
> +	}
> +
> +	if (fw)
> +		intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> +	intel_runtime_pm_put(dev_priv);
> +}
> +
> +static void frequency_sample(struct drm_i915_private *dev_priv)
> +{
> +	if (dev_priv->pmu.enable & BIT_ULL(I915_PMU_ACTUAL_FREQUENCY)) {
> +		u64 val;
> +
> +		val = dev_priv->rps.cur_freq;
> +		if (dev_priv->gt.awake &&
> +		    intel_runtime_pm_get_if_in_use(dev_priv)) {
> +			val = I915_READ_NOTRACE(GEN6_RPSTAT1);
> +			if (INTEL_GEN(dev_priv) >= 9)
> +				val = (val & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
> +			else if (IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8)
> +				val = (val & HSW_CAGF_MASK) >> HSW_CAGF_SHIFT;
> +			else
> +				val = (val & GEN6_CAGF_MASK) >> GEN6_CAGF_SHIFT;
> +			intel_runtime_pm_put(dev_priv);
> +		}
> +		val = intel_gpu_freq(dev_priv, val);
> +		dev_priv->pmu.sample[__I915_SAMPLE_FREQ_ACT] += val * PERIOD;
> +	}
> +
> +	if (dev_priv->pmu.enable & BIT_ULL(I915_PMU_REQUESTED_FREQUENCY)) {
> +		u64 val = intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq);
> +		dev_priv->pmu.sample[__I915_SAMPLE_FREQ_REQ] += val * PERIOD;
> +	}
> +}
> +
> +static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
> +{
> +	struct drm_i915_private *i915 =
> +		container_of(hrtimer, struct drm_i915_private, pmu.timer);
> +
> +	if (i915->pmu.enable == 0)
> +		return HRTIMER_NORESTART;
> +
> +	engines_sample(i915);
> +	frequency_sample(i915);
> +
> +	hrtimer_forward_now(hrtimer, ns_to_ktime(PERIOD));
> +	return HRTIMER_RESTART;
> +}
> +
> +static void i915_pmu_event_destroy(struct perf_event *event)
> +{
> +	WARN_ON(event->parent);
> +}
> +
> +static int engine_event_init(struct perf_event *event)
> +{
> +	struct drm_i915_private *i915 =
> +		container_of(event->pmu, typeof(*i915), pmu.base);
> +	int engine = event->attr.config >> 2;
> +	int sample = event->attr.config & 3;
> +
> +	switch (sample) {
> +	case I915_SAMPLE_QUEUED:
> +	case I915_SAMPLE_BUSY:
> +	case I915_SAMPLE_WAIT:
> +		break;
> +	case I915_SAMPLE_SEMA:
> +		if (INTEL_GEN(i915) < 6)
> +			return -ENODEV;
> +		break;
> +	default:
> +		return -ENOENT;
> +	}
> +
> +	if (engine >= I915_NUM_ENGINES)
> +		return -ENOENT;
> +
> +	if (!i915->engine[engine])
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +static enum hrtimer_restart hrtimer_sample(struct hrtimer *hrtimer)
> +{
> +	struct perf_sample_data data;
> +	struct perf_event *event;
> +	u64 period;
> +
> +	event = container_of(hrtimer, struct perf_event, hw.hrtimer);
> +	if (event->state != PERF_EVENT_STATE_ACTIVE)
> +		return HRTIMER_NORESTART;
> +
> +	event->pmu->read(event);
> +
> +	perf_sample_data_init(&data, 0, event->hw.last_period);
> +	perf_event_overflow(event, &data, NULL);
> +
> +	period = max_t(u64, 10000, event->hw.sample_period);
> +	hrtimer_forward_now(hrtimer, ns_to_ktime(period));
> +	return HRTIMER_RESTART;
> +}
> +
> +static void init_hrtimer(struct perf_event *event)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	if (!is_sampling_event(event))
> +		return;
> +
> +	hrtimer_init(&hwc->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +	hwc->hrtimer.function = hrtimer_sample;
> +
> +	if (event->attr.freq) {
> +		long freq = event->attr.sample_freq;
> +
> +		event->attr.sample_period = NSEC_PER_SEC / freq;
> +		hwc->sample_period = event->attr.sample_period;
> +		local64_set(&hwc->period_left, hwc->sample_period);
> +		hwc->last_period = hwc->sample_period;
> +		event->attr.freq = 0;
> +	}
> +}
> +
> +static int i915_pmu_event_init(struct perf_event *event)
> +{
> +	struct drm_i915_private *i915 =
> +		container_of(event->pmu, typeof(*i915), pmu.base);
> +	int ret;
> +
> +	/* XXX ideally only want pid == -1 && cpu == -1 */
> +
> +	if (event->attr.type != event->pmu->type)
> +		return -ENOENT;
> +
> +	if (has_branch_stack(event))
> +		return -EOPNOTSUPP;
> +
> +	ret = 0;
> +	if (event->attr.config < RING_MAX) {
> +		ret = engine_event_init(event);
> +	} else switch (event->attr.config) {
> +	case I915_PMU_ACTUAL_FREQUENCY:
> +		if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
> +			ret = -ENODEV; /* requires a mutex for sampling! */
> +	case I915_PMU_REQUESTED_FREQUENCY:
> +	case I915_PMU_ENERGY:
> +	case I915_PMU_RC6_RESIDENCY:
> +	case I915_PMU_RC6p_RESIDENCY:
> +	case I915_PMU_RC6pp_RESIDENCY:
> +		if (INTEL_GEN(i915) < 6)
> +			ret = -ENODEV;
> +		break;
> +	}
> +	if (ret)
> +		return ret;
> +
> +	if (!event->parent)
> +		event->destroy = i915_pmu_event_destroy;
> +
> +	init_hrtimer(event);
> +
> +	return 0;
> +}
> +
> +static void i915_pmu_timer_start(struct perf_event *event)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	s64 period;
> +
> +	if (!is_sampling_event(event))
> +		return;
> +
> +	period = local64_read(&hwc->period_left);
> +	if (period) {
> +		if (period < 0)
> +			period = 10000;
> +
> +		local64_set(&hwc->period_left, 0);
> +	} else {
> +		period = max_t(u64, 10000, hwc->sample_period);
> +	}
> +
> +	hrtimer_start_range_ns(&hwc->hrtimer,
> +			       ns_to_ktime(period), 0,
> +			       HRTIMER_MODE_REL_PINNED);
> +}
> +
> +static void i915_pmu_timer_cancel(struct perf_event *event)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	if (!is_sampling_event(event))
> +		return;
> +
> +	local64_set(&hwc->period_left,
> +		    ktime_to_ns(hrtimer_get_remaining(&hwc->hrtimer)));
> +	hrtimer_cancel(&hwc->hrtimer);
> +}
> +
> +static void i915_pmu_enable(struct perf_event *event)
> +{
> +	struct drm_i915_private *i915 =
> +		container_of(event->pmu, typeof(*i915), pmu.base);
> +
> +	if (i915->pmu.enable == 0)
> +		hrtimer_start_range_ns(&i915->pmu.timer,
> +				       ns_to_ktime(PERIOD), 0,
> +				       HRTIMER_MODE_REL_PINNED);
> +
> +	i915->pmu.enable |= BIT_ULL(event->attr.config);
> +
> +	i915_pmu_timer_start(event);
> +}
> +
> +static void i915_pmu_disable(struct perf_event *event)
> +{
> +	struct drm_i915_private *i915 =
> +		container_of(event->pmu, typeof(*i915), pmu.base);
> +
> +	i915->pmu.enable &= ~BIT_ULL(event->attr.config);
> +	i915_pmu_timer_cancel(event);
> +}
> +
> +static int i915_pmu_event_add(struct perf_event *event, int flags)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	if (flags & PERF_EF_START)
> +		i915_pmu_enable(event);
> +
> +	hwc->state = !(flags & PERF_EF_START);
> +
> +	return 0;
> +}
> +
> +static void i915_pmu_event_del(struct perf_event *event, int flags)
> +{
> +	i915_pmu_disable(event);
> +}
> +
> +static void i915_pmu_event_start(struct perf_event *event, int flags)
> +{
> +	i915_pmu_enable(event);
> +}
> +
> +static void i915_pmu_event_stop(struct perf_event *event, int flags)
> +{
> +	i915_pmu_disable(event);
> +}
> +
> +static u64 read_energy_uJ(struct drm_i915_private *dev_priv)
> +{
> +	u64 power;
> +
> +	GEM_BUG_ON(INTEL_GEN(dev_priv) < 6);
> +
> +	intel_runtime_pm_get(dev_priv);
> +
> +	rdmsrl(MSR_RAPL_POWER_UNIT, power);
> +	power = (power & 0x1f00) >> 8;
> +	power = 1000000 >> power; /* convert to uJ */
> +	power *= I915_READ_NOTRACE(MCH_SECP_NRG_STTS);
> +
> +	intel_runtime_pm_put(dev_priv);
> +
> +	return power;
> +}
> +
> +static inline u64 calc_residency(struct drm_i915_private *dev_priv,
> +				 const i915_reg_t reg)
> +{
> +	u64 val, units = 128, div = 100000;
> +
> +	GEM_BUG_ON(INTEL_GEN(dev_priv) < 6);
> +
> +	intel_runtime_pm_get(dev_priv);
> +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> +		div = dev_priv->czclk_freq;
> +		units = 1;
> +		if (I915_READ_NOTRACE(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH)
> +			units <<= 8;
> +	} else if (IS_GEN9_LP(dev_priv)) {
> +		div = 1200;
> +		units = 1;
> +	}
> +	val = I915_READ_NOTRACE(reg);
> +	intel_runtime_pm_put(dev_priv);
> +
> +	val *= units;
> +	return DIV_ROUND_UP_ULL(val, div);
> +}
> +
> +static u64 count_interrupts(struct drm_i915_private *i915)
> +{
> +	/* open-coded kstat_irqs() */
> +	struct irq_desc *desc = irq_to_desc(i915->drm.pdev->irq);
> +	u64 sum = 0;
> +	int cpu;
> +
> +	if (!desc || !desc->kstat_irqs)
> +		return 0;
> +
> +	for_each_possible_cpu(cpu)
> +		sum += *per_cpu_ptr(desc->kstat_irqs, cpu);
> +
> +	return sum;
> +}
> +
> +static void i915_pmu_event_read(struct perf_event *event)
> +{
> +	struct drm_i915_private *i915 =
> +		container_of(event->pmu, typeof(*i915), pmu.base);
> +	u64 val = 0;
> +
> +	if (event->attr.config < 32) {
> +		int engine = event->attr.config >> 2;
> +		int sample = event->attr.config & 3;
> +		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];
> +		break;
> +	case I915_PMU_REQUESTED_FREQUENCY:
> +		val = i915->pmu.sample[__I915_SAMPLE_FREQ_REQ];
> +		break;
> +	case I915_PMU_ENERGY:
> +		val = read_energy_uJ(i915);
> +		break;
> +	case I915_PMU_INTERRUPTS:
> +		val = count_interrupts(i915);
> +		break;
> +
> +	case I915_PMU_RC6_RESIDENCY:
> +		if (!i915->gt.awake)
> +			return;
> +
> +		val = calc_residency(i915, IS_VALLEYVIEW(i915) ? VLV_GT_RENDER_RC6 : GEN6_GT_GFX_RC6);
> +		break;
> +
> +	case I915_PMU_RC6p_RESIDENCY:
> +		if (!i915->gt.awake)
> +			return;
> +
> +		if (!IS_VALLEYVIEW(i915))
> +			val = calc_residency(i915, GEN6_GT_GFX_RC6p);
> +		break;
> +
> +	case I915_PMU_RC6pp_RESIDENCY:
> +		if (!i915->gt.awake)
> +			return;
> +
> +		if (!IS_VALLEYVIEW(i915))
> +			val = calc_residency(i915, GEN6_GT_GFX_RC6pp);
> +		break;
> +	}
> +
> +	local64_set(&event->count, val);
> +}
> +
> +static int i915_pmu_event_event_idx(struct perf_event *event)
> +{
> +	return 0;
> +}
> +
> +void i915_pmu_register(struct drm_i915_private *i915)
> +{
> +	if (INTEL_GEN(i915) <= 2)
> +		return;
> +
> +	i915->pmu.base.task_ctx_nr	= perf_sw_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;
> +
> +	hrtimer_init(&i915->pmu.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +	i915->pmu.timer.function = i915_sample;
> +	i915->pmu.enable = 0;
> +
> +	if (perf_pmu_register(&i915->pmu.base, "i915", -1))
> +		i915->pmu.base.event_init = NULL;
> +}
> +
> +void i915_pmu_unregister(struct drm_i915_private *i915)
> +{
> +	if (!i915->pmu.base.event_init)
> +		return;
> +
> +	i915->pmu.enable = 0;
> +
> +	perf_pmu_unregister(&i915->pmu.base);
> +	i915->pmu.base.event_init = NULL;
> +
> +	hrtimer_cancel(&i915->pmu.timer);
> +}
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index d33c93444c0d..0877b151239d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -245,6 +245,8 @@ struct intel_engine_cs {
>   		I915_SELFTEST_DECLARE(bool mock : 1);
>   	} breadcrumbs;
>   
> +	u64 pmu_sample[4];
> +
>   	/*
>   	 * A pool of objects to use as shadow copies of client batch buffers
>   	 * when the command parser is enabled. Prevents the client from
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 7ccbd6a2bbe0..733774f19a0b 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -86,6 +86,47 @@ enum i915_mocs_table_index {
>   	I915_MOCS_CACHED,
>   };
>   
> +/**
> + * DOC: perf_events exposed by i915 through /sys/bus/event_sources/drivers/i915
> + *
> + */
> +#define I915_SAMPLE_QUEUED	0
> +#define I915_SAMPLE_BUSY	1
> +#define I915_SAMPLE_WAIT	2
> +#define I915_SAMPLE_SEMA	3
> +
> +#define I915_SAMPLE_RCS		0
> +#define I915_SAMPLE_VCS		1
> +#define I915_SAMPLE_BCS		2
> +#define I915_SAMPLE_VECS	3
> +
> +#define __I915_PMU_COUNT(ring, id) ((ring) << 2 | (id))
> +
> +#define I915_PMU_COUNT_RCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_RCS, I915_SAMPLE_BUSY)
> +#define I915_PMU_COUNT_RCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_RCS, I915_SAMPLE_WAIT)
> +#define I915_PMU_COUNT_RCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_RCS, I915_SAMPLE_SEMA)
> +
> +#define I915_PMU_COUNT_VCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_BUSY)
> +#define I915_PMU_COUNT_VCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_WAIT)
> +#define I915_PMU_COUNT_VCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_SEMA)
> +
> +#define I915_PMU_COUNT_BCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_BUSY)
> +#define I915_PMU_COUNT_BCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_WAIT)
> +#define I915_PMU_COUNT_BCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_SEMA)
> +
> +#define I915_PMU_COUNT_VECS_BUSY __I915_PMU_COUNT(I915_SAMPLE_VECS, I915_SAMPLE_BUSY)
> +#define I915_PMU_COUNT_VECS_WAIT __I915_PMU_COUNT(I915_SAMPLE_VECS, I915_SAMPLE_WAIT)
> +#define I915_PMU_COUNT_VECS_SEMA __I915_PMU_COUNT(I915_SAMPLE_VECS, I915_SAMPLE_SEMA)
> +
> +#define I915_PMU_ACTUAL_FREQUENCY 32
> +#define I915_PMU_REQUESTED_FREQUENCY 33
> +#define I915_PMU_ENERGY 34
> +#define I915_PMU_INTERRUPTS 35
> +
> +#define I915_PMU_RC6_RESIDENCY		40
> +#define I915_PMU_RC6p_RESIDENCY	41
> +#define I915_PMU_RC6pp_RESIDENCY	42
> +
>   /* Each region is a minimum of 16k, and there are at most 255 of them.
>    */
>   #define I915_NR_TEX_REGIONS 255	/* table size 2k - maximum due to use
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index e46eba8cd1b7..7b8c6dce1078 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7386,6 +7386,7 @@ int perf_event_overflow(struct perf_event *event,
>   {
>   	return __perf_event_overflow(event, 1, data, regs);
>   }
> +EXPORT_SYMBOL_GPL(perf_event_overflow);
>   
>   /*
>    * Generic software event infrastructure

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

Re: [RFC 11/14] drm/i915: Engine busy time tracking

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-07-19 10:12:33)
> 
> On 18/07/2017 16:19, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2017-07-18 15:36:15)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> Track total time requests have been executing on the hardware.
> >>
> >> To make this cheap it is hidden behind a static branch with the
> >> intention that it is only enabled when there is a consumer
> >> listening. This means that in the default off case the total
> >> cost of the tracking is just a few no-op instructions on the
> >> fast paths.
> > 
> >> +static inline void intel_engine_context_in(struct intel_engine_cs *engine)
> >> +{
> >> +       if (static_branch_unlikely(&i915_engine_stats_key)) {
> >> +               unsigned long flags;
> >> +
> >> +               spin_lock_irqsave(&engine->stats.lock, flags);
> > 
> > What's the purpose of this lock? RMW is ordered by virtue of the tasklet
> > (only one cpu can be doing the rmw at any time). Did I miss another
> > user?
> 
> Hm it should be a plain spin_lock and a _bh variant on the external API.
> 
> But the purpose is to allow atomic sampling of accumulated busy time 
> plus the current engine status.

You can do that with a lockless read, if you treat it as a seqlock and
the total as the latch. Something like

	u64 total, latch, start;

	total = engine->stats.total;
	do {
		latch = total;
		start = engine->stats.start;
		smp_rmb();
		total = engine->stats.total;
	} while (total != latch);
	total += ktime_now() - latch;

Assuming x86-64. If we only have 32b atomic reads, it is less fun.

> >> +               if (engine->stats.ref++ == 0)
> >> +                       engine->stats.start = ktime_get_real_ns();
> > 
> > Use ktime_get_raw() and leave the conversion to ns to the caller.
> 
> You mean both store in ktime_t and don't fetch the wall time but 
> monotonic? Do you say this because perf pmu perhaps needs monotonic or 
> for some other reason?

We only want monotonic (otherwise we will observe time going backwards), but
whether or not we want the clock source compensation? I was under the
impression we could defer that compensation until later.

> > What is the cost of a ktime nowadays?
> 
> I don't see it in the profiles, so not much I think.

readtsc is there, but not enough to worry. The effect on execution
latency is in the noise on initial inspection.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 12/14] drm/i915: Interface for controling engine stats collection

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-07-19 10:30:13)
> 
> On 18/07/2017 16:22, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2017-07-18 15:36:16)
> >> +u64 intel_engine_get_current_busy_ns(struct intel_engine_cs *engine)
> >> +{
> >> +       unsigned long flags;
> >> +       u64 total;
> >> +
> >> +       spin_lock_irqsave(&engine->stats.lock, flags);
> >> +
> >> +       total = engine->stats.total;
> >> +
> >> +       /*
> >> +        * If the engine is executing something at the moment
> >> +        * add it to the total.
> >> +        */
> >> +       if (engine->stats.ref)
> >> +               total += ktime_get_real_ns() - engine->stats.start;
> >> +
> >> +       spin_unlock_irqrestore(&engine->stats.lock, flags);
> > 
> > Answers to another patch found here. I would say this is the other half
> > of the interface and should be kept together.
> 
> Yes, it was an ugly split.
> 
> On 18/07/2017 16:43, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2017-07-18 15:36:16)
> >> +int intel_enable_engine_stats(struct drm_i915_private *dev_priv)
> >> +{
> >> +       if (!i915.enable_execlists)
> >> +               return -ENODEV;
> >> +
> >> +       mutex_lock(&i915_engine_stats_mutex);
> >> +       if (i915_engine_stats_ref++ == 0) {
> >> +               struct intel_engine_cs *engine;
> >> +               enum intel_engine_id id;
> >> +
> >> +               for_each_engine(engine, dev_priv, id) {
> >> +                       memset(&engine->stats, 0, sizeof(engine->stats));
> >> +                       spin_lock_init(&engine->stats.lock);
> >> +               }
> >> +
> >> +               static_branch_enable(&i915_engine_stats_key);
> >> +       }
> >> +       mutex_unlock(&i915_engine_stats_mutex);
> > 
> > I don't think static_branch_enable() is a might_sleep, so it looks like
> > you can rewrite this avoiding the mutex and thus not requiring the
> > worker and then can use the error code here to decide if you need to
> > use the timer instead.
> 
> Perhaps I could get rid of the mutex though by using atomic_inc/dec_return.
> 
> But there is a mutex in jump label handling,

Totally missed it. I wonder why it is a mutex, certainly serialising
enable/disable need something. The comments suggest that once upon a
time (or different arch?) it was much more of a stop_machine().

> so I think the workers have 
> to stay - and it is also beneficial to have delayed static branch disable,
> since the perf core seems to like calling start/stop on the events a lot.
> But it is recommended practice for static branches in any way.

Interesting there is a static_key_slow_dec_deferred. But honestly I
think it is hard to defend a global static_key for a per-device
interface.

> So from that angle I could perhaps even move the delayed disable to this
> patch so it is automatically shared by all callers.
> 
> >> +static DEFINE_MUTEX(i915_engine_stats_mutex);
> >> +static int i915_engine_stats_ref;
> >>   
> >>   /* Haswell does have the CXT_SIZE register however it does not appear to be
> >>    * valid. Now, docs explain in dwords what is in the context object. The full
> >> @@ -1340,6 +1342,57 @@ void intel_engines_mark_idle(struct drm_i915_private *i915)
> >>          }
> >>   }
> >>   
> >> +int intel_enable_engine_stats(struct drm_i915_private *dev_priv)
> > 
> > The pattern I've been trying to use here is
> > 
> >       intel_engine_* - operate on the named engine
> > 
> >       intel_engines_* - operate on all engines
> 
> Ok.
>  
> > Long term though having a global static key is going to be a nasty wart.
> > Joonas will definitely ask the question how much will it cost us to use
> > an engine->bool and what we can do to minimise that cost.
> 
> Why you think it is nasty? Sounds pretty cool to me.

If we enable sampling on one device (engine even!), it affects another.
But the device is the more compelling argument against.
 
> But I think can re-organize the series to start with a normal branch and
> then add the static one on top if so is desired.

Ok. I like the idea of dynamically patching in branches, and hate to be
party pooper!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/14] i915 PMU and engine busy stats

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-07-18 15:36:04)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Rough sketch of the idea I mentioned a few times to various people - merging
> the engine busyness tracking with Chris i915 PMU RFC.
> 
> First patch is the actual PMU RFC by Chris. It is followed by some cleanup
> patches, then come a few improvements, cheap execlists engine busyness tracking,
> debugfs view for the same, and finally the i915 PMU is extended to use this
> instead of timer based mmio sampling.
> 
> This makes it cheaper and also more accurate since engine busyness is not
> derived via sampling.
> 
> But I haven't figure out the perf API yet. For example is it possible to access
> our events in an usable fashion via perf top/stat or something? Do we want to
> make the events discoverable as I did (patch 8).

In my dreams I have gpu activity in the same perf timechart as gpu
activity. But that can be mostly by the request tracepoints, but still
overlaying cpu/gpu activity is desirable and more importantly we want to
coordinate with nouveau/amdgpu so that such interfaces are as agnostic
as possible. There are definitely a bunch of global features in common
for all (engine enumeration & activity, mempool enumeration, size &
activty, power usage?). But the key question is how do we build for the
future? Split the event id range into common/driver?

> I could not find much (any?) kernel API level documentation for perf.

There isn't much indeed. Given that we now have a second pair of eyes go
over the sampling and improve its interaction with i915, we should start
getting PeterZ involved to check the interaction with perf.
 
> Btw patch series actually works since intel-gpu-overlay can use these events
> when they are available.
> 
> Chris Wilson (1):
>   RFC drm/i915: Expose a PMU interface for perf queries

One thing I would like is for any future interface (including this
engine/class/event id) to use the engine class/instance mapping.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 01/14] RFC drm/i915: Expose a PMU interface for perf queries

[Tvrtko Ursulin]

On 19/07/2017 10:53, Kamble, Sagar A wrote:
> Can we reuse calc_residency defined in i915_sysfs.c

Looks like it, that is intel_pm.c/intel_rc6_residency_us.
I will incorporate the change in the series or the patch. Thanks for 
spotting this!

Regards,

Tvrtko

> 
> On 7/18/2017 8:06 PM, Tvrtko Ursulin wrote:
>> From: Chris Wilson <chris@chris-wilson.co.uk>
>>
>> The first goal is to be able to measure GPU (and invidual ring) busyness
>> without having to poll registers from userspace. (Which not only incurs
>> holding the forcewake lock indefinitely, perturbing the system, but also
>> runs the risk of hanging the machine.) As an alternative we can use the
>> perf event counter interface to sample the ring registers periodically
>> and send those results to userspace.
>>
>> To be able to do so, we need to export the two symbols from
>> kernel/events/core.c to register and unregister a PMU device.
>>
>> v2: Use a common timer for the ring sampling.
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/Makefile           |   1 +
>>   drivers/gpu/drm/i915/i915_drv.c         |   2 +
>>   drivers/gpu/drm/i915/i915_drv.h         |  23 ++
>>   drivers/gpu/drm/i915/i915_pmu.c         | 452 
>> ++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_ringbuffer.h |   2 +
>>   include/uapi/drm/i915_drm.h             |  41 +++
>>   kernel/events/core.c                    |   1 +
>>   7 files changed, 522 insertions(+)
>>   create mode 100644 drivers/gpu/drm/i915/i915_pmu.c
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile 
>> b/drivers/gpu/drm/i915/Makefile
>> index f8227318dcaf..1c720013dc42 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -26,6 +26,7 @@ i915-y := i915_drv.o \
>>   i915-$(CONFIG_COMPAT)   += i915_ioc32.o
>>   i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o
>> +i915-$(CONFIG_PERF_EVENTS) += i915_pmu.o
>>   # GEM code
>>   i915-y += i915_cmd_parser.o \
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c 
>> b/drivers/gpu/drm/i915/i915_drv.c
>> index d310d8245dca..f18ce519f6a2 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1194,6 +1194,7 @@ static void i915_driver_register(struct 
>> drm_i915_private *dev_priv)
>>       struct drm_device *dev = &dev_priv->drm;
>>       i915_gem_shrinker_init(dev_priv);
>> +    i915_pmu_register(dev_priv);
>>       /*
>>        * Notify a valid surface after modesetting,
>> @@ -1247,6 +1248,7 @@ static void i915_driver_unregister(struct 
>> drm_i915_private *dev_priv)
>>       intel_opregion_unregister(dev_priv);
>>       i915_perf_unregister(dev_priv);
>> +    i915_pmu_unregister(dev_priv);
>>       i915_teardown_sysfs(dev_priv);
>>       i915_guc_log_unregister(dev_priv);
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 7c6fab08a2e6..de518503e033 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -40,6 +40,7 @@
>>   #include <linux/hash.h>
>>   #include <linux/intel-iommu.h>
>>   #include <linux/kref.h>
>> +#include <linux/perf_event.h>
>>   #include <linux/pm_qos.h>
>>   #include <linux/reservation.h>
>>   #include <linux/shmem_fs.h>
>> @@ -2093,6 +2094,12 @@ struct intel_cdclk_state {
>>       unsigned int cdclk, vco, ref;
>>   };
>> +enum {
>> +    __I915_SAMPLE_FREQ_ACT = 0,
>> +    __I915_SAMPLE_FREQ_REQ,
>> +    __I915_NUM_PMU_SAMPLERS
>> +};
>> +
>>   struct drm_i915_private {
>>       struct drm_device drm;
>> @@ -2591,6 +2598,13 @@ struct drm_i915_private {
>>           int    irq;
>>       } lpe_audio;
>> +    struct {
>> +        struct pmu base;
>> +        struct hrtimer timer;
>> +        u64 enable;
>> +        u64 sample[__I915_NUM_PMU_SAMPLERS];
>> +    } pmu;
>> +
>>       /*
>>        * NOTE: This is the dri1/ums dungeon, don't add stuff here. 
>> Your patch
>>        * will be rejected. Instead look for a better place.
>> @@ -3760,6 +3774,15 @@ extern void i915_perf_fini(struct 
>> drm_i915_private *dev_priv);
>>   extern void i915_perf_register(struct drm_i915_private *dev_priv);
>>   extern void i915_perf_unregister(struct drm_i915_private *dev_priv);
>> +/* i915_pmu.c */
>> +#ifdef CONFIG_PERF_EVENTS
>> +extern void i915_pmu_register(struct drm_i915_private *i915);
>> +extern void i915_pmu_unregister(struct drm_i915_private *i915);
>> +#else
>> +static inline void i915_pmu_register(struct drm_i915_private *i915) {}
>> +static inline void i915_pmu_unregister(struct drm_i915_private *i915) {}
>> +#endif
>> +
>>   /* i915_suspend.c */
>>   extern int i915_save_state(struct drm_i915_private *dev_priv);
>>   extern int i915_restore_state(struct drm_i915_private *dev_priv);
>> diff --git a/drivers/gpu/drm/i915/i915_pmu.c 
>> b/drivers/gpu/drm/i915/i915_pmu.c
>> new file mode 100644
>> index 000000000000..f03ddad44da6
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/i915_pmu.c
>> @@ -0,0 +1,452 @@
>> +#include <linux/perf_event.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#include "i915_drv.h"
>> +#include "intel_ringbuffer.h"
>> +
>> +#define FREQUENCY 200
>> +#define PERIOD max_t(u64, 10000, NSEC_PER_SEC / FREQUENCY)
>> +
>> +#define RING_MASK 0xffffffff
>> +#define RING_MAX 32
>> +
>> +static void engines_sample(struct drm_i915_private *dev_priv)
>> +{
>> +    struct intel_engine_cs *engine;
>> +    enum intel_engine_id id;
>> +    bool fw = false;
>> +
>> +    if ((dev_priv->pmu.enable & RING_MASK) == 0)
>> +        return;
>> +
>> +    if (!dev_priv->gt.awake)
>> +        return;
>> +
>> +    if (!intel_runtime_pm_get_if_in_use(dev_priv))
>> +        return;
>> +
>> +    for_each_engine(engine, dev_priv, id) {
>> +        u32 val;
>> +
>> +        if ((dev_priv->pmu.enable & (0x7 << (4*id))) == 0)
>> +            continue;
>> +
>> +        if (i915_seqno_passed(intel_engine_get_seqno(engine),
>> +                      intel_engine_last_submit(engine)))
>> +            continue;
>> +
>> +        if (!fw) {
>> +            intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>> +            fw = true;
>> +        }
>> +
>> +        engine->pmu_sample[I915_SAMPLE_QUEUED] += PERIOD;
>> +
>> +        val = I915_READ_FW(RING_MI_MODE(engine->mmio_base));
>> +        if (!(val & MODE_IDLE))
>> +            engine->pmu_sample[I915_SAMPLE_BUSY] += PERIOD;
>> +
>> +        val = I915_READ_FW(RING_CTL(engine->mmio_base));
>> +        if (val & RING_WAIT)
>> +            engine->pmu_sample[I915_SAMPLE_WAIT] += PERIOD;
>> +        if (val & RING_WAIT_SEMAPHORE)
>> +            engine->pmu_sample[I915_SAMPLE_SEMA] += PERIOD;
>> +    }
>> +
>> +    if (fw)
>> +        intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>> +    intel_runtime_pm_put(dev_priv);
>> +}
>> +
>> +static void frequency_sample(struct drm_i915_private *dev_priv)
>> +{
>> +    if (dev_priv->pmu.enable & BIT_ULL(I915_PMU_ACTUAL_FREQUENCY)) {
>> +        u64 val;
>> +
>> +        val = dev_priv->rps.cur_freq;
>> +        if (dev_priv->gt.awake &&
>> +            intel_runtime_pm_get_if_in_use(dev_priv)) {
>> +            val = I915_READ_NOTRACE(GEN6_RPSTAT1);
>> +            if (INTEL_GEN(dev_priv) >= 9)
>> +                val = (val & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
>> +            else if (IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8)
>> +                val = (val & HSW_CAGF_MASK) >> HSW_CAGF_SHIFT;
>> +            else
>> +                val = (val & GEN6_CAGF_MASK) >> GEN6_CAGF_SHIFT;
>> +            intel_runtime_pm_put(dev_priv);
>> +        }
>> +        val = intel_gpu_freq(dev_priv, val);
>> +        dev_priv->pmu.sample[__I915_SAMPLE_FREQ_ACT] += val * PERIOD;
>> +    }
>> +
>> +    if (dev_priv->pmu.enable & BIT_ULL(I915_PMU_REQUESTED_FREQUENCY)) {
>> +        u64 val = intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq);
>> +        dev_priv->pmu.sample[__I915_SAMPLE_FREQ_REQ] += val * PERIOD;
>> +    }
>> +}
>> +
>> +static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
>> +{
>> +    struct drm_i915_private *i915 =
>> +        container_of(hrtimer, struct drm_i915_private, pmu.timer);
>> +
>> +    if (i915->pmu.enable == 0)
>> +        return HRTIMER_NORESTART;
>> +
>> +    engines_sample(i915);
>> +    frequency_sample(i915);
>> +
>> +    hrtimer_forward_now(hrtimer, ns_to_ktime(PERIOD));
>> +    return HRTIMER_RESTART;
>> +}
>> +
>> +static void i915_pmu_event_destroy(struct perf_event *event)
>> +{
>> +    WARN_ON(event->parent);
>> +}
>> +
>> +static int engine_event_init(struct perf_event *event)
>> +{
>> +    struct drm_i915_private *i915 =
>> +        container_of(event->pmu, typeof(*i915), pmu.base);
>> +    int engine = event->attr.config >> 2;
>> +    int sample = event->attr.config & 3;
>> +
>> +    switch (sample) {
>> +    case I915_SAMPLE_QUEUED:
>> +    case I915_SAMPLE_BUSY:
>> +    case I915_SAMPLE_WAIT:
>> +        break;
>> +    case I915_SAMPLE_SEMA:
>> +        if (INTEL_GEN(i915) < 6)
>> +            return -ENODEV;
>> +        break;
>> +    default:
>> +        return -ENOENT;
>> +    }
>> +
>> +    if (engine >= I915_NUM_ENGINES)
>> +        return -ENOENT;
>> +
>> +    if (!i915->engine[engine])
>> +        return -ENODEV;
>> +
>> +    return 0;
>> +}
>> +
>> +static enum hrtimer_restart hrtimer_sample(struct hrtimer *hrtimer)
>> +{
>> +    struct perf_sample_data data;
>> +    struct perf_event *event;
>> +    u64 period;
>> +
>> +    event = container_of(hrtimer, struct perf_event, hw.hrtimer);
>> +    if (event->state != PERF_EVENT_STATE_ACTIVE)
>> +        return HRTIMER_NORESTART;
>> +
>> +    event->pmu->read(event);
>> +
>> +    perf_sample_data_init(&data, 0, event->hw.last_period);
>> +    perf_event_overflow(event, &data, NULL);
>> +
>> +    period = max_t(u64, 10000, event->hw.sample_period);
>> +    hrtimer_forward_now(hrtimer, ns_to_ktime(period));
>> +    return HRTIMER_RESTART;
>> +}
>> +
>> +static void init_hrtimer(struct perf_event *event)
>> +{
>> +    struct hw_perf_event *hwc = &event->hw;
>> +
>> +    if (!is_sampling_event(event))
>> +        return;
>> +
>> +    hrtimer_init(&hwc->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>> +    hwc->hrtimer.function = hrtimer_sample;
>> +
>> +    if (event->attr.freq) {
>> +        long freq = event->attr.sample_freq;
>> +
>> +        event->attr.sample_period = NSEC_PER_SEC / freq;
>> +        hwc->sample_period = event->attr.sample_period;
>> +        local64_set(&hwc->period_left, hwc->sample_period);
>> +        hwc->last_period = hwc->sample_period;
>> +        event->attr.freq = 0;
>> +    }
>> +}
>> +
>> +static int i915_pmu_event_init(struct perf_event *event)
>> +{
>> +    struct drm_i915_private *i915 =
>> +        container_of(event->pmu, typeof(*i915), pmu.base);
>> +    int ret;
>> +
>> +    /* XXX ideally only want pid == -1 && cpu == -1 */
>> +
>> +    if (event->attr.type != event->pmu->type)
>> +        return -ENOENT;
>> +
>> +    if (has_branch_stack(event))
>> +        return -EOPNOTSUPP;
>> +
>> +    ret = 0;
>> +    if (event->attr.config < RING_MAX) {
>> +        ret = engine_event_init(event);
>> +    } else switch (event->attr.config) {
>> +    case I915_PMU_ACTUAL_FREQUENCY:
>> +        if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>> +            ret = -ENODEV; /* requires a mutex for sampling! */
>> +    case I915_PMU_REQUESTED_FREQUENCY:
>> +    case I915_PMU_ENERGY:
>> +    case I915_PMU_RC6_RESIDENCY:
>> +    case I915_PMU_RC6p_RESIDENCY:
>> +    case I915_PMU_RC6pp_RESIDENCY:
>> +        if (INTEL_GEN(i915) < 6)
>> +            ret = -ENODEV;
>> +        break;
>> +    }
>> +    if (ret)
>> +        return ret;
>> +
>> +    if (!event->parent)
>> +        event->destroy = i915_pmu_event_destroy;
>> +
>> +    init_hrtimer(event);
>> +
>> +    return 0;
>> +}
>> +
>> +static void i915_pmu_timer_start(struct perf_event *event)
>> +{
>> +    struct hw_perf_event *hwc = &event->hw;
>> +    s64 period;
>> +
>> +    if (!is_sampling_event(event))
>> +        return;
>> +
>> +    period = local64_read(&hwc->period_left);
>> +    if (period) {
>> +        if (period < 0)
>> +            period = 10000;
>> +
>> +        local64_set(&hwc->period_left, 0);
>> +    } else {
>> +        period = max_t(u64, 10000, hwc->sample_period);
>> +    }
>> +
>> +    hrtimer_start_range_ns(&hwc->hrtimer,
>> +                   ns_to_ktime(period), 0,
>> +                   HRTIMER_MODE_REL_PINNED);
>> +}
>> +
>> +static void i915_pmu_timer_cancel(struct perf_event *event)
>> +{
>> +    struct hw_perf_event *hwc = &event->hw;
>> +
>> +    if (!is_sampling_event(event))
>> +        return;
>> +
>> +    local64_set(&hwc->period_left,
>> +            ktime_to_ns(hrtimer_get_remaining(&hwc->hrtimer)));
>> +    hrtimer_cancel(&hwc->hrtimer);
>> +}
>> +
>> +static void i915_pmu_enable(struct perf_event *event)
>> +{
>> +    struct drm_i915_private *i915 =
>> +        container_of(event->pmu, typeof(*i915), pmu.base);
>> +
>> +    if (i915->pmu.enable == 0)
>> +        hrtimer_start_range_ns(&i915->pmu.timer,
>> +                       ns_to_ktime(PERIOD), 0,
>> +                       HRTIMER_MODE_REL_PINNED);
>> +
>> +    i915->pmu.enable |= BIT_ULL(event->attr.config);
>> +
>> +    i915_pmu_timer_start(event);
>> +}
>> +
>> +static void i915_pmu_disable(struct perf_event *event)
>> +{
>> +    struct drm_i915_private *i915 =
>> +        container_of(event->pmu, typeof(*i915), pmu.base);
>> +
>> +    i915->pmu.enable &= ~BIT_ULL(event->attr.config);
>> +    i915_pmu_timer_cancel(event);
>> +}
>> +
>> +static int i915_pmu_event_add(struct perf_event *event, int flags)
>> +{
>> +    struct hw_perf_event *hwc = &event->hw;
>> +
>> +    if (flags & PERF_EF_START)
>> +        i915_pmu_enable(event);
>> +
>> +    hwc->state = !(flags & PERF_EF_START);
>> +
>> +    return 0;
>> +}
>> +
>> +static void i915_pmu_event_del(struct perf_event *event, int flags)
>> +{
>> +    i915_pmu_disable(event);
>> +}
>> +
>> +static void i915_pmu_event_start(struct perf_event *event, int flags)
>> +{
>> +    i915_pmu_enable(event);
>> +}
>> +
>> +static void i915_pmu_event_stop(struct perf_event *event, int flags)
>> +{
>> +    i915_pmu_disable(event);
>> +}
>> +
>> +static u64 read_energy_uJ(struct drm_i915_private *dev_priv)
>> +{
>> +    u64 power;
>> +
>> +    GEM_BUG_ON(INTEL_GEN(dev_priv) < 6);
>> +
>> +    intel_runtime_pm_get(dev_priv);
>> +
>> +    rdmsrl(MSR_RAPL_POWER_UNIT, power);
>> +    power = (power & 0x1f00) >> 8;
>> +    power = 1000000 >> power; /* convert to uJ */
>> +    power *= I915_READ_NOTRACE(MCH_SECP_NRG_STTS);
>> +
>> +    intel_runtime_pm_put(dev_priv);
>> +
>> +    return power;
>> +}
>> +
>> +static inline u64 calc_residency(struct drm_i915_private *dev_priv,
>> +                 const i915_reg_t reg)
>> +{
>> +    u64 val, units = 128, div = 100000;
>> +
>> +    GEM_BUG_ON(INTEL_GEN(dev_priv) < 6);
>> +
>> +    intel_runtime_pm_get(dev_priv);
>> +    if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
>> +        div = dev_priv->czclk_freq;
>> +        units = 1;
>> +        if (I915_READ_NOTRACE(VLV_COUNTER_CONTROL) & 
>> VLV_COUNT_RANGE_HIGH)
>> +            units <<= 8;
>> +    } else if (IS_GEN9_LP(dev_priv)) {
>> +        div = 1200;
>> +        units = 1;
>> +    }
>> +    val = I915_READ_NOTRACE(reg);
>> +    intel_runtime_pm_put(dev_priv);
>> +
>> +    val *= units;
>> +    return DIV_ROUND_UP_ULL(val, div);
>> +}
>> +
>> +static u64 count_interrupts(struct drm_i915_private *i915)
>> +{
>> +    /* open-coded kstat_irqs() */
>> +    struct irq_desc *desc = irq_to_desc(i915->drm.pdev->irq);
>> +    u64 sum = 0;
>> +    int cpu;
>> +
>> +    if (!desc || !desc->kstat_irqs)
>> +        return 0;
>> +
>> +    for_each_possible_cpu(cpu)
>> +        sum += *per_cpu_ptr(desc->kstat_irqs, cpu);
>> +
>> +    return sum;
>> +}
>> +
>> +static void i915_pmu_event_read(struct perf_event *event)
>> +{
>> +    struct drm_i915_private *i915 =
>> +        container_of(event->pmu, typeof(*i915), pmu.base);
>> +    u64 val = 0;
>> +
>> +    if (event->attr.config < 32) {
>> +        int engine = event->attr.config >> 2;
>> +        int sample = event->attr.config & 3;
>> +        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];
>> +        break;
>> +    case I915_PMU_REQUESTED_FREQUENCY:
>> +        val = i915->pmu.sample[__I915_SAMPLE_FREQ_REQ];
>> +        break;
>> +    case I915_PMU_ENERGY:
>> +        val = read_energy_uJ(i915);
>> +        break;
>> +    case I915_PMU_INTERRUPTS:
>> +        val = count_interrupts(i915);
>> +        break;
>> +
>> +    case I915_PMU_RC6_RESIDENCY:
>> +        if (!i915->gt.awake)
>> +            return;
>> +
>> +        val = calc_residency(i915, IS_VALLEYVIEW(i915) ? 
>> VLV_GT_RENDER_RC6 : GEN6_GT_GFX_RC6);
>> +        break;
>> +
>> +    case I915_PMU_RC6p_RESIDENCY:
>> +        if (!i915->gt.awake)
>> +            return;
>> +
>> +        if (!IS_VALLEYVIEW(i915))
>> +            val = calc_residency(i915, GEN6_GT_GFX_RC6p);
>> +        break;
>> +
>> +    case I915_PMU_RC6pp_RESIDENCY:
>> +        if (!i915->gt.awake)
>> +            return;
>> +
>> +        if (!IS_VALLEYVIEW(i915))
>> +            val = calc_residency(i915, GEN6_GT_GFX_RC6pp);
>> +        break;
>> +    }
>> +
>> +    local64_set(&event->count, val);
>> +}
>> +
>> +static int i915_pmu_event_event_idx(struct perf_event *event)
>> +{
>> +    return 0;
>> +}
>> +
>> +void i915_pmu_register(struct drm_i915_private *i915)
>> +{
>> +    if (INTEL_GEN(i915) <= 2)
>> +        return;
>> +
>> +    i915->pmu.base.task_ctx_nr    = perf_sw_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;
>> +
>> +    hrtimer_init(&i915->pmu.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>> +    i915->pmu.timer.function = i915_sample;
>> +    i915->pmu.enable = 0;
>> +
>> +    if (perf_pmu_register(&i915->pmu.base, "i915", -1))
>> +        i915->pmu.base.event_init = NULL;
>> +}
>> +
>> +void i915_pmu_unregister(struct drm_i915_private *i915)
>> +{
>> +    if (!i915->pmu.base.event_init)
>> +        return;
>> +
>> +    i915->pmu.enable = 0;
>> +
>> +    perf_pmu_unregister(&i915->pmu.base);
>> +    i915->pmu.base.event_init = NULL;
>> +
>> +    hrtimer_cancel(&i915->pmu.timer);
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
>> b/drivers/gpu/drm/i915/intel_ringbuffer.h
>> index d33c93444c0d..0877b151239d 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
>> @@ -245,6 +245,8 @@ struct intel_engine_cs {
>>           I915_SELFTEST_DECLARE(bool mock : 1);
>>       } breadcrumbs;
>> +    u64 pmu_sample[4];
>> +
>>       /*
>>        * A pool of objects to use as shadow copies of client batch 
>> buffers
>>        * when the command parser is enabled. Prevents the client from
>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index 7ccbd6a2bbe0..733774f19a0b 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -86,6 +86,47 @@ enum i915_mocs_table_index {
>>       I915_MOCS_CACHED,
>>   };
>> +/**
>> + * DOC: perf_events exposed by i915 through 
>> /sys/bus/event_sources/drivers/i915
>> + *
>> + */
>> +#define I915_SAMPLE_QUEUED    0
>> +#define I915_SAMPLE_BUSY    1
>> +#define I915_SAMPLE_WAIT    2
>> +#define I915_SAMPLE_SEMA    3
>> +
>> +#define I915_SAMPLE_RCS        0
>> +#define I915_SAMPLE_VCS        1
>> +#define I915_SAMPLE_BCS        2
>> +#define I915_SAMPLE_VECS    3
>> +
>> +#define __I915_PMU_COUNT(ring, id) ((ring) << 2 | (id))
>> +
>> +#define I915_PMU_COUNT_RCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_RCS, 
>> I915_SAMPLE_BUSY)
>> +#define I915_PMU_COUNT_RCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_RCS, 
>> I915_SAMPLE_WAIT)
>> +#define I915_PMU_COUNT_RCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_RCS, 
>> I915_SAMPLE_SEMA)
>> +
>> +#define I915_PMU_COUNT_VCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_VCS, 
>> I915_SAMPLE_BUSY)
>> +#define I915_PMU_COUNT_VCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_VCS, 
>> I915_SAMPLE_WAIT)
>> +#define I915_PMU_COUNT_VCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_VCS, 
>> I915_SAMPLE_SEMA)
>> +
>> +#define I915_PMU_COUNT_BCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_BCS, 
>> I915_SAMPLE_BUSY)
>> +#define I915_PMU_COUNT_BCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_BCS, 
>> I915_SAMPLE_WAIT)
>> +#define I915_PMU_COUNT_BCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_BCS, 
>> I915_SAMPLE_SEMA)
>> +
>> +#define I915_PMU_COUNT_VECS_BUSY __I915_PMU_COUNT(I915_SAMPLE_VECS, 
>> I915_SAMPLE_BUSY)
>> +#define I915_PMU_COUNT_VECS_WAIT __I915_PMU_COUNT(I915_SAMPLE_VECS, 
>> I915_SAMPLE_WAIT)
>> +#define I915_PMU_COUNT_VECS_SEMA __I915_PMU_COUNT(I915_SAMPLE_VECS, 
>> I915_SAMPLE_SEMA)
>> +
>> +#define I915_PMU_ACTUAL_FREQUENCY 32
>> +#define I915_PMU_REQUESTED_FREQUENCY 33
>> +#define I915_PMU_ENERGY 34
>> +#define I915_PMU_INTERRUPTS 35
>> +
>> +#define I915_PMU_RC6_RESIDENCY        40
>> +#define I915_PMU_RC6p_RESIDENCY    41
>> +#define I915_PMU_RC6pp_RESIDENCY    42
>> +
>>   /* Each region is a minimum of 16k, and there are at most 255 of them.
>>    */
>>   #define I915_NR_TEX_REGIONS 255    /* table size 2k - maximum due to 
>> use
>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>> index e46eba8cd1b7..7b8c6dce1078 100644
>> --- a/kernel/events/core.c
>> +++ b/kernel/events/core.c
>> @@ -7386,6 +7386,7 @@ int perf_event_overflow(struct perf_event *event,
>>   {
>>       return __perf_event_overflow(event, 1, data, regs);
>>   }
>> +EXPORT_SYMBOL_GPL(perf_event_overflow);
>>   /*
>>    * Generic software event infrastructure
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/14] i915 PMU and engine busy stats

[Tvrtko Ursulin]

On 19/07/2017 13:05, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-07-18 15:36:04)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Rough sketch of the idea I mentioned a few times to various people - merging
>> the engine busyness tracking with Chris i915 PMU RFC.
>>
>> First patch is the actual PMU RFC by Chris. It is followed by some cleanup
>> patches, then come a few improvements, cheap execlists engine busyness tracking,
>> debugfs view for the same, and finally the i915 PMU is extended to use this
>> instead of timer based mmio sampling.
>>
>> This makes it cheaper and also more accurate since engine busyness is not
>> derived via sampling.
>>
>> But I haven't figure out the perf API yet. For example is it possible to access
>> our events in an usable fashion via perf top/stat or something? Do we want to
>> make the events discoverable as I did (patch 8).
> 
> In my dreams I have gpu activity in the same perf timechart as gpu
> activity. But that can be mostly by the request tracepoints, but still
> overlaying cpu/gpu activity is desirable and more importantly we want to
> coordinate with nouveau/amdgpu so that such interfaces are as agnostic
> as possible. There are definitely a bunch of global features in common
> for all (engine enumeration & activity, mempool enumeration, size &
> activty, power usage?). But the key question is how do we build for the
> future? Split the event id range into common/driver?

I don't know if going for common events would be workable. A few metrics 
sounds like it could be generic, but I am not sure there would be more 
than a couple where that would be future proof. Also is the coordination 
effort (no one else seems to implement a perf interface at the moment) 
worth it at the current time? I am not sure.

>> I could not find much (any?) kernel API level documentation for perf.
> 
> There isn't much indeed. Given that we now have a second pair of eyes go
> over the sampling and improve its interaction with i915, we should start
> getting PeterZ involved to check the interaction with perf.

Okay, I guess another cleanup pass and then I can do that.

In the meantime do you have any good understanding of what kind of 
events are we exposing here? They look weird if I record them and look 
with "perf script", and "perf stat" always reports zeroes for them. But 
they still work from the overlay tool. So it is a bit of a mystery to me 
what they really are.
>> Btw patch series actually works since intel-gpu-overlay can use these events
>> when they are available.
>>
>> Chris Wilson (1):
>>    RFC drm/i915: Expose a PMU interface for perf queries
> 
> One thing I would like is for any future interface (including this
> engine/class/event id) to use the engine class/instance mapping.

I was thinking about that myself. I can do it in the next cleanup pass.

Regards,

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

Re: [RFC 12/14] drm/i915: Interface for controling engine stats collection

[Tvrtko Ursulin]

On 19/07/2017 12:04, Chris Wilson wrote:

[snip]

>>> Long term though having a global static key is going to be a nasty wart.
>>> Joonas will definitely ask the question how much will it cost us to use
>>> an engine->bool and what we can do to minimise that cost.
>>
>> Why you think it is nasty? Sounds pretty cool to me.
> 
> If we enable sampling on one device (engine even!), it affects another.
> But the device is the more compelling argument against.

Since you mention engines, I can do it on engine granularity with normal 
branches. It makes sense for the pmu interface to have it per engine.

Then as I said before, I can put in a late patch in the series which 
adds a static key (master enable/disable on or-ed per-engine enables) 
just in case we find it attractive.

Regards,

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

Re: [RFC 01/14] RFC drm/i915: Expose a PMU interface for perf queries

[Ben Widawsky]

On 17-07-18 15:36:05, Tvrtko Ursulin wrote:
>From: Chris Wilson <chris@chris-wilson.co.uk>
>
>The first goal is to be able to measure GPU (and invidual ring) busyness
>without having to poll registers from userspace. (Which not only incurs
>holding the forcewake lock indefinitely, perturbing the system, but also
>runs the risk of hanging the machine.) As an alternative we can use the
>perf event counter interface to sample the ring registers periodically
>and send those results to userspace.
>
>To be able to do so, we need to export the two symbols from
>kernel/events/core.c to register and unregister a PMU device.
>
>v2: Use a common timer for the ring sampling.
>
>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>---
> drivers/gpu/drm/i915/Makefile           |   1 +
> drivers/gpu/drm/i915/i915_drv.c         |   2 +
> drivers/gpu/drm/i915/i915_drv.h         |  23 ++
> drivers/gpu/drm/i915/i915_pmu.c         | 452 ++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_ringbuffer.h |   2 +
> include/uapi/drm/i915_drm.h             |  41 +++
> kernel/events/core.c                    |   1 +
> 7 files changed, 522 insertions(+)
> create mode 100644 drivers/gpu/drm/i915/i915_pmu.c
>
>diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>index f8227318dcaf..1c720013dc42 100644
>--- a/drivers/gpu/drm/i915/Makefile
>+++ b/drivers/gpu/drm/i915/Makefile
>@@ -26,6 +26,7 @@ i915-y := i915_drv.o \
>
> i915-$(CONFIG_COMPAT)   += i915_ioc32.o
> i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o
>+i915-$(CONFIG_PERF_EVENTS) += i915_pmu.o
>
> # GEM code
> i915-y += i915_cmd_parser.o \
>diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>index d310d8245dca..f18ce519f6a2 100644
>--- a/drivers/gpu/drm/i915/i915_drv.c
>+++ b/drivers/gpu/drm/i915/i915_drv.c
>@@ -1194,6 +1194,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
> 	struct drm_device *dev = &dev_priv->drm;
>
> 	i915_gem_shrinker_init(dev_priv);
>+	i915_pmu_register(dev_priv);
>
> 	/*
> 	 * Notify a valid surface after modesetting,
>@@ -1247,6 +1248,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
> 	intel_opregion_unregister(dev_priv);
>
> 	i915_perf_unregister(dev_priv);
>+	i915_pmu_unregister(dev_priv);
>
> 	i915_teardown_sysfs(dev_priv);
> 	i915_guc_log_unregister(dev_priv);
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index 7c6fab08a2e6..de518503e033 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -40,6 +40,7 @@
> #include <linux/hash.h>
> #include <linux/intel-iommu.h>
> #include <linux/kref.h>
>+#include <linux/perf_event.h>
> #include <linux/pm_qos.h>
> #include <linux/reservation.h>
> #include <linux/shmem_fs.h>
>@@ -2093,6 +2094,12 @@ struct intel_cdclk_state {
> 	unsigned int cdclk, vco, ref;
> };
>
>+enum {
>+	__I915_SAMPLE_FREQ_ACT = 0,
>+	__I915_SAMPLE_FREQ_REQ,
>+	__I915_NUM_PMU_SAMPLERS
>+};
>+
> struct drm_i915_private {
> 	struct drm_device drm;
>
>@@ -2591,6 +2598,13 @@ struct drm_i915_private {
> 		int	irq;
> 	} lpe_audio;
>
>+	struct {
>+		struct pmu base;
>+		struct hrtimer timer;
>+		u64 enable;
>+		u64 sample[__I915_NUM_PMU_SAMPLERS];
>+	} pmu;
>+
> 	/*
> 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> 	 * will be rejected. Instead look for a better place.
>@@ -3760,6 +3774,15 @@ extern void i915_perf_fini(struct drm_i915_private *dev_priv);
> extern void i915_perf_register(struct drm_i915_private *dev_priv);
> extern void i915_perf_unregister(struct drm_i915_private *dev_priv);
>
>+/* i915_pmu.c */
>+#ifdef CONFIG_PERF_EVENTS
>+extern void i915_pmu_register(struct drm_i915_private *i915);
>+extern void i915_pmu_unregister(struct drm_i915_private *i915);
>+#else
>+static inline void i915_pmu_register(struct drm_i915_private *i915) {}
>+static inline void i915_pmu_unregister(struct drm_i915_private *i915) {}
>+#endif
>+
> /* i915_suspend.c */
> extern int i915_save_state(struct drm_i915_private *dev_priv);
> extern int i915_restore_state(struct drm_i915_private *dev_priv);
>diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
>new file mode 100644
>index 000000000000..f03ddad44da6
>--- /dev/null
>+++ b/drivers/gpu/drm/i915/i915_pmu.c
>@@ -0,0 +1,452 @@
>+#include <linux/perf_event.h>
>+#include <linux/pm_runtime.h>
>+
>+#include "i915_drv.h"
>+#include "intel_ringbuffer.h"
>+
>+#define FREQUENCY 200
>+#define PERIOD max_t(u64, 10000, NSEC_PER_SEC / FREQUENCY)
>+
>+#define RING_MASK 0xffffffff
>+#define RING_MAX 32
>+
>+static void engines_sample(struct drm_i915_private *dev_priv)
>+{
>+	struct intel_engine_cs *engine;
>+	enum intel_engine_id id;
>+	bool fw = false;
>+
>+	if ((dev_priv->pmu.enable & RING_MASK) == 0)
>+		return;
>+
>+	if (!dev_priv->gt.awake)
>+		return;
>+
>+	if (!intel_runtime_pm_get_if_in_use(dev_priv))
>+		return;
>+
>+	for_each_engine(engine, dev_priv, id) {
>+		u32 val;
>+
>+		if ((dev_priv->pmu.enable & (0x7 << (4*id))) == 0)
>+			continue;
>+
>+		if (i915_seqno_passed(intel_engine_get_seqno(engine),
>+				      intel_engine_last_submit(engine)))
>+			continue;
>+

This seems too clever of an optimization, why not just use MODE_IDLE to be as
accurate as possible and rely as little as possible on software tracking.

>+		if (!fw) {
>+			intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>+			fw = true;
>+		}
>+
>+		engine->pmu_sample[I915_SAMPLE_QUEUED] += PERIOD;
>+
>+		val = I915_READ_FW(RING_MI_MODE(engine->mmio_base));
>+		if (!(val & MODE_IDLE))
>+			engine->pmu_sample[I915_SAMPLE_BUSY] += PERIOD;
>+
>+		val = I915_READ_FW(RING_CTL(engine->mmio_base));
>+		if (val & RING_WAIT)
>+			engine->pmu_sample[I915_SAMPLE_WAIT] += PERIOD;
>+		if (val & RING_WAIT_SEMAPHORE)
>+			engine->pmu_sample[I915_SAMPLE_SEMA] += PERIOD;
>+	}
>+
>+	if (fw)
>+		intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>+	intel_runtime_pm_put(dev_priv);
>+}
>+
>+static void frequency_sample(struct drm_i915_private *dev_priv)
>+{
>+	if (dev_priv->pmu.enable & BIT_ULL(I915_PMU_ACTUAL_FREQUENCY)) {
>+		u64 val;
>+
>+		val = dev_priv->rps.cur_freq;
>+		if (dev_priv->gt.awake &&
>+		    intel_runtime_pm_get_if_in_use(dev_priv)) {
>+			val = I915_READ_NOTRACE(GEN6_RPSTAT1);
>+			if (INTEL_GEN(dev_priv) >= 9)
>+				val = (val & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
>+			else if (IS_HASWELL(dev_priv) || INTEL_GEN(dev_priv) >= 8)
>+				val = (val & HSW_CAGF_MASK) >> HSW_CAGF_SHIFT;
>+			else
>+				val = (val & GEN6_CAGF_MASK) >> GEN6_CAGF_SHIFT;
>+			intel_runtime_pm_put(dev_priv);
>+		}
>+		val = intel_gpu_freq(dev_priv, val);
>+		dev_priv->pmu.sample[__I915_SAMPLE_FREQ_ACT] += val * PERIOD;
>+	}
>+
>+	if (dev_priv->pmu.enable & BIT_ULL(I915_PMU_REQUESTED_FREQUENCY)) {
>+		u64 val = intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq);
>+		dev_priv->pmu.sample[__I915_SAMPLE_FREQ_REQ] += val * PERIOD;
>+	}
>+}
>+
>+static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
>+{
>+	struct drm_i915_private *i915 =
>+		container_of(hrtimer, struct drm_i915_private, pmu.timer);
>+
>+	if (i915->pmu.enable == 0)
>+		return HRTIMER_NORESTART;
>+
>+	engines_sample(i915);
>+	frequency_sample(i915);
>+
>+	hrtimer_forward_now(hrtimer, ns_to_ktime(PERIOD));
>+	return HRTIMER_RESTART;
>+}
>+
>+static void i915_pmu_event_destroy(struct perf_event *event)
>+{
>+	WARN_ON(event->parent);
>+}
>+
>+static int engine_event_init(struct perf_event *event)
>+{
>+	struct drm_i915_private *i915 =
>+		container_of(event->pmu, typeof(*i915), pmu.base);
>+	int engine = event->attr.config >> 2;
>+	int sample = event->attr.config & 3;
>+
>+	switch (sample) {
>+	case I915_SAMPLE_QUEUED:
>+	case I915_SAMPLE_BUSY:
>+	case I915_SAMPLE_WAIT:
>+		break;
>+	case I915_SAMPLE_SEMA:
>+		if (INTEL_GEN(i915) < 6)
>+			return -ENODEV;
>+		break;
>+	default:
>+		return -ENOENT;
>+	}
>+
>+	if (engine >= I915_NUM_ENGINES)
>+		return -ENOENT;
>+
>+	if (!i915->engine[engine])
>+		return -ENODEV;
>+
>+	return 0;
>+}
>+
>+static enum hrtimer_restart hrtimer_sample(struct hrtimer *hrtimer)
>+{
>+	struct perf_sample_data data;
>+	struct perf_event *event;
>+	u64 period;
>+
>+	event = container_of(hrtimer, struct perf_event, hw.hrtimer);
>+	if (event->state != PERF_EVENT_STATE_ACTIVE)
>+		return HRTIMER_NORESTART;
>+
>+	event->pmu->read(event);
>+
>+	perf_sample_data_init(&data, 0, event->hw.last_period);
>+	perf_event_overflow(event, &data, NULL);
>+
>+	period = max_t(u64, 10000, event->hw.sample_period);
>+	hrtimer_forward_now(hrtimer, ns_to_ktime(period));
>+	return HRTIMER_RESTART;
>+}
>+
>+static void init_hrtimer(struct perf_event *event)
>+{
>+	struct hw_perf_event *hwc = &event->hw;
>+
>+	if (!is_sampling_event(event))
>+		return;
>+
>+	hrtimer_init(&hwc->hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>+	hwc->hrtimer.function = hrtimer_sample;
>+
>+	if (event->attr.freq) {
>+		long freq = event->attr.sample_freq;
>+
>+		event->attr.sample_period = NSEC_PER_SEC / freq;
>+		hwc->sample_period = event->attr.sample_period;
>+		local64_set(&hwc->period_left, hwc->sample_period);
>+		hwc->last_period = hwc->sample_period;
>+		event->attr.freq = 0;
>+	}
>+}
>+
>+static int i915_pmu_event_init(struct perf_event *event)
>+{
>+	struct drm_i915_private *i915 =
>+		container_of(event->pmu, typeof(*i915), pmu.base);
>+	int ret;
>+
>+	/* XXX ideally only want pid == -1 && cpu == -1 */
>+
>+	if (event->attr.type != event->pmu->type)
>+		return -ENOENT;
>+
>+	if (has_branch_stack(event))
>+		return -EOPNOTSUPP;
>+
>+	ret = 0;
>+	if (event->attr.config < RING_MAX) {
>+		ret = engine_event_init(event);
>+	} else switch (event->attr.config) {
>+	case I915_PMU_ACTUAL_FREQUENCY:
>+		if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915))
>+			ret = -ENODEV; /* requires a mutex for sampling! */
>+	case I915_PMU_REQUESTED_FREQUENCY:
>+	case I915_PMU_ENERGY:
>+	case I915_PMU_RC6_RESIDENCY:
>+	case I915_PMU_RC6p_RESIDENCY:
>+	case I915_PMU_RC6pp_RESIDENCY:
>+		if (INTEL_GEN(i915) < 6)
>+			ret = -ENODEV;
>+		break;
>+	}
>+	if (ret)
>+		return ret;
>+
>+	if (!event->parent)
>+		event->destroy = i915_pmu_event_destroy;
>+
>+	init_hrtimer(event);
>+
>+	return 0;
>+}
>+
>+static void i915_pmu_timer_start(struct perf_event *event)
>+{
>+	struct hw_perf_event *hwc = &event->hw;
>+	s64 period;
>+
>+	if (!is_sampling_event(event))
>+		return;
>+
>+	period = local64_read(&hwc->period_left);
>+	if (period) {
>+		if (period < 0)
>+			period = 10000;
>+
>+		local64_set(&hwc->period_left, 0);
>+	} else {
>+		period = max_t(u64, 10000, hwc->sample_period);
>+	}
>+
>+	hrtimer_start_range_ns(&hwc->hrtimer,
>+			       ns_to_ktime(period), 0,
>+			       HRTIMER_MODE_REL_PINNED);
>+}
>+
>+static void i915_pmu_timer_cancel(struct perf_event *event)
>+{
>+	struct hw_perf_event *hwc = &event->hw;
>+
>+	if (!is_sampling_event(event))
>+		return;
>+
>+	local64_set(&hwc->period_left,
>+		    ktime_to_ns(hrtimer_get_remaining(&hwc->hrtimer)));
>+	hrtimer_cancel(&hwc->hrtimer);
>+}
>+
>+static void i915_pmu_enable(struct perf_event *event)
>+{
>+	struct drm_i915_private *i915 =
>+		container_of(event->pmu, typeof(*i915), pmu.base);
>+
>+	if (i915->pmu.enable == 0)
>+		hrtimer_start_range_ns(&i915->pmu.timer,
>+				       ns_to_ktime(PERIOD), 0,
>+				       HRTIMER_MODE_REL_PINNED);
>+
>+	i915->pmu.enable |= BIT_ULL(event->attr.config);
>+
>+	i915_pmu_timer_start(event);
>+}
>+
>+static void i915_pmu_disable(struct perf_event *event)
>+{
>+	struct drm_i915_private *i915 =
>+		container_of(event->pmu, typeof(*i915), pmu.base);
>+
>+	i915->pmu.enable &= ~BIT_ULL(event->attr.config);
>+	i915_pmu_timer_cancel(event);
>+}
>+
>+static int i915_pmu_event_add(struct perf_event *event, int flags)
>+{
>+	struct hw_perf_event *hwc = &event->hw;
>+
>+	if (flags & PERF_EF_START)
>+		i915_pmu_enable(event);
>+
>+	hwc->state = !(flags & PERF_EF_START);
>+
>+	return 0;
>+}
>+
>+static void i915_pmu_event_del(struct perf_event *event, int flags)
>+{
>+	i915_pmu_disable(event);
>+}
>+
>+static void i915_pmu_event_start(struct perf_event *event, int flags)
>+{
>+	i915_pmu_enable(event);
>+}
>+
>+static void i915_pmu_event_stop(struct perf_event *event, int flags)
>+{
>+	i915_pmu_disable(event);
>+}
>+
>+static u64 read_energy_uJ(struct drm_i915_private *dev_priv)
>+{
>+	u64 power;
>+
>+	GEM_BUG_ON(INTEL_GEN(dev_priv) < 6);
>+
>+	intel_runtime_pm_get(dev_priv);
>+
>+	rdmsrl(MSR_RAPL_POWER_UNIT, power);
>+	power = (power & 0x1f00) >> 8;
>+	power = 1000000 >> power; /* convert to uJ */
>+	power *= I915_READ_NOTRACE(MCH_SECP_NRG_STTS);
>+
>+	intel_runtime_pm_put(dev_priv);
>+
>+	return power;
>+}
>+
>+static inline u64 calc_residency(struct drm_i915_private *dev_priv,
>+				 const i915_reg_t reg)
>+{
>+	u64 val, units = 128, div = 100000;
>+
>+	GEM_BUG_ON(INTEL_GEN(dev_priv) < 6);
>+
>+	intel_runtime_pm_get(dev_priv);
>+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
>+		div = dev_priv->czclk_freq;
>+		units = 1;
>+		if (I915_READ_NOTRACE(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH)
>+			units <<= 8;
>+	} else if (IS_GEN9_LP(dev_priv)) {
>+		div = 1200;
>+		units = 1;
>+	}
>+	val = I915_READ_NOTRACE(reg);
>+	intel_runtime_pm_put(dev_priv);
>+
>+	val *= units;
>+	return DIV_ROUND_UP_ULL(val, div);
>+}
>+
>+static u64 count_interrupts(struct drm_i915_private *i915)
>+{
>+	/* open-coded kstat_irqs() */
>+	struct irq_desc *desc = irq_to_desc(i915->drm.pdev->irq);
>+	u64 sum = 0;
>+	int cpu;
>+
>+	if (!desc || !desc->kstat_irqs)
>+		return 0;
>+
>+	for_each_possible_cpu(cpu)
>+		sum += *per_cpu_ptr(desc->kstat_irqs, cpu);
>+
>+	return sum;
>+}
>+
>+static void i915_pmu_event_read(struct perf_event *event)
>+{
>+	struct drm_i915_private *i915 =
>+		container_of(event->pmu, typeof(*i915), pmu.base);
>+	u64 val = 0;
>+
>+	if (event->attr.config < 32) {
>+		int engine = event->attr.config >> 2;
>+		int sample = event->attr.config & 3;
>+		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];
>+		break;
>+	case I915_PMU_REQUESTED_FREQUENCY:
>+		val = i915->pmu.sample[__I915_SAMPLE_FREQ_REQ];
>+		break;
>+	case I915_PMU_ENERGY:
>+		val = read_energy_uJ(i915);
>+		break;
>+	case I915_PMU_INTERRUPTS:
>+		val = count_interrupts(i915);
>+		break;
>+
>+	case I915_PMU_RC6_RESIDENCY:
>+		if (!i915->gt.awake)
>+			return;
>+
>+		val = calc_residency(i915, IS_VALLEYVIEW(i915) ? VLV_GT_RENDER_RC6 : GEN6_GT_GFX_RC6);
>+		break;
>+
>+	case I915_PMU_RC6p_RESIDENCY:
>+		if (!i915->gt.awake)
>+			return;
>+
>+		if (!IS_VALLEYVIEW(i915))
>+			val = calc_residency(i915, GEN6_GT_GFX_RC6p);
>+		break;
>+
>+	case I915_PMU_RC6pp_RESIDENCY:
>+		if (!i915->gt.awake)
>+			return;
>+
>+		if (!IS_VALLEYVIEW(i915))
>+			val = calc_residency(i915, GEN6_GT_GFX_RC6pp);
>+		break;
>+	}
>+
>+	local64_set(&event->count, val);
>+}
>+
>+static int i915_pmu_event_event_idx(struct perf_event *event)
>+{
>+	return 0;
>+}
>+
>+void i915_pmu_register(struct drm_i915_private *i915)
>+{
>+	if (INTEL_GEN(i915) <= 2)
>+		return;
>+
>+	i915->pmu.base.task_ctx_nr	= perf_sw_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;
>+
>+	hrtimer_init(&i915->pmu.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>+	i915->pmu.timer.function = i915_sample;
>+	i915->pmu.enable = 0;
>+
>+	if (perf_pmu_register(&i915->pmu.base, "i915", -1))
>+		i915->pmu.base.event_init = NULL;
>+}
>+
>+void i915_pmu_unregister(struct drm_i915_private *i915)
>+{
>+	if (!i915->pmu.base.event_init)
>+		return;
>+
>+	i915->pmu.enable = 0;
>+
>+	perf_pmu_unregister(&i915->pmu.base);
>+	i915->pmu.base.event_init = NULL;
>+
>+	hrtimer_cancel(&i915->pmu.timer);
>+}
>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
>index d33c93444c0d..0877b151239d 100644
>--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
>@@ -245,6 +245,8 @@ struct intel_engine_cs {
> 		I915_SELFTEST_DECLARE(bool mock : 1);
> 	} breadcrumbs;
>
>+	u64 pmu_sample[4];
>+
> 	/*
> 	 * A pool of objects to use as shadow copies of client batch buffers
> 	 * when the command parser is enabled. Prevents the client from
>diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>index 7ccbd6a2bbe0..733774f19a0b 100644
>--- a/include/uapi/drm/i915_drm.h
>+++ b/include/uapi/drm/i915_drm.h
>@@ -86,6 +86,47 @@ enum i915_mocs_table_index {
> 	I915_MOCS_CACHED,
> };
>
>+/**
>+ * DOC: perf_events exposed by i915 through /sys/bus/event_sources/drivers/i915
>+ *
>+ */
>+#define I915_SAMPLE_QUEUED	0
>+#define I915_SAMPLE_BUSY	1
>+#define I915_SAMPLE_WAIT	2
>+#define I915_SAMPLE_SEMA	3
>+
>+#define I915_SAMPLE_RCS		0
>+#define I915_SAMPLE_VCS		1
>+#define I915_SAMPLE_BCS		2
>+#define I915_SAMPLE_VECS	3
>+
>+#define __I915_PMU_COUNT(ring, id) ((ring) << 2 | (id))
>+
>+#define I915_PMU_COUNT_RCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_RCS, I915_SAMPLE_BUSY)
>+#define I915_PMU_COUNT_RCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_RCS, I915_SAMPLE_WAIT)
>+#define I915_PMU_COUNT_RCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_RCS, I915_SAMPLE_SEMA)
>+
>+#define I915_PMU_COUNT_VCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_BUSY)
>+#define I915_PMU_COUNT_VCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_WAIT)
>+#define I915_PMU_COUNT_VCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_VCS, I915_SAMPLE_SEMA)
>+
>+#define I915_PMU_COUNT_BCS_BUSY __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_BUSY)
>+#define I915_PMU_COUNT_BCS_WAIT __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_WAIT)
>+#define I915_PMU_COUNT_BCS_SEMA __I915_PMU_COUNT(I915_SAMPLE_BCS, I915_SAMPLE_SEMA)
>+
>+#define I915_PMU_COUNT_VECS_BUSY __I915_PMU_COUNT(I915_SAMPLE_VECS, I915_SAMPLE_BUSY)
>+#define I915_PMU_COUNT_VECS_WAIT __I915_PMU_COUNT(I915_SAMPLE_VECS, I915_SAMPLE_WAIT)
>+#define I915_PMU_COUNT_VECS_SEMA __I915_PMU_COUNT(I915_SAMPLE_VECS, I915_SAMPLE_SEMA)
>+
>+#define I915_PMU_ACTUAL_FREQUENCY 32
>+#define I915_PMU_REQUESTED_FREQUENCY 33

This one seems less than useful.

>+#define I915_PMU_ENERGY 34
>+#define I915_PMU_INTERRUPTS 35
>+
>+#define I915_PMU_RC6_RESIDENCY		40
>+#define I915_PMU_RC6p_RESIDENCY	41
>+#define I915_PMU_RC6pp_RESIDENCY	42
>+
> /* Each region is a minimum of 16k, and there are at most 255 of them.
>  */
> #define I915_NR_TEX_REGIONS 255	/* table size 2k - maximum due to use
>diff --git a/kernel/events/core.c b/kernel/events/core.c
>index e46eba8cd1b7..7b8c6dce1078 100644
>--- a/kernel/events/core.c
>+++ b/kernel/events/core.c
>@@ -7386,6 +7386,7 @@ int perf_event_overflow(struct perf_event *event,
> {
> 	return __perf_event_overflow(event, 1, data, regs);
> }
>+EXPORT_SYMBOL_GPL(perf_event_overflow);
>
> /*
>  * Generic software event infrastructure
>-- 
>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

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

[Ben Widawsky]

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

Re: [RFC 07/14] drm/i915/pmu: Add fake regs

[Ben Widawsky]

On 17-07-18 15:36:11, Tvrtko Ursulin wrote:
>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
>Without this I can get a null ptr deref when trying to access
>our events with perf.
>
>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This definitely seems unsafe, but should be squashed in somewhere earlier...

>---
> drivers/gpu/drm/i915/i915_pmu.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
>index 4d61a1e72ee6..4195d89b1c82 100644
>--- a/drivers/gpu/drm/i915/i915_pmu.c
>+++ b/drivers/gpu/drm/i915/i915_pmu.c
>@@ -183,8 +183,11 @@ static int engine_event_init(struct perf_event *event)
> 	return 0;
> }
>
>+static DEFINE_PER_CPU(struct pt_regs, i915_pmu_pt_regs);
>+
> static enum hrtimer_restart hrtimer_sample(struct hrtimer *hrtimer)
> {
>+	struct pt_regs *regs = this_cpu_ptr(&i915_pmu_pt_regs);
> 	struct perf_sample_data data;
> 	struct perf_event *event;
> 	u64 period;
>@@ -196,7 +199,7 @@ static enum hrtimer_restart hrtimer_sample(struct hrtimer *hrtimer)
> 	event->pmu->read(event);
>
> 	perf_sample_data_init(&data, 0, event->hw.last_period);
>-	perf_event_overflow(event, &data, NULL);
>+	perf_event_overflow(event, &data, regs);
>
> 	period = max_t(u64, 10000, event->hw.sample_period);
> 	hrtimer_forward_now(hrtimer, ns_to_ktime(period));
>-- 
>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

Re: [RFC 12/14] drm/i915: Interface for controling engine stats collection

[Ben Widawsky]

On 17-07-19 10:34:14, Tvrtko Ursulin wrote:
>
>Hi Ben,
>
>On 18/07/2017 15:36, Tvrtko Ursulin wrote:
>>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>>Enables other i915 components to enable and disable
>>the facility as needed.
>>
>>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>---
>>  drivers/gpu/drm/i915/intel_engine_cs.c  | 53 +++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_ringbuffer.h |  5 ++++
>>  2 files changed, 58 insertions(+)
>>
>>diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
>>index 3e5e08c6b5ef..03e7459bad06 100644
>>--- a/drivers/gpu/drm/i915/intel_engine_cs.c
>>+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
>>@@ -29,6 +29,8 @@
>>  #include "intel_lrc.h"
>>  DEFINE_STATIC_KEY_FALSE(i915_engine_stats_key);
>>+static DEFINE_MUTEX(i915_engine_stats_mutex);
>>+static int i915_engine_stats_ref;
>>  /* Haswell does have the CXT_SIZE register however it does not appear to be
>>   * valid. Now, docs explain in dwords what is in the context object. The full
>>@@ -1340,6 +1342,57 @@ void intel_engines_mark_idle(struct drm_i915_private *i915)
>>  	}
>>  }
>>+int intel_enable_engine_stats(struct drm_i915_private *dev_priv)
>>+{
>>+	if (!i915.enable_execlists)
>>+		return -ENODEV;
>>+
>>+	mutex_lock(&i915_engine_stats_mutex);
>>+	if (i915_engine_stats_ref++ == 0) {
>>+		struct intel_engine_cs *engine;
>>+		enum intel_engine_id id;
>>+
>>+		for_each_engine(engine, dev_priv, id) {
>>+			memset(&engine->stats, 0, sizeof(engine->stats));
>>+			spin_lock_init(&engine->stats.lock);
>>+		}
>>+
>>+		static_branch_enable(&i915_engine_stats_key);
>>+	}
>>+	mutex_unlock(&i915_engine_stats_mutex);
>>+
>>+	return 0;
>>+}
>>+
>>+void intel_disable_engine_stats(void)
>>+{
>>+	mutex_lock(&i915_engine_stats_mutex);
>>+	if (--i915_engine_stats_ref == 0)
>>+		static_branch_disable(&i915_engine_stats_key);
>>+	mutex_unlock(&i915_engine_stats_mutex);
>>+}
>>+
>>+u64 intel_engine_get_current_busy_ns(struct intel_engine_cs *engine)
>>+{
>>+	unsigned long flags;
>>+	u64 total;
>>+
>>+	spin_lock_irqsave(&engine->stats.lock, flags);
>>+
>>+	total = engine->stats.total;
>>+
>>+	/*
>>+	 * If the engine is executing something at the moment
>>+	 * add it to the total.
>>+	 */
>>+	if (engine->stats.ref)
>>+		total += ktime_get_real_ns() - engine->stats.start;
>>+
>>+	spin_unlock_irqrestore(&engine->stats.lock, flags);
>>+
>>+	return total;
>>+}
>>+
>>  #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
>>  #include "selftests/mock_engine.c"
>>  #endif
>>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
>>index 2eb1e970ad06..e0f495a6d0d9 100644
>>--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
>>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
>>@@ -776,4 +776,9 @@ static inline void intel_engine_context_out(struct intel_engine_cs *engine)
>>  	}
>>  }
>>+int intel_enable_engine_stats(struct drm_i915_private *i915);
>>+void intel_disable_engine_stats(void);
>>+
>>+u64 intel_engine_get_current_busy_ns(struct intel_engine_cs *engine);
>
>If we exported these symbols for other modules to use, what kind of 
>API would they need? Presumably not per-engine but something to give 
>the aggregated busyness of all engines? Or I have misunderstood you 
>that there is this requirement?
>
>Regards,
>
>Tvrtko

No misunderstanding. For our current usage, busyness of all engines would be
easiest. If one of the engines doesn't contribute much to the total TDP though,
it wouldn't need to actually be included, so we could perhaps leave room for
per-engine.

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

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

[Tvrtko Ursulin]

On 25/07/2017 02:18, Ben Widawsky wrote:
> 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?

It is to decouple the ABI namespace from the driver internal enumeration 
of engines and so allow the latter to be refactored in the future, and 
at the same time fix the fist patch to index engines other than RCS 
correctly.

I did not bother with smart commit message for the first few patches 
since they really should be squashed with the first one from Chris.

Regards,

Tvrtko


>> 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
>>
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 07/14] drm/i915/pmu: Add fake regs

[Tvrtko Ursulin]

On 25/07/2017 02:20, Ben Widawsky wrote:
> On 17-07-18 15:36:11, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Without this I can get a null ptr deref when trying to access
>> our events with perf.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> This definitely seems unsafe, but should be squashed in somewhere 
> earlier...

Yep, the same for all patches at least up to this one. I did not want to 
just take over Chris' patch, especially since the uncertainty on this 
one however is that I am not sure whether the perf interactions are 
correct. Action on me here, as discussed in cover letter discussion, is 
to try to get some clarification from Peter Zijlstra and what and how 
exactly we should be doing. Now that I have the internal API 
clarification from you I can tidy the series, respin, and do that.

Regards,

Tvrtko

> 
>> ---
>> drivers/gpu/drm/i915/i915_pmu.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_pmu.c 
>> b/drivers/gpu/drm/i915/i915_pmu.c
>> index 4d61a1e72ee6..4195d89b1c82 100644
>> --- a/drivers/gpu/drm/i915/i915_pmu.c
>> +++ b/drivers/gpu/drm/i915/i915_pmu.c
>> @@ -183,8 +183,11 @@ static int engine_event_init(struct perf_event 
>> *event)
>>     return 0;
>> }
>>
>> +static DEFINE_PER_CPU(struct pt_regs, i915_pmu_pt_regs);
>> +
>> static enum hrtimer_restart hrtimer_sample(struct hrtimer *hrtimer)
>> {
>> +    struct pt_regs *regs = this_cpu_ptr(&i915_pmu_pt_regs);
>>     struct perf_sample_data data;
>>     struct perf_event *event;
>>     u64 period;
>> @@ -196,7 +199,7 @@ static enum hrtimer_restart hrtimer_sample(struct 
>> hrtimer *hrtimer)
>>     event->pmu->read(event);
>>
>>     perf_sample_data_init(&data, 0, event->hw.last_period);
>> -    perf_event_overflow(event, &data, NULL);
>> +    perf_event_overflow(event, &data, regs);
>>
>>     period = max_t(u64, 10000, event->hw.sample_period);
>>     hrtimer_forward_now(hrtimer, ns_to_ktime(period));
>> -- 
>> 2.9.4
>>
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 00/14] i915 PMU and engine busy stats

[Tvrtko Ursulin]

On 20/07/2017 10:03, Tvrtko Ursulin wrote:
> On 19/07/2017 13:05, Chris Wilson wrote:
>> Quoting Tvrtko Ursulin (2017-07-18 15:36:04)
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> Rough sketch of the idea I mentioned a few times to various people - 
>>> merging
>>> the engine busyness tracking with Chris i915 PMU RFC.
>>>
>>> First patch is the actual PMU RFC by Chris. It is followed by some 
>>> cleanup
>>> patches, then come a few improvements, cheap execlists engine 
>>> busyness tracking,
>>> debugfs view for the same, and finally the i915 PMU is extended to 
>>> use this
>>> instead of timer based mmio sampling.
>>>
>>> This makes it cheaper and also more accurate since engine busyness is 
>>> not
>>> derived via sampling.
>>>
>>> But I haven't figure out the perf API yet. For example is it possible 
>>> to access
>>> our events in an usable fashion via perf top/stat or something? Do we 
>>> want to
>>> make the events discoverable as I did (patch 8).
>>
>> In my dreams I have gpu activity in the same perf timechart as gpu
>> activity. But that can be mostly by the request tracepoints, but still
>> overlaying cpu/gpu activity is desirable and more importantly we want to
>> coordinate with nouveau/amdgpu so that such interfaces are as agnostic
>> as possible. There are definitely a bunch of global features in common
>> for all (engine enumeration & activity, mempool enumeration, size &
>> activty, power usage?). But the key question is how do we build for the
>> future? Split the event id range into common/driver?
> 
> I don't know if going for common events would be workable. A few metrics 
> sounds like it could be generic, but I am not sure there would be more 
> than a couple where that would be future proof. Also is the coordination 
> effort (no one else seems to implement a perf interface at the moment) 
> worth it at the current time? I am not sure.
> 
>>> I could not find much (any?) kernel API level documentation for perf.
>>
>> There isn't much indeed. Given that we now have a second pair of eyes go
>> over the sampling and improve its interaction with i915, we should start
>> getting PeterZ involved to check the interaction with perf.
> 
> Okay, I guess another cleanup pass and then I can do that.
> 
> In the meantime do you have any good understanding of what kind of 
> events are we exposing here? They look weird if I record them and look 
> with "perf script", and "perf stat" always reports zeroes for them. But 
> they still work from the overlay tool. So it is a bit of a mystery to me 
> what they really are.
>>> Btw patch series actually works since intel-gpu-overlay can use these 
>>> events
>>> when they are available.
>>>
>>> Chris Wilson (1):
>>>    RFC drm/i915: Expose a PMU interface for perf queries
>>
>> One thing I would like is for any future interface (including this
>> engine/class/event id) to use the engine class/instance mapping.
> 
> I was thinking about that myself. I can do it in the next cleanup pass.

Although to do this I think it will make more sense for me to squash a 
bunch of improvements into your patch, and to start working on it 
directly. Your thoughts on this? Do you mind if I start working on the 
original patch bumping its version number for all the additions?

This would mean squashing in probably the first eight patches from this 
series. Followed by reworking it towards class-instance. And the rc6 
residency consolidation Sagar suggested.

How do we keep shared authorship in this case? Can we have two From: 
lines at the top?

Regards,

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

Re: [RFC 00/14] i915 PMU and engine busy stats

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-07-26 11:34:49)
> 
> On 20/07/2017 10:03, Tvrtko Ursulin wrote:
> > On 19/07/2017 13:05, Chris Wilson wrote:
> >> Quoting Tvrtko Ursulin (2017-07-18 15:36:04)
> >>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>
> >>> Rough sketch of the idea I mentioned a few times to various people - 
> >>> merging
> >>> the engine busyness tracking with Chris i915 PMU RFC.
> >>>
> >>> First patch is the actual PMU RFC by Chris. It is followed by some 
> >>> cleanup
> >>> patches, then come a few improvements, cheap execlists engine 
> >>> busyness tracking,
> >>> debugfs view for the same, and finally the i915 PMU is extended to 
> >>> use this
> >>> instead of timer based mmio sampling.
> >>>
> >>> This makes it cheaper and also more accurate since engine busyness is 
> >>> not
> >>> derived via sampling.
> >>>
> >>> But I haven't figure out the perf API yet. For example is it possible 
> >>> to access
> >>> our events in an usable fashion via perf top/stat or something? Do we 
> >>> want to
> >>> make the events discoverable as I did (patch 8).
> >>
> >> In my dreams I have gpu activity in the same perf timechart as gpu
> >> activity. But that can be mostly by the request tracepoints, but still
> >> overlaying cpu/gpu activity is desirable and more importantly we want to
> >> coordinate with nouveau/amdgpu so that such interfaces are as agnostic
> >> as possible. There are definitely a bunch of global features in common
> >> for all (engine enumeration & activity, mempool enumeration, size &
> >> activty, power usage?). But the key question is how do we build for the
> >> future? Split the event id range into common/driver?
> > 
> > I don't know if going for common events would be workable. A few metrics 
> > sounds like it could be generic, but I am not sure there would be more 
> > than a couple where that would be future proof. Also is the coordination 
> > effort (no one else seems to implement a perf interface at the moment) 
> > worth it at the current time? I am not sure.
> > 
> >>> I could not find much (any?) kernel API level documentation for perf.
> >>
> >> There isn't much indeed. Given that we now have a second pair of eyes go
> >> over the sampling and improve its interaction with i915, we should start
> >> getting PeterZ involved to check the interaction with perf.
> > 
> > Okay, I guess another cleanup pass and then I can do that.
> > 
> > In the meantime do you have any good understanding of what kind of 
> > events are we exposing here? They look weird if I record them and look 
> > with "perf script", and "perf stat" always reports zeroes for them. But 
> > they still work from the overlay tool. So it is a bit of a mystery to me 
> > what they really are.
> >>> Btw patch series actually works since intel-gpu-overlay can use these 
> >>> events
> >>> when they are available.
> >>>
> >>> Chris Wilson (1):
> >>>    RFC drm/i915: Expose a PMU interface for perf queries
> >>
> >> One thing I would like is for any future interface (including this
> >> engine/class/event id) to use the engine class/instance mapping.
> > 
> > I was thinking about that myself. I can do it in the next cleanup pass.
> 
> Although to do this I think it will make more sense for me to squash a 
> bunch of improvements into your patch, and to start working on it 
> directly. Your thoughts on this? Do you mind if I start working on the 
> original patch bumping its version number for all the additions?

Don't mind in the slightest. I thought you had broken them out for a
quick series of "oh, yes, completely missed that" to be squashed in when
the patch is ready, and you feel comfortable on signing off on the whole
thing.
 
> This would mean squashing in probably the first eight patches from this 
> series. Followed by reworking it towards class-instance. And the rc6 
> residency consolidation Sagar suggested.
> 
> How do we keep shared authorship in this case? Can we have two From: 
> lines at the top?

Take it. If you want leave a based on a patch by me, but 90% of the work
is in the last 10% writing the patch, so you will be doing more work to
bring the interface to a reality than I did.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx