Hi-
By BIOS (1.2.3 on a Dell XPS 13 9350) seems to want to enable C1E
auto-promotion (ugh!), which results in this difference across
suspend/resume according to turbostat:
-cpu3: MSR_IA32_POWER_CTL: 0x0024005d (C1E auto-promotion: DISabled)
+cpu3: MSR_IA32_POWER_CTL: 0x0024005f (C1E auto-promotion: ENabled)
Should intel_idle learn to re-disable idle promotion on resume?
--Andy
> By BIOS (1.2.3 on a Dell XPS 13 9350) seems to want to enable C1E
> auto-promotion (ugh!), which results in this difference across
> suspend/resume according to turbostat:
>
> -cpu3: MSR_IA32_POWER_CTL: 0x0024005d (C1E auto-promotion: DISabled)
> +cpu3: MSR_IA32_POWER_CTL: 0x0024005f (C1E auto-promotion: ENabled)
>
> Should intel_idle learn to re-disable idle promotion on resume?
Yes, it seems that way.
Go ahead and send a patch, or file a bug at bugzilla.kernel.org
and we'll get to it.
thanks,
-len
Firmware that enables auto-promotion / auto-demotion flags we don't
like will probably re-enable them after suspend/resume. Disable
them again after resume so they stay fixed.
I've seen this on my Dell XPS 13 9350.
Signed-off-by: Andy Lutomirski <[email protected]>
---
drivers/idle/intel_idle.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 338df09ad60b..e3d7d8bbc843 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -61,6 +61,7 @@
#include <linux/notifier.h>
#include <linux/cpu.h>
#include <linux/module.h>
+#include <linux/syscore_ops.h>
#include <asm/cpu_device_id.h>
#include <asm/mwait.h>
#include <asm/msr.h>
@@ -1026,6 +1027,15 @@ void intel_idle_state_table_update(void)
return;
}
+static void intel_idle_resume(void)
+{
+ on_each_cpu(fix_this_cpu, NULL, 1);
+}
+
+static struct syscore_ops intel_idle_syscore_ops = {
+ .resume = intel_idle_resume,
+};
+
/*
* intel_idle_cpuidle_driver_init()
* allocate, initialize cpuidle_states
@@ -1119,6 +1129,7 @@ static int __init intel_idle_init(void)
if (retval)
return retval;
+ register_syscore_ops(&intel_idle_syscore_ops);
intel_idle_cpuidle_driver_init();
retval = cpuidle_register_driver(&intel_idle_driver);
if (retval) {
@@ -1153,6 +1164,7 @@ static void __exit intel_idle_exit(void)
{
intel_idle_cpuidle_devices_uninit();
cpuidle_unregister_driver(&intel_idle_driver);
+ unregister_syscore_ops(&intel_idle_syscore_ops);
cpu_notifier_register_begin();
--
2.5.0
The demotion policy config registers are per-package and these CPUs
only exist in single-package variants AFAIK, so it's not stricly
necessary to fix these MSRs on each core, but fixing them on resume
is still likely to be helpful, so move them into fix_this_cpu.
Signed-off-by: Andy Lutomirski <[email protected]>
---
drivers/idle/intel_idle.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index e3d7d8bbc843..cbb02d28dc48 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -826,6 +826,11 @@ static void fix_this_cpu(void *dummy)
msr_bits &= ~0x2;
wrmsrl(MSR_IA32_POWER_CTL, msr_bits);
}
+
+ if (icpu->byt_auto_demotion_disable_flag) {
+ wrmsrl(MSR_CC6_DEMOTION_POLICY_CONFIG, 0);
+ wrmsrl(MSR_MC6_DEMOTION_POLICY_CONFIG, 0);
+ }
}
static const struct idle_cpu idle_cpu_nehalem = {
@@ -1084,11 +1089,6 @@ static int __init intel_idle_cpuidle_driver_init(void)
drv->state_count += 1;
}
- if (icpu->byt_auto_demotion_disable_flag) {
- wrmsrl(MSR_CC6_DEMOTION_POLICY_CONFIG, 0);
- wrmsrl(MSR_MC6_DEMOTION_POLICY_CONFIG, 0);
- }
-
return 0;
}
--
2.5.0
I can't usefully test the fourth patch. I'm reasonably confident
that the other three work, though, and I tested them on a laptop
that doesn't preserve the C1E auto-promotion flag across
suspend/resume.
Andy Lutomirski (4):
intel_idle: Consolidate auto-promotion/auto-demotion fixups
intel_idle: Remove a broadcast MSR fixup at boot
intel_idle: Fix MSRs after resume
intel_idle: Move BYT/CHT auto-demotion fixup into fix_this_cpu
drivers/idle/intel_idle.c | 56 ++++++++++++++++++++++++-----------------------
1 file changed, 29 insertions(+), 27 deletions(-)
--
2.5.0
This eliminates some duplicate code and will make it easier
add fixup calls in places where they're currently missing.
Signed-off-by: Andy Lutomirski <[email protected]>
---
drivers/idle/intel_idle.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index cd4510a63375..32b3e6049994 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -810,21 +810,21 @@ static struct notifier_block cpu_hotplug_notifier = {
.notifier_call = cpu_hotplug_notify,
};
-static void auto_demotion_disable(void *dummy)
+static void fix_this_cpu(void *dummy)
{
unsigned long long msr_bits;
- rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
- msr_bits &= ~(icpu->auto_demotion_disable_flags);
- wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
-}
-static void c1e_promotion_disable(void *dummy)
-{
- unsigned long long msr_bits;
+ if (icpu->auto_demotion_disable_flags) {
+ rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
+ msr_bits &= ~(icpu->auto_demotion_disable_flags);
+ wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
+ }
- rdmsrl(MSR_IA32_POWER_CTL, msr_bits);
- msr_bits &= ~0x2;
- wrmsrl(MSR_IA32_POWER_CTL, msr_bits);
+ if (icpu->disable_promotion_to_c1e) {
+ rdmsrl(MSR_IA32_POWER_CTL, msr_bits);
+ msr_bits &= ~0x2;
+ wrmsrl(MSR_IA32_POWER_CTL, msr_bits);
+ }
}
static const struct idle_cpu idle_cpu_nehalem = {
@@ -1074,16 +1074,12 @@ static int __init intel_idle_cpuidle_driver_init(void)
drv->state_count += 1;
}
- if (icpu->auto_demotion_disable_flags)
- on_each_cpu(auto_demotion_disable, NULL, 1);
-
if (icpu->byt_auto_demotion_disable_flag) {
wrmsrl(MSR_CC6_DEMOTION_POLICY_CONFIG, 0);
wrmsrl(MSR_MC6_DEMOTION_POLICY_CONFIG, 0);
}
- if (icpu->disable_promotion_to_c1e) /* each-cpu is redundant */
- on_each_cpu(c1e_promotion_disable, NULL, 1);
+ on_each_cpu(fix_this_cpu, NULL, 1);
return 0;
}
@@ -1108,11 +1104,7 @@ static int intel_idle_cpu_init(int cpu)
return -EIO;
}
- if (icpu->auto_demotion_disable_flags)
- smp_call_function_single(cpu, auto_demotion_disable, NULL, 1);
-
- if (icpu->disable_promotion_to_c1e)
- smp_call_function_single(cpu, c1e_promotion_disable, NULL, 1);
+ smp_call_function_single(cpu, fix_this_cpu, NULL, 1);
return 0;
}
--
2.5.0
intel_idle already fixes MSRs on each CPU when it registers for that
CPU, so it doesn't need to separately broadcast to all CPUs on
startup.
Signed-off-by: Andy Lutomirski <[email protected]>
---
drivers/idle/intel_idle.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 32b3e6049994..338df09ad60b 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1079,8 +1079,6 @@ static int __init intel_idle_cpuidle_driver_init(void)
wrmsrl(MSR_MC6_DEMOTION_POLICY_CONFIG, 0);
}
- on_each_cpu(fix_this_cpu, NULL, 1);
-
return 0;
}
--
2.5.0
On Sun, Mar 13, 2016 at 9:31 PM, Brown, Len <[email protected]> wrote:
>> By BIOS (1.2.3 on a Dell XPS 13 9350) seems to want to enable C1E
>> auto-promotion (ugh!), which results in this difference across
>> suspend/resume according to turbostat:
>>
>> -cpu3: MSR_IA32_POWER_CTL: 0x0024005d (C1E auto-promotion: DISabled)
>> +cpu3: MSR_IA32_POWER_CTL: 0x0024005f (C1E auto-promotion: ENabled)
>>
>> Should intel_idle learn to re-disable idle promotion on resume?
>
> Yes, it seems that way.
>
> Go ahead and send a patch, or file a bug at bugzilla.kernel.org
> and we'll get to it.
Sent. The only other differences I see across suspend/resume in
turbostat --debug (with taskset -c 0 to suppress spurious junk) are:
--- pre-susp.txt 2016-03-13 22:21:39.889337697 -0700
+++ post-susp.txt 2016-03-13 21:38:20.782503438 -0700
@@ -24,8 +24,8 @@
cpu0: MSR_NHM_SNB_PKG_CST_CFG_CTL: 0x1e008006 (UNdemote-C3,
UNdemote-C1, demote-C3, demote-C1, locked: pkg-cstate-limit=6: pc8)
cpu0: MSR_PM_ENABLE: 0x00000001 (HWP)
cpu0: MSR_HWP_CAPABILITIES: 0x0108171c (high 0x1c guar 0x17 eff 0x8 low 0x1)
-cpu0: MSR_HWP_REQUEST: 0x80001c04 (min 0x4 max 0x1c des 0x0 epp 0x80
window 0x0 pkg 0x0)
-cpu0: MSR_HWP_INTERRUPT: 0x00000001 (EN_Guaranteed_Perf_Change,
Dis_Excursion_Min)
+cpu0: MSR_HWP_REQUEST: 0x8000ff01 (min 0x1 max 0xff des 0x0 epp 0x80
window 0x0 pkg 0x0)
+cpu0: MSR_HWP_INTERRUPT: 0x00000000 (Dis_Guaranteed_Perf_Change,
Dis_Excursion_Min)
cpu0: MSR_HWP_STATUS: 0x00000000 (No-Guaranteed_Perf_Change, No-Excursion_Min)
cpu0: MSR_IA32_ENERGY_PERF_BIAS: 0x00000006 (balanced)
cpu0: MSR_RAPL_POWER_UNIT: 0x000a0e03 (0.125000 Watts, 0.000061
Joules, 0.000977 sec.)
@@ -36,6 +36,6 @@
cpu0: MSR_DRAM_POWER_LIMIT: 0x5400de00000000 (UNlocked)
cpu0: DRAM Limit: DISabled (0.000000 Watts, 0.000977 sec, clamp DISabled)
cpu0: MSR_IA32_TEMPERATURE_TARGET: 0x00640000 (100 C)
-cpu0: MSR_IA32_PACKAGE_THERM_STATUS: 0x88360000 (46 C)
-cpu0: MSR_IA32_THERM_STATUS: 0x88360000 (46 C +/- 1)
-cpu1: MSR_IA32_THERM_STATUS: 0x88360000 (46 C +/- 1)
+cpu0: MSR_IA32_PACKAGE_THERM_STATUS: 0x883c0000 (40 C)
+cpu0: MSR_IA32_THERM_STATUS: 0x883c0000 (40 C +/- 1)
+cpu1: MSR_IA32_THERM_STATUS: 0x88400000 (36 C +/- 1)
Are either of those potentially interesting?
On Sun, Mar 13, 2016 at 9:48 PM, Andy Lutomirski <[email protected]> wrote:
> Firmware that enables auto-promotion / auto-demotion flags we don't
> like will probably re-enable them after suspend/resume. Disable
> them again after resume so they stay fixed.
>
> I've seen this on my Dell XPS 13 9350.
This isn't right -- syscore's resume hook is called too early for
smp_call_function_many. I think the right way is through a device's
dev_pm_ops (SET_SYSTEM_SLEEP_PM_OPS), but there don't seem to be any
devices associated with intel_idle.
--Andy
>
> Signed-off-by: Andy Lutomirski <[email protected]>
> ---
> drivers/idle/intel_idle.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 338df09ad60b..e3d7d8bbc843 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -61,6 +61,7 @@
> #include <linux/notifier.h>
> #include <linux/cpu.h>
> #include <linux/module.h>
> +#include <linux/syscore_ops.h>
> #include <asm/cpu_device_id.h>
> #include <asm/mwait.h>
> #include <asm/msr.h>
> @@ -1026,6 +1027,15 @@ void intel_idle_state_table_update(void)
> return;
> }
>
> +static void intel_idle_resume(void)
> +{
> + on_each_cpu(fix_this_cpu, NULL, 1);
> +}
> +
> +static struct syscore_ops intel_idle_syscore_ops = {
> + .resume = intel_idle_resume,
> +};
> +
> /*
> * intel_idle_cpuidle_driver_init()
> * allocate, initialize cpuidle_states
> @@ -1119,6 +1129,7 @@ static int __init intel_idle_init(void)
> if (retval)
> return retval;
>
> + register_syscore_ops(&intel_idle_syscore_ops);
> intel_idle_cpuidle_driver_init();
> retval = cpuidle_register_driver(&intel_idle_driver);
> if (retval) {
> @@ -1153,6 +1164,7 @@ static void __exit intel_idle_exit(void)
> {
> intel_idle_cpuidle_devices_uninit();
> cpuidle_unregister_driver(&intel_idle_driver);
> + unregister_syscore_ops(&intel_idle_syscore_ops);
>
> cpu_notifier_register_begin();
>
> --
> 2.5.0
>
--
Andy Lutomirski
AMA Capital Management, LLC