i915 PMU and engine busy stats

PATCH Superseded Tvrtko Ursulin 5 participants 21 messages Sep 18, 2017 – Sep 20, 2017

Subsystems (3)

DRM drivers DRM misc Intel i915

lore

Share & tools

lore

Tvrtko Ursulin

author maintainer

2017-09-18 11:38 UTC #0 lore raw

v4 cover i915 PMU and engine busy stats

From: Tvrtko Ursulin <[email protected]>

Fourth spin of the i915 PMU series. Now with the RFC tag removed.

This is a cleaned up and bugfixed version with incorporated inital review feedback. Some patches have been dropped for time being.

Patches 1-2 are small refactors to make the following work easier.

Patch 3 is the main bit.

Patch 4 is a small optimisation on top, to only run the sampling timer when it is required. (Depending on GPU awake status and active PMU counters.)

Patches 5-7 add software engine busyness tracking, which is then used from the PMU in patch 7. This allows more efficient and more accurate engine busyness metric.

And finally patch 8 is an additional optimisation which hides the whole cost of the software engine busyness tracking behind a single nop instruction in the off case.

Tvrtko Ursulin (8):
drm/i915: Convert intel_rc6_residency_us to ns drm/i915: Extract intel_get_cagf
drm/i915/pmu: Expose a PMU interface for perf queries drm/i915/pmu: Suspend sampling when GPU is idle drm/i915: Wrap context schedule notification drm/i915: Engine busy time tracking drm/i915/pmu: Wire up engine busy stats to PMU drm/i915: Gate engine stats collection with a static key

 drivers/gpu/drm/i915/Makefile           |   1 +
 drivers/gpu/drm/i915/i915_debugfs.c     |   9 +-
 drivers/gpu/drm/i915/i915_drv.c         |   2 +
 drivers/gpu/drm/i915/i915_drv.h         |  96 +++-
 drivers/gpu/drm/i915/i915_gem.c         |   1 +
 drivers/gpu/drm/i915/i915_gem_request.c |   1 +
 drivers/gpu/drm/i915/i915_pmu.c         | 853 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h         |   3 +
 drivers/gpu/drm/i915/i915_sysfs.c       |  20 +-
 drivers/gpu/drm/i915/intel_engine_cs.c  | 111 +++++
 drivers/gpu/drm/i915/intel_lrc.c        |  19 +-
 drivers/gpu/drm/i915/intel_pm.c         |  41 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c |  25 +
 drivers/gpu/drm/i915/intel_ringbuffer.h | 147 ++++++
 include/uapi/drm/i915_drm.h             |  57 +++
 15 files changed, 1351 insertions(+), 35 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_pmu.c

Signature · 6 lines

2.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Tvrtko Ursulin

author maintainer

2017-09-18 11:38 UTC #1 lore raw

PATCH v4 1/8 drm/i915: Convert intel_rc6_residency_us to ns

From: Tvrtko Ursulin <[email protected]>

Will be used for exposing the PMU counters.

v2:
* Move intel_runtime_pm_get/put to the callers. (Chris Wilson)
* Restore full unit conversion precision.

Signed-off-by Tvrtko Ursulin <[email protected]>

---
 drivers/gpu/drm/i915/i915_drv.h   |  8 +++++++-
 drivers/gpu/drm/i915/i915_sysfs.c |  9 +++++++--
 drivers/gpu/drm/i915/intel_pm.c   | 27 +++++++++++++--------------
 3 files changed, 27 insertions(+), 17 deletions(-)

drivers/gpu/drm/i915/i915_drv.h +7 -1

index 8b735b69f6ed..4571cbff13a9 100644
@@ -4125,9 +4125,15 @@ void vlv_phy_reset_lanes(struct intel_encoder *encoder);
 
 int intel_gpu_freq(struct drm_i915_private *dev_priv, int val);
 int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
-u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv,
+u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv,
 			   const i915_reg_t reg);
 
+static inline u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv,
+					 const i915_reg_t reg)
+{
+	return DIV_ROUND_UP_ULL(intel_rc6_residency_ns(dev_priv, reg), 1000);
+}
+
 #define I915_READ8(reg)		dev_priv->uncore.funcs.mmio_readb(dev_priv, (reg), true)
 #define I915_WRITE8(reg, val)	dev_priv->uncore.funcs.mmio_writeb(dev_priv, (reg), (val), true)

drivers/gpu/drm/i915/i915_sysfs.c +7 -2

index d61c8727f756..1a34d32d0092 100644
@@ -42,8 +42,13 @@ static inline struct drm_i915_private *kdev_minor_to_i915(struct device *kdev)
 static u32 calc_residency(struct drm_i915_private *dev_priv,
 			  i915_reg_t reg)
 {
-	return DIV_ROUND_CLOSEST_ULL(intel_rc6_residency_us(dev_priv, reg),
-				     1000);
+	u64 res;
+
+	intel_runtime_pm_get(dev_priv);
+	res = intel_rc6_residency_us(dev_priv, reg);
+	intel_runtime_pm_put(dev_priv);
+
+	return DIV_ROUND_CLOSEST_ULL(res, 1000);
 }
 
 static ssize_t

drivers/gpu/drm/i915/intel_pm.c +13 ~~-14~~

index 94624ede3479..9c4588369452 100644
@@ -9342,34 +9342,33 @@ static u64 vlv_residency_raw(struct drm_i915_private *dev_priv,
 	return lower | (u64)upper << 8;
 }
 
-u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv,
+u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv,
 			   const i915_reg_t reg)
 {
-	u64 time_hw, units, div;
+	u64 time_hw;
+	u32 mul, div;
 
 	if (!intel_enable_rc6())
 		return 0;
 
-	intel_runtime_pm_get(dev_priv);
-
 	/* On VLV and CHV, residency time is in CZ units rather than 1.28us */
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		units = 1000;
+		mul = 1000000;
 		div = dev_priv->czclk_freq;
-
 		time_hw = vlv_residency_raw(dev_priv, reg);
-	} else if (IS_GEN9_LP(dev_priv)) {
-		units = 1000;
-		div = 1200;		/* 833.33ns */
 
-		time_hw = I915_READ(reg);
 	} else {
-		units = 128000; /* 1.28us */
-		div = 100000;
+		/* 833.33ns units on Gen9LP, 1.28us elsewhere. */
+		if (IS_GEN9_LP(dev_priv)) {
+			mul = 10000;
+			div = 12;
+		} else {
+			mul = 1280;
+			div = 1;
+		}
 
 		time_hw = I915_READ(reg);
 	}
 
