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

["Nilawar, Badal" <badal.nilawar@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 6307 bytes --]


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

> +	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;
>   };

[-- Attachment #2: Type: text/html, Size: 8026 bytes --]