2024-01-17 10:46:09

by Sibi Sankar

[permalink] [raw]
Subject: [PATCH V2 2/4] firmware: arm_scmi: Add perf_freq_xlate interface

Add a new perf_freq_xlate interface to the existing perf_ops to translate
a given perf index to frequency.

This can be used by the cpufreq driver and framework to determine the
throttled frequency from a given perf index and apply HW pressure
accordingly.

Signed-off-by: Sibi Sankar <[email protected]>
---

v2:
* Rename opp_xlate -> freq_xlate [Viresh]

drivers/firmware/arm_scmi/perf.c | 21 +++++++++++++++++++++
include/linux/scmi_protocol.h | 3 +++
2 files changed, 24 insertions(+)

diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index ae7681eda276..e286f04ee6e3 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -977,6 +977,26 @@ static int scmi_notify_support(const struct scmi_protocol_handle *ph, u32 domain
return 0;
}

+static int scmi_perf_freq_xlate(const struct scmi_protocol_handle *ph, u32 domain,
+ int idx, unsigned long *freq)
+{
+ struct perf_dom_info *dom;
+
+ dom = scmi_perf_domain_lookup(ph, domain);
+ if (IS_ERR(dom))
+ return PTR_ERR(dom);
+
+ if (idx >= dom->opp_count)
+ return -ERANGE;
+
+ if (!dom->level_indexing_mode)
+ *freq = dom->opp[idx].perf * dom->mult_factor;
+ else
+ *freq = dom->opp[idx].indicative_freq * dom->mult_factor;
+
+ return 0;
+}
+
static const struct scmi_perf_proto_ops perf_proto_ops = {
.num_domains_get = scmi_perf_num_domains_get,
.info_get = scmi_perf_info_get,
@@ -992,6 +1012,7 @@ static const struct scmi_perf_proto_ops perf_proto_ops = {
.fast_switch_possible = scmi_fast_switch_possible,
.power_scale_get = scmi_power_scale_get,
.perf_notify_support = scmi_notify_support,
+ .perf_freq_xlate = scmi_perf_freq_xlate,
};

static int scmi_perf_set_notify_enabled(const struct scmi_protocol_handle *ph,
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index b0947d004826..6221d391386c 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -145,6 +145,7 @@ struct scmi_perf_notify_info {
* @power_scale_mw_get: indicates if the power values provided are in milliWatts
* or in some other (abstract) scale
* @perf_notify_support: indicates if limit and level change notification is supported
+ * @perf_freq_xlate: translates the given perf index to frequency in Hz
*/
struct scmi_perf_proto_ops {
int (*num_domains_get)(const struct scmi_protocol_handle *ph);
@@ -173,6 +174,8 @@ struct scmi_perf_proto_ops {
enum scmi_power_scale (*power_scale_get)(const struct scmi_protocol_handle *ph);
int (*perf_notify_support)(const struct scmi_protocol_handle *ph, u32 domain,
struct scmi_perf_notify_info *info);
+ int (*perf_freq_xlate)(const struct scmi_protocol_handle *ph, u32 domain,
+ int idx, unsigned long *freq);
};

/**
--
2.34.1



2024-01-29 15:54:11

by Cristian Marussi

[permalink] [raw]
Subject: Re: [PATCH V2 2/4] firmware: arm_scmi: Add perf_freq_xlate interface

On Wed, Jan 17, 2024 at 04:11:14PM +0530, Sibi Sankar wrote:
> Add a new perf_freq_xlate interface to the existing perf_ops to translate
> a given perf index to frequency.
>
> This can be used by the cpufreq driver and framework to determine the
> throttled frequency from a given perf index and apply HW pressure
> accordingly.
>
> Signed-off-by: Sibi Sankar <[email protected]>
> ---
>
> v2:
> * Rename opp_xlate -> freq_xlate [Viresh]
>
> drivers/firmware/arm_scmi/perf.c | 21 +++++++++++++++++++++
> include/linux/scmi_protocol.h | 3 +++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
> index ae7681eda276..e286f04ee6e3 100644
> --- a/drivers/firmware/arm_scmi/perf.c
> +++ b/drivers/firmware/arm_scmi/perf.c
> @@ -977,6 +977,26 @@ static int scmi_notify_support(const struct scmi_protocol_handle *ph, u32 domain
> return 0;
> }
>
> +static int scmi_perf_freq_xlate(const struct scmi_protocol_handle *ph, u32 domain,
> + int idx, unsigned long *freq)
> +{
> + struct perf_dom_info *dom;
> +
> + dom = scmi_perf_domain_lookup(ph, domain);
> + if (IS_ERR(dom))
> + return PTR_ERR(dom);
> +
> + if (idx >= dom->opp_count)
> + return -ERANGE;
> +
> + if (!dom->level_indexing_mode)
> + *freq = dom->opp[idx].perf * dom->mult_factor;
> + else
> + *freq = dom->opp[idx].indicative_freq * dom->mult_factor;
> +
> + return 0;
> +}

Potentially, once the dom info are exposed you can also drop this
accessor and move all of these checks/calculations in the the SCMI cpufreq
driver...but maybe Sudeep/Viresh prefer to keep this accessor exposed
like others.

Thanks,
Cristian

2024-01-31 12:09:47

by Cristian Marussi

[permalink] [raw]
Subject: Re: [PATCH V2 2/4] firmware: arm_scmi: Add perf_freq_xlate interface

On Wed, Jan 17, 2024 at 04:11:14PM +0530, Sibi Sankar wrote:
> Add a new perf_freq_xlate interface to the existing perf_ops to translate
> a given perf index to frequency.
>
> This can be used by the cpufreq driver and framework to determine the
> throttled frequency from a given perf index and apply HW pressure
> accordingly.
>
> Signed-off-by: Sibi Sankar <[email protected]>
> ---
>
> v2:
> * Rename opp_xlate -> freq_xlate [Viresh]
>
> drivers/firmware/arm_scmi/perf.c | 21 +++++++++++++++++++++
> include/linux/scmi_protocol.h | 3 +++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
> index ae7681eda276..e286f04ee6e3 100644
> --- a/drivers/firmware/arm_scmi/perf.c
> +++ b/drivers/firmware/arm_scmi/perf.c
> @@ -977,6 +977,26 @@ static int scmi_notify_support(const struct scmi_protocol_handle *ph, u32 domain
> return 0;
> }
>
> +static int scmi_perf_freq_xlate(const struct scmi_protocol_handle *ph, u32 domain,
> + int idx, unsigned long *freq)
> +{
> + struct perf_dom_info *dom;
> +
> + dom = scmi_perf_domain_lookup(ph, domain);
> + if (IS_ERR(dom))
> + return PTR_ERR(dom);
> +
> + if (idx >= dom->opp_count)
> + return -ERANGE;
> +
> + if (!dom->level_indexing_mode)
> + *freq = dom->opp[idx].perf * dom->mult_factor;
> + else
> + *freq = dom->opp[idx].indicative_freq * dom->mult_factor;
> +

As said elsewhere the plan would be to change slightly the SCMI core to
avoid the need for this patch and the previous one (while NOT exposing
too much Perf info)...

.. anyway just looking at the above freq calc logic in this patch, be
aware that as it stands it seems to me broken, since the idx you use to
peek into the opp array comes (in the next patch) from the range_max
carried by the notification and that can be, indeed, a perf_level OR a
perf_index BUT it is absolutely NOT guaranteed to be an index into the
opp[] array...so it may work in your case if you have a platform
defining level or indexes matching the opp[] indexes BUT it is not true
in general. (but as said, this will be handled by the core and possibly
this patch dropped...)

Thanks,
Cristian