Re: [PATCH v2 2/2] drm/xe: Add sysfs for GT/IA bias

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Tue, Jul 07, 2026 at 12:48:05AM +0200, Michal Wajdeczko wrote:
> 
> 
> On 7/6/2026 11:36 PM, Vinay Belgaumkar wrote:
> > GT_IA_PERF_BIAS_REG indicates power budget between IA and GT.
> > Lower 16 bits correspond to IA and upper to GT. Higher value
> > indicates more bias towards that plane. The values are in U1.15
> > format.
> > 
> > $ cat /sys/class/drm/card1/device/tile0/gt0/freq0/gt_ia_bias
> > GT Bias: 1.000
> > IA Bias: 0.500
> 
> "Attributes should be ASCII text files, preferably with only one value per file"
> 
> see https://docs.kernel.org/6.10/filesystems/sysfs.html#attributes

indeed. But I think the main question here is, do we really want this
to be a stable API?!

Why don't we go with the debugfs?
So we print the Hex and the decoded values in the single file
and we accept the hex as input file?

If we go with sysfs, I wonder if we shouldn't go with decoded,
but then having 2 separate bias files gt_bias, ia_bias.

But also, if we go with stable API we need a whole documentation, explaining
the use cases where that might be useful and its limitations. I don't want
to keep receiving bug reports because people are randomly touching these
files and getting nothing because they are not in a limited TDP scenario.

And even worse, having to debug other corner case bugs because people
decided to touch this. So, debugfs?!

