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

[Iddamsetty, Aravind]

On 18-07-2023 10:37, Dixit, Ashutosh wrote:
> On Thu, 06 Jul 2023 20:53:47 -0700, Iddamsetty, Aravind wrote:
>>
> 
> Hi Aravind,
> 
> Back to this review.
> 
>>>> On 06-07-2023 08:09, Dixit, Ashutosh wrote:
>>>>> On Tue, 27 Jun 2023 05:21:13 -0700, Aravind Iddamsetty wrote:
>>>>
>>>>>> +static u64 __engine_group_busyness_read(struct xe_gt *gt, u64 config)
>>>>>> +{
>>>>>> +	u64 val = 0;
>>>>>> +
>>>>>> +	switch (config) {
>>>>>> +	case XE_PMU_RENDER_GROUP_BUSY(0):
>>>>>> +		val = xe_mmio_read32(gt, XE_OAG_RENDER_BUSY_FREE);
>>>>>> +		break;
>>>>>> +	case XE_PMU_COPY_GROUP_BUSY(0):
>>>>>> +		val = xe_mmio_read32(gt, XE_OAG_BLT_BUSY_FREE);
>>>>>> +		break;
>>>>>> +	case XE_PMU_MEDIA_GROUP_BUSY(0):
>>>>>> +		val = xe_mmio_read32(gt, XE_OAG_ANY_MEDIA_FF_BUSY_FREE);
>>>>>> +		break;
>>>>>> +	case XE_PMU_ANY_ENGINE_GROUP_BUSY(0):
>>>>>> +		val = xe_mmio_read32(gt, XE_OAG_RC0_ANY_ENGINE_BUSY_FREE);
>>>>>> +		break;
>>>>>> +	default:
>>>>>> +		drm_warn(&gt->tile->xe->drm, "unknown pmu event\n");
>>>>>> +	}
>>>>>> +
>>>>>> +	return xe_gt_clock_interval_to_ns(gt, val * 16);
>>>>>> +}
>>>>>
>>>>> A few questions on just the above function first:
>>>>>
>>>>> 1. OK so these registers won't be available to VF's, but any idea what
>>>>>    these counts are when VF's are active?
>>>>
>>>> VF's cannot access the registers but the counters will be incrementing
>>>> if respective engines are busy and can be monitored from PF and that
>>>> will be across all VFs.
>>>
>>> Ok, good.
>>>
>>>>
>>>>>
>>>>> 2. When would these 32 bit registers overflow? Let us say a group is
>>>>>    continuously busy and we are running at 1 GHz, the registers would
>>>>>    overflow in 4 seconds (max value 4G)?
>>>>
>>>> Based on BSPEC:52071 they use MHZ clock assuming the default 24MHz, it
>>>> would take around 5726 secs to overflow.
>>>
>>> OK, overflow should not be an issue then. Though I have seen 19.2 and 38.4
>>> MHz in OA. Also, if these are OAG registers, OA timestamp freq can be
>>> different from CS timestamp freq, so not sure if that needs to be
>>> handled. See i915_perf_oa_timestamp_frequency() in i915.
>>
>> so that is handled by below calculation
> 
>>>>
>>>>>
>>>>> 3. What is the multiplication by 16 (not factored above in 2.)? I don't see
>>>>>    that in Bspec.
>>>>
>>>> These counters are incremented based on crystal clock frequency and we
>>>> need to convert to CS time base hence a 16x mul. BSPEC:52071
>>>
>>> Hmm still don't see the 16x mul in BSPEC:52071. Anyway.
>>
>> lets say the frequency is 24MHz so the counter increments every
>> 1333.333ns(granularity) and corresponding cs timestamp base for this
>> frequency is 83.333ns, 1333.333/83.333 = 16 and this true for rest of
>> the frequency selections as well. hence we multiply the counter x 16.
> 
> OK, I finally see in Bspec:71028: "Unit in GPMusec. Refer to Timestamp
> Bases for details" which is a pointer to Bspec: 52071. So the 16x multiply
> looks correct (and these registers are not running at OA freq otherwise
> we'd have to use i915_perf_oa_timestamp_frequency()). Hopefully it's
> correct. Thanks.

Thanks,
Aravind.