-	intel_runtime_pm_put(dev_priv);
-	return DIV_ROUND_UP_ULL(time_hw * units, div);
+	return DIV_ROUND_UP_ULL(time_hw * mul, div);
 }

Signature · 6 lines

2.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Tvrtko Ursulin

author maintainer

2017-09-18 11:38 UTC #2 lore raw

PATCH v4 2/8 drm/i915: Extract intel_get_cagf

In reply to Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

Code to be shared between debugfs and the PMU implementation.

v2: Checkpatch cleanup.
v3: Also consolidate i915_sysfs.c/gt_act_freq_mhz_show.

Signed-off-by Tvrtko Ursulin <[email protected]>

Reviewed-by Chris Wilson <[email protected]> (v2)

---
 drivers/gpu/drm/i915/i915_debugfs.c |  9 ++-------
 drivers/gpu/drm/i915/i915_drv.h     |  2 ++
 drivers/gpu/drm/i915/i915_sysfs.c   | 11 +++--------
 drivers/gpu/drm/i915/intel_pm.c     | 14 ++++++++++++++
 4 files changed, 21 insertions(+), 15 deletions(-)

drivers/gpu/drm/i915/i915_debugfs.c +2 -7

index 62133dd303ac..98a735ecaeaf 100644
@@ -1113,13 +1113,8 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
 		rpdownei = I915_READ(GEN6_RP_CUR_DOWN_EI) & GEN6_CURIAVG_MASK;
 		rpcurdown = I915_READ(GEN6_RP_CUR_DOWN) & GEN6_CURBSYTAVG_MASK;
 		rpprevdown = I915_READ(GEN6_RP_PREV_DOWN) & GEN6_CURBSYTAVG_MASK;
-		if (INTEL_GEN(dev_priv) >= 9)
-			cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
-		else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-			cagf = (rpstat & HSW_CAGF_MASK) >> HSW_CAGF_SHIFT;
-		else
-			cagf = (rpstat & GEN6_CAGF_MASK) >> GEN6_CAGF_SHIFT;
-		cagf = intel_gpu_freq(dev_priv, cagf);
+		cagf = intel_gpu_freq(dev_priv,
+				      intel_get_cagf(dev_priv, rpstat));
 
 		intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);

drivers/gpu/drm/i915/i915_drv.h +2 -0

index 4571cbff13a9..7d3f5a5c2454 100644
@@ -4134,6 +4134,8 @@ static inline u64 intel_rc6_residency_us(struct drm_i915_private *dev_priv,
 	return DIV_ROUND_UP_ULL(intel_rc6_residency_ns(dev_priv, reg), 1000);
 }
 
+u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat1);
+
 #define I915_READ8(reg)		dev_priv->uncore.funcs.mmio_readb(dev_priv, (reg), true)
 #define I915_WRITE8(reg, val)	dev_priv->uncore.funcs.mmio_writeb(dev_priv, (reg), (val), true)

drivers/gpu/drm/i915/i915_sysfs.c +3 -8

index 1a34d32d0092..2f74ae55332d 100644
@@ -257,14 +257,9 @@ static ssize_t gt_act_freq_mhz_show(struct device *kdev,
 		freq = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
 		ret = intel_gpu_freq(dev_priv, (freq >> 8) & 0xff);
 	} else {
-		u32 rpstat = I915_READ(GEN6_RPSTAT1);
-		if (INTEL_GEN(dev_priv) >= 9)
-			ret = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
-		else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
-			ret = (rpstat & HSW_CAGF_MASK) >> HSW_CAGF_SHIFT;
-		else
-			ret = (rpstat & GEN6_CAGF_MASK) >> GEN6_CAGF_SHIFT;
-		ret = intel_gpu_freq(dev_priv, ret);
+		ret = intel_gpu_freq(dev_priv,
+				     intel_get_cagf(dev_priv,
+						    I915_READ(GEN6_RPSTAT1)));
 	}
 	mutex_unlock(&dev_priv->rps.hw_lock);

drivers/gpu/drm/i915/intel_pm.c +14 -0

index 9c4588369452..97a7fd74da3e 100644
@@ -9372,3 +9372,17 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv,
 
 	return DIV_ROUND_UP_ULL(time_hw * mul, div);
 }
+
+u32 intel_get_cagf(struct drm_i915_private *dev_priv, u32 rpstat)
+{
+	u32 cagf;
+
+	if (INTEL_GEN(dev_priv) >= 9)
+		cagf = (rpstat & GEN9_CAGF_MASK) >> GEN9_CAGF_SHIFT;
+	else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+		cagf = (rpstat & HSW_CAGF_MASK) >> HSW_CAGF_SHIFT;
+	else
+		cagf = (rpstat & GEN6_CAGF_MASK) >> GEN6_CAGF_SHIFT;
+
+	return  cagf;
+}

Signature · 6 lines

2.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Tvrtko Ursulin

author maintainer

2017-09-18 11:38 UTC #3 lore raw

PATCH v4 3/8 drm/i915/pmu: Expose a PMU interface for perf queries

In reply to Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

From: Chris Wilson <[email protected]>
From: Tvrtko Ursulin <[email protected]>
From: Dmitry Rogozhkin <[email protected]>

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.

v1-v2 (Chris Wilson):

v2: Use a common timer for the ring sampling.

v3: (Tvrtko Ursulin)
* Decouple uAPI from i915 engine ids.
* Complete uAPI defines.
* Refactor some code to helpers for clarity.
* Skip sampling disabled engines.
* Expose counters in sysfs.
* Pass in fake regs to avoid null ptr deref in perf core.
* Convert to class/instance uAPI.
* Use shared driver code for rc6 residency, power and frequency.

v4: (Dmitry Rogozhkin)
* Register PMU with .task_ctx_nr=perf_invalid_context
* Expose cpumask for the PMU with the single CPU in the mask
* Properly support pmu->stop(): it should call pmu->read()
* Properly support pmu->del(): it should call stop(event, PERF_EF_UPDATE)
* Introduce refcounting of event subscriptions.
* Make pmu.busy_stats a refcounter to avoid busy stats going away with some deleted event.
* Expose cpumask for i915 PMU to avoid multiple events creation of the same type followed by counter aggregation by perf-stat.
* Track CPUs getting online/offline to migrate perf context. If (likely) cpumask will initially set CPU0, CONFIG_BOOTPARAM_HOTPLUG_CPU0 will be needed to see effect of CPU status tracking.
* End result is that only global events are supported and perf stat works correctly.
* Deny perf driver level sampling - it is prohibited for uncore PMU.

v5: (Tvrtko Ursulin)

* Don't hardcode number of engine samplers.
* Rewrite event ref-counting for correctness and simplicity.
* Store initial counter value when starting already enabled events to correctly report values to all listeners.
* Fix RC6 residency readout.
* Comments, GPL header.

v6:
* Add missing entry to v4 changelog.
* Fix accounting in CPU hotplug case by copying the approach from arch/x86/events/intel/cstate.c. (Dmitry Rogozhkin)

v7:
* Log failure message only on failure.
* Remove CPU hotplug notification state on unregister.

v8:
* Fix error unwind on failed registration.
* Checkpatch cleanup.

v9:
* Drop the energy metric, it is available via intel_rapl_perf. (Ville Syrjälä)
* Use HAS_RC6(p). (Chris Wilson)
* Handle unsupported non-engine events. (Dmitry Rogozhkin)
* Rebase for intel_rc6_residency_ns needing caller managed runtime pm.
* Drop HAS_RC6 checks from the read callback since creating those events will be rejected at init time already.
* Add counter units to sysfs so perf stat output is nicer.
* Cleanup the attribute tables for brevity and readability.

Signed-off-by Chris Wilson <[email protected]>

Signed-off-by Tvrtko Ursulin <[email protected]>

Signed-off-by Dmitry Rogozhkin <[email protected]>

Cc Tvrtko Ursulin <[email protected]>

Cc Chris Wilson <[email protected]>

Cc Dmitry Rogozhkin <[email protected]>

Cc Peter Zijlstra <[email protected]>

---
 drivers/gpu/drm/i915/Makefile           |   1 +
 drivers/gpu/drm/i915/i915_drv.c         |   2 +
 drivers/gpu/drm/i915/i915_drv.h         |  78 ++++
 drivers/gpu/drm/i915/i915_pmu.c         | 705 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h         |   3 +
 drivers/gpu/drm/i915/intel_engine_cs.c  |  10 +
 drivers/gpu/drm/i915/intel_ringbuffer.c |  25 ++
 drivers/gpu/drm/i915/intel_ringbuffer.h |  25 ++
 include/uapi/drm/i915_drm.h             |  57 +++
 9 files changed, 906 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/i915_pmu.c

drivers/gpu/drm/i915/Makefile +1 -0

index 1cb8059a3a16..7b3a0eca62b6 100644
@@ -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 \

drivers/gpu/drm/i915/i915_drv.c +2 -0

index 5c111ea96e80..b1f96eb1be16 100644
@@ -1196,6 +1196,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,

@@ -1250,6 +1251,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);

drivers/gpu/drm/i915/i915_drv.h +78 -0

108 lines not on this page · raw message

drivers/gpu/drm/i915/i915_pmu.c new file +705 -0

705 lines not on this page · raw message

drivers/gpu/drm/i915/i915_reg.h +3 -0

index 94b40a469afd..971d6a7fd9ac 100644
@@ -186,6 +186,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define VIDEO_ENHANCEMENT_CLASS	2
 #define COPY_ENGINE_CLASS	3
 #define OTHER_CLASS		4
+#define MAX_ENGINE_CLASS	4
+
+#define MAX_ENGINE_INSTANCE    1
 
 /* PCI config space */

drivers/gpu/drm/i915/intel_engine_cs.c +10 -0

index 3d135c3cd380..82510d971c7d 100644
@@ -198,6 +198,15 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 	GEM_BUG_ON(info->class >= ARRAY_SIZE(intel_engine_classes));
 	class_info = &intel_engine_classes[info->class];
 
+	if (GEM_WARN_ON(info->class > MAX_ENGINE_CLASS))
+		return -EINVAL;
+
+	if (GEM_WARN_ON(info->instance > MAX_ENGINE_INSTANCE))
+		return -EINVAL;
+
+	if (GEM_WARN_ON(dev_priv->engine_class[info->class][info->instance]))
+		return -EINVAL;
+
 	GEM_BUG_ON(dev_priv->engine[id]);
 	engine = kzalloc(sizeof(*engine), GFP_KERNEL);
 	if (!engine)

@@ -225,6 +234,7 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 
 	ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier);
 
+	dev_priv->engine_class[info->class][info->instance] = engine;
 	dev_priv->engine[id] = engine;
 	return 0;
 }

drivers/gpu/drm/i915/intel_ringbuffer.c +25 -0

index 22e5ea8516b6..825d5af2d967 100644
@@ -2163,3 +2163,28 @@ int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine)
 
 	return intel_init_ring_buffer(engine);
 }
+
+static u8 user_class_map[I915_ENGINE_CLASS_MAX] = {
+	[I915_ENGINE_CLASS_OTHER] = OTHER_CLASS,
+	[I915_ENGINE_CLASS_RENDER] = RENDER_CLASS,
+	[I915_ENGINE_CLASS_COPY] = COPY_ENGINE_CLASS,
+	[I915_ENGINE_CLASS_VIDEO] = VIDEO_DECODE_CLASS,
+	[I915_ENGINE_CLASS_VIDEO_ENHANCE] = VIDEO_ENHANCEMENT_CLASS,
+};
+
+struct intel_engine_cs *
+intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance)
+{
+	if (class >= ARRAY_SIZE(user_class_map))
+		return NULL;
+
+	class = user_class_map[class];
+
+	if (WARN_ON_ONCE(class > MAX_ENGINE_CLASS))
+		return NULL;
+
+	if (instance > MAX_ENGINE_INSTANCE)
+		return NULL;
+
+	return i915->engine_class[class][instance];
+}

drivers/gpu/drm/i915/intel_ringbuffer.h +25 -0

index abf171c3cb9c..cbd1734759b8 100644
@@ -245,6 +245,28 @@ struct intel_engine_cs {
 		I915_SELFTEST_DECLARE(bool mock : 1);
 	} breadcrumbs;
 
