| Message ID | 20220716062655.704893-1-dedekind1@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | intel_idle: make SPR C1 and C1E be independent | expand |
On Sat, Jul 16, 2022 at 8:27 AM Artem Bityutskiy <dedekind1@gmail.com> wrote: > > From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> > > This patch partially reverts the changes made by the following commit: > > da0e58c038e6 intel_idle: add 'preferred_cstates' module argument > > As that commit describes, on early Sapphire Rapids Xeon platforms the C1 and > C1E states were mutually exclusive, so that users could only have either C1 and > C6, or C1E and C6. > > However, Intel firmware engineers managed to remove this limitation and make C1 > and C1E to be completely independent, just like on previous Xeon platforms. > > Therefore, this patch: > * Removes commentary describing the old, and now non-existing SPR C1E > limitation. > * Marks SPR C1E as available by default. > * Removes the 'preferred_cstates' parameter handling for SPR. Both C1 and > C1E will be available regardless of 'preferred_cstates' value. > > We expect that all SPR systems are shipping with new firmware, which includes > the C1/C1E improvement. > > Cc: stable@vger.kernel.org # v5.18+ > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> > --- > drivers/idle/intel_idle.c | 24 +----------------------- > 1 file changed, 1 insertion(+), 23 deletions(-) > > diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c > index 424ef470223d..ba2b485a03ed 100644 > --- a/drivers/idle/intel_idle.c > +++ b/drivers/idle/intel_idle.c > @@ -879,16 +879,6 @@ static struct cpuidle_state adl_l_cstates[] __initdata = { > .enter = NULL } > }; > > -/* > - * On Sapphire Rapids Xeon C1 has to be disabled if C1E is enabled, and vice > - * versa. On SPR C1E is enabled only if "C1E promotion" bit is set in > - * MSR_IA32_POWER_CTL. But in this case there effectively no C1, because C1 > - * requests are promoted to C1E. If the "C1E promotion" bit is cleared, then > - * both C1 and C1E requests end up with C1, so there is effectively no C1E. > - * > - * By default we enable C1 and disable C1E by marking it with > - * 'CPUIDLE_FLAG_UNUSABLE'. > - */ > static struct cpuidle_state spr_cstates[] __initdata = { > { > .name = "C1", > @@ -901,8 +891,7 @@ static struct cpuidle_state spr_cstates[] __initdata = { > { > .name = "C1E", > .desc = "MWAIT 0x01", > - .flags = MWAIT2flg(0x01) | CPUIDLE_FLAG_ALWAYS_ENABLE | > - CPUIDLE_FLAG_UNUSABLE, > + .flags = MWAIT2flg(0x01) | CPUIDLE_FLAG_ALWAYS_ENABLE, > .exit_latency = 2, > .target_residency = 4, > .enter = &intel_idle, > @@ -1724,17 +1713,6 @@ static void __init spr_idle_state_table_update(void) > { > unsigned long long msr; > > - /* Check if user prefers C1E over C1. */ > - if ((preferred_states_mask & BIT(2)) && > - !(preferred_states_mask & BIT(1))) { > - /* Disable C1 and enable C1E. */ > - spr_cstates[0].flags |= CPUIDLE_FLAG_UNUSABLE; > - spr_cstates[1].flags &= ~CPUIDLE_FLAG_UNUSABLE; > - > - /* Enable C1E using the "C1E promotion" bit. */ > - c1e_promotion = C1E_PROMOTION_ENABLE; > - } > - > /* > * By default, the C6 state assumes the worst-case scenario of package > * C6. However, if PC6 is disabled, we update the numbers to match > -- Applied as 5.20 material, thanks!
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 424ef470223d..ba2b485a03ed 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -879,16 +879,6 @@ static struct cpuidle_state adl_l_cstates[] __initdata = { .enter = NULL } }; -/* - * On Sapphire Rapids Xeon C1 has to be disabled if C1E is enabled, and vice - * versa. On SPR C1E is enabled only if "C1E promotion" bit is set in - * MSR_IA32_POWER_CTL. But in this case there effectively no C1, because C1 - * requests are promoted to C1E. If the "C1E promotion" bit is cleared, then - * both C1 and C1E requests end up with C1, so there is effectively no C1E. - * - * By default we enable C1 and disable C1E by marking it with - * 'CPUIDLE_FLAG_UNUSABLE'. - */ static struct cpuidle_state spr_cstates[] __initdata = { { .name = "C1", @@ -901,8 +891,7 @@ static struct cpuidle_state spr_cstates[] __initdata = { { .name = "C1E", .desc = "MWAIT 0x01", - .flags = MWAIT2flg(0x01) | CPUIDLE_FLAG_ALWAYS_ENABLE | - CPUIDLE_FLAG_UNUSABLE, + .flags = MWAIT2flg(0x01) | CPUIDLE_FLAG_ALWAYS_ENABLE, .exit_latency = 2, .target_residency = 4, .enter = &intel_idle, @@ -1724,17 +1713,6 @@ static void __init spr_idle_state_table_update(void) { unsigned long long msr; - /* Check if user prefers C1E over C1. */ - if ((preferred_states_mask & BIT(2)) && - !(preferred_states_mask & BIT(1))) { - /* Disable C1 and enable C1E. */ - spr_cstates[0].flags |= CPUIDLE_FLAG_UNUSABLE; - spr_cstates[1].flags &= ~CPUIDLE_FLAG_UNUSABLE; - - /* Enable C1E using the "C1E promotion" bit. */ - c1e_promotion = C1E_PROMOTION_ENABLE; - } - /* * By default, the C6 state assumes the worst-case scenario of package * C6. However, if PC6 is disabled, we update the numbers to match