[Intel-xe] [PATCH] drm/xe/pmu: Drop interrupt pmu event

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Mon Oct 23 15:44:50 UTC 2023


On 23/10/2023 16:15, Jani Nikula wrote:
> On Tue, 17 Oct 2023, Aravind Iddamsetty <aravind.iddamsetty at linux.intel.com> wrote:
>> Drop interrupt event from PMU as that is not useful and not being used
>> by any UMD.
> 
> Is the i915 interrupt event used by anyone? Could it be dropped too?

At least intel_gpu_top does so removing it would break old binaries on 
new kernels. There also might be user scripts which use perf directly 
and it would be quite hard to find them all, tthat is, prove or disprove 
their existance.

Regards,

Tvrtko


>>
>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
>> Cc: Francois Dugast <francois.dugast at intel.com>
>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty at linux.intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_irq.c       | 18 ------------------
>>   drivers/gpu/drm/xe/xe_pmu.c       |  9 ---------
>>   drivers/gpu/drm/xe/xe_pmu_types.h |  8 --------
>>   include/uapi/drm/xe_drm.h         | 13 ++++++-------
>>   4 files changed, 6 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>> index 61350ed32c61..29f539f1b94b 100644
>> --- a/drivers/gpu/drm/xe/xe_irq.c
>> +++ b/drivers/gpu/drm/xe/xe_irq.c
>> @@ -27,20 +27,6 @@
>>   #define IIR(offset)				XE_REG(offset + 0x8)
>>   #define IER(offset)				XE_REG(offset + 0xc)
>>   
>> -/*
>> - * Interrupt statistic for PMU. Increments the counter only if the
>> - * interrupt originated from the GPU so interrupts from a device which
>> - * shares the interrupt line are not accounted.
>> - */
>> -static __always_inline void xe_pmu_irq_stats(struct xe_device *xe)
>> -{
>> -	/*
>> -	 * A clever compiler translates that into INC. A not so clever one
>> -	 * should at least prevent store tearing.
>> -	 */
>> -	WRITE_ONCE(xe->pmu.irq_count, xe->pmu.irq_count + 1);
>> -}
>> -
>>   static void assert_iir_is_zero(struct xe_gt *mmio, struct xe_reg reg)
>>   {
>>   	u32 val = xe_mmio_read32(mmio, reg);
>> @@ -361,8 +347,6 @@ static irqreturn_t xelp_irq_handler(int irq, void *arg)
>>   
>>   	xe_display_irq_enable(xe, gu_misc_iir);
>>   
>> -	xe_pmu_irq_stats(xe);
>> -
>>   	return IRQ_HANDLED;
>>   }
>>   
>> @@ -459,8 +443,6 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
>>   	dg1_intr_enable(xe, false);
>>   	xe_display_irq_enable(xe, gu_misc_iir);
>>   
>> -	xe_pmu_irq_stats(xe);
>> -
>>   	return IRQ_HANDLED;
>>   }
>>   
>> diff --git a/drivers/gpu/drm/xe/xe_pmu.c b/drivers/gpu/drm/xe/xe_pmu.c
>> index abfc0b3aeac4..318bbc9c1c9b 100644
>> --- a/drivers/gpu/drm/xe/xe_pmu.c
>> +++ b/drivers/gpu/drm/xe/xe_pmu.c
>> @@ -114,10 +114,6 @@ config_status(struct xe_device *xe, u64 config)
>>   		return -ENOENT;
>>   
>>   	switch (config_counter(config)) {
>> -	case XE_PMU_INTERRUPTS(0):
>> -		if (gt_id)
>> -			return -ENOENT;
>> -		break;
>>   	case XE_PMU_RENDER_GROUP_BUSY(0):
>>   	case XE_PMU_COPY_GROUP_BUSY(0):
>>   	case XE_PMU_ANY_ENGINE_GROUP_BUSY(0):
>> @@ -181,13 +177,9 @@ static u64 __xe_pmu_event_read(struct perf_event *event)
>>   	const unsigned int gt_id = config_gt_id(event->attr.config);
>>   	const u64 config = event->attr.config;
>>   	struct xe_gt *gt = xe_device_get_gt(xe, gt_id);
>> -	struct xe_pmu *pmu = &xe->pmu;
>>   	u64 val;
>>   
>>   	switch (config_counter(config)) {
>> -	case XE_PMU_INTERRUPTS(0):
>> -		val = READ_ONCE(pmu->irq_count);
>> -		break;
>>   	case XE_PMU_RENDER_GROUP_BUSY(0):
>>   	case XE_PMU_COPY_GROUP_BUSY(0):
>>   	case XE_PMU_ANY_ENGINE_GROUP_BUSY(0):
>> @@ -361,7 +353,6 @@ create_event_attributes(struct xe_pmu *pmu)
>>   		const char *unit;
>>   		bool global;
>>   	} events[] = {
>> -		__global_event(0, "interrupts", NULL),
>>   		__event(1, "render-group-busy", "ns"),
>>   		__event(2, "copy-group-busy", "ns"),
>>   		__event(3, "media-group-busy", "ns"),
>> diff --git a/drivers/gpu/drm/xe/xe_pmu_types.h b/drivers/gpu/drm/xe/xe_pmu_types.h
>> index 4ccc7e9042f6..9cadbd243f57 100644
>> --- a/drivers/gpu/drm/xe/xe_pmu_types.h
>> +++ b/drivers/gpu/drm/xe/xe_pmu_types.h
>> @@ -51,14 +51,6 @@ struct xe_pmu {
>>   	 *
>>   	 */
>>   	u64 sample[XE_PMU_MAX_GT][__XE_NUM_PMU_SAMPLERS];
>> -	/**
>> -	 * @irq_count: Number of interrupts
>> -	 *
>> -	 * Intentionally unsigned long to avoid atomics or heuristics on 32bit.
>> -	 * 4e9 interrupts are a lot and postprocessing can really deal with an
>> -	 * occasional wraparound easily. It's 32bit after all.
>> -	 */
>> -	unsigned long irq_count;
>>   	/**
>>   	 * @events_attr_group: Device events attribute group.
>>   	 */
>> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
>> index 24bf8f0f52e8..be43ef0b92e6 100644
>> --- a/include/uapi/drm/xe_drm.h
>> +++ b/include/uapi/drm/xe_drm.h
>> @@ -1041,7 +1041,7 @@ struct drm_xe_vm_madvise {
>>    * in 'struct perf_event_attr' as part of perf_event_open syscall to read a
>>    * particular event.
>>    *
>> - * For example to open the XE_PMU_INTERRUPTS(0):
>> + * For example to open the XE_PMU_RENDER_GROUP_BUSY(0):
>>    *
>>    * .. code-block:: C
>>    *
>> @@ -1055,7 +1055,7 @@ struct drm_xe_vm_madvise {
>>    *	attr.read_format = PERF_FORMAT_TOTAL_TIME_ENABLED;
>>    *	attr.use_clockid = 1;
>>    *	attr.clockid = CLOCK_MONOTONIC;
>> - *	attr.config = XE_PMU_INTERRUPTS(0);
>> + *	attr.config = XE_PMU_RENDER_GROUP_BUSY(0);
>>    *
>>    *	fd = syscall(__NR_perf_event_open, &attr, -1, cpu, -1, 0);
>>    */
>> @@ -1068,11 +1068,10 @@ struct drm_xe_vm_madvise {
>>   #define ___XE_PMU_OTHER(gt, x) \
>>   	(((__u64)(x)) | ((__u64)(gt) << __XE_PMU_GT_SHIFT))
>>   
>> -#define XE_PMU_INTERRUPTS(gt)			___XE_PMU_OTHER(gt, 0)
>> -#define XE_PMU_RENDER_GROUP_BUSY(gt)		___XE_PMU_OTHER(gt, 1)
>> -#define XE_PMU_COPY_GROUP_BUSY(gt)		___XE_PMU_OTHER(gt, 2)
>> -#define XE_PMU_MEDIA_GROUP_BUSY(gt)		___XE_PMU_OTHER(gt, 3)
>> -#define XE_PMU_ANY_ENGINE_GROUP_BUSY(gt)	___XE_PMU_OTHER(gt, 4)
>> +#define XE_PMU_RENDER_GROUP_BUSY(gt)		___XE_PMU_OTHER(gt, 0)
>> +#define XE_PMU_COPY_GROUP_BUSY(gt)		___XE_PMU_OTHER(gt, 1)
>> +#define XE_PMU_MEDIA_GROUP_BUSY(gt)		___XE_PMU_OTHER(gt, 2)
>> +#define XE_PMU_ANY_ENGINE_GROUP_BUSY(gt)	___XE_PMU_OTHER(gt, 3)
>>   
>>   #if defined(__cplusplus)
>>   }
> 


More information about the Intel-xe mailing list