| Message ID | 20200617094332.8391-2-nicola.mazzucato@arm.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [1/2] firmware: arm_scmi: Add fast_switch_possible() api | expand |
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
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 :)
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
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 --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);
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(-)