[PATCH] firmware: arm_scmi: perf: ignore an implausible sustained frequency

[PATCH] firmware: arm_scmi: perf: ignore an implausible sustained frequency

[Charlie Garner]

On a Dell Inspiron 14 Plus 7441 (Snapdragon X Plus X1P64100, soc_id 615)
the SCMI firmware reports a sustained frequency below the lowest available
OPP for performance domains NCC1 and NCC2. It reports zero for both
sustained_freq_khz and sustained_perf_level there, while NCC0 reports
3417600 kHz at level 12. All three domains use level indexing mode, so
mult_factor is fixed at 1000 and the OPP frequencies come from
indicative_freq; this is not a units or mult_factor problem.

scmi_dvfs_device_opps_add() then flags every OPP in NCC1 and NCC2 as turbo:

	data.turbo = freq > dom->sustained_freq_khz * 1000UL;

cpufreq_frequency_table_cpuinfo() skips boost-flagged entries and fails
when none are left:

	if ((!cpufreq_boost_enabled() || !policy->boost_enabled)
	    && (pos->flags & CPUFREQ_BOOST_FREQ))
		continue;
	...
	if (min_freq == ~0)
		return -EINVAL;

cpufreq_policy_online() drops the policy on that error without logging
anything. Only one of the three performance domains ends up with a policy.
The part has 10 cores (the x1e80100 DT describes 12, CPUs 7 and 11 fail to
boot): 4 of them can scale, the other 6 get no policy, no governor, and no
cpufreq cooling device.

This cannot be worked around by enabling boost. policy->boost_enabled is
still 0 during that validation, and both places that set it -
boost_supported in cpufreq_table_validate_and_sort(), boost_enabled in
cpufreq_online() - run after the call that already returned -EINVAL. A
domain whose OPPs are all flagged turbo can therefore never get a policy.

Confirmed by probing dev_pm_opp_add_dynamic() on the affected machine: all
13 OPPs are added with turbo=0 for domain NCC0 and turbo=1 for every CPU in
NCC1 and NCC2, across an identical 710400-3417600 kHz table. The raw domain
attributes above were read the same way, with a kprobe on
scmi_dvfs_device_opps_add() fetching the perf_dom_info fields.

A sustained frequency below the lowest OPP carries no information - it
cannot separate sustained levels from boost levels. Treat it as "this
domain has no turbo levels" instead of letting it disable the domain
entirely, and say so once with a FW_BUG warning, since the failure is
otherwise silent. A sustained frequency equal to the lowest OPP is left
alone, it already leaves that OPP non-turbo.

Fixes: a897575e79d7 ("firmware: arm_scmi: Add support for marking certain frequencies as turbo")
Cc: stable@vger.kernel.org
Signed-off-by: Charlie Garner <charlie@akao.au>
---

Notes:
    The SCMI quirks framework (quirks.c) would also work here, and this
    platform already has two quirks enabled. I went with a generic check
    because a sustained frequency below every OPP is meaningless on any
    platform, and the check is a no-op for firmware that reports a sane value.
    Happy to turn it into a quirk if you'd prefer that.

 drivers/firmware/arm_scmi/perf.c | 34 ++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index 4583d02bee1c..94f644996f9d 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -861,11 +861,20 @@ static void scmi_perf_domain_init_fc(const struct scmi_protocol_handle *ph,
 	dom->fc_info = fc;
 }
 
