Re: [Intel-gfx] [PATCH 6/6] drm/i915/pmu: Export counters from all tiles

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Fri, May 05, 2023 at 05:58:16PM -0700, Umesh Nerlige Ramappa wrote:
>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
>Start exporting frequency and RC6 counters from all tiles.
>
>Existing counters keep their names and config values and new one use the
>namespace added in the previous patch, with the "-gtN" added to their
>names.
>
>Interrupts counter is an odd one off. Because it is the global device
>counters (not only GT) we choose not to add per tile versions for now.

UMD specific changes link needed here as well. With that:

Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

Thanks,
Umesh
>
>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>---
> drivers/gpu/drm/i915/i915_pmu.c | 87 ++++++++++++++++++++++-----------
> 1 file changed, 59 insertions(+), 28 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
>index 12b2f3169abf..284e5c5b97bb 100644
>--- a/drivers/gpu/drm/i915/i915_pmu.c
>+++ b/drivers/gpu/drm/i915/i915_pmu.c
>@@ -546,8 +546,9 @@ config_status(struct drm_i915_private *i915, u64 config)
> 	struct intel_gt *gt = to_gt(i915);
>
> 	unsigned int gt_id = config_gt_id(config);
>+	unsigned int max_gt_id = HAS_EXTRA_GT_LIST(i915) ? 1 : 0;
>
>-	if (gt_id)
>+	if (gt_id > max_gt_id)
> 		return -ENOENT;
>
> 	switch (config_counter(config)) {
>@@ -561,6 +562,8 @@ config_status(struct drm_i915_private *i915, u64 config)
> 			return -ENODEV;
> 		break;
> 	case I915_PMU_INTERRUPTS:
>+		if (gt_id)
>+			return -ENOENT;
> 		break;
> 	case I915_PMU_RC6_RESIDENCY:
> 		if (!gt->rc6.supported)
>@@ -930,11 +933,20 @@ static const struct attribute_group i915_pmu_cpumask_attr_group = {
> 	.attrs = i915_cpumask_attrs,
> };
>
>-#define __event(__config, __name, __unit) \
>+#define __event(__counter, __name, __unit) \
> { \
>-	.config = (__config), \
>+	.counter = (__counter), \
> 	.name = (__name), \
> 	.unit = (__unit), \
>+	.global = false, \
>+}
>+
>+#define __global_event(__counter, __name, __unit) \
>+{ \
>+	.counter = (__counter), \
>+	.name = (__name), \
>+	.unit = (__unit), \
>+	.global = true, \
> }
>
> #define __engine_event(__sample, __name) \
>@@ -973,15 +985,16 @@ create_event_attributes(struct i915_pmu *pmu)
> {
> 	struct drm_i915_private *i915 = container_of(pmu, typeof(*i915), pmu);
> 	static const struct {
>-		u64 config;
>+		unsigned int counter;
> 		const char *name;
> 		const char *unit;
>+		bool global;
> 	} events[] = {
>-		__event(I915_PMU_ACTUAL_FREQUENCY, "actual-frequency", "M"),
>-		__event(I915_PMU_REQUESTED_FREQUENCY, "requested-frequency", "M"),
>-		__event(I915_PMU_INTERRUPTS, "interrupts", NULL),
>-		__event(I915_PMU_RC6_RESIDENCY, "rc6-residency", "ns"),
>-		__event(I915_PMU_SOFTWARE_GT_AWAKE_TIME, "software-gt-awake-time", "ns"),
>+		__event(0, "actual-frequency", "M"),
>+		__event(1, "requested-frequency", "M"),
>+		__global_event(2, "interrupts", NULL),
>+		__event(3, "rc6-residency", "ns"),
>+		__event(4, "software-gt-awake-time", "ns"),
> 	};
> 	static const struct {
> 		enum drm_i915_pmu_engine_sample sample;
>@@ -996,12 +1009,17 @@ create_event_attributes(struct i915_pmu *pmu)
> 	struct i915_ext_attribute *i915_attr = NULL, *i915_iter;
> 	struct attribute **attr = NULL, **attr_iter;
> 	struct intel_engine_cs *engine;
>-	unsigned int i;
>+	struct intel_gt *gt;
>+	unsigned int i, j;
>
> 	/* Count how many counters we will be exposing. */
>-	for (i = 0; i < ARRAY_SIZE(events); i++) {
>-		if (!config_status(i915, events[i].config))
>-			count++;
>+	for_each_gt(gt, i915, j) {
>+		for (i = 0; i < ARRAY_SIZE(events); i++) {
>+			u64 config = ___I915_PMU_OTHER(j, events[i].counter);
>+
>+			if (!config_status(i915, config))
>+				count++;
>+		}
> 	}
>
> 	for_each_uabi_engine(engine, i915) {
>@@ -1031,26 +1049,39 @@ create_event_attributes(struct i915_pmu *pmu)
> 	attr_iter = attr;
>
> 	/* Initialize supported non-engine counters. */
>-	for (i = 0; i < ARRAY_SIZE(events); i++) {
>-		char *str;
>-
>-		if (config_status(i915, events[i].config))
>-			continue;
>-
>-		str = kstrdup(events[i].name, GFP_KERNEL);
>-		if (!str)
>-			goto err;
>+	for_each_gt(gt, i915, j) {
>+		for (i = 0; i < ARRAY_SIZE(events); i++) {
>+			u64 config = ___I915_PMU_OTHER(j, events[i].counter);
>+			char *str;
>
>-		*attr_iter++ = &i915_iter->attr.attr;
>-		i915_iter = add_i915_attr(i915_iter, str, events[i].config);
>+			if (config_status(i915, config))
>+				continue;
>
>-		if (events[i].unit) {
>-			str = kasprintf(GFP_KERNEL, "%s.unit", events[i].name);
>+			if (events[i].global || !HAS_EXTRA_GT_LIST(i915))
>+				str = kstrdup(events[i].name, GFP_KERNEL);
>+			else
>+				str = kasprintf(GFP_KERNEL, "%s-gt%u",
>+						events[i].name, j);
> 			if (!str)
> 				goto err;
>
>-			*attr_iter++ = &pmu_iter->attr.attr;
>-			pmu_iter = add_pmu_attr(pmu_iter, str, events[i].unit);
>+			*attr_iter++ = &i915_iter->attr.attr;
>+			i915_iter = add_i915_attr(i915_iter, str, config);
>+
>+			if (events[i].unit) {
>+				if (events[i].global || !HAS_EXTRA_GT_LIST(i915))
>+					str = kasprintf(GFP_KERNEL, "%s.unit",
>+							events[i].name);
>+				else
>+					str = kasprintf(GFP_KERNEL, "%s-gt%u.unit",
>+							events[i].name, j);
>+				if (!str)
>+					goto err;
>+
>+				*attr_iter++ = &pmu_iter->attr.attr;
>+				pmu_iter = add_pmu_attr(pmu_iter, str,
>+							events[i].unit);
>+			}
> 		}
> 	}
>
>-- 
>2.36.1
>