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

Rodrigo Vivi rodrigo.vivi at kernel.org
Fri May 5 13:14:26 UTC 2023


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.

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