diff mbox series

[v2,4/4] cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()

Message ID 20250117101457.1530653-5-zhenglifeng1@huawei.com
State Accepted
Commit 2b16c631832df6cf8782fb1fdc7df8a4f03f4f16
Headers show
Series cpufreq: Fix some boost errors related to CPU online and offline. | expand

Commit Message

zhenglifeng (A) Jan. 17, 2025, 10:14 a.m. UTC
At the end of cpufreq_online() in cpufreq.c, set_boost is executed and the
per-policy boost flag is set to mirror the cpufreq_driver boost. So it is
not necessary to run set_boost in acpi_cpufreq_cpu_init().

Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
---
 drivers/cpufreq/acpi-cpufreq.c | 5 -----
 1 file changed, 5 deletions(-)

Comments

Viresh Kumar Jan. 21, 2025, 6:14 a.m. UTC | #1
On 17-01-25, 18:14, Lifeng Zheng wrote:
> At the end of cpufreq_online() in cpufreq.c, set_boost is executed and the
> per-policy boost flag is set to mirror the cpufreq_driver boost. So it is
> not necessary to run set_boost in acpi_cpufreq_cpu_init().
> 
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>  drivers/cpufreq/acpi-cpufreq.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index c9ebacf5c88e..f4b5e455f173 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -891,11 +891,6 @@ 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) {
> -		set_boost(policy, acpi_cpufreq_driver.boost_enabled);
> -		policy->boost_enabled = acpi_cpufreq_driver.boost_enabled;
> -	}
> -
>  	return result;
>  
>  err_unreg:

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

There are more cleanups in drivers that can be done though. I will try
that once this series is merged.
zhenglifeng (A) Jan. 24, 2025, 9:11 a.m. UTC | #2
On 2025/1/24 16:59, Viresh Kumar wrote:

> On 21-01-25, 11:44, Viresh Kumar wrote:
>> There are more cleanups in drivers that can be done though. I will try
>> that once this series is merged.
> 
> https://lore.kernel.org/751338633b070ee570c3c7da053bd6b9497ee50e.1737707712.git.viresh.kumar@linaro.org
> 

Nice!
Viresh Kumar April 15, 2025, 10:29 a.m. UTC | #3
On 17-01-25, 18:14, Lifeng Zheng wrote:
> At the end of cpufreq_online() in cpufreq.c, set_boost is executed and the
> per-policy boost flag is set to mirror the cpufreq_driver boost. So it is
> not necessary to run set_boost in acpi_cpufreq_cpu_init().
> 
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>  drivers/cpufreq/acpi-cpufreq.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index c9ebacf5c88e..f4b5e455f173 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -891,11 +891,6 @@ 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) {
> -		set_boost(policy, acpi_cpufreq_driver.boost_enabled);
> -		policy->boost_enabled = acpi_cpufreq_driver.boost_enabled;
> -	}
> -
>  	return result;
>  
>  err_unreg:

https://bugzilla.kernel.org/show_bug.cgi?id=220013

"
On kernel 6.13.8, disabling boost by setting
/sys/devices/system/cpu/cpufreq/boost to 0 would persist after
resuming from suspend. After updating to 6.14.2, the system is able to
enter boost states after resuming from suspend despite the boost flag
still being set to 0. Toggling it to 1 and then back to 0 in this
state re-disables boost. My system uses the acpi-cpufreq driver.
"

This bug report is filed and git bisect points to this commit.

Rafael, I think the commit in question did the right thing and there
is something else in the driver that is causing the issue here.

I think the problem here is cpufreq_boost_down_prep(), which gets
called during suspend path and enables the boost.

But since the boost was never enabled from flag's point of view, it
never gets disabled again on resume.

I have suggested following for now to check if this is the case or
not:

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 924314cdeebc..d8599ae7922f 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -538,6 +538,7 @@ 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.
         */
+       policy->boost_enabled = true;
        return boost_set_msr(1);
 }
diff mbox series

Patch

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index c9ebacf5c88e..f4b5e455f173 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -891,11 +891,6 @@  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) {
-		set_boost(policy, acpi_cpufreq_driver.boost_enabled);
-		policy->boost_enabled = acpi_cpufreq_driver.boost_enabled;
-	}
-
 	return result;
 
 err_unreg: