[Bug 220013] New: [REGRESSION, BISECTED] acpi-cpufreq: Boost disablement not being restored after resume from suspend

[Bug 220013] New: [REGRESSION, BISECTED] acpi-cpufreq: Boost disablement not being restored after resume from suspend

[bugzilla-daemon]

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

            Bug ID: 220013
           Summary: [REGRESSION, BISECTED] acpi-cpufreq: Boost disablement
                    not being restored after resume from suspend
           Product: Power Management
           Version: 2.5
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P3
         Component: cpufreq
          Assignee: linux-pm@vger.kernel.org
          Reporter: nic.c3.14@gmail.com
        Regression: No

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.

Hardware: AMD Ryzen 7 2700U

Steps to Reproduce:
1. Disable boost by writing 0 to /sys/devices/system/cpu/cpufreq/boost
2. Place the system into S3 suspend
3. Wake the system. /sys/devices/system/cpu/cpufreq/boost` should still be 0.
4. Place a load on the processor to cause the system to enter a boost state,
such as running `stress -c 4`.
5. The processor boosts

I have bisected this to commit 2b16c631832df6cf8782fb1fdc7df8a4f03f4f16
("cpufreq: ACPI: Remove set_boost in acpi_cpufreq_cpu_init()").

This issue persists with mainline master (commit 004a365eb8b9 ("Merge tag
'erofs-for-6.15-rc2-fixes' of
git://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs") at the time of
testing). Reverting commit 2b16c631832d restores the previous behavior (not a
clean revert; needed to merge with be6b8681a0e4c1477a2c1cb155f7b9188aa88acb
("cpufreq: acpi: Set policy->boost_supported"))

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.

[Bug 220013] [REGRESSION, BISECTED] acpi-cpufreq: Boost disablement not being restored after resume from suspend

[bugzilla-daemon]

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

Nicholas Chin (nic.c3.14@gmail.com) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Bisected commit-id|                            |2b16c631832df6cf8782fb1fdc7
                   |                            |df8a4f03f4f16
         Regression|No                          |Yes

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.

[Bug 220013] [REGRESSION, BISECTED] acpi-cpufreq: Boost disablement not being restored after resume from suspend

[bugzilla-daemon]

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

Artem S. Tashkinov (aros@gmx.com) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |viresh.kumar@linaro.org

--- Comment #1 from Artem S. Tashkinov (aros@gmx.com) ---
I cannot CC Lifeng Zheng, CC'ing Viresh Kumar

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.

[Bug 220013] [REGRESSION, BISECTED] acpi-cpufreq: Boost disablement not being restored after resume from suspend

[bugzilla-daemon]

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

--- Comment #2 from Viresh Kumar (viresh.kumar@linaro.org) ---
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);
 }

Can you try this change please ?

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.

[Bug 220013] [REGRESSION, BISECTED] acpi-cpufreq: Boost disablement not being restored after resume from suspend

[bugzilla-daemon]

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

--- Comment #3 from Nicholas Chin (nic.c3.14@gmail.com) ---
(In reply to Viresh Kumar from comment #2)
> 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);
>  }
> 
> Can you try this change please ?

The suggested change fails to compile with the following error:

drivers/cpufreq/acpi-cpufreq.c: In function ‘cpufreq_boost_down_prep’:          
drivers/cpufreq/acpi-cpufreq.c:541:9: error: ‘policy’ undeclared (first use in
this function)                                                                  
  541 |         policy->boost_enabled = true;                                   
      |         ^~~~~~                                                          
drivers/cpufreq/acpi-cpufreq.c:541:9: note: each undeclared identifier is
reported only once for each function it appears in
make[4]: *** [scripts/Makefile.build:203: drivers/cpufreq/acpi-cpufreq.o] Error
1
make[3]: *** [scripts/Makefile.build:461: drivers/cpufreq] Error 2
make[3]: *** Waiting for unfinished jobs....

since policy is not a global variable and is normally passed as a pointer to
the various functions in acpi-cpufreq.c.

It appears that cpufreq_boost_down_prep() is only called in
acpi_cpufreq_cpu_exit(), so I tried the following which should be functionally
the same as the previously suggested change:

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 924314cdeebc..36e2ff3188c9 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -931,6 +931,7 @@ static void acpi_cpufreq_cpu_exit(struct cpufreq_policy
*policy)

        pr_debug("%s\n", __func__);

+       policy->boost_enabled = true;
        cpufreq_boost_down_prep(policy->cpu);
        policy->fast_switch_possible = false;
        policy->driver_data = NULL;

With this change, boost disablement is restored after resume from suspend.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.

[Bug 220013] [REGRESSION, BISECTED] acpi-cpufreq: Boost disablement not being restored after resume from suspend

[bugzilla-daemon]

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

--- Comment #4 from Viresh Kumar (viresh.kumar@linaro.org) ---
Sorry about the build failure earlier.

Thanks for testing this out. I have sent a formal fix now and cc'd you.

https://lore.kernel.org/all/2c788c2ca0cab09a8ef4e384f272af928a880b0e.1744781329.git.viresh.kumar@linaro.org/

Please test that once and provide your Tested-by to get it merged.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.

[Bug 220013] [REGRESSION, BISECTED] acpi-cpufreq: Boost disablement not being restored after resume from suspend

[bugzilla-daemon]

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

--- Comment #5 from Nicholas Chin (nic.c3.14@gmail.com) ---
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.

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.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are the assignee for the bug.