+	struct {
+		/**
+		 * @enable: Bitmask of enable sample events on this engine.
+		 *
+		 * Bits correspond to sample event types, for instance
+		 * I915_SAMPLE_QUEUED is bit 0 etc.
+		 */
+		u32 enable;
+		/**
+		 * @enable_count: Reference count for the enabled samplers.
+		 *
+		 * Index number corresponds to the bit number from @enable.
+		 */
+		unsigned int enable_count[I915_PMU_SAMPLE_BITS];
+		/**
+		 * @sample: Counter value for sampling events.
+		 *
+		 * Our internal timer stores the current counter in this field.
+		 */
+		u64 sample[I915_ENGINE_SAMPLE_MAX];
+	} pmu;
+
 	/*
 	 * A pool of objects to use as shadow copies of client batch buffers
 	 * when the command parser is enabled. Prevents the client from

@@ -743,4 +765,7 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915);
 
 bool intel_engine_can_store_dword(struct intel_engine_cs *engine);
 
+struct intel_engine_cs *
+intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance);
+
 #endif /* _INTEL_RINGBUFFER_H_ */

include/uapi/drm/i915_drm.h +57 -0

63 lines not on this page · raw message

Signature · 6 lines

2.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Tvrtko Ursulin

author maintainer

2017-09-20 16:02 UTC #4 lore raw

PATCH v10 3/8 drm/i915/pmu: Expose a PMU interface for perf queries

From: Tvrtko Ursulin <[email protected]>

From: Chris Wilson <[email protected]>
From: Tvrtko Ursulin <[email protected]>
From: Dmitry Rogozhkin <[email protected]>

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.

v1-v2 (Chris Wilson):

v2: Use a common timer for the ring sampling.

v5: (Tvrtko Ursulin)

v6:
* Add missing entry to v4 changelog.
* Fix accounting in CPU hotplug case by copying the approach from arch/x86/events/intel/cstate.c. (Dmitry Rogozhkin)

v7:
* Log failure message only on failure.
* Remove CPU hotplug notification state on unregister.

v8:
* Fix error unwind on failed registration.
* Checkpatch cleanup.

v10:
* Fixed queued accounting.

Signed-off-by Chris Wilson <[email protected]>

Signed-off-by Tvrtko Ursulin <[email protected]>

Signed-off-by Dmitry Rogozhkin <[email protected]>

Cc Tvrtko Ursulin <[email protected]>

Cc Chris Wilson <[email protected]>

Cc Dmitry Rogozhkin <[email protected]>

Cc Peter Zijlstra <[email protected]>

---
 drivers/gpu/drm/i915/Makefile           |   1 +
 drivers/gpu/drm/i915/i915_drv.c         |   2 +
 drivers/gpu/drm/i915/i915_drv.h         |  78 ++++
 drivers/gpu/drm/i915/i915_pmu.c         | 707 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h         |   3 +
 drivers/gpu/drm/i915/intel_engine_cs.c  |  10 +
 drivers/gpu/drm/i915/intel_ringbuffer.c |  25 ++
 drivers/gpu/drm/i915/intel_ringbuffer.h |  25 ++
 include/uapi/drm/i915_drm.h             |  57 +++
 9 files changed, 908 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/i915_pmu.c

drivers/gpu/drm/i915/Makefile +1 -0

index 5182e3d5557d..5c6013961b3b 100644
@@ -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 \

drivers/gpu/drm/i915/i915_drv.c +2 -0

index 5c111ea96e80..b1f96eb1be16 100644
@@ -1196,6 +1196,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,

@@ -1250,6 +1251,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);

drivers/gpu/drm/i915/i915_drv.h +78 -0

108 lines not on this page · raw message

drivers/gpu/drm/i915/i915_pmu.c new file +707 -0

707 lines not on this page · raw message

drivers/gpu/drm/i915/i915_reg.h +3 -0

index 94b40a469afd..971d6a7fd9ac 100644
@@ -186,6 +186,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define VIDEO_ENHANCEMENT_CLASS	2
 #define COPY_ENGINE_CLASS	3
 #define OTHER_CLASS		4
+#define MAX_ENGINE_CLASS	4
+
+#define MAX_ENGINE_INSTANCE    1
 
 /* PCI config space */

drivers/gpu/drm/i915/intel_engine_cs.c +10 -0

index 3d135c3cd380..82510d971c7d 100644
@@ -198,6 +198,15 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 	GEM_BUG_ON(info->class >= ARRAY_SIZE(intel_engine_classes));
 	class_info = &intel_engine_classes[info->class];
 
+	if (GEM_WARN_ON(info->class > MAX_ENGINE_CLASS))
+		return -EINVAL;
+
+	if (GEM_WARN_ON(info->instance > MAX_ENGINE_INSTANCE))
+		return -EINVAL;
+
+	if (GEM_WARN_ON(dev_priv->engine_class[info->class][info->instance]))
+		return -EINVAL;
+
 	GEM_BUG_ON(dev_priv->engine[id]);
 	engine = kzalloc(sizeof(*engine), GFP_KERNEL);
 	if (!engine)

@@ -225,6 +234,7 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
 
 	ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier);
 
+	dev_priv->engine_class[info->class][info->instance] = engine;
 	dev_priv->engine[id] = engine;
 	return 0;
 }

drivers/gpu/drm/i915/intel_ringbuffer.c +25 -0

index 85e64a45d0bf..3b424cc5610b 100644
@@ -2183,3 +2183,28 @@ int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine)
 
 	return intel_init_ring_buffer(engine);
 }
+
+static u8 user_class_map[I915_ENGINE_CLASS_MAX] = {
+	[I915_ENGINE_CLASS_OTHER] = OTHER_CLASS,
+	[I915_ENGINE_CLASS_RENDER] = RENDER_CLASS,
+	[I915_ENGINE_CLASS_COPY] = COPY_ENGINE_CLASS,
+	[I915_ENGINE_CLASS_VIDEO] = VIDEO_DECODE_CLASS,
+	[I915_ENGINE_CLASS_VIDEO_ENHANCE] = VIDEO_ENHANCEMENT_CLASS,
+};
+
+struct intel_engine_cs *
+intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance)
+{
+	if (class >= ARRAY_SIZE(user_class_map))
+		return NULL;
+
+	class = user_class_map[class];
+
+	if (WARN_ON_ONCE(class > MAX_ENGINE_CLASS))
+		return NULL;
+
+	if (instance > MAX_ENGINE_INSTANCE)
+		return NULL;
+
+	return i915->engine_class[class][instance];
+}

drivers/gpu/drm/i915/intel_ringbuffer.h +25 -0

index 138116a3b537..e20dfb1429c0 100644
@@ -245,6 +245,28 @@ struct intel_engine_cs {
 		I915_SELFTEST_DECLARE(bool mock : 1);
 	} breadcrumbs;
 
+	struct {
+		/**
+		 * @enable: Bitmask of enable sample events on this engine.
+		 *
+		 * Bits correspond to sample event types, for instance
+		 * I915_SAMPLE_QUEUED is bit 0 etc.
+		 */
+		u32 enable;
+		/**
+		 * @enable_count: Reference count for the enabled samplers.
+		 *
+		 * Index number corresponds to the bit number from @enable.
+		 */
+		unsigned int enable_count[I915_PMU_SAMPLE_BITS];
+		/**
+		 * @sample: Counter value for sampling events.
+		 *
+		 * Our internal timer stores the current counter in this field.
+		 */
+		u64 sample[I915_ENGINE_SAMPLE_MAX];
+	} pmu;
+
 	/*
 	 * A pool of objects to use as shadow copies of client batch buffers
 	 * when the command parser is enabled. Prevents the client from

@@ -751,4 +773,7 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915);
 
 bool intel_engine_can_store_dword(struct intel_engine_cs *engine);
 
+struct intel_engine_cs *
+intel_engine_lookup_user(struct drm_i915_private *i915, u8 class, u8 instance);
+
 #endif /* _INTEL_RINGBUFFER_H_ */

include/uapi/drm/i915_drm.h +57 -0

63 lines not on this page · raw message

Signature · 6 lines

2.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Tvrtko Ursulin

author maintainer

2017-09-18 11:38 UTC #5 lore raw

PATCH v4 4/8 drm/i915/pmu: Suspend sampling when GPU is idle

In reply to Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

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.

v2: Rebase and limit timer even more.
v3: Rebase.
v4: Rebase.
v5: Skip action if perf PMU failed to register.
v6: Checkpatch cleanup.
v7:
* Add a common helper to start the timer if needed. (Chris Wilson)
* Add comment explaining bitwise logic in pmu_needs_timer.

Signed-off-by Tvrtko Ursulin <[email protected]>

---
 drivers/gpu/drm/i915/i915_drv.h         |  8 +++
 drivers/gpu/drm/i915/i915_gem.c         |  1 +
 drivers/gpu/drm/i915/i915_gem_request.c |  1 +
 drivers/gpu/drm/i915/i915_pmu.c         | 88 +++++++++++++++++++++++++++++----
 4 files changed, 88 insertions(+), 10 deletions(-)

drivers/gpu/drm/i915/i915_drv.h +8 -0

index 824372037672..0e0df3b72944 100644
@@ -2252,6 +2252,10 @@ struct i915_pmu {
 	 */
 	unsigned int enable_count[I915_PMU_MASK_BITS];
 	/**
+	 * @timer_enabled: Should the internal sampling timer be running.
+	 */
+	bool timer_enabled;
+	/**
 	 * @sample: Current counter value for i915 events which need sampling.
 	 *
 	 * These counters are updated from the i915 PMU sampling timer.

@@ -4002,9 +4006,13 @@ extern void i915_perf_unregister(struct drm_i915_private *dev_priv);
 #ifdef CONFIG_PERF_EVENTS
 void i915_pmu_register(struct drm_i915_private *i915);
 void i915_pmu_unregister(struct drm_i915_private *i915);
+void i915_pmu_gt_idle(struct drm_i915_private *i915);
+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 */

drivers/gpu/drm/i915/i915_gem.c +1 -0

index 3250dfaa192b..27ae16abed86 100644
@@ -3227,6 +3227,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;

drivers/gpu/drm/i915/i915_gem_request.c +1 -0

index 813a3b546d6e..18a1e379253e 100644
@@ -258,6 +258,7 @@ static void mark_busy(struct drm_i915_private *i915)
 	i915_update_gfx_val(i915);
 	if (INTEL_GEN(i915) >= 6)
 		gen6_rps_busy(i915);
+	i915_pmu_gt_active(i915);
 
 	queue_delayed_work(i915->wq,
 			   &i915->gt.retire_work,

drivers/gpu/drm/i915/i915_pmu.c +78 ~~-10~~

119 lines not on this page · raw message

Signature · 6 lines

2.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Tvrtko Ursulin

author maintainer

2017-09-18 11:38 UTC #6 lore raw

PATCH v4 5/8 drm/i915: Wrap context schedule notification

In reply to Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

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

Signed-off-by Tvrtko Ursulin <[email protected]>

---
 drivers/gpu/drm/i915/intel_lrc.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

drivers/gpu/drm/i915/intel_lrc.c +14 -3

index 1960ba5ff9e4..ba91db43f163 100644
@@ -301,6 +301,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)
 {

@@ -346,7 +358,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));

@@ -612,8 +624,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);

Signature · 6 lines

2.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Tvrtko Ursulin

author maintainer

2017-09-18 11:38 UTC #7 lore raw

PATCH v4 6/8 drm/i915: Engine busy time tracking

In reply to Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

Track total time requests have been executing on the hardware.

We add new kernel API to allow software tracking of time GPU engines are spending executing requests.

Both per-engine and global API is added with the latter also being exported for use by external users.

v2:
* Squashed with the internal API.
* Dropped static key.
* Made per-engine.
* Store time in monotonic ktime.

v3: Moved stats clearing to disable.

v4:
* Comments.
* Don't export the API just yet.

v5: Whitespace cleanup.

v6:
* Rename ref to active.
* Drop engine aggregate stats for now.
* Account initial busy period after enabling stats.

Signed-off-by Tvrtko Ursulin <[email protected]>

---
 drivers/gpu/drm/i915/intel_engine_cs.c  | 84 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c        |  2 +
 drivers/gpu/drm/i915/intel_ringbuffer.h | 92 +++++++++++++++++++++++++++++++++
 3 files changed, 178 insertions(+)

drivers/gpu/drm/i915/intel_engine_cs.c +84 -0

index 82510d971c7d..cbf978d39052 100644
@@ -232,6 +232,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_class[info->class][info->instance] = engine;

@@ -1556,6 +1558,88 @@ bool intel_engine_can_store_dword(struct intel_engine_cs *engine)
 	}
 }
 
