Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1356806 > unrolled thread

C1E auto-promotion suspend/resume

Back to article view | Back to linux.kernel

Contents

  C1E auto-promotion suspend/resume Andy Lutomirski <luto@amacapital.net> - 2016-03-14 02:40 +0100
    RE: C1E auto-promotion suspend/resume "Brown, Len" <len.brown@intel.com> - 2016-03-14 05:40 +0100
      [PATCH 0/4] intel_idle: Improve MSR fixup resume handling Andy Lutomirski <luto@kernel.org> - 2016-03-14 05:50 +0100
        [PATCH 3/4] intel_idle: Fix MSRs after resume Andy Lutomirski <luto@kernel.org> - 2016-03-14 05:50 +0100
        [PATCH 4/4] intel_idle: Move BYT/CHT auto-demotion fixup into fix_this_cpu Andy Lutomirski <luto@kernel.org> - 2016-03-14 05:50 +0100
        [PATCH 1/4] intel_idle: Consolidate auto-promotion/auto-demotion fixups Andy Lutomirski <luto@kernel.org> - 2016-03-14 05:50 +0100
        [PATCH 2/4] intel_idle: Remove a broadcast MSR fixup at boot Andy Lutomirski <luto@kernel.org> - 2016-03-14 05:50 +0100
      Re: C1E auto-promotion suspend/resume Andy Lutomirski <luto@amacapital.net> - 2016-03-14 06:30 +0100

#1356806 — C1E auto-promotion suspend/resume

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

[toc] | [next] | [standalone]

#1356892

> 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

[toc] | [prev] | [next] | [standalone]

#1356899 — [PATCH 0/4] intel_idle: Improve MSR fixup resume handling

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

[toc] | [prev] | [next] | [standalone]

#1356905 — [PATCH 3/4] intel_idle: Fix MSRs after resume

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 <luto@kernel.org>
---
 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

[toc] | [prev] | [next] | [standalone]

#1356907 — [PATCH 4/4] intel_idle: Move BYT/CHT auto-demotion fixup into fix_this_cpu

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 <luto@kernel.org>
---
 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

[toc] | [prev] | [next] | [standalone]

#1356910 — [PATCH 1/4] intel_idle: Consolidate auto-promotion/auto-demotion fixups

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 <luto@kernel.org>
---
 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

[toc] | [prev] | [next] | [standalone]

#1356923 — [PATCH 2/4] intel_idle: Remove a broadcast MSR fixup at boot

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 <luto@kernel.org>
---
 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

[toc] | [prev] | [next] | [standalone]

#1356942

On Sun, Mar 13, 2016 at 9:31 PM, Brown, Len <len.brown@intel.com> 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?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web