[PATCH v14 7/7] drm/xe/pmu: Add GT C6 events

[Lucas De Marchi]

On Wed, Jan 22, 2025 at 05:32:59AM -0500, Rodrigo Vivi wrote:
>On Tue, Jan 21, 2025 at 10:23:41PM -0800, Lucas De Marchi wrote:
>> From: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
>>
>> Provide a PMU interface for GT C6 residency counters. The implementation
>> is ported over from the i915 PMU code.
>
>This is not valid anymore right?! Perhaps rephrase to show that the API
>design itself was taken from there?

yep, re-phrased:

Provide a PMU interface for GT C6 residency counters. The interface is  
similar to the one available for i915, but gt is passed in the config   
when creating the event.                                                

>
>> Residency is provided in units of
>> ms(like sysfs entry in - /sys/class/drm/card0/device/tile0/gt0/gtidle).
>>
>> Sample usage and output:
>>
>> 	$ perf list | grep gt-c6
>> 	  xe_0000_00_02.0/gt-c6-residency/                   [Kernel PMU event]
>>
>> 	$ tail /sys/bus/event_source/devices/xe_0000_00_02.0/events/gt-c6-residency*
>> 	==> /sys/bus/event_source/devices/xe_0000_00_02.0/events/gt-c6-residency <==
>> 	event=0x01
>>
>> 	==> /sys/bus/event_source/devices/xe_0000_00_02.0/events/gt-c6-residency.unit <==
>> 	ms
>>
>> 	$ perf stat -e xe_0000_00_02.0/gt-c6-residency,gt=0/ -I1000
>> 	#           time             counts unit events
>> 	     1.001196056              1,001 ms   xe_0000_00_02.0/gt-c6-residency,gt=0/
>> 	     2.005216219              1,003 ms   xe_0000_00_02.0/gt-c6-residency,gt=0/
>>
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_pmu.c | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_pmu.c b/drivers/gpu/drm/xe/xe_pmu.c
>> index 8d938d67c1f2c..a2e4addd3dd7e 100644
>> --- a/drivers/gpu/drm/xe/xe_pmu.c
>> +++ b/drivers/gpu/drm/xe/xe_pmu.c
>> @@ -11,6 +11,7 @@
>>  #include "xe_device.h"
>>  #include "xe_force_wake.h"
>>  #include "xe_gt_clock.h"
>> +#include "xe_gt_idle.h"
>>  #include "xe_gt_printk.h"
>>  #include "xe_mmio.h"
>>  #include "xe_macros.h"
>> @@ -122,12 +123,16 @@ static int xe_pmu_event_init(struct perf_event *event)
>>  static u64 __xe_pmu_event_read(struct perf_event *event)
>>  {
>>  	struct xe_gt *gt = event_to_gt(event);
>> -	u64 val = 0;
>>
>>  	if (!gt)
>>  		return 0;
>>
>> -	return val;
>> +	switch (config_to_event_id(event->attr.config)) {
>> +	case XE_PMU_EVENT_GT_C6_RESIDENCY:
>> +		return xe_gt_idle_residency_msec(&gt->gtidle);
>> +	}
>> +
>> +	return 0;
>>  }
>>
>>  static void xe_pmu_event_update(struct perf_event *event)
>> @@ -268,6 +273,10 @@ static const struct attribute_group pmu_events_attr_group = {
>>
>>  static void set_supported_events(struct xe_pmu *pmu)
>>  {
>> +	struct xe_device *xe = container_of(pmu, typeof(*xe), pmu);
>> +
>> +	if (!xe->info.skip_guc_pc)
>> +		pmu->supported_events |= BIT_ULL(XE_PMU_EVENT_GT_C6_RESIDENCY);
>>  }
>
>A feeling that it would be better to squash this to the other attribute patch,
>but I understand the reasons...

well... yeah. That part alone is now bigger than this with all the
simplifications and the previous code with dynamic attributes is simply
gone. I'm not comfortable adding others' signoff on a completely new
code.  It's based more on other drivers already in the tree than the
previous implementation for xe (or i915).

>
>Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>

thanks
Lucas De Marchi

>
>>
>>  /**
>> --
>> 2.48.0
>>