Message ID | 20240626204723.6237-1-mario.limonciello@amd.com (mailing list archive) |
---|---|
State | In Next |
Delegated to: | Rafael Wysocki |
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. >> --
On Thu, Jun 27, 2024 at 9:12 PM Mario Limonciello <mario.limonciello@amd.com> wrote: > > 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. Applied as 6.10-rc material along with the [2/2]. I've added a Fixes: tag to the second patch and "Cc: stable" tags to both patches. Thanks! > > > >> 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.