> 
> > 
> > This interface will allow us to observe power budget changes while
> > running workloads and also, if needed, tune it.
> > 
> > v2: Decode the bias values (Rodrigo)
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > ---
> >  drivers/gpu/drm/xe/regs/xe_gt_regs.h |  2 +
> >  drivers/gpu/drm/xe/xe_gt_freq.c      | 71 ++++++++++++++++++++++++++++
> >  2 files changed, 73 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > index 08251c7a1a4b..41c5c5f20964 100644
> > --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > @@ -634,6 +634,8 @@
> >  #define GT_GFX_RC6_LOCKED			XE_REG(0x138104)
> >  #define GT_GFX_RC6				XE_REG(0x138108)
> >  
> > +#define GT_IA_PERF_BIAS_REG			XE_REG(0x138158)
> > +
> >  #define GT0_PERF_LIMIT_REASONS			XE_REG(0x1381a8)
> >  /* Common performance limit reason bits - available on all platforms */
> >  #define   GT0_PERF_LIMIT_REASONS_MASK		0xde3
> > diff --git a/drivers/gpu/drm/xe/xe_gt_freq.c b/drivers/gpu/drm/xe/xe_gt_freq.c
> > index a40dd074106f..08c0be8defd0 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_freq.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_freq.c
> > @@ -11,10 +11,13 @@
> >  #include <drm/drm_managed.h>
> >  #include <drm/drm_print.h>
> >  
> > +#include <regs/xe_gt_regs.h>
> 
> use regular "" include
> 
> > +#include "xe_gt.h"
> >  #include "xe_gt_sysfs.h"
> >  #include "xe_gt_throttle.h"
> >  #include "xe_gt_types.h"
> >  #include "xe_guc_pc.h"
> > +#include "xe_mmio.h"
> >  #include "xe_pm.h"
> >  
> >  /**
> > @@ -242,6 +245,61 @@ static ssize_t power_profile_store(struct kobject *kobj,
> >  }
> >  static struct kobj_attribute attr_power_profile = __ATTR_RW(power_profile);
> >  
> > +static ssize_t gt_ia_bias_show(struct kobject *kobj,
> > +			       struct kobj_attribute *attr,
> > +			       char *buf)
> > +{
> > +	struct device *dev = kobj_to_dev(kobj);
> > +	struct xe_gt *gt = kobj_to_gt(kobj->parent);
> > +	u32 val;
> > +	u32 ia_frac, gt_frac;
> > +	u16 ia_raw, gt_raw;
> > +	u16 ia_frac_bits, gt_frac_bits;
> > +	u16 ia_int, gt_int;
> > +
> > +	guard(xe_pm_runtime)(dev_to_xe(dev));
> > +	val = xe_mmio_read32(&gt->mmio, GT_IA_PERF_BIAS_REG);
> > +
> > +	ia_raw = val & 0xFFFF;
> > +	gt_raw = (val >> 16) & 0xFFFF;
> 
> use REG_FIELD_GET instead
> 
> > +
> > +	/* Uppermost bit is integer value */
> > +	ia_int = ia_raw >> 15;
> > +	gt_int = gt_raw >> 15;
> > +
> > +	/* Rest are fraction bits */
> > +	ia_frac_bits = ia_raw & 0x7FFF;
> > +	gt_frac_bits = gt_raw & 0x7FFF;
> 
> maybe it is worth to define some Q1.15 helpers?
> 
> > +
> > +	/* Up to 3 decimal places */
> > +	ia_frac = ((uint64_t)(ia_frac_bits * 1000)) / 0x8000;
> > +	gt_frac = ((uint64_t)(gt_frac_bits * 1000)) / 0x8000;
> > +
> > +	return sysfs_emit(buf, "GT Bias: %u.%03u\nIA Bias: %u.%03u\n",
> > +			  gt_int, gt_frac, ia_int, ia_frac);
> > +}
> > +
> > +static ssize_t gt_ia_bias_store(struct kobject *kobj,
> > +				struct kobj_attribute *attr,
> > +				const char *buff, size_t count)
> > +{
> > +	struct device *dev = kobj_to_dev(kobj);
> > +	struct xe_gt *gt = kobj_to_gt(kobj->parent);
> > +	u32 val;
> > +	int ret;
> > +
> > +	ret = kstrtou32(buff, 0, &val);
> 
> hmm, shouldn't we accept decoded values as we show them?
> now it's inconsistent and admin must guess how to encode it properly

right. it should be consistent, easy to use and not so dependent on hw and fw.

> 
> > +	if (ret)
> > +		return ret;
> > +
> > +	guard(xe_pm_runtime)(dev_to_xe(dev));
> > +	xe_mmio_write32(&gt->mmio, GT_IA_PERF_BIAS_REG, val);
> > +
> > +	return count;
> > +}
> > +
> > +static struct kobj_attribute attr_gt_ia_bias = __ATTR_RW(gt_ia_bias);
> > +
> >  static const struct attribute *freq_attrs[] = {
> >  	&attr_act_freq.attr,
> >  	&attr_cur_freq.attr,
> > @@ -258,8 +316,13 @@ static const struct attribute *freq_attrs[] = {
> >  static void freq_fini(void *arg)
> >  {
> >  	struct kobject *kobj = arg;
> > +	struct device *dev = kobj_to_dev(kobj);
> > +	struct xe_gt *gt = kobj_to_gt(kobj->parent);
> >  
> >  	sysfs_remove_files(kobj, freq_attrs);
> > +
> > +	if (xe_gt_is_main_type(gt) && !IS_DGFX(dev_to_xe(dev)))
> > +		sysfs_remove_file(kobj, &attr_gt_ia_bias.attr);
> >  	kobject_put(kobj);
> >  }
> >  
> > @@ -289,6 +352,14 @@ int xe_gt_freq_init(struct xe_gt *gt)
> >  		return err;
> >  	}
> >  
> > +	if (xe_gt_is_main_type(gt) && !IS_DGFX(xe)) {
> > +		err = sysfs_create_file(gt->freq, &attr_gt_ia_bias.attr);
> > +		if (err) {
> > +			kobject_put(gt->freq);
> > +			return err;
> > +		}
> > +	}
> > +
> >  	err = devm_add_action_or_reset(xe->drm.dev, freq_fini, gt->freq);
> >  	if (err)
> >  		return err;
>