+static unsigned long scmi_perf_opp_freq(const struct perf_dom_info *dom,
+					int idx)
+{
+	if (!dom->level_indexing_mode)
+		return dom->opp[idx].perf * dom->mult_factor;
+
+	return dom->opp[idx].indicative_freq * dom->mult_factor;
+}
+
 static int scmi_dvfs_device_opps_add(const struct scmi_protocol_handle *ph,
 				     struct device *dev, u32 domain)
 {
 	int idx, ret;
-	unsigned long freq;
+	unsigned long freq, sustained_hz, lowest_hz = ULONG_MAX;
 	struct dev_pm_opp_data data = {};
 	struct perf_dom_info *dom;
 
@@ -873,14 +882,27 @@ static int scmi_dvfs_device_opps_add(const struct scmi_protocol_handle *ph,
 	if (IS_ERR(dom))
 		return PTR_ERR(dom);
 
+	for (idx = 0; idx < dom->opp_count; idx++)
+		lowest_hz = min(lowest_hz, scmi_perf_opp_freq(dom, idx));
+
+	/*
+	 * A sustained frequency below every OPP would mark all of them as
+	 * turbo. Such a value cannot separate sustained levels from boost
+	 * levels, so ignore it and treat the domain as having no turbo OPPs.
+	 */
+	sustained_hz = dom->sustained_freq_khz * 1000UL;
+	if (dom->opp_count && sustained_hz < lowest_hz) {
+		dev_warn_once(dev, FW_BUG
+			      "[%d][%s]: sustained freq %lu Hz below lowest OPP %lu Hz, ignored\n",
+			      domain, dom->info.name, sustained_hz, lowest_hz);
+		sustained_hz = ULONG_MAX;
+	}
+
 	for (idx = 0; idx < dom->opp_count; idx++) {
-		if (!dom->level_indexing_mode)
-			freq = dom->opp[idx].perf * dom->mult_factor;
-		else
-			freq = dom->opp[idx].indicative_freq * dom->mult_factor;
+		freq = scmi_perf_opp_freq(dom, idx);
 
 		/* All OPPs above the sustained frequency are treated as turbo */
-		data.turbo = freq > dom->sustained_freq_khz * 1000UL;
+		data.turbo = freq > sustained_hz;
 
 		data.level = dom->opp[idx].perf;
 		data.freq = freq;

base-commit: 60a89ec8d8f56dcd99611cb054fbf7d0e864cf4e
-- 
2.55.0

Re: [PATCH] firmware: arm_scmi: perf: ignore an implausible sustained frequency

[Sudeep Holla]

On Thu, Sep 24, 2026 at 12:52:45AM +0930, Charlie Garner wrote:
> On a Dell Inspiron 14 Plus 7441 (Snapdragon X Plus X1P64100, soc_id 615)
> the SCMI firmware reports a sustained frequency below the lowest available
> OPP for performance domains NCC1 and NCC2. It reports zero for both
> sustained_freq_khz and sustained_perf_level there, while NCC0 reports
> 3417600 kHz at level 12. All three domains use level indexing mode, so
> mult_factor is fixed at 1000 and the OPP frequencies come from
> indicative_freq; this is not a units or mult_factor problem.
> 
> scmi_dvfs_device_opps_add() then flags every OPP in NCC1 and NCC2 as turbo:
> 
> 	data.turbo = freq > dom->sustained_freq_khz * 1000UL;
> 
> cpufreq_frequency_table_cpuinfo() skips boost-flagged entries and fails
> when none are left:
> 
> 	if ((!cpufreq_boost_enabled() || !policy->boost_enabled)
> 	    && (pos->flags & CPUFREQ_BOOST_FREQ))
> 		continue;
> 	...
> 	if (min_freq == ~0)
> 		return -EINVAL;
> 
> cpufreq_policy_online() drops the policy on that error without logging
> anything. Only one of the three performance domains ends up with a policy.
> The part has 10 cores (the x1e80100 DT describes 12, CPUs 7 and 11 fail to
> boot): 4 of them can scale, the other 6 get no policy, no governor, and no
> cpufreq cooling device.
> 
> This cannot be worked around by enabling boost. policy->boost_enabled is
> still 0 during that validation, and both places that set it -
> boost_supported in cpufreq_table_validate_and_sort(), boost_enabled in
> cpufreq_online() - run after the call that already returned -EINVAL. A
> domain whose OPPs are all flagged turbo can therefore never get a policy.
> 
> Confirmed by probing dev_pm_opp_add_dynamic() on the affected machine: all
> 13 OPPs are added with turbo=0 for domain NCC0 and turbo=1 for every CPU in
> NCC1 and NCC2, across an identical 710400-3417600 kHz table. The raw domain
> attributes above were read the same way, with a kprobe on
> scmi_dvfs_device_opps_add() fetching the perf_dom_info fields.
> 
> A sustained frequency below the lowest OPP carries no information - it
> cannot separate sustained levels from boost levels. Treat it as "this
> domain has no turbo levels" instead of letting it disable the domain
> entirely, and say so once with a FW_BUG warning, since the failure is
> otherwise silent. A sustained frequency equal to the lowest OPP is left
> alone, it already leaves that OPP non-turbo.
> 
> Fixes: a897575e79d7 ("firmware: arm_scmi: Add support for marking certain frequencies as turbo")
> Cc: stable@vger.kernel.org
> Signed-off-by: Charlie Garner <charlie@akao.au>
> ---
> 
> Notes:
>     The SCMI quirks framework (quirks.c) would also work here, and this
>     platform already has two quirks enabled. I went with a generic check
>     because a sustained frequency below every OPP is meaningless on any
>     platform, and the check is a no-op for firmware that reports a sane value.
>     Happy to turn it into a quirk if you'd prefer that.
> 

I am unable to decide which is better approach. I am more inclined to quirks
so that other systems are not penalised by unnecessary checks but at the same
time it seems like a sensible generic check.

Cristian,

Any strong opinion/inclination ?

>  drivers/firmware/arm_scmi/perf.c | 34 ++++++++++++++++++++++++++------
>  1 file changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
> index 4583d02bee1c..94f644996f9d 100644
> --- a/drivers/firmware/arm_scmi/perf.c
> +++ b/drivers/firmware/arm_scmi/perf.c
> @@ -861,11 +861,20 @@ static void scmi_perf_domain_init_fc(const struct scmi_protocol_handle *ph,
>  	dom->fc_info = fc;
>  }
>  
> +static unsigned long scmi_perf_opp_freq(const struct perf_dom_info *dom,
> +					int idx)
> +{
> +	if (!dom->level_indexing_mode)
> +		return dom->opp[idx].perf * dom->mult_factor;
> +
> +	return dom->opp[idx].indicative_freq * dom->mult_factor;
> +}
> +
>  static int scmi_dvfs_device_opps_add(const struct scmi_protocol_handle *ph,
>  				     struct device *dev, u32 domain)
>  {
>  	int idx, ret;
> -	unsigned long freq;
> +	unsigned long freq, sustained_hz, lowest_hz = ULONG_MAX;
>  	struct dev_pm_opp_data data = {};
>  	struct perf_dom_info *dom;
>  
> @@ -873,14 +882,27 @@ static int scmi_dvfs_device_opps_add(const struct scmi_protocol_handle *ph,
>  	if (IS_ERR(dom))
>  		return PTR_ERR(dom);
>  
> +	for (idx = 0; idx < dom->opp_count; idx++)
> +		lowest_hz = min(lowest_hz, scmi_perf_opp_freq(dom, idx));
> +

Just wondering how badly the perf values are also screwed up on this system ?
If we are adding it as generic, I am thinking of just using the sorted
opp list and lowest_hz = scmi_perf_opp_freq(dom, 0), no ? It avoids
unnecessary loop above.

-- 
Regards,
Sudeep

Re: [PATCH] firmware: arm_scmi: perf: ignore an implausible sustained frequency

[Cristian Marussi]

On Fri, Sep 25, 2026 at 11:00:46AM +0100, Sudeep Holla wrote:
> On Thu, Sep 24, 2026 at 12:52:45AM +0930, Charlie Garner wrote:
> > On a Dell Inspiron 14 Plus 7441 (Snapdragon X Plus X1P64100, soc_id 615)
> > the SCMI firmware reports a sustained frequency below the lowest available
> > OPP for performance domains NCC1 and NCC2. It reports zero for both
> > sustained_freq_khz and sustained_perf_level there, while NCC0 reports
> > 3417600 kHz at level 12. All three domains use level indexing mode, so
> > mult_factor is fixed at 1000 and the OPP frequencies come from
> > indicative_freq; this is not a units or mult_factor problem.
> > 

Hi

seems like I did not receive the original mail anywhhere...

> > scmi_dvfs_device_opps_add() then flags every OPP in NCC1 and NCC2 as turbo:
> > 
> > 	data.turbo = freq > dom->sustained_freq_khz * 1000UL;
> > 
> > cpufreq_frequency_table_cpuinfo() skips boost-flagged entries and fails
> > when none are left:
> > 
> > 	if ((!cpufreq_boost_enabled() || !policy->boost_enabled)
> > 	    && (pos->flags & CPUFREQ_BOOST_FREQ))
> > 		continue;
> > 	...
> > 	if (min_freq == ~0)
> > 		return -EINVAL;
> > 
> > cpufreq_policy_online() drops the policy on that error without logging
> > anything. Only one of the three performance domains ends up with a policy.
> > The part has 10 cores (the x1e80100 DT describes 12, CPUs 7 and 11 fail to
> > boot): 4 of them can scale, the other 6 get no policy, no governor, and no
> > cpufreq cooling device.
> > 
> > This cannot be worked around by enabling boost. policy->boost_enabled is
> > still 0 during that validation, and both places that set it -
> > boost_supported in cpufreq_table_validate_and_sort(), boost_enabled in
> > cpufreq_online() - run after the call that already returned -EINVAL. A
> > domain whose OPPs are all flagged turbo can therefore never get a policy.
> > 
> > Confirmed by probing dev_pm_opp_add_dynamic() on the affected machine: all
> > 13 OPPs are added with turbo=0 for domain NCC0 and turbo=1 for every CPU in
> > NCC1 and NCC2, across an identical 710400-3417600 kHz table. The raw domain
> > attributes above were read the same way, with a kprobe on
> > scmi_dvfs_device_opps_add() fetching the perf_dom_info fields.
> > 
> > A sustained frequency below the lowest OPP carries no information - it
> > cannot separate sustained levels from boost levels. Treat it as "this
> > domain has no turbo levels" instead of letting it disable the domain
> > entirely, and say so once with a FW_BUG warning, since the failure is
> > otherwise silent. A sustained frequency equal to the lowest OPP is left
> > alone, it already leaves that OPP non-turbo.
> > 
> > Fixes: a897575e79d7 ("firmware: arm_scmi: Add support for marking certain frequencies as turbo")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Charlie Garner <charlie@akao.au>
> > ---
> > 
> > Notes:
> >     The SCMI quirks framework (quirks.c) would also work here, and this
> >     platform already has two quirks enabled. I went with a generic check
> >     because a sustained frequency below every OPP is meaningless on any
> >     platform, and the check is a no-op for firmware that reports a sane value.
> >     Happy to turn it into a quirk if you'd prefer that.
> > 
> 
> I am unable to decide which is better approach. I am more inclined to quirks
> so that other systems are not penalised by unnecessary checks but at the same
> time it seems like a sensible generic check.
> 
> Cristian,
> 
> Any strong opinion/inclination ?

No. It is probably an out of spec behaviour that makes sense to spot and
rectify in the original code instead of being doomed to add a string of quirks,
but I am more concerned about upcoming SCMI v4.0 QoS Perf extensions spec which
is public BUT still to come in the implementation, since it makes an extensive
usage of sustained_freq for its own calculation AFAICR

So while now this is flagged as FW_BUG and we carry on because it only impacts
boost, in the future we probably need to shutdown an entire set of features
when this is spotted at discovery, since for anything else that boost I dont
think there is a safe way to guess the proper AND safe sustained levels if
the FW is rotten...

But I suppose we'll take care of that when that time will come...

Thanks,
Cristian