[Intel-xe] [PATCH v3 2/2] drm/xe/pmu: Enable PMU interface

[Iddamsetty, Aravind]

On 09-08-2023 12:58, Dixit, Ashutosh wrote:

Hi Ashutosh,

> On Tue, 08 Aug 2023 04:54:36 -0700, Aravind Iddamsetty wrote:
>>
> 
> Hi Aravind,
> 
> Spotted a few remaining things. See if it's possible to fix these up and
> send another version.
> 
>> diff --git a/drivers/gpu/drm/xe/xe_pmu.c b/drivers/gpu/drm/xe/xe_pmu.c
>> new file mode 100644
>> index 000000000000..9637f8283641
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_pmu.c
>> @@ -0,0 +1,673 @@

<snip>
>> +static u64 __engine_group_busyness_read(struct xe_gt *gt, int sample_type)
>> +{
>> +	u64 val = 0;
>> +
> 
> What is the forcewake domain for these registers? Don't we need to get
> forcewake before reading these. Something like:
> 
>         XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL));

based on  BSPEC:67609 these belong to GT power domain, so acquiring that
should be sufficient.
> 
>> +	switch (sample_type) {
>> +	case __XE_SAMPLE_RENDER_GROUP_BUSY:
>> +		val = xe_mmio_read32(gt, XE_OAG_RENDER_BUSY_FREE);
>> +		break;
>> +	case __XE_SAMPLE_COPY_GROUP_BUSY:
>> +		val = xe_mmio_read32(gt, XE_OAG_BLT_BUSY_FREE);
>> +		break;
>> +	case __XE_SAMPLE_MEDIA_GROUP_BUSY:
>> +		val = xe_mmio_read32(gt, XE_OAG_ANY_MEDIA_FF_BUSY_FREE);
>> +		break;
>> +	case __XE_SAMPLE_ANY_ENGINE_GROUP_BUSY:
>> +		val = xe_mmio_read32(gt, XE_OAG_RC0_ANY_ENGINE_BUSY_FREE);
>> +		break;
>> +	default:
>> +		drm_warn(&gt->tile->xe->drm, "unknown pmu event\n");
>> +	}
> 
> And similarly here:
> 
>         XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL));
> 

Thanks,
Aravind.

<snip>
> 
> Thanks.
> --
> Ashutosh