[Intel-gfx] [PATCH 6/6] drm/i915/pmu: Export counters from all tiles
From: Tvrtko Ursulin
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.
Signed-off-by: Tvrtko Ursulin
Signed-off-by: Aravind Iddamsetty
Reviewed-by: Umesh Nerlige Ramappa
Signed-off-by: Umesh Nerlige Ramappa
---
drivers/gpu/drm/i915/i915_pmu.c | 82 ++---
1 file changed, 55 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index b3dd9e51c5cc..12345fd0b2cd 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -927,11 +927,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) \
@@ -970,15 +979,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;
@@ -993,12 +1003,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) {
@@ -1028,26 +1043,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++ = _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++ = _iter->attr.attr;
- pmu_iter = add_pmu_attr(pmu_iter, str, events[i].unit);
+
[Intel-gfx] [PATCH 6/6] drm/i915/pmu: Export counters from all tiles
From: Tvrtko Ursulin
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.
Signed-off-by: Tvrtko Ursulin
Signed-off-by: Aravind Iddamsetty
Signed-off-by: Umesh Nerlige Ramappa
---
drivers/gpu/drm/i915/i915_pmu.c | 82 ++---
1 file changed, 55 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index b3dd9e51c5cc..12345fd0b2cd 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -927,11 +927,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) \
@@ -970,15 +979,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;
@@ -993,12 +1003,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) {
@@ -1028,26 +1043,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++ = _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++ = _iter->attr.attr;
- pmu_iter = add_pmu_attr(pmu_iter, str, events[i].unit);
+ *attr_iter++ = _iter->attr.attr;
+
Re: [Intel-gfx] [PATCH 6/6] drm/i915/pmu: Export counters from all tiles
On Fri, 12 May 2023 11:53:32 -0700, Umesh Nerlige Ramappa wrote:
>
> On Fri, May 12, 2023 at 10:08:58AM -0700, Dixit, Ashutosh wrote:
> > On Fri, 12 May 2023 03:57:35 -0700, Tvrtko Ursulin wrote:
> >>
> >>
> >> On 11/05/2023 19:57, Dixit, Ashutosh wrote:
> >> > On Fri, 05 May 2023 17:58:16 -0700, Umesh Nerlige Ramappa wrote:
> >> >>
> >> >
> >> > One drive-by comment:
> >> >
> >> >> 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;
> >> >
> >> > But in Patch 5 we have:
> >> >
> >> > #define I915_PMU_MAX_GTS (4)
> >>
> >> AFAIR that one is just to size the internal arrays, while max_gt_id is to
> >> report to userspace which events are present.
> >
> > Hmm, apart from the #defines's in i915_drm.h in Patch 5, not seeing
> > anything else reported to userspace about which events are present.
>
> Ex: We have only gt0 and gt1 on MTL. When user configures an event (sets
> event id, tile id etc on the config parameter) and calls the
> perf_event_open, it results in i915_pmu_event_init() -> config_status()
> which will return an ENOENT if the event was for say gt2 or gt3. This is
> for runtime check only.
Ah ok, sorry I missed that. In that case what we have above is fine. xe has
a tile_count field but in i915 there's no easy way to find number of gt's,
short of using for_each_gt() and incrementing a count. That seems like an
overkill. So maybe what we have above is fine.
Thanks.
--
Ashutosh
>
> >
> > Also, we already have I915_MAX_GT, we shouldn't need I915_PMU_MAX_GTS, or
> > at least:
> >
> > #define I915_PMU_MAX_GTS I915_MAX_GT
> >
> > Better to use things uniformly. If we want I915_PMU_MAX_GTS to be 2 instead
> > of I915_MAX_GT (but why?, below is just a check) let's do
> >
> > #define I915_PMU_MAX_GTS 2
> >
> > And use that in the code above. But I think we should just use I915_MAX_GT.
>
> Agree,
>
> Thanks,
> Umesh
> >
> > Thanks.
> > --
> > Ashutosh
> >
> >
> >> >
> >> >>
> >> >> - 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)
Re: [Intel-gfx] [PATCH 6/6] drm/i915/pmu: Export counters from all tiles
On Fri, May 12, 2023 at 10:08:58AM -0700, Dixit, Ashutosh wrote:
On Fri, 12 May 2023 03:57:35 -0700, Tvrtko Ursulin wrote:
On 11/05/2023 19:57, Dixit, Ashutosh wrote:
> On Fri, 05 May 2023 17:58:16 -0700, Umesh Nerlige Ramappa wrote:
>>
>
> One drive-by comment:
>
>> 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;
>
> But in Patch 5 we have:
>
> #define I915_PMU_MAX_GTS (4)
AFAIR that one is just to size the internal arrays, while max_gt_id is to
report to userspace which events are present.
Hmm, apart from the #defines's in i915_drm.h in Patch 5, not seeing
anything else reported to userspace about which events are present.
Ex: We have only gt0 and gt1 on MTL. When user configures an event (sets
event id, tile id etc on the config parameter) and calls the
perf_event_open, it results in i915_pmu_event_init() -> config_status()
which will return an ENOENT if the event was for say gt2 or gt3. This is
for runtime check only.
Also, we already have I915_MAX_GT, we shouldn't need I915_PMU_MAX_GTS, or
at least:
#define I915_PMU_MAX_GTS I915_MAX_GT
Better to use things uniformly. If we want I915_PMU_MAX_GTS to be 2 instead
of I915_MAX_GT (but why?, below is just a check) let's do
#define I915_PMU_MAX_GTS 2
And use that in the code above. But I think we should just use I915_MAX_GT.
Agree,
Thanks,
Umesh
Thanks.
--
Ashutosh
>
>>
>> - 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)
Re: [Intel-gfx] [PATCH 6/6] drm/i915/pmu: Export counters from all tiles
On Fri, 12 May 2023 03:57:35 -0700, Tvrtko Ursulin wrote:
>
>
> On 11/05/2023 19:57, Dixit, Ashutosh wrote:
> > On Fri, 05 May 2023 17:58:16 -0700, Umesh Nerlige Ramappa wrote:
> >>
> >
> > One drive-by comment:
> >
> >> 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;
> >
> > But in Patch 5 we have:
> >
> > #define I915_PMU_MAX_GTS (4)
>
> AFAIR that one is just to size the internal arrays, while max_gt_id is to
> report to userspace which events are present.
Hmm, apart from the #defines's in i915_drm.h in Patch 5, not seeing
anything else reported to userspace about which events are present.
Also, we already have I915_MAX_GT, we shouldn't need I915_PMU_MAX_GTS, or
at least:
#define I915_PMU_MAX_GTS I915_MAX_GT
Better to use things uniformly. If we want I915_PMU_MAX_GTS to be 2 instead
of I915_MAX_GT (but why?, below is just a check) let's do
#define I915_PMU_MAX_GTS 2
And use that in the code above. But I think we should just use I915_MAX_GT.
Thanks.
--
Ashutosh
> >
> >>
> >> - 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)
Re: [Intel-gfx] [PATCH 6/6] drm/i915/pmu: Export counters from all tiles
On 11/05/2023 19:57, Dixit, Ashutosh wrote:
On Fri, 05 May 2023 17:58:16 -0700, Umesh Nerlige Ramappa wrote:
One drive-by comment:
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;
But in Patch 5 we have:
#define I915_PMU_MAX_GTS (4)
AFAIR that one is just to size the internal arrays, while max_gt_id is
to report to userspace which events are present.
Regards,
Tvrtko
- 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)
Thanks.
--
Ashutosh
Re: [Intel-gfx] [PATCH 6/6] drm/i915/pmu: Export counters from all tiles
On Fri, 05 May 2023 17:58:16 -0700, Umesh Nerlige Ramappa wrote:
>
One drive-by comment:
> 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;
But in Patch 5 we have:
#define I915_PMU_MAX_GTS (4)
>
> - 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)
Thanks.
--
Ashutosh
Re: [Intel-gfx] [PATCH 6/6] drm/i915/pmu: Export counters from all tiles
On 06/05/2023 01:58, Umesh Nerlige Ramappa wrote:
From: Tvrtko Ursulin
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.
Signed-off-by: Tvrtko Ursulin
Signed-off-by: Aravind Iddamsetty
Signed-off-by: Umesh Nerlige Ramappa
---
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)
Would this be clearer as:
unsigned in num_gts = 1 + (unsigned int)HAS_EXTRA_GT_LIST(i915);
if (gt_id >= num_gts)
?
Just thinking out loud, no real opinion either way.
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;
-
Re: [Intel-gfx] [PATCH 6/6] drm/i915/pmu: Export counters from all tiles
On Fri, May 05, 2023 at 05:58:16PM -0700, Umesh Nerlige Ramappa wrote:
From: Tvrtko Ursulin
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
Thanks,
Umesh
Signed-off-by: Tvrtko Ursulin
Signed-off-by: Aravind Iddamsetty
Signed-off-by: Umesh Nerlige Ramappa
---
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++ = _iter->attr.attr;
-
[Intel-gfx] [PATCH 6/6] drm/i915/pmu: Export counters from all tiles
From: Tvrtko Ursulin
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.
Signed-off-by: Tvrtko Ursulin
Signed-off-by: Aravind Iddamsetty
Signed-off-by: Umesh Nerlige Ramappa
---
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++ = _iter->attr.attr;
- i915_iter = add_i915_attr(i915_iter, str, events[i].config);
+ if (config_status(i915, config))
+ continue;
-
