2021-12-11 16:33:52

by srinivas pandruvada

[permalink] [raw]
Subject: [PATCH] x86: intel_epb: Allow model specific normal EPB value

The current EPB "normal" is defined as 6 and set whenever power-up EPB
value is 0. This setting resulted in the desired out of box power and
performance for several CPU generations. But this value is not suitable
for AlderLake mobile CPUs, as this resulted in higher uncore power.
Since EPB is model specific, this is not unreasonable to have different
behavior.

Allow a capability where "normal" EPB can be redefined. For AlderLake
mobile CPUs this desired normal value is 7.

Signed-off-by: Srinivas Pandruvada <[email protected]>
---
arch/x86/kernel/cpu/intel_epb.c | 45 +++++++++++++++++++++++----------
1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_epb.c b/arch/x86/kernel/cpu/intel_epb.c
index f4dd73396f28..fbaf12e43f41 100644
--- a/arch/x86/kernel/cpu/intel_epb.c
+++ b/arch/x86/kernel/cpu/intel_epb.c
@@ -16,6 +16,7 @@
#include <linux/syscore_ops.h>
#include <linux/pm.h>

+#include <asm/cpu_device_id.h>
#include <asm/cpufeature.h>
#include <asm/msr.h>

@@ -58,6 +59,22 @@ static DEFINE_PER_CPU(u8, saved_epb);
#define EPB_SAVED 0x10ULL
#define MAX_EPB EPB_MASK

+enum energy_perf_value_index {
+ EPB_INDEX_PERFORMANCE,
+ EPB_INDEX_BALANCE_PERFORMANCE,
+ EPB_INDEX_NORMAL,
+ EPB_INDEX_BALANCE_POWERSAVE,
+ EPB_INDEX_POWERSAVE,
+};
+
+static u8 energ_perf_values[] = {
+ [EPB_INDEX_PERFORMANCE] = ENERGY_PERF_BIAS_PERFORMANCE,
+ [EPB_INDEX_BALANCE_PERFORMANCE] = ENERGY_PERF_BIAS_BALANCE_PERFORMANCE,
+ [EPB_INDEX_NORMAL] = ENERGY_PERF_BIAS_NORMAL,
+ [EPB_INDEX_BALANCE_POWERSAVE] = ENERGY_PERF_BIAS_BALANCE_POWERSAVE,
+ [EPB_INDEX_POWERSAVE] = ENERGY_PERF_BIAS_POWERSAVE,
+};
+
static int intel_epb_save(void)
{
u64 epb;
@@ -90,7 +107,7 @@ static void intel_epb_restore(void)
*/
val = epb & EPB_MASK;
if (val == ENERGY_PERF_BIAS_PERFORMANCE) {
- val = ENERGY_PERF_BIAS_NORMAL;
+ val = energ_perf_values[EPB_INDEX_NORMAL];
pr_warn_once("ENERGY_PERF_BIAS: Set to 'normal', was 'performance'\n");
}
}
@@ -103,18 +120,11 @@ static struct syscore_ops intel_epb_syscore_ops = {
};

static const char * const energy_perf_strings[] = {
- "performance",
- "balance-performance",
- "normal",
- "balance-power",
- "power"
-};
-static const u8 energ_perf_values[] = {
- ENERGY_PERF_BIAS_PERFORMANCE,
- ENERGY_PERF_BIAS_BALANCE_PERFORMANCE,
- ENERGY_PERF_BIAS_NORMAL,
- ENERGY_PERF_BIAS_BALANCE_POWERSAVE,
- ENERGY_PERF_BIAS_POWERSAVE
+ [EPB_INDEX_PERFORMANCE] = "performance",
+ [EPB_INDEX_BALANCE_PERFORMANCE] = "balance-performance",
+ [EPB_INDEX_NORMAL] = "normal",
+ [EPB_INDEX_BALANCE_POWERSAVE] = "balance-power",
+ [EPB_INDEX_POWERSAVE] = "power",
};

