Re: [PATCH] drm/xe/guc: Add SLPC power profile interface

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

On Fri, Aug 29, 2025 at 09:43:08AM -0700, Vinay Belgaumkar wrote:
> GuC has an interface to set a power profile for the SLPC algorithm.
> Base mode is default and ensures a balanced performance, power_saving
> mode has conservative up/down thresholds and is suitable for use with
> apps that typically need to be power efficient. This will result in
> lower GT frequencies, thus consuming lower power.
> 
> Selected power profile will be displayed in this format:
> 
> $ cat power_profile
> 
>   [base]    power_saving
> 
> $ echo power_saving > power_profile
> $ cat power_profile
> 
>   base    [power_saving]
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  drivers/gpu/drm/xe/abi/guc_actions_slpc_abi.h |  5 +++
>  drivers/gpu/drm/xe/xe_gt_freq.c               | 42 +++++++++++++++++++
>  drivers/gpu/drm/xe/xe_guc_pc.c                | 37 ++++++++++++++++
>  drivers/gpu/drm/xe/xe_guc_pc.h                |  1 +
>  drivers/gpu/drm/xe/xe_guc_pc_types.h          |  2 +
>  5 files changed, 87 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/abi/guc_actions_slpc_abi.h b/drivers/gpu/drm/xe/abi/guc_actions_slpc_abi.h
> index b28c8fa061f7..c1d16acbb732 100644
> --- a/drivers/gpu/drm/xe/abi/guc_actions_slpc_abi.h
> +++ b/drivers/gpu/drm/xe/abi/guc_actions_slpc_abi.h
> @@ -210,6 +210,11 @@ struct slpc_shared_data {
>  	u8 reserved_mode_definition[4096];
>  } __packed;
>  
> +enum slpc_power_profiles {
> +	SLPC_POWER_PROFILES_BASE = 0x0,
> +	SLPC_POWER_PROFILES_POWER_SAVING = 0x1
> +};
> +
>  /**
>   * DOC: SLPC H2G MESSAGE FORMAT
>   *
> diff --git a/drivers/gpu/drm/xe/xe_gt_freq.c b/drivers/gpu/drm/xe/xe_gt_freq.c
> index 60d9354e7dbf..cf26004b1efb 100644
> --- a/drivers/gpu/drm/xe/xe_gt_freq.c
> +++ b/drivers/gpu/drm/xe/xe_gt_freq.c
> @@ -11,6 +11,7 @@
>  #include <drm/drm_managed.h>
>  #include <drm/drm_print.h>
>  
> +#include "abi/guc_actions_slpc_abi.h"

We should think on a way to keep this only inside the xe_guc_pc component instead
of leaking the guc slpc api to other components...

>  #include "xe_gt_sysfs.h"
>  #include "xe_gt_throttle.h"
>  #include "xe_gt_types.h"
> @@ -227,6 +228,46 @@ static ssize_t max_freq_store(struct kobject *kobj,
>  }
>  static struct kobj_attribute attr_max_freq = __ATTR_RW(max_freq);
>  
> +static ssize_t power_profile_show(struct kobject *kobj,
> +				  struct kobj_attribute *attr,
> +				  char *buff)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct xe_guc_pc *pc = dev_to_pc(dev);
> +
> +	switch (pc->power_profile) {
> +	case SLPC_POWER_PROFILES_BASE:
> +		return sysfs_emit(buff, "[%s]    %s\n", "base", "power_saving");
> +	case SLPC_POWER_PROFILES_POWER_SAVING:
> +		return sysfs_emit(buff, "%s    [%s]\n", "base", "power_saving");
> +	}

perhaps encapsulating this in a xe_guc_pc function, or perhaps having this
outside of the freq sysfs dir?
or finding a way so the guc_pc can register a file in this sysfs but
keep the entire flow there?

> +
> +	return sysfs_emit(buff, "%u\n", pc->power_profile);
> +}
> +
> +static ssize_t power_profile_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);
> +	char power_saving[] = "power_saving";
> +	char base[] = "base";
> +	int err;
> +	u32 val;
> +
> +	if (!strncmp(buff, power_saving, sizeof(power_saving) - 1))
> +		val = SLPC_POWER_PROFILES_POWER_SAVING;
> +	else if (!strncmp(buff, base, sizeof(base) - 1))
> +		val = SLPC_POWER_PROFILES_BASE;
> +	else
> +		return -EINVAL;
> +
> +	err = xe_guc_pc_set_power_profile(pc, val);
> +	return err ?: count;
> +}
> +static struct kobj_attribute attr_power_profile = __ATTR_RW(power_profile);
> +
>  static const struct attribute *freq_attrs[] = {
>  	&attr_act_freq.attr,
>  	&attr_cur_freq.attr,
> @@ -236,6 +277,7 @@ static const struct attribute *freq_attrs[] = {
>  	&attr_rpn_freq.attr,
>  	&attr_min_freq.attr,
>  	&attr_max_freq.attr,
> +	&attr_power_profile.attr,

we should not do this. We should only show the file if GuC PC is enabled.

>  	NULL
>  };
>  
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
> index 88557e86d637..c71d95959dd2 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> @@ -79,6 +79,11 @@
>   * Xe driver enables SLPC with all of its defaults features and frequency
>   * selection, which varies per platform.
>   *
> + * Power profiles add another level of control to SLPC. When power saving
> + * profile is chosen, SLPC will use conservative thresholds to ramp frequency,
> + * thus saving power. Base profile is default and ensures balanced performance
> + * for any workload.
> + *
>   * Render-C States:
>   * ================
>   *
> @@ -1171,6 +1176,31 @@ static int pc_action_set_strategy(struct xe_guc_pc *pc, u32 val)
>  	return ret;
>  }
>  
> +int xe_guc_pc_set_power_profile(struct xe_guc_pc *pc, u32 val)
> +{
> +
> +	int ret = 0;
> +
> +	if (val > SLPC_POWER_PROFILES_POWER_SAVING)
> +		return -EINVAL;
> +
> +	guard(mutex)(&pc->freq_lock);
> +	xe_pm_runtime_get(pc_to_xe(pc));
> +
> +	ret = pc_action_set_param(pc,
> +				  SLPC_PARAM_POWER_PROFILE,
> +				  val);
> +	if (ret)
> +		xe_gt_err_once(pc_to_gt(pc), "Failed to set power profile to %d: %pe\n",
> +			       val, ERR_PTR(ret));
> +	else
> +		pc->power_profile = val;
> +
> +	xe_pm_runtime_put(pc_to_xe(pc));
> +
> +	return ret;
> +}
> +
>  /**
>   * xe_guc_pc_start - Start GuC's Power Conservation component
>   * @pc: Xe_GuC_PC instance
> @@ -1249,6 +1279,11 @@ int xe_guc_pc_start(struct xe_guc_pc *pc)
>  	/* Enable SLPC Optimized Strategy for compute */
>  	ret = pc_action_set_strategy(pc, SLPC_OPTIMIZED_STRATEGY_COMPUTE);
>  
> +	/* Set cached value of power_profile */
> +	ret = xe_guc_pc_set_power_profile(pc, pc->power_profile);
> +	if (unlikely(ret))
> +		xe_gt_err(gt, "Failed to set SLPC power profile: %pe\n", ERR_PTR(ret));
> +
>  out:
>  	xe_force_wake_put(gt_to_fw(gt), fw_ref);
>  	return ret;
> @@ -1327,6 +1362,8 @@ int xe_guc_pc_init(struct xe_guc_pc *pc)
>  
>  	pc->bo = bo;
>  
> +	pc->power_profile = SLPC_POWER_PROFILES_BASE;
> +
>  	return devm_add_action_or_reset(xe->drm.dev, xe_guc_pc_fini_hw, pc);
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.h b/drivers/gpu/drm/xe/xe_guc_pc.h
> index 52ecdd5ddbff..5242bfa8aaa3 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_power_profile(struct xe_guc_pc *pc, u32 val);
>  
>  enum xe_gt_idle_state xe_guc_pc_c_status(struct xe_guc_pc *pc);
>  u64 xe_guc_pc_rc6_residency(struct xe_guc_pc *pc);
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc_types.h b/drivers/gpu/drm/xe/xe_guc_pc_types.h
> index c02053948a57..5e4ea53fbee6 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;
> +	/** @power_profile: Base or power_saving profile */
> +	u32 power_profile;
>  };
>  
>  #endif	/* _XE_GUC_PC_TYPES_H_ */
> -- 
> 2.38.1
>