[PATCH 1/2] firmware: arm_scmi: Add fast_switch_possible() api

[PATCH 1/2] firmware: arm_scmi: Add fast_switch_possible() api

[Nicola Mazzucato]

Add a new fast_switch_possible interface to the existing
perf_ops api to export the information of whether or not
fast_switch is possible in this driver.

This can be used by the CPUFreq driver and framework to
choose proper mechanism for frequency change.

Suggested-by: Lukasz Luba <lukasz.luba@arm.com>
Signed-off-by: Nicola Mazzucato <nicola.mazzucato@arm.com>
---
 drivers/firmware/arm_scmi/perf.c | 12 ++++++++++++
 include/linux/scmi_protocol.h    |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index eadc171e254b..ef747a9bb948 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -697,6 +697,17 @@ static int scmi_dvfs_est_power_get(const struct scmi_handle *handle, u32 domain,
 	return ret;
 }
 
+static bool scmi_fast_switch_possible(const struct scmi_handle *handle,
+				   struct device *dev)
+{
+	struct perf_dom_info *dom;
+	struct scmi_perf_info *pi = handle->perf_priv;
+
+	dom = pi->dom_info + scmi_dev_domain_id(dev);
+
+	return (dom->fc_info && dom->fc_info->level_set_addr);
+}
+
 static struct scmi_perf_ops perf_ops = {
 	.limits_set = scmi_perf_limits_set,
 	.limits_get = scmi_perf_limits_get,
@@ -708,6 +719,7 @@ static struct scmi_perf_ops perf_ops = {
 	.freq_set = scmi_dvfs_freq_set,
 	.freq_get = scmi_dvfs_freq_get,
 	.est_power_get = scmi_dvfs_est_power_get,
+	.fast_switch_possible = scmi_fast_switch_possible,
 };
 
 static int scmi_perf_protocol_init(struct scmi_handle *handle)
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index ce2f5c28b2df..19e50b89117e 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -118,6 +118,8 @@ struct scmi_perf_ops {
 			unsigned long *rate, bool poll);
 	int (*est_power_get)(const struct scmi_handle *handle, u32 domain,
 			     unsigned long *rate, unsigned long *power);
+	bool (*fast_switch_possible)(const struct scmi_handle *handle,
+			struct device *dev);
 };
 
 /**
-- 
2.27.0

[PATCH 2/2] cpufreq: arm_scmi: Set fast_switch_possible conditionally

[Nicola Mazzucato]

Currently the fast_switch_possible flag is set unconditionally
to true. Based on this, schedutil does not create a
thread for frequency switching and would always use the
fast switch path.
However, if the platform does not support frequency
fast switch, this may cause the governor to attempt an
operation that is not supported by the platform.

Fix this by correctly retrieve the fast_switch capability
from the driver which knows if the platform can support
this feature.

Suggested-by: Lukasz Luba <lukasz.luba@arm.com>
Signed-off-by: Nicola Mazzucato <nicola.mazzucato@arm.com>
---
 drivers/cpufreq/scmi-cpufreq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c
index 61623e2ff149..1cf688fcb56b 100644
--- a/drivers/cpufreq/scmi-cpufreq.c
+++ b/drivers/cpufreq/scmi-cpufreq.c
@@ -198,7 +198,8 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy)
 
 	policy->cpuinfo.transition_latency = latency;
 
-	policy->fast_switch_possible = true;
+	policy->fast_switch_possible =
+		handle->perf_ops->fast_switch_possible(handle, cpu_dev);
 
 	em_register_perf_domain(policy->cpus, nr_opp, &em_cb);
 
-- 
2.27.0

Re: [PATCH 2/2] cpufreq: arm_scmi: Set fast_switch_possible conditionally

[Sudeep Holla]

On Wed, Jun 17, 2020 at 10:43:32AM +0100, Nicola Mazzucato wrote:
> Currently the fast_switch_possible flag is set unconditionally
> to true. Based on this, schedutil does not create a
> thread for frequency switching and would always use the
> fast switch path.
> However, if the platform does not support frequency
> fast switch, this may cause the governor to attempt an
> operation that is not supported by the platform.
>
> Fix this by correctly retrieve the fast_switch capability
> from the driver which knows if the platform can support
> this feature.
>

Hi Viresh,

This is first step towards avoiding polling based cpufreq set if firmware
has fast access registers that bypass normal mailbox based messaging.

If you happy with this and provide ack, I will take this along with scmi
changes via ARM SoC. Hope that is fine by you.

--
Regards,
Sudeep

Re: [PATCH 2/2] cpufreq: arm_scmi: Set fast_switch_possible conditionally

[Viresh Kumar]

On 17-06-20, 13:47, Sudeep Holla wrote:
> This is first step towards avoiding polling based cpufreq set if firmware
> has fast access registers that bypass normal mailbox based messaging.
> 
> If you happy with this and provide ack, I will take this along with scmi
> changes via ARM SoC. Hope that is fine by you.

Sudeep,

I am not sure how it concerns me frankly :)

AFAICT, this is enabling fast switch based on some mechanism (internal
to scmi) and so either the cpufreq driver will have fast-switch
enabled or not, and both are fine by the cpufreq core.

And so I am confused on why my Ack is important here :)

-- 
viresh

Re: [PATCH 2/2] cpufreq: arm_scmi: Set fast_switch_possible conditionally

[Sudeep Holla]

On Thu, Jun 18, 2020 at 11:44:20AM +0530, Viresh Kumar wrote:
> On 17-06-20, 13:47, Sudeep Holla wrote:
> > This is first step towards avoiding polling based cpufreq set if firmware
> > has fast access registers that bypass normal mailbox based messaging.
> >
> > If you happy with this and provide ack, I will take this along with scmi
> > changes via ARM SoC. Hope that is fine by you.
>
> Sudeep,
>
> I am not sure how it concerns me frankly :)
>

Sorry I wasn't clear.

> AFAICT, this is enabling fast switch based on some mechanism (internal
> to scmi) and so either the cpufreq driver will have fast-switch
> enabled or not, and both are fine by the cpufreq core.
>

Indeed.

> And so I am confused on why my Ack is important here :)
>

Generally ARM SoC team expects a stamp from other subsystem maintainers
if they are pulling it. I understand there is more firmware aspect than
cpufreq aspect here, but still we may need your stamp to this 😉 for
logistic reasons.

--
Regards,
Sudeep

Re: [PATCH 2/2] cpufreq: arm_scmi: Set fast_switch_possible conditionally

[Viresh Kumar]

On 17-06-20, 10:43, Nicola Mazzucato wrote:
> Currently the fast_switch_possible flag is set unconditionally
> to true. Based on this, schedutil does not create a
> thread for frequency switching and would always use the
> fast switch path.
> However, if the platform does not support frequency
> fast switch, this may cause the governor to attempt an
> operation that is not supported by the platform.
> 
> Fix this by correctly retrieve the fast_switch capability
> from the driver which knows if the platform can support
> this feature.
> 
> Suggested-by: Lukasz Luba <lukasz.luba@arm.com>
> Signed-off-by: Nicola Mazzucato <nicola.mazzucato@arm.com>
> ---
>  drivers/cpufreq/scmi-cpufreq.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c
> index 61623e2ff149..1cf688fcb56b 100644
> --- a/drivers/cpufreq/scmi-cpufreq.c
> +++ b/drivers/cpufreq/scmi-cpufreq.c
> @@ -198,7 +198,8 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy)
>  
>  	policy->cpuinfo.transition_latency = latency;
>  
> -	policy->fast_switch_possible = true;
> +	policy->fast_switch_possible =
> +		handle->perf_ops->fast_switch_possible(handle, cpu_dev);
>  
>  	em_register_perf_domain(policy->cpus, nr_opp, &em_cb);

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

-- 
viresh

Re: [PATCH 1/2] firmware: arm_scmi: Add fast_switch_possible() api

[Sudeep Holla]

On Wed, 17 Jun 2020 10:43:31 +0100, Nicola Mazzucato wrote:
> Add a new fast_switch_possible interface to the existing
> perf_ops api to export the information of whether or not
> fast_switch is possible in this driver.
> 
> This can be used by the CPUFreq driver and framework to
> choose proper mechanism for frequency change.


Applied to sudeep.holla/linux (for-next/scmi), thanks!

[1/2] firmware: arm_scmi: Add fast_switch_possible() interface
      https://git.kernel.org/sudeep.holla/c/1909872ff2
[2/2] cpufreq: arm_scmi: Set fast_switch_possible conditionally
      https://git.kernel.org/sudeep.holla/c/fb3571276b

--
Regards,
Sudeep