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

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

On Fri, May 05, 2023 at 05:58:16PM -0700, Umesh Nerlige Ramappa wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> 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@xxxxxxxxx>

Thanks,
Umesh
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@xxxxxxxxx>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx>
> ---
> 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