diff mbox series

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

Message ID 20200617094332.8391-2-nicola.mazzucato@arm.com (mailing list archive)
State New, archived
Headers show
Series [1/2] firmware: arm_scmi: Add fast_switch_possible() api | expand

Commit Message

Nicola Mazzucato June 17, 2020, 9:43 a.m. UTC
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(-)

Comments

Sudeep Holla June 17, 2020, 12:47 p.m. UTC | #1
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
Viresh Kumar June 18, 2020, 6:14 a.m. UTC | #2
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 :)
Sudeep Holla June 18, 2020, 8:08 a.m. UTC | #3
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 
Viresh Kumar June 18, 2020, 9:54 a.m. UTC | #4
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>
diff mbox series

Patch

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);