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

Riana Tauro riana.tauro at intel.com
Mon May 8 09:54:57 UTC 2023



On 5/5/2023 6:44 PM, Rodrigo Vivi wrote:
> On Fri, May 05, 2023 at 01:14:29PM +0530, Riana Tauro wrote:
>>
>>
>> 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 at intel.com>
>>>>> Sent: Wednesday, May 3, 2023 12:13 PM
>>>>> To: intel-xe at lists.freedesktop.org
>>>>> Cc: Tauro, Riana <riana.tauro at intel.com>; Gupta, Anshuman
>>>>> <anshuman.gupta at intel.com>; Vivi, Rodrigo <rodrigo.vivi at intel.com>; Dixit,
>>>>> Ashutosh <ashutosh.dixit at intel.com>; Nilawar, Badal
>>>>> <badal.nilawar at 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 at 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?
> 
> No, that was not what I meant. I'm sorry for not being clear.
> 
> We should have only one interface. Let's not duplicate stuff and minimize the interfaces.
> 
> What I tried to say was to leave all the RC6 stuff inside xe_guc_pc... All RC6 related
> documentation or MMIO operations. Then, on that component you export functions that
> will be needed by this new idle component.
> 
> This new idle component is only responsible for the idle interface... informative
> and or control... sysfs comes here then.
> 
> Later the same shall happen with the freq stuff.  Freq should be exposed through
> devfreq or hwmon... and for that we will have to create a new component. However
> all the related low-level freq management will remain in the xe_guc_pc.

Sorry for the misunderstanding Thanks for the clarification.
Will move the low level functions to guc_pc.

Thanks
Riana
> 
>>
>>>
>>>> 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
>>>>


More information about the Intel-xe mailing list