Re: [Intel-xe] [RFC PATCH 2/3] drm/xe : add rc6_residency in ms

[Riana Tauro <riana.tauro@intel.com>]

On 5/4/2023 9:27 PM, Rodrigo Vivi wrote:
> On Thu, May 04, 2023 at 06:55:48AM -0400, Gupta, Anshuman wrote:
>>
>>
>>> -----Original Message-----
>>> From: Tauro, Riana <riana.tauro@intel.com>
>>> Sent: Wednesday, May 3, 2023 12:13 PM
>>> To: intel-xe@lists.freedesktop.org
>>> Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
>>> <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Dixit,
>>> Ashutosh <ashutosh.dixit@intel.com>; Nilawar, Badal
>>> <badal.nilawar@intel.com>
>>> Subject: [RFC PATCH 2/3] drm/xe : add rc6_residency in ms
>>>
>>> add rc6_residency in ms instead of rc6 residency counter.
>>> Handle wrap around for the counter
>>> The counter can still wrap as it relies on the frequency of counter being read
>>>
>>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>>> ---
>>>   drivers/gpu/drm/xe/xe_idle.c | 46 +++++++++++++++++++++++++++++----
>>> ---
>>>   1 file changed, 37 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_idle.c b/drivers/gpu/drm/xe/xe_idle.c
>>> index 231bdb45a6b7..a7e97ddd7253 100644
>>> --- a/drivers/gpu/drm/xe/xe_idle.c
>>> +++ b/drivers/gpu/drm/xe/xe_idle.c
>>> @@ -11,6 +11,8 @@
>>>   #include "xe_idle.h"
>>>   #include "xe_mmio.h"
>>>
>>> +#define XE_RC6_MULTIPLIER      1280
>>> +
>>>   /*
>>>    * Render-C States:
>>>    * ================
>>> @@ -27,8 +29,7 @@
>>>    *
>>>    * device/gt#/gpu_idle/rc* *read-only* files:
>>>    * - rc_status: Provide the actual immediate status of Render-C: (rc0 or rc6)
>>> - * - rc6_residency: Provide the rc6_residency in units of 1.28 uSec
>>> - *		    Prone to overflows.
>>> + * - rc6_residency_ms: Provide the rc6_residency in ms
>>>    */
>>>
>>>   static struct xe_gt *idle_to_gt(struct xe_idle *idle) @@ -51,6 +52,35 @@
>>> static struct kobj_type xe_idle_kobj_type = {
>>>   	.sysfs_ops = &kobj_sysfs_ops,
>>>   };
>>>
>>> +static u64 rc6_residency_us(struct xe_idle *idle) {
>>> +	struct xe_gt *gt = idle_to_gt(idle);
>>> +	u64 cur_residency, delta, overflow_residency, prev_residency;
>>> +
>>> +	overflow_residency = BIT_ULL(32);
>>> +	cur_residency = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
>> We shall use function pointer here to get the residency, so underlying function can read from
>> appropriate counter and  abstract it from sysfs show function, same comment for converting to
>> ms as well.
> 
> Yes, please!
> 
> Leave this new component as clean and free from the 'RC' stuff as possible.
> Leave all RC stuff inside xe_guc_pc and then this new infra is specific for
> the report and control so you can make this as generic as possible and likely
> extend to other stuff.
> And contain the platform stuff to the lower level.
Hi Rodrigo

So do we have two entries for residency? rc6_residency under 
device/gt#/rc and also displayed as idle_residency under 
device/gt#/gpuidle?

> 
>> Br,
>> Anshuman Gupta.
>>> +
>>> +	/*
>>> +	 * Counter wrap handling
>>> +	 * Store previous hw counter values for counter wrap-around
>>> handling
>>> +	 * Relying on sufficient frequency of queries otherwise counters can
>>> still wrap.
>>> +	 */
>>> +	prev_residency = idle->prev_rc6_residency;
>>> +	idle->prev_rc6_residency = cur_residency;
>>> +
>>> +	/* RC6 delta */
>>> +	if (cur_residency >= prev_residency)
>>> +		delta = cur_residency - prev_residency;
>>> +	else
>>> +		delta = cur_residency + (overflow_residency -
>>> prev_residency);
>>> +
>>> +	/* Add delta to RC6 extended raw driver copy. */
>>> +	cur_residency = idle->cur_rc6_residency + delta;
>>> +	idle->cur_rc6_residency = cur_residency;
>>> +
>>> +	return mul_u64_u32_div(cur_residency, XE_RC6_MULTIPLIER, 1000);
>>> }
>>> +
>>>   static ssize_t
>>>   rc_status_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
>>> { @@ -76,11 +106,10 @@ static const struct kobj_attribute rc_status =
>>> __ATTR(rc_status, 0444, rc_status_show, NULL);
>>>
>>>   static ssize_t
>>> -rc6_residency_show(struct kobject *kobj, struct kobj_attribute *attr, char
>>> *buf)
>>> +rc6_residency_ms_show(struct kobject *kobj, struct kobj_attribute
>>> +*attr, char *buf)
>>>   {
>>>   	struct xe_idle *idle = kobj_to_idle(kobj);
>>>   	struct xe_gt *gt = idle_to_gt(idle);
>>> -	u32 reg;
>>>   	ssize_t ret;
>>>
>>>   	xe_device_mem_access_get(gt_to_xe(gt));
>>> @@ -88,8 +117,7 @@ rc6_residency_show(struct kobject *kobj, struct
>>> kobj_attribute *attr, char *buf)
>>>   	if (ret)
>>>   		goto out;
>>>
>>> -	reg = xe_mmio_read32(gt, GEN6_GT_GFX_RC6.reg);
>>> -	ret = sysfs_emit(buf, "%u\n", reg);
>>> +	ret = sysfs_emit(buf, "%llu\n",
>>> +DIV_ROUND_UP_ULL(rc6_residency_us(&gt->idle), 1000));
>>>
>>>   	XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt),
>>> XE_FORCEWAKE_ALL));
>>>   out:
>>> @@ -97,12 +125,12 @@ rc6_residency_show(struct kobject *kobj, struct
>>> kobj_attribute *attr, char *buf)
>>>   	return ret;
>>>   }
>>>
>>> -static const struct kobj_attribute rc6_residency = -__ATTR(rc6_residency,
>>> 0444, rc6_residency_show, NULL);
>>> +static const struct kobj_attribute rc6_residency_ms =
>>> +__ATTR(rc6_residency_ms, 0444, rc6_residency_ms_show, NULL);
>>>
>>>   static const struct attribute *idle_attrs[] = {
>>>   	&rc_status.attr,
>>> -	&rc6_residency.attr,
>>> +	&rc6_residency_ms.attr,
>>>   	NULL,
>>>   };
>>>
>>> --
>>> 2.40.0
>>