| Message ID | 20240227165309.620422-1-quic_sibis@quicinc.com |
|---|---|
| Headers | show |
| Series | Fix per-policy boost behavior | expand |
Hi Sibi, Thanks for pointing this issue out. However, I can't clearly see how the existing code fails. cpufreq_frequency_table_cpuinfo() checks cpufreq_boost_enabled(), and that should be already set in cpufreq_boost_trigger_state() before calling cpufreq_boost_set_sw(), so presumably cpufreq_boost_set_sw() is supposed to work as expected. Can you explain this a bit further? Cheers, Jie On 28/02/2024 00:53, Sibi Sankar wrote: > Fix per-policy boost behavior by incorporating per-policy boost flag > in the policy->max calculation and setting the correct per-policy > boost_enabled value on devices that use cpufreq_enable_boost_support(). > > Logs reported-by Dietmar Eggemann [1]: > > [1] https://lore.kernel.org/lkml/265e5f2c-9b45-420f-89b1-44369aeb8418@arm.com/ > > Sibi Sankar (2): > cpufreq: Fix per-policy boost behavior on SoCs using > cpufreq_boost_set_sw > cpufreq: apple-soc: Align per-policy and global boost flags > > drivers/cpufreq/apple-soc-cpufreq.c | 1 + > drivers/cpufreq/cpufreq.c | 15 +++++++++------ > drivers/cpufreq/freq_table.c | 2 +- > 3 files changed, 11 insertions(+), 7 deletions(-) >
On 27-02-24, 22:23, Sibi Sankar wrote: > Fix per-policy boost behavior by incorporating per-policy boost flag > in the policy->max calculation and setting the correct per-policy > boost_enabled value on devices that use cpufreq_enable_boost_support(). I don't see the problem explained anywhere and the patches look incorrect too. The drivers aren't supposed to update the policy->boose_enabled value.
On 2/28/24 10:37, Viresh Kumar wrote: > On 27-02-24, 22:23, Sibi Sankar wrote: >> Fix per-policy boost behavior by incorporating per-policy boost flag >> in the policy->max calculation and setting the correct per-policy >> boost_enabled value on devices that use cpufreq_enable_boost_support(). > > I don't see the problem explained anywhere and the patches look > incorrect too. The drivers aren't supposed to update the > policy->boose_enabled value. Hey Viresh, Thanks for taking time to review the series. In the existing code, per-policy flags doesn't have any impact i.e. if cpufreq_driver boost is enabled and one or more of the per-policy boost is disabled, the cpufreq driver will behave as if boost is enabled. I had to update the policy->boost_enabled value because we seem to allow enabling cpufreq_driver.boost_enabled from the driver, but I can drop that because it was just for book keeping. I didn't want to include redundant info from another mail thread that I referenced in the cover letter, but will add more info in the re-spin. -Sibi >
On 28-02-24, 10:44, Sibi Sankar wrote: > In the existing code, per-policy flags doesn't have any impact i.e. > if cpufreq_driver boost is enabled and one or more of the per-policy > boost is disabled, the cpufreq driver will behave as if boost is > enabled. I see. Good catch. The first patch is fine, just explain the problem properly and mention that no one is checking the policy->boost_enabled field. It is never read. > I had to update the policy->boost_enabled value because we seem > to allow enabling cpufreq_driver.boost_enabled from the driver, but I > can drop that because it was just for book keeping. So with cpufreq_driver->boost_enabled at init time, policy's boost_enabled must be set too. Do that in the core during initialization of the policy instead. > I didn't want > to include redundant info from another mail thread that I referenced in > the cover letter, but will add more info in the re-spin. You don't have to, but you need to explain the exact problem in a bit more detail since it wasn't obvious here.
Fix per-policy boost behavior by incorporating per-policy boost flag in the policy->max calculation and setting the correct per-policy boost_enabled value on devices that use cpufreq_enable_boost_support(). Logs reported-by Dietmar Eggemann [1]: [1] https://lore.kernel.org/lkml/265e5f2c-9b45-420f-89b1-44369aeb8418@arm.com/ Sibi Sankar (2): cpufreq: Fix per-policy boost behavior on SoCs using cpufreq_boost_set_sw cpufreq: apple-soc: Align per-policy and global boost flags drivers/cpufreq/apple-soc-cpufreq.c | 1 + drivers/cpufreq/cpufreq.c | 15 +++++++++------ drivers/cpufreq/freq_table.c | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-)