2021-12-16 18:33:11

by Srinivas Pandruvada

[permalink] [raw]
Subject: [PATCH] cpufreq: intel_pstate: Update EPP for AlderLake mobile

There is an expectation from users that they can get frequency specified
by cpufreq/cpuinfo_max_freq when conditions permit. But with AlderLake
mobile it may not be possible. This is possible that frequency is clipped
based on the system power-up EPP value. In this case users can update
cpufreq/energy_performance_preference to some performance oriented EPP to
limit clipping of frequencies.

To get out of box behavior as the prior generations of CPUs, update EPP
for AlderLake mobile CPUs on boot. On prior generations of CPUs EPP = 128
was enough to get maximum frequency, but with AlderLake mobile the
equivalent EPP is 102. Since EPP is model specific, this is possible that
they have different meaning on each generation of CPU.

The current EPP string "balance_performance" corresponds to EPP = 128.
Change the EPP corresponding to "balance_performance" to 102 for only
AlderLake mobile CPUs and update this on each CPU during boot.

To implement reuse epp_values[] array and update the modified EPP at the
index for BALANCE_PERFORMANCE. Add a dummy EPP_INDEX_DEFAULT to
epp_values[] to match indexes in the energy_perf_strings[].

After HWP PM is enabled also update EPP when "balance_performance" is
redefined for the very first time after the boot on each CPU. On
subsequent suspend/resume or offline/online the old EPP is restored,
so no specific action is needed.