static ssize_t energy_perf_bias_show(struct device *dev,
@@ -193,13 +203,22 @@ static int intel_epb_offline(unsigned int cpu)
return 0;
}

+static const struct x86_cpu_id intel_epb_normal[] = {
+ X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, 7),
+ {}
+};
+
static __init int intel_epb_init(void)
{
+ const struct x86_cpu_id *id = x86_match_cpu(intel_epb_normal);
int ret;

if (!boot_cpu_has(X86_FEATURE_EPB))
return -ENODEV;

+ if (id)
+ energ_perf_values[EPB_INDEX_NORMAL] = id->driver_data;
+
ret = cpuhp_setup_state(CPUHP_AP_X86_INTEL_EPB_ONLINE,
"x86/intel/epb:online", intel_epb_online,
intel_epb_offline);
--
2.31.1



2021-12-13 10:05:52

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] x86: intel_epb: Allow model specific normal EPB value

On Sat, Dec 11, 2021 at 5:33 PM Srinivas Pandruvada
<[email protected]> wrote:
>
> The current EPB "normal" is defined as 6 and set whenever power-up EPB
> value is 0. This setting resulted in the desired out of box power and
> performance for several CPU generations. But this value is not suitable
> for AlderLake mobile CPUs, as this resulted in higher uncore power.
> Since EPB is model specific, this is not unreasonable to have different
> behavior.
>
> Allow a capability where "normal" EPB can be redefined. For AlderLake
> mobile CPUs this desired normal value is 7.
>
> Signed-off-by: Srinivas Pandruvada <[email protected]>

Reviewed-by: Rafael J. Wysocki <[email protected]>

