[PATCH 4/5] thermal: intel: int340x: Add module parameter to change slider offset

Srinivas Pandruvada posted 5 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH 4/5] thermal: intel: int340x: Add module parameter to change slider offset
Posted by Srinivas Pandruvada 1 month, 3 weeks ago
The slider offset value allows the SoC to automatically switch slider
positions in range [SOC_SLIDER … (SOC_SLIDER + slider offset)] based on
internal algorithms to improve power efficiency.

By default, the SoC slider offset is set to 0. This means that SoC is not
allowed to switch slider position.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 .../processor_thermal_soc_slider.c            | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_soc_slider.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_soc_slider.c
index ffc538c9b9e3..bd4ff26a488b 100644
--- a/drivers/thermal/intel/int340x_thermal/processor_thermal_soc_slider.c
+++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_soc_slider.c
@@ -87,6 +87,40 @@ static const struct kernel_param_ops slider_def_balance_ops = {
 module_param_cb(slider_balance, &slider_def_balance_ops, NULL, 0644);
 MODULE_PARM_DESC(slider_balance, "Set slider default value for balance.");
 
+static u8 slider_offset;
+
+static int slider_def_offset_set(const char *arg, const struct kernel_param *kp)
+{
+	u8 offset;
+	int ret;
+
+	guard(mutex)(&slider_param_lock);
+
+	ret = kstrtou8(arg, 16, &offset);
+	if (!ret) {
+		if (offset > SOC_SLIDER_VALUE_MAXIMUM)
+			return -EINVAL;
+
+		slider_offset = offset;
+	}
+
+	return ret;
+}
+
+static int slider_def_offset_get(char *buf, const struct kernel_param *kp)
+{
+	guard(mutex)(&slider_param_lock);
+	return sysfs_emit(buf, "%02x\n", slider_offset);
+}
+
+static const struct kernel_param_ops slider_offset_ops = {
+	.set = slider_def_offset_set,
+	.get = slider_def_offset_get,
+};
+
+module_param_cb(slider_offset, &slider_offset_ops, NULL, 0644);
+MODULE_PARM_DESC(slider_offset, "Set slider offset.");
+
 /* Convert from platform power profile option to SoC slider value */
 static int convert_profile_to_power_slider(enum platform_profile_option profile)
 {
@@ -120,6 +154,8 @@ static int convert_power_slider_to_profile(u8 slider)
 #define SLIDER_MASK		GENMASK_ULL(2, 0)
 #define SLIDER_ENABLE_BIT	7
 
+#define SLIDER_OFFSET_MASK	GENMASK_ULL(6, 4)
+
 static void set_soc_power_profile(struct proc_thermal_device *proc_priv, int slider)
 {
 	u64 val;
@@ -128,6 +164,11 @@ static void set_soc_power_profile(struct proc_thermal_device *proc_priv, int sli
 	val &= ~SLIDER_MASK;
 	val |= FIELD_PREP(SLIDER_MASK, slider);
 	val |= BIT(SLIDER_ENABLE_BIT);
+
+	/* Set the slider offset from module params */
+	val &= ~SLIDER_OFFSET_MASK;
+	val |= FIELD_PREP(SLIDER_OFFSET_MASK, slider_offset);
+
 	writeq(val, proc_priv->mmio_base + SOC_POWER_SLIDER_OFFSET);
 }
 
-- 
2.43.0

Re: [PATCH 4/5] thermal: intel: int340x: Add module parameter to change slider offset
Posted by Rafael J. Wysocki 1 month, 1 week ago
On Wed, Aug 13, 2025 at 8:55 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> The slider offset value allows the SoC to automatically switch slider
> positions in range [SOC_SLIDER … (SOC_SLIDER + slider offset)] based on
> internal algorithms to improve power efficiency.

So SOC_SLIDER is the value set by the user (or the default value if
the user has not set it) and the "slider offset" allows the SoC to
adjust the slider value by setting it above SOC_SLIDER up to and
including SOC_SLIDER + slider offset?

If so, I would add a comment to this effect next to the slider_offset
module param definition.

> By default, the SoC slider offset is set to 0. This means that SoC is not
> allowed to switch slider position.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  .../processor_thermal_soc_slider.c            | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_soc_slider.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_soc_slider.c
> index ffc538c9b9e3..bd4ff26a488b 100644
> --- a/drivers/thermal/intel/int340x_thermal/processor_thermal_soc_slider.c
> +++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_soc_slider.c
> @@ -87,6 +87,40 @@ static const struct kernel_param_ops slider_def_balance_ops = {
>  module_param_cb(slider_balance, &slider_def_balance_ops, NULL, 0644);
>  MODULE_PARM_DESC(slider_balance, "Set slider default value for balance.");
>
> +static u8 slider_offset;
> +
> +static int slider_def_offset_set(const char *arg, const struct kernel_param *kp)
> +{
> +       u8 offset;
> +       int ret;
> +
> +       guard(mutex)(&slider_param_lock);
> +
> +       ret = kstrtou8(arg, 16, &offset);
> +       if (!ret) {
> +               if (offset > SOC_SLIDER_VALUE_MAXIMUM)
> +                       return -EINVAL;
> +
> +               slider_offset = offset;
> +       }
> +
> +       return ret;
> +}
> +
> +static int slider_def_offset_get(char *buf, const struct kernel_param *kp)
> +{
> +       guard(mutex)(&slider_param_lock);
> +       return sysfs_emit(buf, "%02x\n", slider_offset);
> +}
> +
> +static const struct kernel_param_ops slider_offset_ops = {
> +       .set = slider_def_offset_set,
> +       .get = slider_def_offset_get,
> +};
> +
> +module_param_cb(slider_offset, &slider_offset_ops, NULL, 0644);
> +MODULE_PARM_DESC(slider_offset, "Set slider offset.");

The _DESC is a bit short IMO.  I would say something like "Maximum
offset by which the slider can be increased automatically above the
prescribed value".

> +
>  /* Convert from platform power profile option to SoC slider value */
>  static int convert_profile_to_power_slider(enum platform_profile_option profile)
>  {
> @@ -120,6 +154,8 @@ static int convert_power_slider_to_profile(u8 slider)
>  #define SLIDER_MASK            GENMASK_ULL(2, 0)
>  #define SLIDER_ENABLE_BIT      7
>
> +#define SLIDER_OFFSET_MASK     GENMASK_ULL(6, 4)
> +
>  static void set_soc_power_profile(struct proc_thermal_device *proc_priv, int slider)
>  {
>         u64 val;
> @@ -128,6 +164,11 @@ static void set_soc_power_profile(struct proc_thermal_device *proc_priv, int sli
>         val &= ~SLIDER_MASK;
>         val |= FIELD_PREP(SLIDER_MASK, slider);
>         val |= BIT(SLIDER_ENABLE_BIT);
> +
> +       /* Set the slider offset from module params */
> +       val &= ~SLIDER_OFFSET_MASK;
> +       val |= FIELD_PREP(SLIDER_OFFSET_MASK, slider_offset);
> +
>         writeq(val, proc_priv->mmio_base + SOC_POWER_SLIDER_OFFSET);
>  }
>
> --