Signed-off-by: Srinivas Pandruvada <[email protected]>
---
drivers/cpufreq/intel_pstate.c | 79 ++++++++++++++++++++++++----------
1 file changed, 57 insertions(+), 22 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index dec2a5649ac1..1408411f4618 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -664,19 +664,29 @@ static int intel_pstate_set_epb(int cpu, s16 pref)
* 3 balance_power
* 4 power
*/
+
+enum energy_perf_value_index {
+ EPP_INDEX_DEFAULT = 0,
+ EPP_INDEX_PERFORMANCE,
+ EPP_INDEX_BALANCE_PERFORMANCE,
+ EPP_INDEX_BALANCE_POWERSAVE,
+ EPP_INDEX_POWERSAVE,
+};
+
static const char * const energy_perf_strings[] = {
- "default",
- "performance",
- "balance_performance",
- "balance_power",
- "power",
+ [EPP_INDEX_DEFAULT] = "default",
+ [EPP_INDEX_PERFORMANCE] = "performance",
+ [EPP_INDEX_BALANCE_PERFORMANCE] = "balance_performance",
+ [EPP_INDEX_BALANCE_POWERSAVE] = "balance_power",
+ [EPP_INDEX_POWERSAVE] = "power",
NULL
};
-static const unsigned int epp_values[] = {
- HWP_EPP_PERFORMANCE,
- HWP_EPP_BALANCE_PERFORMANCE,
- HWP_EPP_BALANCE_POWERSAVE,
- HWP_EPP_POWERSAVE
+static unsigned int epp_values[] = {
+ [EPP_INDEX_DEFAULT] = 0, /* Unused index */
+ [EPP_INDEX_PERFORMANCE] = HWP_EPP_PERFORMANCE,
+ [EPP_INDEX_BALANCE_PERFORMANCE] = HWP_EPP_BALANCE_PERFORMANCE,
+ [EPP_INDEX_BALANCE_POWERSAVE] = HWP_EPP_BALANCE_POWERSAVE,
+ [EPP_INDEX_POWERSAVE] = HWP_EPP_POWERSAVE,
};

static int intel_pstate_get_energy_pref_index(struct cpudata *cpu_data, int *raw_epp)
@@ -690,14 +700,14 @@ static int intel_pstate_get_energy_pref_index(struct cpudata *cpu_data, int *raw
return epp;

if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
- if (epp == HWP_EPP_PERFORMANCE)
- return 1;
- if (epp == HWP_EPP_BALANCE_PERFORMANCE)
- return 2;
- if (epp == HWP_EPP_BALANCE_POWERSAVE)
- return 3;
- if (epp == HWP_EPP_POWERSAVE)
- return 4;
+ if (epp == epp_values[EPP_INDEX_PERFORMANCE])
+ return EPP_INDEX_PERFORMANCE;
+ if (epp == epp_values[EPP_INDEX_BALANCE_PERFORMANCE])
+ return EPP_INDEX_BALANCE_PERFORMANCE;
+ if (epp == epp_values[EPP_INDEX_BALANCE_POWERSAVE])
+ return EPP_INDEX_BALANCE_POWERSAVE;
+ if (epp == epp_values[EPP_INDEX_POWERSAVE])
+ return EPP_INDEX_POWERSAVE;
*raw_epp = epp;
return 0;
} else if (boot_cpu_has(X86_FEATURE_EPB)) {
@@ -757,7 +767,7 @@ static int intel_pstate_set_energy_pref_index(struct cpudata *cpu_data,
if (use_raw)
epp = raw_epp;
else if (epp == -EINVAL)
- epp = epp_values[pref_index - 1];
+ epp = epp_values[pref_index];

/*
* To avoid confusion, refuse to set EPP to any values different
@@ -843,7 +853,7 @@ static ssize_t store_energy_performance_preference(
* upfront.
*/
if (!raw)
- epp = ret ? epp_values[ret - 1] : cpu->epp_default;
+ epp = ret ? epp_values[ret] : cpu->epp_default;

if (cpu->epp_cached != epp) {
int err;
@@ -1679,10 +1689,18 @@ static void intel_pstate_hwp_enable(struct cpudata *cpudata)
wrmsrl_on_cpu(cpudata->cpu, MSR_HWP_INTERRUPT, 0x00);

wrmsrl_on_cpu(cpudata->cpu, MSR_PM_ENABLE, 0x1);
- if (cpudata->epp_default == -EINVAL)
- cpudata->epp_default = intel_pstate_get_epp(cpudata, 0);

intel_pstate_enable_hwp_interrupt(cpudata);
+
+ if (cpudata->epp_default >= 0)
+ return;
+
+ if (epp_values[EPP_INDEX_BALANCE_PERFORMANCE] == HWP_EPP_BALANCE_PERFORMANCE) {
+ cpudata->epp_default = intel_pstate_get_epp(cpudata, 0);
+ } else {
+ cpudata->epp_default = epp_values[EPP_INDEX_BALANCE_PERFORMANCE];
+ intel_pstate_set_epp(cpudata, cpudata->epp_default);
+ }
}

static int atom_get_min_pstate(void)
@@ -3349,6 +3367,16 @@ static bool intel_pstate_hwp_is_enabled(void)
return !!(value & 0x1);
}

+static const struct x86_cpu_id intel_epp_balance_perf[] = {
+ /*
+ * Set EPP value as 102, this is the max suggested EPP
+ * which can result in one core turbo frequency for
+ * AlderLake Mobile CPUs.
+ */
+ X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, 102),
+ {}
+};
+
static int __init intel_pstate_init(void)
{
static struct cpudata **_all_cpu_data;
@@ -3438,6 +3466,13 @@ static int __init intel_pstate_init(void)

intel_pstate_sysfs_expose_params();

+ if (hwp_active) {
+ const struct x86_cpu_id *id = x86_match_cpu(intel_epp_balance_perf);
+
+ if (id)
+ epp_values[EPP_INDEX_BALANCE_PERFORMANCE] = id->driver_data;
+ }
+
mutex_lock(&intel_pstate_driver_lock);
rc = intel_pstate_register_driver(default_driver);
mutex_unlock(&intel_pstate_driver_lock);
--
2.31.1



2021-12-17 15:45:23

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] cpufreq: intel_pstate: Update EPP for AlderLake mobile

On Thu, Dec 16, 2021 at 7:33 PM Srinivas Pandruvada
<[email protected]> wrote:
>
> There is an expectation from users that they can get frequency specified
> by cpufreq/cpuinfo_max_freq when conditions permit. But with AlderLake
> mobile it may not be possible. This is possible that frequency is clipped
> based on the system power-up EPP value. In this case users can update
> cpufreq/energy_performance_preference to some performance oriented EPP to
> limit clipping of frequencies.
>
> To get out of box behavior as the prior generations of CPUs, update EPP
> for AlderLake mobile CPUs on boot. On prior generations of CPUs EPP = 128
> was enough to get maximum frequency, but with AlderLake mobile the
> equivalent EPP is 102. Since EPP is model specific, this is possible that
> they have different meaning on each generation of CPU.
>
> The current EPP string "balance_performance" corresponds to EPP = 128.
> Change the EPP corresponding to "balance_performance" to 102 for only
> AlderLake mobile CPUs and update this on each CPU during boot.
>
> To implement reuse epp_values[] array and update the modified EPP at the
> index for BALANCE_PERFORMANCE. Add a dummy EPP_INDEX_DEFAULT to
> epp_values[] to match indexes in the energy_perf_strings[].
>
> After HWP PM is enabled also update EPP when "balance_performance" is
> redefined for the very first time after the boot on each CPU. On
> subsequent suspend/resume or offline/online the old EPP is restored,
> so no specific action is needed.
>
> Signed-off-by: Srinivas Pandruvada <[email protected]>

Applied as 5.17 material, thanks!

> ---
> drivers/cpufreq/intel_pstate.c | 79 ++++++++++++++++++++++++----------
> 1 file changed, 57 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index dec2a5649ac1..1408411f4618 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -664,19 +664,29 @@ static int intel_pstate_set_epb(int cpu, s16 pref)
> * 3 balance_power
> * 4 power
> */
> +
> +enum energy_perf_value_index {
> + EPP_INDEX_DEFAULT = 0,
> + EPP_INDEX_PERFORMANCE,
> + EPP_INDEX_BALANCE_PERFORMANCE,
> + EPP_INDEX_BALANCE_POWERSAVE,
> + EPP_INDEX_POWERSAVE,
> +};
> +
> static const char * const energy_perf_strings[] = {
> - "default",
> - "performance",
> - "balance_performance",
> - "balance_power",
> - "power",
> + [EPP_INDEX_DEFAULT] = "default",
> + [EPP_INDEX_PERFORMANCE] = "performance",
> + [EPP_INDEX_BALANCE_PERFORMANCE] = "balance_performance",
> + [EPP_INDEX_BALANCE_POWERSAVE] = "balance_power",
> + [EPP_INDEX_POWERSAVE] = "power",
> NULL
> };
> -static const unsigned int epp_values[] = {
> - HWP_EPP_PERFORMANCE,
> - HWP_EPP_BALANCE_PERFORMANCE,
> - HWP_EPP_BALANCE_POWERSAVE,
> - HWP_EPP_POWERSAVE
> +static unsigned int epp_values[] = {
> + [EPP_INDEX_DEFAULT] = 0, /* Unused index */
> + [EPP_INDEX_PERFORMANCE] = HWP_EPP_PERFORMANCE,
> + [EPP_INDEX_BALANCE_PERFORMANCE] = HWP_EPP_BALANCE_PERFORMANCE,
> + [EPP_INDEX_BALANCE_POWERSAVE] = HWP_EPP_BALANCE_POWERSAVE,
> + [EPP_INDEX_POWERSAVE] = HWP_EPP_POWERSAVE,
> };
>
> static int intel_pstate_get_energy_pref_index(struct cpudata *cpu_data, int *raw_epp)
> @@ -690,14 +700,14 @@ static int intel_pstate_get_energy_pref_index(struct cpudata *cpu_data, int *raw
> return epp;
>
> if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
> - if (epp == HWP_EPP_PERFORMANCE)
> - return 1;
> - if (epp == HWP_EPP_BALANCE_PERFORMANCE)
> - return 2;
> - if (epp == HWP_EPP_BALANCE_POWERSAVE)
> - return 3;
> - if (epp == HWP_EPP_POWERSAVE)
> - return 4;
> + if (epp == epp_values[EPP_INDEX_PERFORMANCE])
> + return EPP_INDEX_PERFORMANCE;
> + if (epp == epp_values[EPP_INDEX_BALANCE_PERFORMANCE])
> + return EPP_INDEX_BALANCE_PERFORMANCE;
> + if (epp == epp_values[EPP_INDEX_BALANCE_POWERSAVE])
> + return EPP_INDEX_BALANCE_POWERSAVE;
> + if (epp == epp_values[EPP_INDEX_POWERSAVE])
> + return EPP_INDEX_POWERSAVE;
> *raw_epp = epp;
> return 0;
> } else if (boot_cpu_has(X86_FEATURE_EPB)) {
> @@ -757,7 +767,7 @@ static int intel_pstate_set_energy_pref_index(struct cpudata *cpu_data,
> if (use_raw)
> epp = raw_epp;
> else if (epp == -EINVAL)
> - epp = epp_values[pref_index - 1];
> + epp = epp_values[pref_index];
>
> /*
> * To avoid confusion, refuse to set EPP to any values different
> @@ -843,7 +853,7 @@ static ssize_t store_energy_performance_preference(
> * upfront.
> */
> if (!raw)
> - epp = ret ? epp_values[ret - 1] : cpu->epp_default;
> + epp = ret ? epp_values[ret] : cpu->epp_default;
>
> if (cpu->epp_cached != epp) {
> int err;
> @@ -1679,10 +1689,18 @@ static void intel_pstate_hwp_enable(struct cpudata *cpudata)
> wrmsrl_on_cpu(cpudata->cpu, MSR_HWP_INTERRUPT, 0x00);
>
> wrmsrl_on_cpu(cpudata->cpu, MSR_PM_ENABLE, 0x1);
> - if (cpudata->epp_default == -EINVAL)
> - cpudata->epp_default = intel_pstate_get_epp(cpudata, 0);
>
> intel_pstate_enable_hwp_interrupt(cpudata);
> +
> + if (cpudata->epp_default >= 0)
> + return;
> +
> + if (epp_values[EPP_INDEX_BALANCE_PERFORMANCE] == HWP_EPP_BALANCE_PERFORMANCE) {
> + cpudata->epp_default = intel_pstate_get_epp(cpudata, 0);
> + } else {
> + cpudata->epp_default = epp_values[EPP_INDEX_BALANCE_PERFORMANCE];
> + intel_pstate_set_epp(cpudata, cpudata->epp_default);
> + }
> }
>
> static int atom_get_min_pstate(void)
> @@ -3349,6 +3367,16 @@ static bool intel_pstate_hwp_is_enabled(void)
> return !!(value & 0x1);
> }
>
> +static const struct x86_cpu_id intel_epp_balance_perf[] = {
> + /*
> + * Set EPP value as 102, this is the max suggested EPP
> + * which can result in one core turbo frequency for
> + * AlderLake Mobile CPUs.
> + */
> + X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, 102),
> + {}
> +};
> +
> static int __init intel_pstate_init(void)
> {
> static struct cpudata **_all_cpu_data;
> @@ -3438,6 +3466,13 @@ static int __init intel_pstate_init(void)
>
> intel_pstate_sysfs_expose_params();
>
> + if (hwp_active) {
> + const struct x86_cpu_id *id = x86_match_cpu(intel_epp_balance_perf);
> +
> + if (id)
> + epp_values[EPP_INDEX_BALANCE_PERFORMANCE] = id->driver_data;
> + }
> +
> mutex_lock(&intel_pstate_driver_lock);
> rc = intel_pstate_register_driver(default_driver);
> mutex_unlock(&intel_pstate_driver_lock);
> --
> 2.31.1
>

2021-12-29 10:22:34

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [PATCH] cpufreq: intel_pstate: Update EPP for AlderLake mobile

On Thu, 2021-12-16 at 10:33 -0800, Srinivas Pandruvada wrote:
> There is an expectation from users that they can get frequency specified
> by cpufreq/cpuinfo_max_freq when conditions permit. But with AlderLake
> mobile it may not be possible. This is possible that frequency is clipped
> based on the system power-up EPP value. In this case users can update
> cpufreq/energy_performance_preference to some performance oriented EPP to
> limit clipping of frequencies.
>
> To get out of box behavior as the prior generations of CPUs, update EPP
> for AlderLake mobile CPUs on boot. On prior generations of CPUs EPP = 128
> was enough to get maximum frequency, but with AlderLake mobile the
> equivalent EPP is 102. Since EPP is model specific, this is possible that
> they have different meaning on each generation of CPU.
>
> The current EPP string "balance_performance" corresponds to EPP = 128.
> Change the EPP corresponding to "balance_performance" to 102 for only
> AlderLake mobile CPUs and update this on each CPU during boot.
>
> To implement reuse epp_values[] array and update the modified EPP at the
> index for BALANCE_PERFORMANCE. Add a dummy EPP_INDEX_DEFAULT to
> epp_values[] to match indexes in the energy_perf_strings[].
>
> After HWP PM is enabled also update EPP when "balance_performance" is
> redefined for the very first time after the boot on each CPU. On
> subsequent suspend/resume or offline/online the old EPP is restored,
> so no specific action is needed.
>
> Signed-off-by: Srinivas Pandruvada <[email protected]>


I think this patch alone makes things a bit inconsistent, because the
'x86_energy_perf_policy' does not get adjusted.

Ideally, constants like HWP_EPP_BALANCE_PERFORMANCE should go away from
arch/x86/include/asm/msr-index.h, along with all users.

Artem.