> ---
> arch/x86/kernel/cpu/intel_epb.c | 45 +++++++++++++++++++++++----------
> 1 file changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/intel_epb.c b/arch/x86/kernel/cpu/intel_epb.c
> index f4dd73396f28..fbaf12e43f41 100644
> --- a/arch/x86/kernel/cpu/intel_epb.c
> +++ b/arch/x86/kernel/cpu/intel_epb.c
> @@ -16,6 +16,7 @@
> #include <linux/syscore_ops.h>
> #include <linux/pm.h>
>
> +#include <asm/cpu_device_id.h>
> #include <asm/cpufeature.h>
> #include <asm/msr.h>
>
> @@ -58,6 +59,22 @@ static DEFINE_PER_CPU(u8, saved_epb);
> #define EPB_SAVED 0x10ULL
> #define MAX_EPB EPB_MASK
>
> +enum energy_perf_value_index {
> + EPB_INDEX_PERFORMANCE,
> + EPB_INDEX_BALANCE_PERFORMANCE,
> + EPB_INDEX_NORMAL,
> + EPB_INDEX_BALANCE_POWERSAVE,
> + EPB_INDEX_POWERSAVE,
> +};
> +
> +static u8 energ_perf_values[] = {
> + [EPB_INDEX_PERFORMANCE] = ENERGY_PERF_BIAS_PERFORMANCE,
> + [EPB_INDEX_BALANCE_PERFORMANCE] = ENERGY_PERF_BIAS_BALANCE_PERFORMANCE,
> + [EPB_INDEX_NORMAL] = ENERGY_PERF_BIAS_NORMAL,
> + [EPB_INDEX_BALANCE_POWERSAVE] = ENERGY_PERF_BIAS_BALANCE_POWERSAVE,
> + [EPB_INDEX_POWERSAVE] = ENERGY_PERF_BIAS_POWERSAVE,
> +};
> +
> static int intel_epb_save(void)
> {
> u64 epb;
> @@ -90,7 +107,7 @@ static void intel_epb_restore(void)
> */
> val = epb & EPB_MASK;
> if (val == ENERGY_PERF_BIAS_PERFORMANCE) {
> - val = ENERGY_PERF_BIAS_NORMAL;
> + val = energ_perf_values[EPB_INDEX_NORMAL];
> pr_warn_once("ENERGY_PERF_BIAS: Set to 'normal', was 'performance'\n");
> }
> }
> @@ -103,18 +120,11 @@ static struct syscore_ops intel_epb_syscore_ops = {
> };
>
> static const char * const energy_perf_strings[] = {
> - "performance",
> - "balance-performance",
> - "normal",
> - "balance-power",
> - "power"
> -};
> -static const u8 energ_perf_values[] = {
> - ENERGY_PERF_BIAS_PERFORMANCE,
> - ENERGY_PERF_BIAS_BALANCE_PERFORMANCE,
> - ENERGY_PERF_BIAS_NORMAL,
> - ENERGY_PERF_BIAS_BALANCE_POWERSAVE,
> - ENERGY_PERF_BIAS_POWERSAVE
> + [EPB_INDEX_PERFORMANCE] = "performance",
> + [EPB_INDEX_BALANCE_PERFORMANCE] = "balance-performance",
> + [EPB_INDEX_NORMAL] = "normal",
> + [EPB_INDEX_BALANCE_POWERSAVE] = "balance-power",
> + [EPB_INDEX_POWERSAVE] = "power",
> };
>
> static ssize_t energy_perf_bias_show(struct device *dev,
> @@ -193,13 +203,22 @@ static int intel_epb_offline(unsigned int cpu)
> return 0;
> }
>
> +static const struct x86_cpu_id intel_epb_normal[] = {
> + X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, 7),
> + {}
> +};
> +
> static __init int intel_epb_init(void)
> {
> + const struct x86_cpu_id *id = x86_match_cpu(intel_epb_normal);
> int ret;
>
> if (!boot_cpu_has(X86_FEATURE_EPB))
> return -ENODEV;
>
> + if (id)
> + energ_perf_values[EPB_INDEX_NORMAL] = id->driver_data;
> +
> ret = cpuhp_setup_state(CPUHP_AP_X86_INTEL_EPB_ONLINE,
> "x86/intel/epb:online", intel_epb_online,
> intel_epb_offline);
> --
> 2.31.1
>

2022-01-04 15:27:05

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] x86: intel_epb: Allow model specific normal EPB value

On Mon, Dec 13, 2021 at 11:00 AM Rafael J. Wysocki <[email protected]> wrote:
>
> On Sat, Dec 11, 2021 at 5:33 PM Srinivas Pandruvada
> <[email protected]> wrote:
> >
> > The current EPB "normal" is defined as 6 and set whenever power-up EPB
> > value is 0. This setting resulted in the desired out of box power and
> > performance for several CPU generations. But this value is not suitable
> > for AlderLake mobile CPUs, as this resulted in higher uncore power.
> > Since EPB is model specific, this is not unreasonable to have different
> > behavior.
> >
> > Allow a capability where "normal" EPB can be redefined. For AlderLake
> > mobile CPUs this desired normal value is 7.
> >
> > Signed-off-by: Srinivas Pandruvada <[email protected]>
>
> Reviewed-by: Rafael J. Wysocki <[email protected]>

And since there don't seem to be any concerns regarding this, I'm
going to apply it.