+/**
+ * intel_enable_engine_stats() - Enable engine busy tracking on engine
+ * @engine: engine to enable stats collection
+ *
+ * Start collecting the engine busyness data for @engine.
+ *
+ * Returns 0 on success or a negative error code.
+ */
+int intel_enable_engine_stats(struct intel_engine_cs *engine)
+{
+	unsigned long flags;
+
+	if (!i915.enable_execlists)
+		return -ENODEV;
+
+	spin_lock_irqsave(&engine->stats.lock, flags);
+	if (engine->stats.enabled == ~0)
+		goto busy;
+	if (engine->stats.enabled++ == 0)
+		engine->stats.enabled_at = ktime_get();
+	spin_unlock_irqrestore(&engine->stats.lock, flags);
+
+	return 0;
+
+busy:
+	spin_unlock_irqrestore(&engine->stats.lock, flags);
+
+	return -EBUSY;
+}
+
+/**
+ * intel_disable_engine_stats() - Disable engine busy tracking on engine
+ * @engine: engine to disable stats collection
+ *
+ * Stops collecting the engine busyness data for @engine.
+ */
+void intel_disable_engine_stats(struct intel_engine_cs *engine)
+{
+	unsigned long flags;
+
+	if (!i915.enable_execlists)
+		return;
+
+	spin_lock_irqsave(&engine->stats.lock, flags);
+	WARN_ON_ONCE(engine->stats.enabled == 0);
+	if (--engine->stats.enabled == 0) {
+		engine->stats.enabled_at = 0;
+		engine->stats.active = 0;
+		engine->stats.start = 0;
+		engine->stats.total = 0;
+	}
+	spin_unlock_irqrestore(&engine->stats.lock, flags);
+}
+
+/**
+ * intel_engine_get_busy_time() - Return current accumulated engine busyness
+ * @engine: engine to report on
+ *
+ * Returns accumulated time @engine was busy since engine stats were enabled.
+ */
+ktime_t intel_engine_get_busy_time(struct intel_engine_cs *engine)
+{
+	ktime_t total;
+	unsigned long flags;
+
+	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.active)
+		total = ktime_add(total,
+				  ktime_sub(ktime_get(), 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

drivers/gpu/drm/i915/intel_lrc.c +2 -0

index ba91db43f163..7f2861014448 100644
@@ -304,12 +304,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);
 }

drivers/gpu/drm/i915/intel_ringbuffer.h +92 -0

102 lines not on this page · raw message

Signature · 6 lines

2.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Tvrtko Ursulin

author maintainer

2017-09-18 11:38 UTC #8 lore raw

PATCH v4 7/8 drm/i915/pmu: Wire up engine busy stats to PMU

In reply to Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

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.

v2: Rebase.
v3:
* Rebase, comments.
* Leave engine busyness controls out of workers.
v4: Checkpatch cleanup.
v5: Added comment to pmu_needs_timer change.

Signed-off-by Tvrtko Ursulin <[email protected]>

---
 drivers/gpu/drm/i915/i915_pmu.c         | 40 ++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_ringbuffer.h |  5 +++++
 2 files changed, 42 insertions(+), 3 deletions(-)

drivers/gpu/drm/i915/i915_pmu.c +37 -3

index b7de6fe3cac7..ffba21eeb5d0 100644
@@ -90,6 +90,11 @@ static unsigned int event_enabled_bit(struct perf_event *event)
 	return config_enabled_bit(event->attr.config);
 }
 
+static bool supports_busy_stats(void)
+{
+	return i915.enable_execlists;
+}
+
 static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active)
 {
 	u64 enable;

@@ -115,6 +120,12 @@ static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active)
 	 */
 	if (!gpu_active)
 		enable &= ~ENGINE_SAMPLE_MASK;
+	/**
+	 * Also there is software busyness tracking available we do not
+	 * need the timer for I915_SAMPLE_BUSY counter.
+	 */
+	else if (supports_busy_stats())
+		enable &= ~BIT(I915_SAMPLE_BUSY);
 
 	/**
 	 * If some bits remain it means we need the sampling timer running.

@@ -192,7 +203,8 @@ static void engines_sample(struct drm_i915_private *dev_priv)
 		if (enable & BIT(I915_SAMPLE_QUEUED))
 			engine->pmu.sample[I915_SAMPLE_QUEUED] += PERIOD;
 
-		if (enable & BIT(I915_SAMPLE_BUSY)) {
+		if ((enable & BIT(I915_SAMPLE_BUSY)) &&
+		    !engine->pmu.busy_stats) {
 			u32 val;
 
 			fw = grab_forcewake(dev_priv, fw);

@@ -385,6 +397,9 @@ static u64 __i915_pmu_event_read(struct perf_event *event)
 
 		if (WARN_ON_ONCE(!engine)) {
 			/* Do nothing */
+		} else if (sample == I915_SAMPLE_BUSY &&
+			   engine->pmu.busy_stats) {
+			val = ktime_to_ns(intel_engine_get_busy_time(engine));
 		} else {
 			val = engine->pmu.sample[sample];
 		}

@@ -438,6 +453,12 @@ static void i915_pmu_event_read(struct perf_event *event)
 	local64_add(new - prev, &event->count);
 }
 
+static bool engine_needs_busy_stats(struct intel_engine_cs *engine)
+{
+	return supports_busy_stats() &&
+	       (engine->pmu.enable & BIT(I915_SAMPLE_BUSY));
+}
+
 static void i915_pmu_enable(struct perf_event *event)
 {
 	struct drm_i915_private *i915 =

@@ -477,7 +498,14 @@ static void i915_pmu_enable(struct perf_event *event)
 
 		GEM_BUG_ON(sample >= I915_PMU_SAMPLE_BITS);
 		GEM_BUG_ON(engine->pmu.enable_count[sample] == ~0);
-		engine->pmu.enable_count[sample]++;
+		if (engine->pmu.enable_count[sample]++ == 0) {
+			if (engine_needs_busy_stats(engine) &&
+			    !engine->pmu.busy_stats) {
+				engine->pmu.busy_stats =
+					intel_enable_engine_stats(engine) == 0;
+				WARN_ON_ONCE(!engine->pmu.busy_stats);
+			}
+		}
 	}
 
 	/*

@@ -513,8 +541,14 @@ static void i915_pmu_disable(struct perf_event *event)
 		 * Decrement the reference count and clear the enabled
 		 * bitmask when the last listener on an event goes away.
 		 */
-		if (--engine->pmu.enable_count[sample] == 0)
+		if (--engine->pmu.enable_count[sample] == 0) {
 			engine->pmu.enable &= ~BIT(sample);
+			if (!engine_needs_busy_stats(engine) &&
+			    engine->pmu.busy_stats) {
+				engine->pmu.busy_stats = false;
+				intel_disable_engine_stats(engine);
+			}
+		}
 	}
 
 	GEM_BUG_ON(bit >= I915_PMU_MASK_BITS);

