| Message ID | 2c788c2ca0cab09a8ef4e384f272af928a880b0e.1744781329.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | cpufreq: acpi: Don't enable boost on policy exit | expand |
On 2025/4/16 13:29, Viresh Kumar wrote: > The boost-related code in cpufreq has undergone several changes over the > years, but this particular piece remained unchanged and is now outdated. > > The cpufreq core currently manages boost settings during initialization, > and only when necessary. As such, there's no longer a need to enable > boost explicitly when entering system suspend. > > Previously, this wasn’t causing issues because boost settings were > force-updated during policy initialization. However, commit 2b16c631832d > ("cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()") changed > that behavior—correctly—by avoiding unnecessary updates. > > As a result of this change, if boost was disabled prior to suspend, it > remains disabled on resume—as expected. But due to the current code > forcibly enabling boost at suspend time, the system ends up with boost > frequencies enabled after resume, even if the global boost flag was > disabled. This contradicts the intended behavior. > > Fix this by not enabling boost on policy exit. > > Fixes: 2b16c631832d ("cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()") > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220013 > Reported-by: Nicholas Chin <nic.c3.14@gmail.com> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > drivers/cpufreq/acpi-cpufreq.c | 23 +++-------------------- > 1 file changed, 3 insertions(+), 20 deletions(-) > > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c > index 924314cdeebc..85b5a88f723f 100644 > --- a/drivers/cpufreq/acpi-cpufreq.c > +++ b/drivers/cpufreq/acpi-cpufreq.c > @@ -89,8 +89,9 @@ static bool boost_state(unsigned int cpu) > return false; > } > > -static int boost_set_msr(bool enable) > +static void boost_set_msr_each(void *p_en) > { > + bool enable = (bool) p_en; > u32 msr_addr; > u64 msr_mask, val; > > @@ -107,7 +108,7 @@ static int boost_set_msr(bool enable) > msr_mask = MSR_K7_HWCR_CPB_DIS; > break; > default: > - return -EINVAL; > + return; > } > > rdmsrl(msr_addr, val); > @@ -118,14 +119,6 @@ static int boost_set_msr(bool enable) > val |= msr_mask; > > wrmsrl(msr_addr, val); > - return 0; > -} > - > -static void boost_set_msr_each(void *p_en) > -{ > - bool enable = (bool) p_en; > - > - boost_set_msr(enable); > } > > static int set_boost(struct cpufreq_policy *policy, int val) > @@ -532,15 +525,6 @@ static void free_acpi_perf_data(void) > free_percpu(acpi_perf_data); > } > > -static int cpufreq_boost_down_prep(unsigned int cpu) > -{ > - /* > - * Clear the boost-disable bit on the CPU_DOWN path so that > - * this cpu cannot block the remaining ones from boosting. > - */ > - return boost_set_msr(1); > -} > - > /* > * acpi_cpufreq_early_init - initialize ACPI P-States library > * > @@ -931,7 +915,6 @@ static void acpi_cpufreq_cpu_exit(struct cpufreq_policy *policy) > > pr_debug("%s\n", __func__); > > - cpufreq_boost_down_prep(policy->cpu); > policy->fast_switch_possible = false; > policy->driver_data = NULL; > acpi_processor_unregister_performance(data->acpi_perf_cpu); Nice! I wonder why this cpufreq_boost_down_prep() was needed at the beginning. Reviewed-by: Lifeng Zheng <zhenglifeng1@huawei.com>
On Wed, Apr 16, 2025 at 10:47 AM zhenglifeng (A) <zhenglifeng1@huawei.com> wrote: > > On 2025/4/16 13:29, Viresh Kumar wrote: > > > The boost-related code in cpufreq has undergone several changes over the > > years, but this particular piece remained unchanged and is now outdated. > > > > The cpufreq core currently manages boost settings during initialization, > > and only when necessary. As such, there's no longer a need to enable > > boost explicitly when entering system suspend. > > > > Previously, this wasn’t causing issues because boost settings were > > force-updated during policy initialization. However, commit 2b16c631832d > > ("cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()") changed > > that behavior—correctly—by avoiding unnecessary updates. > > > > As a result of this change, if boost was disabled prior to suspend, it > > remains disabled on resume—as expected. But due to the current code > > forcibly enabling boost at suspend time, the system ends up with boost > > frequencies enabled after resume, even if the global boost flag was > > disabled. This contradicts the intended behavior. > > > > Fix this by not enabling boost on policy exit. > > > > Fixes: 2b16c631832d ("cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()") > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220013 > > Reported-by: Nicholas Chin <nic.c3.14@gmail.com> > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > --- > > drivers/cpufreq/acpi-cpufreq.c | 23 +++-------------------- > > 1 file changed, 3 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c > > index 924314cdeebc..85b5a88f723f 100644 > > --- a/drivers/cpufreq/acpi-cpufreq.c > > +++ b/drivers/cpufreq/acpi-cpufreq.c > > @@ -89,8 +89,9 @@ static bool boost_state(unsigned int cpu) > > return false; > > } > > > > -static int boost_set_msr(bool enable) > > +static void boost_set_msr_each(void *p_en) > > { > > + bool enable = (bool) p_en; > > u32 msr_addr; > > u64 msr_mask, val; > > > > @@ -107,7 +108,7 @@ static int boost_set_msr(bool enable) > > msr_mask = MSR_K7_HWCR_CPB_DIS; > > break; > > default: > > - return -EINVAL; > > + return; > > } > > > > rdmsrl(msr_addr, val); > > @@ -118,14 +119,6 @@ static int boost_set_msr(bool enable) > > val |= msr_mask; > > > > wrmsrl(msr_addr, val); > > - return 0; > > -} > > - > > -static void boost_set_msr_each(void *p_en) > > -{ > > - bool enable = (bool) p_en; > > - > > - boost_set_msr(enable); > > } > > > > static int set_boost(struct cpufreq_policy *policy, int val) > > @@ -532,15 +525,6 @@ static void free_acpi_perf_data(void) > > free_percpu(acpi_perf_data); > > } > > > > -static int cpufreq_boost_down_prep(unsigned int cpu) > > -{ > > - /* > > - * Clear the boost-disable bit on the CPU_DOWN path so that > > - * this cpu cannot block the remaining ones from boosting. > > - */ > > - return boost_set_msr(1); > > -} > > - > > /* > > * acpi_cpufreq_early_init - initialize ACPI P-States library > > * > > @@ -931,7 +915,6 @@ static void acpi_cpufreq_cpu_exit(struct cpufreq_policy *policy) > > > > pr_debug("%s\n", __func__); > > > > - cpufreq_boost_down_prep(policy->cpu); > > policy->fast_switch_possible = false; > > policy->driver_data = NULL; > > acpi_processor_unregister_performance(data->acpi_perf_cpu); > > Nice! > > I wonder why this cpufreq_boost_down_prep() was needed at the beginning. > > Reviewed-by: Lifeng Zheng <zhenglifeng1@huawei.com> Applied as 6.15-rc material, thanks!
> The boost-related code in cpufreq has undergone several changes over the > years, but this particular piece remained unchanged and is now outdated. > > The cpufreq core currently manages boost settings during initialization, > and only when necessary. As such, there's no longer a need to enable > boost explicitly when entering system suspend. > > Previously, this wasn’t causing issues because boost settings were > force-updated during policy initialization. However, commit 2b16c631832d > ("cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()") changed > that behavior—correctly—by avoiding unnecessary updates. > > As a result of this change, if boost was disabled prior to suspend, it > remains disabled on resume—as expected. But due to the current code > forcibly enabling boost at suspend time, the system ends up with boost > frequencies enabled after resume, even if the global boost flag was > disabled. This contradicts the intended behavior. > > Fix this by not enabling boost on policy exit. > > Fixes: 2b16c631832d ("cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()") > Closes:https://bugzilla.kernel.org/show_bug.cgi?id=220013 > Reported-by: Nicholas Chin<nic.c3.14@gmail.com> > Signed-off-by: Viresh Kumar<viresh.kumar@linaro.org> > --- > drivers/cpufreq/acpi-cpufreq.c | 23 +++-------------------- > 1 file changed, 3 insertions(+), 20 deletions(-) > > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c > index 924314cdeebc..85b5a88f723f 100644 > --- a/drivers/cpufreq/acpi-cpufreq.c > +++ b/drivers/cpufreq/acpi-cpufreq.c > @@ -89,8 +89,9 @@ static bool boost_state(unsigned int cpu) > return false; > } > > -static int boost_set_msr(bool enable) > +static void boost_set_msr_each(void *p_en) > { > + bool enable = (bool) p_en; > u32 msr_addr; > u64 msr_mask, val; > > @@ -107,7 +108,7 @@ static int boost_set_msr(bool enable) > msr_mask = MSR_K7_HWCR_CPB_DIS; > break; > default: > - return -EINVAL; > + return; > } > > rdmsrl(msr_addr, val); > @@ -118,14 +119,6 @@ static int boost_set_msr(bool enable) > val |= msr_mask; > > wrmsrl(msr_addr, val); > - return 0; > -} > - > -static void boost_set_msr_each(void *p_en) > -{ > - bool enable = (bool) p_en; > - > - boost_set_msr(enable); > } > > static int set_boost(struct cpufreq_policy *policy, int val) > @@ -532,15 +525,6 @@ static void free_acpi_perf_data(void) > free_percpu(acpi_perf_data); > } > > -static int cpufreq_boost_down_prep(unsigned int cpu) > -{ > - /* > - * Clear the boost-disable bit on the CPU_DOWN path so that > - * this cpu cannot block the remaining ones from boosting. > - */ > - return boost_set_msr(1); > -} > - > /* > * acpi_cpufreq_early_init - initialize ACPI P-States library > * > @@ -931,7 +915,6 @@ static void acpi_cpufreq_cpu_exit(struct cpufreq_policy *policy) > > pr_debug("%s\n", __func__); > > - cpufreq_boost_down_prep(policy->cpu); > policy->fast_switch_possible = false; > policy->driver_data = NULL; > acpi_processor_unregister_performance(data->acpi_perf_cpu); Unfortunately the issue I reported still seems to be present after applying this patch. Upon resuming from suspend, the system is still entering boost states descpite the boost flag being set to 0.
On 16-04-25, 19:54, Nicholas Chin wrote: > Unfortunately the issue I reported still seems to be present after > applying this patch. Upon resuming from suspend, the system is still > entering boost states descpite the boost flag being set to 0. Okay, so this is what we know so far: - Force synchronizing (disabling here) boost state at resume was making this work earlier. - Setting the boost flag to "enabled" state during resume works as well, as that makes the cpufreq core disable the boost frequencies again. - This patch (though doing the correct thing) doesn't work. This is one of the known places where boost was getting enabled before going to suspend though. - This means that some other part of kernel (or hardware ?) is enabling the boost frequencies at suspend (or early resume), which the kernel was disabling earlier and not anymore. Rafael, any suggestions ?
Copying more information from Bugzilla here (Nicholas, it would be faster if you can put all your observations here first, more people are looking at emails than bugzilla). > Nicholas Chin wrote: > I did some more testing and debugging and it seems like when > cpufreq_online() runs after waking the system, policy->boost_enabled > and cpufreq_boost_enabled() are both 0, so the set_boost() at the end > of that function is never run. Right, this is what I wanted to do with the $Subject patch. Don't update boost anymore in suspend/resume > cpufreq_boost_enabled() being 0 indicates that the MSR has boosting > disabled, but when I read out that MSR using rdmsr the bit seems to > indicate that it is actually enabled (I am aware of the inverted logic > of that bit). set_boost() seems to be the only place in the kernel > that causes that MSR to be modified, and I didn't see any extra calls > to it in my debug logs, so it seems like something else (outside the > kernel?) is setting that MSR. And this is what I feel too, something else in kernel or outside of it is doing something tricky.
On Thu, Apr 17, 2025 at 7:02 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 16-04-25, 19:54, Nicholas Chin wrote: > > Unfortunately the issue I reported still seems to be present after > > applying this patch. Upon resuming from suspend, the system is still > > entering boost states descpite the boost flag being set to 0. > > Okay, so this is what we know so far: > > - Force synchronizing (disabling here) boost state at resume was > making this work earlier. > > - Setting the boost flag to "enabled" state during resume works as > well, as that makes the cpufreq core disable the boost frequencies > again. > > - This patch (though doing the correct thing) doesn't work. This is > one of the known places where boost was getting enabled before going > to suspend though. > > - This means that some other part of kernel (or hardware ?) is > enabling the boost frequencies at suspend (or early resume), which > the kernel was disabling earlier and not anymore. > > Rafael, any suggestions ? This a resume from S3, so the platform firmware may enable the boost in its resume path.
On Thu, Apr 17, 2025 at 7:09 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > Copying more information from Bugzilla here (Nicholas, it would be > faster if you can put all your observations here first, more people > are looking at emails than bugzilla). > > > Nicholas Chin wrote: > > I did some more testing and debugging and it seems like when > > cpufreq_online() runs after waking the system, policy->boost_enabled > > and cpufreq_boost_enabled() are both 0, so the set_boost() at the end > > of that function is never run. > > Right, this is what I wanted to do with the $Subject patch. Don't > update boost anymore in suspend/resume This is going to work for suspend-to-idle, but not necessarily for S3. BTW, the patch is correct IMV, so I'm not going to drop it, but it looks like something more is needed on top of it. > > cpufreq_boost_enabled() being 0 indicates that the MSR has boosting > > disabled, but when I read out that MSR using rdmsr the bit seems to > > indicate that it is actually enabled (I am aware of the inverted logic > > of that bit). set_boost() seems to be the only place in the kernel > > that causes that MSR to be modified, and I didn't see any extra calls > > to it in my debug logs, so it seems like something else (outside the > > kernel?) is setting that MSR. > > And this is what I feel too, something else in kernel or outside of it > is doing something tricky. On a resume from S3, you actually don't know if the platform firmware has preserved the configuration from before the suspend transition. It may not.
On Thu, Apr 17, 2025 at 2:39 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Thu, Apr 17, 2025 at 7:09 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > Copying more information from Bugzilla here (Nicholas, it would be > > faster if you can put all your observations here first, more people > > are looking at emails than bugzilla). > > > > > Nicholas Chin wrote: > > > I did some more testing and debugging and it seems like when > > > cpufreq_online() runs after waking the system, policy->boost_enabled > > > and cpufreq_boost_enabled() are both 0, so the set_boost() at the end > > > of that function is never run. > > > > Right, this is what I wanted to do with the $Subject patch. Don't > > update boost anymore in suspend/resume > > This is going to work for suspend-to-idle, but not necessarily for S3. > > BTW, the patch is correct IMV, so I'm not going to drop it, but it > looks like something more is needed on top of it. But the changelog isn't because the patch really doesn't address the issue at hand, which is most likely related to the platform firmware clearing the "boost disable" bit. Frankly, I'd rather get back to the state from before commit 2b16c631832d ("cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()") and start over with the knowledge that the platform firmware may scribble on the MSR before the kernel gets control back. On a way back from system suspend the MSR needs to be put back in sync with the boost settings in the kernel.
On Thu, 17 Apr 2025 at 21:23, Rafael J. Wysocki <rafael@kernel.org> wrote: > But the changelog isn't because the patch really doesn't address the > issue at hand, which is most likely related to the platform firmware > clearing the "boost disable" bit. I think the patch and changelog were still correct as the driver was also enabling the boost at exit(). So it fixes the problem, but not fully. > Frankly, I'd rather get back to the state from before commit > 2b16c631832d ("cpufreq: ACPI: Remove set_boost in > acpi_cpufreq_cpu_init()") and start over with the knowledge that the > platform firmware may scribble on the MSR before the kernel gets > control back. > > On a way back from system suspend the MSR needs to be put back in sync > with the boost settings in the kernel. What about something like this instead ? Nicholas, can you give this a try along with the $Subject patch (both patches should be applied) ? diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c index 924314cdeebc..71557f2ac22a 100644 --- a/drivers/cpufreq/acpi-cpufreq.c +++ b/drivers/cpufreq/acpi-cpufreq.c @@ -909,8 +909,10 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy) if (perf->states[0].core_frequency * 1000 != freq_table[0].frequency) pr_warn(FW_WARN "P-state 0 is not max freq\n"); - if (acpi_cpufreq_driver.set_boost) + if (acpi_cpufreq_driver.set_boost) { policy->boost_supported = true; + policy->boost_enabled = boost_state(cpu); + } return result;
On 2025-04-17 23:58, Viresh Kumar wrote: > What about something like this instead ? Nicholas, can you give this a try > along with the $Subject patch (both patches should be applied) ? > > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c > index 924314cdeebc..71557f2ac22a 100644 > --- a/drivers/cpufreq/acpi-cpufreq.c > +++ b/drivers/cpufreq/acpi-cpufreq.c > @@ -909,8 +909,10 @@ static int acpi_cpufreq_cpu_init(struct > cpufreq_policy *policy) > if (perf->states[0].core_frequency * 1000 != freq_table[0].frequency) > pr_warn(FW_WARN "P-state 0 is not max freq\n"); > > - if (acpi_cpufreq_driver.set_boost) > + if (acpi_cpufreq_driver.set_boost) { > policy->boost_supported = true; > + policy->boost_enabled = boost_state(cpu); > + } > > return result; Thanks, applying this patch along with the $Subject patch works.
On Fri, Apr 18, 2025 at 7:06 PM Nicholas Chin <nic.c3.14@gmail.com> wrote: > > On 2025-04-17 23:58, Viresh Kumar wrote: > > What about something like this instead ? Nicholas, can you give this a try > > along with the $Subject patch (both patches should be applied) ? > > > > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c > > index 924314cdeebc..71557f2ac22a 100644 > > --- a/drivers/cpufreq/acpi-cpufreq.c > > +++ b/drivers/cpufreq/acpi-cpufreq.c > > @@ -909,8 +909,10 @@ static int acpi_cpufreq_cpu_init(struct > > cpufreq_policy *policy) > > if (perf->states[0].core_frequency * 1000 != freq_table[0].frequency) > > pr_warn(FW_WARN "P-state 0 is not max freq\n"); > > > > - if (acpi_cpufreq_driver.set_boost) > > + if (acpi_cpufreq_driver.set_boost) { > > policy->boost_supported = true; > > + policy->boost_enabled = boost_state(cpu); So it updates policy->boost_enabled in accordance with the current setting in the MSR. IMO it would be better to update the MSR in accordance with policy->boost_enabled or users may get confused if their boost settings change after a suspend-resume cycle. Or have I got lost completely? > > + } > > > > return result; > > Thanks, applying this patch along with the $Subject patch works.
On Sat, 19 Apr 2025 at 00:58, Rafael J. Wysocki <rafael@kernel.org> wrote: > So it updates policy->boost_enabled in accordance with the current > setting in the MSR. Yes. > IMO it would be better to update the MSR in accordance with > policy->boost_enabled or users may get confused if their boost > settings change after a suspend-resume cycle. Or have I got lost > completely? I wrote this patch based on the sync that happens at the end of cpufreq_online(): /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */ if (cpufreq_driver->set_boost && policy->boost_supported && policy->boost_enabled != cpufreq_boost_enabled()) { policy->boost_enabled = cpufreq_boost_enabled(); ret = cpufreq_driver->set_boost(policy, policy->boost_enabled); if (ret) { /* If the set_boost fails, the online operation is not affected */ pr_info("%s: CPU%d: Cannot %s BOOST\n", __func__, policy->cpu, str_enable_disable(policy->boost_enabled)); policy->boost_enabled = !policy->boost_enabled; } } So my patch works as the cpufreq core force-syncs the state at init (pretty much what the driver was doing before). Though I now wonder if this code (in cpufreq_online()) is really doing the right thing or not. So if global boost is enabled before suspend with policy boost being disabled, the policy boost will be enabled on resume. -- Viresh
On 2025/4/19 15:54, Viresh Kumar wrote: > On Sat, 19 Apr 2025 at 00:58, Rafael J. Wysocki <rafael@kernel.org> wrote: > >> So it updates policy->boost_enabled in accordance with the current >> setting in the MSR. > > Yes. > >> IMO it would be better to update the MSR in accordance with >> policy->boost_enabled or users may get confused if their boost >> settings change after a suspend-resume cycle. Or have I got lost >> completely? > > I wrote this patch based on the sync that happens at the end of > cpufreq_online(): > > /* Let the per-policy boost flag mirror the cpufreq_driver > boost during init */ > if (cpufreq_driver->set_boost && policy->boost_supported && > policy->boost_enabled != cpufreq_boost_enabled()) { > policy->boost_enabled = cpufreq_boost_enabled(); > ret = cpufreq_driver->set_boost(policy, policy->boost_enabled); > if (ret) { > /* If the set_boost fails, the online > operation is not affected */ > pr_info("%s: CPU%d: Cannot %s BOOST\n", > __func__, policy->cpu, > str_enable_disable(policy->boost_enabled)); > policy->boost_enabled = !policy->boost_enabled; > } > } > > So my patch works as the cpufreq core force-syncs the state at init > (pretty much what the driver was doing before). > > Though I now wonder if this code (in cpufreq_online()) is really doing > the right thing or not. So if global boost is enabled before suspend with > policy boost being disabled, the policy boost will be enabled on resume. Yes, the policy boost will be forcibly set to mirror the global boost. This indicates that the global boost value is the default value of policy boost each time the CPU goes online. Otherwise, we'll meet things like: 1. The global boost is set to disabled after a CPU going offline but the policy boost is still be enabled after the CPU going online again. 2. The global boost is set to enabled after a CPU going offline and the rest of the online CPUs are all boost enabled. However, the offline CPU remains in the boost disabled state after it going online again. Users have to set its boost state separately. IMV, a user set the global boost means "I want all policy boost/unboost", every CPU going online after that should follow this order. So I think the code in cpufreq_online() is doing the right thing. BTW, I think there is optimization can be done in cpufreq_boost_trigger_state(). Currently, Nothing will happend if users set global boost flag to true when this flag is already true. But I think it's better to set all policies to boost in this situation. It might make more sense to think of this as a refresh operation. This is just my idea. I'd like to hear your opinion. > > -- > Viresh
diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c index 924314cdeebc..85b5a88f723f 100644 --- a/drivers/cpufreq/acpi-cpufreq.c +++ b/drivers/cpufreq/acpi-cpufreq.c @@ -89,8 +89,9 @@ static bool boost_state(unsigned int cpu) return false; } -static int boost_set_msr(bool enable) +static void boost_set_msr_each(void *p_en) { + bool enable = (bool) p_en; u32 msr_addr; u64 msr_mask, val; @@ -107,7 +108,7 @@ static int boost_set_msr(bool enable) msr_mask = MSR_K7_HWCR_CPB_DIS; break; default: - return -EINVAL; + return; } rdmsrl(msr_addr, val); @@ -118,14 +119,6 @@ static int boost_set_msr(bool enable) val |= msr_mask; wrmsrl(msr_addr, val); - return 0; -} - -static void boost_set_msr_each(void *p_en) -{ - bool enable = (bool) p_en; - - boost_set_msr(enable); } static int set_boost(struct cpufreq_policy *policy, int val) @@ -532,15 +525,6 @@ static void free_acpi_perf_data(void) free_percpu(acpi_perf_data); } -static int cpufreq_boost_down_prep(unsigned int cpu) -{ - /* - * Clear the boost-disable bit on the CPU_DOWN path so that - * this cpu cannot block the remaining ones from boosting. - */ - return boost_set_msr(1); -} - /* * acpi_cpufreq_early_init - initialize ACPI P-States library * @@ -931,7 +915,6 @@ static void acpi_cpufreq_cpu_exit(struct cpufreq_policy *policy) pr_debug("%s\n", __func__); - cpufreq_boost_down_prep(policy->cpu); policy->fast_switch_possible = false; policy->driver_data = NULL; acpi_processor_unregister_performance(data->acpi_perf_cpu);
The boost-related code in cpufreq has undergone several changes over the years, but this particular piece remained unchanged and is now outdated. The cpufreq core currently manages boost settings during initialization, and only when necessary. As such, there's no longer a need to enable boost explicitly when entering system suspend. Previously, this wasn’t causing issues because boost settings were force-updated during policy initialization. However, commit 2b16c631832d ("cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()") changed that behavior—correctly—by avoiding unnecessary updates. As a result of this change, if boost was disabled prior to suspend, it remains disabled on resume—as expected. But due to the current code forcibly enabling boost at suspend time, the system ends up with boost frequencies enabled after resume, even if the global boost flag was disabled. This contradicts the intended behavior. Fix this by not enabling boost on policy exit. Fixes: 2b16c631832d ("cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()") Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220013 Reported-by: Nicholas Chin <nic.c3.14@gmail.com> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/cpufreq/acpi-cpufreq.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-)