Re: [PATCH] drm/xe/guc_pc: Add ignore efficient frequency support

["Anirban, Sk" <sk.anirban@intel.com>]

Hi,

On 17-09-2025 10:59, Nilawar, Badal wrote:
>
>
> On 16-09-2025 19:35, Sk Anirban wrote:
>> When setting the minimum frequency, GuC automatically adjusts it to
>> efficient levels, potentially overriding user requests.
>> This patch adds the slpc_ignore_eff_freq sysfs interface to disable
>> this behavior, allowing users to enforce their exact minimum frequency
>> settings.
>> Signed-off-by: Sk Anirban<sk.anirban@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_gt_freq.c      | 36 ++++++++++++++++++++
>>   drivers/gpu/drm/xe/xe_guc_pc.c       | 49 ++++++++++++++++++++++++++--
>>   drivers/gpu/drm/xe/xe_guc_pc.h       |  1 +
>>   drivers/gpu/drm/xe/xe_guc_pc_types.h |  2 ++
>>   4 files changed, 86 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_freq.c b/drivers/gpu/drm/xe/xe_gt_freq.c
>> index 4ff1b6b58d6b..fb16fa5c6a0f 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_freq.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_freq.c
>> @@ -227,6 +227,41 @@ static ssize_t max_freq_store(struct kobject *kobj,
>>   }
>>   static struct kobj_attribute attr_max_freq = __ATTR_RW(max_freq);
>>   
>> +static ssize_t slpc_ignore_eff_freq_show(struct kobject *kobj,
>> +					 struct kobj_attribute *attr, char *buf)
>> +{
>> +	struct device *dev = kobj_to_dev(kobj);
>> +	struct xe_guc_pc *pc = dev_to_pc(dev);
>> +
>> +	return sysfs_emit(buf, "%u\n", pc->ignore_eff_freq);
>> +}
>> +
>> +static ssize_t slpc_ignore_eff_freq_store(struct kobject *kobj,
>> +					  struct kobj_attribute *attr,
>> +					  const char *buff, size_t count)
>> +{
>> +	struct device *dev = kobj_to_dev(kobj);
>> +	struct xe_guc_pc *pc = dev_to_pc(dev);
>> +	u32 val;
>> +	ssize_t ret;
>> +
>> +	ret = kstrtou32(buff, 0, &val);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (val > 1)
>> +		return -EINVAL;
>> +
>> +	xe_pm_runtime_get(dev_to_xe(dev));
>> +	ret = xe_guc_pc_set_ignore_eff_freq(pc, val);
>> +	xe_pm_runtime_put(dev_to_xe(dev));
>> +	if (ret)
>> +		return ret;
>> +
>> +	return count;
>> +}
>> +static struct kobj_attribute attr_slpc_ignore_eff_freq = __ATTR_RW(slpc_ignore_eff_freq);
>> +
>>   static ssize_t power_profile_show(struct kobject *kobj,
>>   				  struct kobj_attribute *attr,
>>   				  char *buff)
>> @@ -263,6 +298,7 @@ static const struct attribute *freq_attrs[] = {
>>   	&attr_rpn_freq.attr,
>>   	&attr_min_freq.attr,
>>   	&attr_max_freq.attr,
>> +	&attr_slpc_ignore_eff_freq.attr,
>>   	&attr_power_profile.attr,
>>   	NULL
>>   };
>> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
>> index 53fdf59524c4..0229a89e3ff8 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
>> @@ -336,6 +336,8 @@ static void pc_set_cur_freq(struct xe_guc_pc *pc, u32 freq)
>>   
>>   static int pc_set_min_freq(struct xe_guc_pc *pc, u32 freq)
>>   {
>> +	int ret;
>> +
>>   	/*
>>   	 * Let's only check for the rpn-rp0 range. If max < min,
>>   	 * min becomes a fixed request.
>> @@ -347,8 +349,10 @@ static int pc_set_min_freq(struct xe_guc_pc *pc, u32 freq)
>>   	 * GuC policy is to elevate minimum frequency to the efficient levels
>>   	 * Our goal is to have the admin choices respected.
>>   	 */
>> -	pc_action_set_param(pc, SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
>> -			    freq < pc->rpe_freq);
> This code already respects the user’s request by disabling efficient 
> frequency when the requested frequency is lower than RPE. So, why is 
> there a need for an additional sysfs entry to disable efficient frequency?
>
> Regards,
> Badal
Currently, the SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY parameter is only 
set when pc_set_min_freq() is called—and even then, only if the 
requested minimum frequency is below RPe. However, after the driver 
loads or when the requested frequency is above RPe, there's no way to 
manually enable this parameter. As a result, we see frequency 
fluctuations during IGT testing.

Thanks,
Anirban

>> +	ret = pc_action_set_param(pc, SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
>> +				  freq < pc->rpe_freq || pc->ignore_eff_freq);
>> +	if (ret)
>> +		xe_gt_err(pc_to_gt(pc), "Failed to set efficient freq\n");
>>   
>>   	return pc_action_set_param(pc,
>>   				   SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
>> @@ -737,6 +741,45 @@ int xe_guc_pc_set_max_freq(struct xe_guc_pc *pc, u32 freq)
>>   	return xe_guc_pc_set_max_freq_locked(pc, freq);
>>   }
>>   
>> +int xe_guc_pc_set_ignore_eff_freq(struct xe_guc_pc *pc, bool val)
>> +{
>> +	int ret;
>> +	u32 min_freq;
>> +
>> +	guard(mutex)(&pc->freq_lock);
>> +
>> +	if (!pc->freq_ready)
>> +		return -EAGAIN;
>> +
>> +	if (val) {
>> +		ret = xe_guc_pc_get_min_freq_locked(pc, &min_freq);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	ret = pc_action_set_param(pc, SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY, val);
>> +	if (ret) {
>> +		xe_gt_err(pc_to_gt(pc), "Failed to set ignore efficient freq(%d): %pe\n",
>> +			  val, ERR_PTR(ret));
>> +		return ret;
>> +	}
>> +
>> +	pc->ignore_eff_freq = val;
>> +
>> +	if (val) {
>> +		ret = pc_action_set_param(pc,
>> +					  SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
>> +					  min_freq);
>> +		if (ret) {
>> +			xe_gt_err(pc_to_gt(pc), "Failed to set min freq to %u: %pe\n",
>> +				  min_freq, ERR_PTR(ret));
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   /**
>>    * xe_guc_pc_c_status - get the current GT C state
>>    * @pc: XE_GuC_PC instance
>> @@ -1382,6 +1425,8 @@ int xe_guc_pc_init(struct xe_guc_pc *pc)
>>   	if (err)
>>   		return err;
>>   
>> +	pc->ignore_eff_freq = false;
>> +
>>   	bo = xe_managed_bo_create_pin_map(xe, tile, size,
>>   					  XE_BO_FLAG_VRAM_IF_DGFX(tile) |
>>   					  XE_BO_FLAG_GGTT |
>> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.h b/drivers/gpu/drm/xe/xe_guc_pc.h
>> index 0e31396f103c..3a87d0da609d 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_pc.h
>> +++ b/drivers/gpu/drm/xe/xe_guc_pc.h
>> @@ -31,6 +31,7 @@ int xe_guc_pc_get_min_freq(struct xe_guc_pc *pc, u32 *freq);
>>   int xe_guc_pc_set_min_freq(struct xe_guc_pc *pc, u32 freq);
>>   int xe_guc_pc_get_max_freq(struct xe_guc_pc *pc, u32 *freq);
>>   int xe_guc_pc_set_max_freq(struct xe_guc_pc *pc, u32 freq);
>> +int xe_guc_pc_set_ignore_eff_freq(struct xe_guc_pc *pc, bool val);
>>   int xe_guc_pc_set_power_profile(struct xe_guc_pc *pc, const char *buf);
>>   void xe_guc_pc_get_power_profile(struct xe_guc_pc *pc, char *profile);
>>   
>> diff --git a/drivers/gpu/drm/xe/xe_guc_pc_types.h b/drivers/gpu/drm/xe/xe_guc_pc_types.h
>> index 5e4ea53fbee6..c3b44598507b 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_pc_types.h
>> +++ b/drivers/gpu/drm/xe/xe_guc_pc_types.h
>> @@ -37,6 +37,8 @@ struct xe_guc_pc {
>>   	struct mutex freq_lock;
>>   	/** @freq_ready: Only handle freq changes, if they are really ready */
>>   	bool freq_ready;
>> +	/** @ignore_eff_freq: Ignore efficient frequency */
>> +	bool ignore_eff_freq;
>>   	/** @power_profile: Base or power_saving profile */
>>   	u32 power_profile;
>>   };