> > ---
> > arch/x86/kernel/cpu/intel_epb.c | 45 +++++++++++++++++++++++----------
> > 1 file changed, 32 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/intel_epb.c b/arch/x86/kernel/cpu/intel_epb.c
> > index f4dd73396f28..fbaf12e43f41 100644
> > --- a/arch/x86/kernel/cpu/intel_epb.c
> > +++ b/arch/x86/kernel/cpu/intel_epb.c
> > @@ -16,6 +16,7 @@
> > #include <linux/syscore_ops.h>
> > #include <linux/pm.h>
> >
> > +#include <asm/cpu_device_id.h>
> > #include <asm/cpufeature.h>
> > #include <asm/msr.h>
> >
> > @@ -58,6 +59,22 @@ static DEFINE_PER_CPU(u8, saved_epb);
> > #define EPB_SAVED 0x10ULL
> > #define MAX_EPB EPB_MASK
> >
> > +enum energy_perf_value_index {
> > + EPB_INDEX_PERFORMANCE,
> > + EPB_INDEX_BALANCE_PERFORMANCE,
> > + EPB_INDEX_NORMAL,
> > + EPB_INDEX_BALANCE_POWERSAVE,
> > + EPB_INDEX_POWERSAVE,
> > +};
> > +
> > +static u8 energ_perf_values[] = {
> > + [EPB_INDEX_PERFORMANCE] = ENERGY_PERF_BIAS_PERFORMANCE,
> > + [EPB_INDEX_BALANCE_PERFORMANCE] = ENERGY_PERF_BIAS_BALANCE_PERFORMANCE,
> > + [EPB_INDEX_NORMAL] = ENERGY_PERF_BIAS_NORMAL,
> > + [EPB_INDEX_BALANCE_POWERSAVE] = ENERGY_PERF_BIAS_BALANCE_POWERSAVE,
> > + [EPB_INDEX_POWERSAVE] = ENERGY_PERF_BIAS_POWERSAVE,
> > +};
> > +
> > static int intel_epb_save(void)
> > {
> > u64 epb;
> > @@ -90,7 +107,7 @@ static void intel_epb_restore(void)
> > */
> > val = epb & EPB_MASK;
> > if (val == ENERGY_PERF_BIAS_PERFORMANCE) {
> > - val = ENERGY_PERF_BIAS_NORMAL;
> > + val = energ_perf_values[EPB_INDEX_NORMAL];
> > pr_warn_once("ENERGY_PERF_BIAS: Set to 'normal', was 'performance'\n");
> > }
> > }
> > @@ -103,18 +120,11 @@ static struct syscore_ops intel_epb_syscore_ops = {
> > };
> >
> > static const char * const energy_perf_strings[] = {
> > - "performance",
> > - "balance-performance",
> > - "normal",
> > - "balance-power",
> > - "power"
> > -};
> > -static const u8 energ_perf_values[] = {
> > - ENERGY_PERF_BIAS_PERFORMANCE,
> > - ENERGY_PERF_BIAS_BALANCE_PERFORMANCE,
> > - ENERGY_PERF_BIAS_NORMAL,
> > - ENERGY_PERF_BIAS_BALANCE_POWERSAVE,
> > - ENERGY_PERF_BIAS_POWERSAVE
> > + [EPB_INDEX_PERFORMANCE] = "performance",
> > + [EPB_INDEX_BALANCE_PERFORMANCE] = "balance-performance",
> > + [EPB_INDEX_NORMAL] = "normal",
> > + [EPB_INDEX_BALANCE_POWERSAVE] = "balance-power",
> > + [EPB_INDEX_POWERSAVE] = "power",
> > };
> >
> > static ssize_t energy_perf_bias_show(struct device *dev,
> > @@ -193,13 +203,22 @@ static int intel_epb_offline(unsigned int cpu)
> > return 0;
> > }
> >
> > +static const struct x86_cpu_id intel_epb_normal[] = {
> > + X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, 7),
> > + {}
> > +};
> > +
> > static __init int intel_epb_init(void)
> > {
> > + const struct x86_cpu_id *id = x86_match_cpu(intel_epb_normal);
> > int ret;
> >
> > if (!boot_cpu_has(X86_FEATURE_EPB))
> > return -ENODEV;
> >
> > + if (id)
> > + energ_perf_values[EPB_INDEX_NORMAL] = id->driver_data;
> > +
> > ret = cpuhp_setup_state(CPUHP_AP_X86_INTEL_EPB_ONLINE,
> > "x86/intel/epb:online", intel_epb_online,
> > intel_epb_offline);
> > --
> > 2.31.1
> >