drivers/gpu/drm/i915/intel_ringbuffer.h +5 -0

index ae0a4c498a0f..cb58ce34ab13 100644
@@ -265,6 +265,11 @@ struct intel_engine_cs {
 		 * Our internal timer stores the current counter in this field.
 		 */
 		u64 sample[I915_ENGINE_SAMPLE_MAX];
+		/**
+		 * @busy_stats: Has enablement of engine stats tracking been
+		 * 		requested.
+		 */
+		bool busy_stats;
 	} pmu;
 
 	/*

Signature · 6 lines

2.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Chris Wilson

2017-09-18 14:59 UTC #9 lore raw

Re: [PATCH 7/8] drm/i915/pmu: Wire up engine busy stats to PMU

Quoting Tvrtko Ursulin (2017-09-18 12:38:13)

Quoted 17 lines

drivers/gpu/drm/i915/i915_pmu.c +5 -0

index b7de6fe3cac7..ffba21eeb5d0 100644
@@ -90,6 +90,11 @@ static unsigned int event_enabled_bit(struct perf_event *event)
        return config_enabled_bit(event->attr.config);
 }
 
+static bool supports_busy_stats(void)
+{
+       return i915.enable_execlists;
+}
+
 static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active)
 {
        u64 enable;

drivers/gpu/drm/i915/i915_pmu.c static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active) · line 123

Quoted 4 lines

drivers/gpu/drm/i915/i915_pmu.c +1 -0

@@ -115,6 +120,12 @@ static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active)
         */
        if (!gpu_active)
                enable &= ~ENGINE_SAMPLE_MASK;
+       /**

/** kerneldoc?
-Chris
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Tvrtko Ursulin

2017-09-19 08:46 UTC #10 lore raw

Re: [PATCH 7/8] drm/i915/pmu: Wire up engine busy stats to PMU

Quoted 9 lines · Chris Wilson

Quoting Tvrtko Ursulin (2017-09-18 12:38:13)

Quoted 8 lines

drivers/gpu/drm/i915/i915_pmu.c +0 -0

index b7de6fe3cac7..ffba21eeb5d0 100644
@@ -90,6 +90,11 @@ static unsigned int event_enabled_bit(struct perf_event *event)
         return config_enabled_bit(event->attr.config);
  }

Quoted 15 lines

drivers/gpu/drm/i915/i915_pmu.c static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active) · line 123

Quoted 13 lines

drivers/gpu/drm/i915/i915_pmu.c +6 -0

+static bool supports_busy_stats(void)
+{
+       return i915.enable_execlists;
+}
+
  static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active)
  {
         u64 enable;

@@ -115,6 +120,12 @@ static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active)
          */
         if (!gpu_active)
                 enable &= ~ENGINE_SAMPLE_MASK;
+       /**

/** kerneldoc?

I've managed to sprinkle quite a few of those around while added comments. Finger memory, what can I say.. Another thing on the todo list.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Tvrtko Ursulin

author maintainer

2017-09-18 11:38 UTC #11 lore raw

PATCH v4 8/8 drm/i915: Gate engine stats collection with a static key

In reply to Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

This reduces the cost of the software engine busyness tracking to a single no-op instruction when there are no listeners.

v2: Rebase and some comments.
v3: Rebase.
v4: Checkpatch fixes.
v5: Rebase.
v6: Use system_long_wq to avoid being blocked by struct_mutex
users.

Signed-off-by Tvrtko Ursulin <[email protected]>

---
 drivers/gpu/drm/i915/i915_pmu.c         |  54 +++++++++++++++--
 drivers/gpu/drm/i915/intel_engine_cs.c  |  17 ++++++
 drivers/gpu/drm/i915/intel_ringbuffer.h | 101 ++++++++++++++++++++------------
 3 files changed, 130 insertions(+), 42 deletions(-)

drivers/gpu/drm/i915/i915_pmu.c +50 -4

index ffba21eeb5d0..6d22172b8fb0 100644
@@ -499,11 +499,17 @@ static void i915_pmu_enable(struct perf_event *event)
 		GEM_BUG_ON(sample >= I915_PMU_SAMPLE_BITS);
 		GEM_BUG_ON(engine->pmu.enable_count[sample] == ~0);
 		if (engine->pmu.enable_count[sample]++ == 0) {
+			/*
+			 * Enable engine busy stats tracking if needed or
+			 * alternatively cancel the scheduled disabling of the
+			 * same.
+			 */
 			if (engine_needs_busy_stats(engine) &&
 			    !engine->pmu.busy_stats) {
-				engine->pmu.busy_stats =
-					intel_enable_engine_stats(engine) == 0;
-				WARN_ON_ONCE(!engine->pmu.busy_stats);
+				engine->pmu.busy_stats = true;
+				if (!cancel_delayed_work(&engine->pmu.disable_busy_stats))
+					queue_work(system_long_wq,
+						   &engine->pmu.enable_busy_stats);
 			}
 		}
 	}

@@ -546,7 +552,15 @@ static void i915_pmu_disable(struct perf_event *event)
 			if (!engine_needs_busy_stats(engine) &&
 			    engine->pmu.busy_stats) {
 				engine->pmu.busy_stats = false;
-				intel_disable_engine_stats(engine);
+				/*
+				 * We request a delayed disable to handle the
+				 * rapid on/off cycles on events which can
+				 * happen when tools like perf stat start in a
+				 * nicer way.
+				 */
+				queue_delayed_work(system_long_wq,
+						   &engine->pmu.disable_busy_stats,
+						   round_jiffies_up_relative(HZ));
 			}
 		}
 	}

@@ -737,9 +751,27 @@ static int i915_pmu_cpu_offline(unsigned int cpu, struct hlist_node *node)
 	return 0;
 }
 
