[PATCH V2] cpufreq: Fix per-policy boost behavior on SoCs using cpufreq_boost_set_sw

[PATCH V2] cpufreq: Fix per-policy boost behavior on SoCs using cpufreq_boost_set_sw

[Sibi Sankar]

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. Fix this by incorporating per-policy boost flag in the policy->max
calculus used in cpufreq_frequency_table_cpuinfo and setting the default
per-policy boost to mirror the cpufreq_driver boost flag.

Fixes: 218a06a79d9a ("cpufreq: Support per-policy performance boost")
Reported-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---

v2:
* Enable per-policy boost flag in the core instead. [Viresh]
* Add more details regarding the bug. [Viresh]
* Drop cover-letter and patch 2.

Logs reported-by Dietmar Eggemann:
https://lore.kernel.org/lkml/265e5f2c-9b45-420f-89b1-44369aeb8418@arm.com/

 drivers/cpufreq/cpufreq.c    | 19 +++++++++++++------
 drivers/cpufreq/freq_table.c |  2 +-
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index f6f8d7f450e7..c04b2ba2993a 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -653,14 +653,16 @@ static ssize_t store_local_boost(struct cpufreq_policy *policy,
 	if (policy->boost_enabled == enable)
 		return count;
 
+	policy->boost_enabled = enable;
+
 	cpus_read_lock();
 	ret = cpufreq_driver->set_boost(policy, enable);
 	cpus_read_unlock();
 
-	if (ret)
+	if (ret) {
+		policy->boost_enabled = !policy->boost_enabled;
 		return ret;
-
-	policy->boost_enabled = enable;
+	}
 
 	return count;
 }
@@ -1428,6 +1430,10 @@ static int cpufreq_online(unsigned int cpu)
 			goto out_free_policy;
 		}
 
+		/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
+		if (cpufreq_driver->boost_enabled)
+			policy->boost_enabled = policy_has_boost_freq(policy) ? true : false;
+
 		/*
 		 * The initialization has succeeded and the policy is online.
 		 * If there is a problem with its frequency table, take it
@@ -2769,11 +2775,12 @@ int cpufreq_boost_trigger_state(int state)
 
 	cpus_read_lock();
 	for_each_active_policy(policy) {
+		policy->boost_enabled = state;
 		ret = cpufreq_driver->set_boost(policy, state);
-		if (ret)
+		if (ret) {
+			policy->boost_enabled = !policy->boost_enabled;
 			goto err_reset_state;
-
-		policy->boost_enabled = state;
+		}
 	}
 	cpus_read_unlock();
 
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index c4d4643b6ca6..c17dc51a5a02 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -40,7 +40,7 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
 	cpufreq_for_each_valid_entry(pos, table) {
 		freq = pos->frequency;
 
-		if (!cpufreq_boost_enabled()
+		if ((!cpufreq_boost_enabled() || !policy->boost_enabled)
 		    && (pos->flags & CPUFREQ_BOOST_FREQ))
 			continue;
 
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2] cpufreq: Fix per-policy boost behavior on SoCs using cpufreq_boost_set_sw

[Viresh Kumar]

On 08-03-24, 16:06, Sibi Sankar wrote:
> +		/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> +		if (cpufreq_driver->boost_enabled)
> +			policy->boost_enabled = policy_has_boost_freq(policy) ? true : false;

Can be written as:

policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy);

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

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2] cpufreq: Fix per-policy boost behavior on SoCs using cpufreq_boost_set_sw

[Dhruva Gole]

Hi,

On Mar 11, 2024 at 10:20:44 +0530, Viresh Kumar wrote:
> On 08-03-24, 16:06, Sibi Sankar wrote:
> > +		/* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> > +		if (cpufreq_driver->boost_enabled)
> > +			policy->boost_enabled = policy_has_boost_freq(policy) ? true : false;
> 
> Can be written as:
> 
> policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy);

I would like to recommend the same.

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

Rest looks fine,
Reviewed-by: Dhruva Gole <d-gole@ti.com>

-- 
Best regards,
Dhruva Gole <d-gole@ti.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel