Message ID | 20240626204723.6237-1-mario.limonciello@amd.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [v2,1/2] cpufreq: Allow drivers to advertise boost enabled | expand |
On Wed, Jun 26, 2024 at 10:47 PM Mario Limonciello <mario.limonciello@amd.com> wrote: > > The behavior introduced in commit f37a4d6b4a2c ("cpufreq: Fix per-policy > boost behavior on SoCs using cpufreq_boost_set_sw()") sets up the boost > policy incorrectly when boost has been enabled by the platform firmware > initially even if a driver sets the policy up. > > This is because policy_has_boost_freq() assumes that there is a frequency > table set up by the driver and that the boost frequencies are advertised > in that table. This assumption doesn't work for acpi-cpufreq or > amd-pstate. Only use this check to enable boost if it's not already > enabled instead of also disabling it if alreayd enabled. > > Reviewed-by: Sibi Sankar <quic_sibis@quicinc.com> > Reviewed-by: Dhruva Gole <d-gole@ti.com> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> > Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> > Fixes: f37a4d6b4a2c ("cpufreq: Fix per-policy boost behavior on SoCs using cpufreq_boost_set_sw()") CC: stable I suppose? > Suggested-by: Viresh Kumar <viresh.kumar@linaro.org> > Suggested-by: Gautham R. Shenoy <gautham.shenoy@amd.com> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > Cc: Sibi Sankar <quic_sibis@quicinc.com> > Cc: Dietmar Eggemann <dietmar.eggemann@arm.com> > Cc: Viresh Kumar <viresh.kumar@linaro.org> > Cc: Dhruva Gole <d-gole@ti.com> > Cc: Yipeng Zou <zouyipeng@huawei.com> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > v1->v2 > * Pick up tags > --- > drivers/cpufreq/cpufreq.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 1fdabb660231..270ea04fb616 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -1430,7 +1430,8 @@ static int cpufreq_online(unsigned int cpu) > } > > /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */ > - policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy); > + if (cpufreq_boost_enabled() && policy_has_boost_freq(policy)) > + policy->boost_enabled = true; > > /* > * The initialization has succeeded and the policy is online. > --
On 6/27/2024 04:59, Rafael J. Wysocki wrote: > On Wed, Jun 26, 2024 at 10:47 PM Mario Limonciello > <mario.limonciello@amd.com> wrote: >> >> The behavior introduced in commit f37a4d6b4a2c ("cpufreq: Fix per-policy >> boost behavior on SoCs using cpufreq_boost_set_sw()") sets up the boost >> policy incorrectly when boost has been enabled by the platform firmware >> initially even if a driver sets the policy up. >> >> This is because policy_has_boost_freq() assumes that there is a frequency >> table set up by the driver and that the boost frequencies are advertised >> in that table. This assumption doesn't work for acpi-cpufreq or >> amd-pstate. Only use this check to enable boost if it's not already >> enabled instead of also disabling it if alreayd enabled. >> >> Reviewed-by: Sibi Sankar <quic_sibis@quicinc.com> >> Reviewed-by: Dhruva Gole <d-gole@ti.com> >> Acked-by: Viresh Kumar <viresh.kumar@linaro.org> >> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> >> Fixes: f37a4d6b4a2c ("cpufreq: Fix per-policy boost behavior on SoCs using cpufreq_boost_set_sw()") > > CC: stable I suppose? Yes, I didn't realize f37a4d6b4a2c came in 6.9, I had assumed it was 6.10. But since it's 6.9, yes if you can please add stable tag when committing. > >> Suggested-by: Viresh Kumar <viresh.kumar@linaro.org> >> Suggested-by: Gautham R. Shenoy <gautham.shenoy@amd.com> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> Cc: Sibi Sankar <quic_sibis@quicinc.com> >> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com> >> Cc: Viresh Kumar <viresh.kumar@linaro.org> >> Cc: Dhruva Gole <d-gole@ti.com> >> Cc: Yipeng Zou <zouyipeng@huawei.com> >> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> v1->v2 >> * Pick up tags >> --- >> drivers/cpufreq/cpufreq.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c >> index 1fdabb660231..270ea04fb616 100644 >> --- a/drivers/cpufreq/cpufreq.c >> +++ b/drivers/cpufreq/cpufreq.c >> @@ -1430,7 +1430,8 @@ static int cpufreq_online(unsigned int cpu) >> } >> >> /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */ >> - policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy); >> + if (cpufreq_boost_enabled() && policy_has_boost_freq(policy)) >> + policy->boost_enabled = true; >> >> /* >> * The initialization has succeeded and the policy is online. >> --
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 1fdabb660231..270ea04fb616 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1430,7 +1430,8 @@ static int cpufreq_online(unsigned int cpu) } /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */ - policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy); + if (cpufreq_boost_enabled() && policy_has_boost_freq(policy)) + policy->boost_enabled = true; /* * The initialization has succeeded and the policy is online.