+static void __enable_busy_stats(struct work_struct *work)
+{
+	struct intel_engine_cs *engine =
+		container_of(work, typeof(*engine), pmu.enable_busy_stats);
+
+	WARN_ON_ONCE(intel_enable_engine_stats(engine));
+}
+
+static void __disable_busy_stats(struct work_struct *work)
+{
+	struct intel_engine_cs *engine =
+	       container_of(work, typeof(*engine), pmu.disable_busy_stats.work);
+
+	intel_disable_engine_stats(engine);
+}
+
 void i915_pmu_register(struct drm_i915_private *i915)
 {
 	int ret;
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
 
 	if (INTEL_GEN(i915) <= 2) {
 		DRM_INFO("PMU not supported for this GPU.");

@@ -773,6 +805,12 @@ void i915_pmu_register(struct drm_i915_private *i915)
 	i915->pmu.timer.function = i915_sample;
 	i915->pmu.enable = 0;
 
+	for_each_engine(engine, i915, id) {
+		INIT_WORK(&engine->pmu.enable_busy_stats, __enable_busy_stats);
+		INIT_DELAYED_WORK(&engine->pmu.disable_busy_stats,
+				  __disable_busy_stats);
+	}
+
 	ret = perf_pmu_register(&i915->pmu.base, "i915", -1);
 	if (ret == 0)
 		return;

@@ -791,6 +829,9 @@ void i915_pmu_register(struct drm_i915_private *i915)
 
 void i915_pmu_unregister(struct drm_i915_private *i915)
 {
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
+
 	if (!i915->pmu.base.event_init)
 		return;

@@ -802,6 +843,11 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
 
 	hrtimer_cancel(&i915->pmu.timer);
 
+	for_each_engine(engine, i915, id) {
+		flush_work(&engine->pmu.enable_busy_stats);
+		flush_delayed_work(&engine->pmu.disable_busy_stats);
+	}
+
 	perf_pmu_unregister(&i915->pmu.base);
 	i915->pmu.base.event_init = NULL;
 }

drivers/gpu/drm/i915/intel_engine_cs.c +17 -0

index cbf978d39052..490b5254f2c0 100644
@@ -21,6 +21,7 @@
  * IN THE SOFTWARE.
  *
  */
+#include <linux/static_key.h>
 
 #include "i915_drv.h"
 #include "intel_ringbuffer.h"

@@ -1558,6 +1559,10 @@ bool intel_engine_can_store_dword(struct intel_engine_cs *engine)
 	}
 }
 
+DEFINE_STATIC_KEY_FALSE(i915_engine_stats_key);
+static DEFINE_MUTEX(i915_engine_stats_mutex);
+static int i915_engine_stats_ref;
+
 /**
  * intel_enable_engine_stats() - Enable engine busy tracking on engine
  * @engine: engine to enable stats collection

@@ -1573,6 +1578,8 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine)
 	if (!i915.enable_execlists)
 		return -ENODEV;
 
+	mutex_lock(&i915_engine_stats_mutex);
+
 	spin_lock_irqsave(&engine->stats.lock, flags);
 	if (engine->stats.enabled == ~0)
 		goto busy;

@@ -1580,10 +1587,16 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine)
 		engine->stats.enabled_at = ktime_get();
 	spin_unlock_irqrestore(&engine->stats.lock, flags);
 
+	if (i915_engine_stats_ref++ == 0)
+		static_branch_enable(&i915_engine_stats_key);
+
+	mutex_unlock(&i915_engine_stats_mutex);
+
 	return 0;
 
 busy:
 	spin_unlock_irqrestore(&engine->stats.lock, flags);
+	mutex_unlock(&i915_engine_stats_mutex);
 
 	return -EBUSY;
 }

@@ -1601,6 +1614,7 @@ void intel_disable_engine_stats(struct intel_engine_cs *engine)
 	if (!i915.enable_execlists)
 		return;
 
+	mutex_lock(&i915_engine_stats_mutex);
 	spin_lock_irqsave(&engine->stats.lock, flags);
 	WARN_ON_ONCE(engine->stats.enabled == 0);
 	if (--engine->stats.enabled == 0) {

@@ -1610,6 +1624,9 @@ void intel_disable_engine_stats(struct intel_engine_cs *engine)
 		engine->stats.total = 0;
 	}
 	spin_unlock_irqrestore(&engine->stats.lock, flags);
+	if (--i915_engine_stats_ref == 0)
+		static_branch_disable(&i915_engine_stats_key);
+	mutex_unlock(&i915_engine_stats_mutex);
 }
 
 /**

drivers/gpu/drm/i915/intel_ringbuffer.h +63 ~~-38~~

128 lines not on this page · raw message

Signature · 6 lines

2.9.5

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Rogozhkin, Dmitry V

2017-09-19 00:38 UTC #12 lore raw

Re: [PATCH 8/8] drm/i915: Gate engine stats collection with a static key

Quoted 82 lines · Tvrtko Ursulin

static inline void intel_engine_context_out(struct intel_engine_cs
*engine)

 {
        unsigned long flags;

drivers/gpu/drm/i915/intel_ringbuffer.h +0 -9

-       if (READ_ONCE(engine->stats.enabled) == 0)
-               return;
-
-       spin_lock_irqsave(&engine->stats.lock, flags);
-
-       if (engine->stats.enabled > 0) {
-               ktime_t last, now = ktime_get();
-
-               if (engine->stats.active && --engine->stats.active ==

0) {

(quoted diff) +0 -2

-                       /*
-                        * Decrement the active context count and in

case GPU

drivers/gpu/drm/i915/intel_ringbuffer.h +0 -5

-                        * is now idle add up to the running total.
-                        */
-                       last = ktime_sub(now, engine->stats.start);
-
-                       engine->stats.total =

ktime_add(engine->stats.total,

drivers/gpu/drm/i915/intel_ringbuffer.h +0 -4

-                                                       last);
-               } else if (engine->stats.active == 0) {
-                       /*
-                        * After turning on engine stats, context out

might be
- * the first event in which case we account
from the

drivers/gpu/drm/i915/intel_ringbuffer.h +0 -3

-                        * time stats gathering was turned on.
-                        */
-                       last = ktime_sub(now,

engine->stats.enabled_at);

(quoted diff) +0 -2

-
-                       engine->stats.total =

ktime_add(engine->stats.total,

drivers/gpu/drm/i915/intel_ringbuffer.h +13 -1

-                                                       last);
+       if (static_branch_unlikely(&i915_engine_stats_key)) {
+               if (READ_ONCE(engine->stats.enabled) == 0)
+                       return;
+
+               spin_lock_irqsave(&engine->stats.lock, flags);
+
+               if (engine->stats.enabled > 0) {
+                       ktime_t last, now = ktime_get();
+
+                       if (engine->stats.active &&
+                           --engine->stats.active == 0) {
+                               /*
+                                * Decrement the active context count

and in
+ * case GPU is now idle add up to the
running