diff mbox series

[2/2] firmware: arm_scmi: Fix possible frequency truncation when using level indexing mode

Message ID 20231130204343.503076-2-sudeep.holla@arm.com (mailing list archive)
State Not Applicable
Headers show
Series [1/2] firmware: arm_scmi: Fix frequency truncation by promoting multiplier to u64 | expand

Commit Message

Sudeep Holla Nov. 30, 2023, 8:43 p.m. UTC
The multiplier is already promoted to u64, however the frequency
calculations done when using level indexing mode doesn't use the
multiplier computed. It instead hardcodes the multiplier value of 1000
at all the usage sites.

Clean that up by assigning the multiplier value of 1000 when using
the perf level indexing mode and upadte the frequency calculations to
use the multiplier instead. It should fix the possible frequency
truncation for all the values greater than or equal to 4GHz.

Fixes: 31c7c1397a33 ("firmware: arm_scmi: Add v3.2 perf level indexing mode support")
Reported-by: Sibi Sankar <quic_sibis@quicinc.com>
Closes: https://lore.kernel.org/all/20231129065748.19871-3-quic_sibis@quicinc.com/
Cc: Cristian Marussi <cristian.marussi@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/perf.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

--
2.43.0

Comments

Cristian Marussi Dec. 1, 2023, 4:23 p.m. UTC | #1
On Thu, Nov 30, 2023 at 08:43:43PM +0000, Sudeep Holla wrote:
> The multiplier is already promoted to u64, however the frequency
> calculations done when using level indexing mode doesn't use the
> multiplier computed. It instead hardcodes the multiplier value of 1000
> at all the usage sites.
> 
> Clean that up by assigning the multiplier value of 1000 when using
> the perf level indexing mode and upadte the frequency calculations to

				   ^update

> use the multiplier instead. It should fix the possible frequency
> truncation for all the values greater than or equal to 4GHz.
> 
> Fixes: 31c7c1397a33 ("firmware: arm_scmi: Add v3.2 perf level indexing mode support")
> Reported-by: Sibi Sankar <quic_sibis@quicinc.com>
> Closes: https://lore.kernel.org/all/20231129065748.19871-3-quic_sibis@quicinc.com/
> Cc: Cristian Marussi <cristian.marussi@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---

Other than the typo, LGTM.

Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>

Thanks,
Cristian

>  drivers/firmware/arm_scmi/perf.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
> index 8ce449922e55..875dcb71bb65 100644
> --- a/drivers/firmware/arm_scmi/perf.c
> +++ b/drivers/firmware/arm_scmi/perf.c
> @@ -268,7 +268,8 @@ scmi_perf_domain_attributes_get(const struct scmi_protocol_handle *ph,
>  		dom_info->sustained_perf_level =
>  					le32_to_cpu(attr->sustained_perf_level);
>  		if (!dom_info->sustained_freq_khz ||
> -		    !dom_info->sustained_perf_level)
> +		    !dom_info->sustained_perf_level ||
> +		    dom_info->level_indexing_mode)
>  			/* CPUFreq converts to kHz, hence default 1000 */
>  			dom_info->mult_factor =	1000;
>  		else
> @@ -806,7 +807,7 @@ static int scmi_dvfs_device_opps_add(const struct scmi_protocol_handle *ph,
>  		if (!dom->level_indexing_mode)
>  			freq = dom->opp[idx].perf * dom->mult_factor;
>  		else
> -			freq = dom->opp[idx].indicative_freq * 1000;
> +			freq = dom->opp[idx].indicative_freq * dom->mult_factor;
> 
>  		data.level = dom->opp[idx].perf;
>  		data.freq = freq;
> @@ -853,7 +854,8 @@ static int scmi_dvfs_freq_set(const struct scmi_protocol_handle *ph, u32 domain,
>  	} else {
>  		struct scmi_opp *opp;
> 
> -		opp = LOOKUP_BY_FREQ(dom->opps_by_freq, freq / 1000);
> +		opp = LOOKUP_BY_FREQ(dom->opps_by_freq,
> +				     freq / dom->mult_factor);
>  		if (!opp)
>  			return -EIO;
> 
> @@ -887,7 +889,7 @@ static int scmi_dvfs_freq_get(const struct scmi_protocol_handle *ph, u32 domain,
>  		if (!opp)
>  			return -EIO;
> 
> -		*freq = opp->indicative_freq * 1000;
> +		*freq = opp->indicative_freq * dom->mult_factor;
>  	}
> 
>  	return ret;
> @@ -910,7 +912,7 @@ static int scmi_dvfs_est_power_get(const struct scmi_protocol_handle *ph,
>  		if (!dom->level_indexing_mode)
>  			opp_freq = opp->perf * dom->mult_factor;
>  		else
> -			opp_freq = opp->indicative_freq * 1000;
> +			opp_freq = opp->indicative_freq * dom->mult_factor;
> 
>  		if (opp_freq < *freq)
>  			continue;
> --
> 2.43.0
>
diff mbox series

Patch

diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index 8ce449922e55..875dcb71bb65 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -268,7 +268,8 @@  scmi_perf_domain_attributes_get(const struct scmi_protocol_handle *ph,
 		dom_info->sustained_perf_level =
 					le32_to_cpu(attr->sustained_perf_level);
 		if (!dom_info->sustained_freq_khz ||
-		    !dom_info->sustained_perf_level)
+		    !dom_info->sustained_perf_level ||
+		    dom_info->level_indexing_mode)
 			/* CPUFreq converts to kHz, hence default 1000 */
 			dom_info->mult_factor =	1000;
 		else
@@ -806,7 +807,7 @@  static int scmi_dvfs_device_opps_add(const struct scmi_protocol_handle *ph,
 		if (!dom->level_indexing_mode)
 			freq = dom->opp[idx].perf * dom->mult_factor;
 		else
-			freq = dom->opp[idx].indicative_freq * 1000;
+			freq = dom->opp[idx].indicative_freq * dom->mult_factor;

 		data.level = dom->opp[idx].perf;
 		data.freq = freq;
@@ -853,7 +854,8 @@  static int scmi_dvfs_freq_set(const struct scmi_protocol_handle *ph, u32 domain,
 	} else {
 		struct scmi_opp *opp;

-		opp = LOOKUP_BY_FREQ(dom->opps_by_freq, freq / 1000);
+		opp = LOOKUP_BY_FREQ(dom->opps_by_freq,
+				     freq / dom->mult_factor);
 		if (!opp)
 			return -EIO;

@@ -887,7 +889,7 @@  static int scmi_dvfs_freq_get(const struct scmi_protocol_handle *ph, u32 domain,
 		if (!opp)
 			return -EIO;

-		*freq = opp->indicative_freq * 1000;
+		*freq = opp->indicative_freq * dom->mult_factor;
 	}

 	return ret;
@@ -910,7 +912,7 @@  static int scmi_dvfs_est_power_get(const struct scmi_protocol_handle *ph,
 		if (!dom->level_indexing_mode)
 			opp_freq = opp->perf * dom->mult_factor;
 		else
-			opp_freq = opp->indicative_freq * 1000;
+			opp_freq = opp->indicative_freq * dom->mult_factor;

 		if (opp_freq < *freq)
 			continue;