* [PATCH 0/3] scmi-cpufreq: Set transition_delay_us @ 2024-02-22 13:56 ` Pierre Gondois 0 siblings, 0 replies; 24+ messages in thread From: Pierre Gondois @ 2024-02-22 13:56 UTC (permalink / raw) To: linux-kernel Cc: Cristian Marussi, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Pierre Gondois, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm policy's fields definitions: `transition_delay_us`: The minimum amount of time between two consecutive freq. requests for one policy. `transition_latency`: Delta between freq. change request and effective freq. change on the hardware. cpufreq_policy_transition_delay_us() uses the `transition_delay_us` value if available. Otherwise a value is induced from the policy's `transition_latency`. The scmi-cpufreq driver doesn't populate the `transition_delay_us`. Values matching the definition are available through the SCMI specification. Add support to fetch these values and use them in the scmi-cpufreq driver. Pierre Gondois (3): firmware: arm_scmi: Populate perf commands rate_limit firmware: arm_scmi: Populate fast channel rate_limit cpufreq: scmi: Set transition_delay_us drivers/cpufreq/scmi-cpufreq.c | 26 +++++++++++++ drivers/firmware/arm_scmi/driver.c | 5 ++- drivers/firmware/arm_scmi/perf.c | 53 +++++++++++++++++++++++++-- drivers/firmware/arm_scmi/powercap.c | 12 ++++-- drivers/firmware/arm_scmi/protocols.h | 4 +- include/linux/scmi_protocol.h | 8 ++++ 6 files changed, 98 insertions(+), 10 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 0/3] scmi-cpufreq: Set transition_delay_us @ 2024-02-22 13:56 ` Pierre Gondois 0 siblings, 0 replies; 24+ messages in thread From: Pierre Gondois @ 2024-02-22 13:56 UTC (permalink / raw) To: linux-kernel Cc: Cristian Marussi, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Pierre Gondois, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm policy's fields definitions: `transition_delay_us`: The minimum amount of time between two consecutive freq. requests for one policy. `transition_latency`: Delta between freq. change request and effective freq. change on the hardware. cpufreq_policy_transition_delay_us() uses the `transition_delay_us` value if available. Otherwise a value is induced from the policy's `transition_latency`. The scmi-cpufreq driver doesn't populate the `transition_delay_us`. Values matching the definition are available through the SCMI specification. Add support to fetch these values and use them in the scmi-cpufreq driver. Pierre Gondois (3): firmware: arm_scmi: Populate perf commands rate_limit firmware: arm_scmi: Populate fast channel rate_limit cpufreq: scmi: Set transition_delay_us drivers/cpufreq/scmi-cpufreq.c | 26 +++++++++++++ drivers/firmware/arm_scmi/driver.c | 5 ++- drivers/firmware/arm_scmi/perf.c | 53 +++++++++++++++++++++++++-- drivers/firmware/arm_scmi/powercap.c | 12 ++++-- drivers/firmware/arm_scmi/protocols.h | 4 +- include/linux/scmi_protocol.h | 8 ++++ 6 files changed, 98 insertions(+), 10 deletions(-) -- 2.25.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/3] firmware: arm_scmi: Populate perf commands rate_limit 2024-02-22 13:56 ` Pierre Gondois @ 2024-02-22 13:56 ` Pierre Gondois -1 siblings, 0 replies; 24+ messages in thread From: Pierre Gondois @ 2024-02-22 13:56 UTC (permalink / raw) To: linux-kernel Cc: Cristian Marussi, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Pierre Gondois, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm Arm SCMI spec. v3.2, s4.5.3.4 PERFORMANCE_DOMAIN_ATTRIBUTES defines a per-domain rate_limit for performance requests: """ Rate Limit in microseconds, indicating the minimum time required between successive requests. A value of 0 indicates that this field is not supported by the platform. This field does not apply to FastChannels. """" The field is first defined in SCMI v1.0. Add support to fetch this value and advertise it through a rate_limit_get() callback. Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> --- drivers/firmware/arm_scmi/perf.c | 21 +++++++++++++++++++++ include/linux/scmi_protocol.h | 4 ++++ 2 files changed, 25 insertions(+) diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index 211e8e0aef2c..37c80376bd0a 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -153,6 +153,7 @@ struct perf_dom_info { bool perf_fastchannels; bool level_indexing_mode; u32 opp_count; + u32 rate_limit_us; u32 sustained_freq_khz; u32 sustained_perf_level; unsigned long mult_factor; @@ -266,6 +267,8 @@ scmi_perf_domain_attributes_get(const struct scmi_protocol_handle *ph, if (PROTOCOL_REV_MAJOR(version) >= 0x4) dom_info->level_indexing_mode = SUPPORTS_LEVEL_INDEXING(flags); + dom_info->rate_limit_us = le32_to_cpu(attr->rate_limit_us) & + GENMASK(19, 0); dom_info->sustained_freq_khz = le32_to_cpu(attr->sustained_freq_khz); dom_info->sustained_perf_level = @@ -855,6 +858,23 @@ scmi_dvfs_transition_latency_get(const struct scmi_protocol_handle *ph, return dom->opp[dom->opp_count - 1].trans_latency_us * 1000; } +static int +scmi_dvfs_rate_limit_get(const struct scmi_protocol_handle *ph, + u32 domain, u32 *rate_limit) +{ + struct perf_dom_info *dom; + + if (!rate_limit) + return -EINVAL; + + dom = scmi_perf_domain_lookup(ph, domain); + if (IS_ERR(dom)) + return PTR_ERR(dom); + + *rate_limit = dom->rate_limit_us; + return 0; +} + static int scmi_dvfs_freq_set(const struct scmi_protocol_handle *ph, u32 domain, unsigned long freq, bool poll) { @@ -970,6 +990,7 @@ static const struct scmi_perf_proto_ops perf_proto_ops = { .level_set = scmi_perf_level_set, .level_get = scmi_perf_level_get, .transition_latency_get = scmi_dvfs_transition_latency_get, + .rate_limit_get = scmi_dvfs_rate_limit_get, .device_opps_add = scmi_dvfs_device_opps_add, .freq_set = scmi_dvfs_freq_set, .freq_get = scmi_dvfs_freq_get, diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index f2f05fb42d28..acd956ffcb84 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -128,6 +128,8 @@ struct scmi_perf_domain_info { * @level_set: sets the performance level of a domain * @level_get: gets the performance level of a domain * @transition_latency_get: gets the DVFS transition latency for a given device + * @rate_limit_get: gets the minimum time (us) required between successive + * requests * @device_opps_add: adds all the OPPs for a given device * @freq_set: sets the frequency for a given device using sustained frequency * to sustained performance level mapping @@ -154,6 +156,8 @@ struct scmi_perf_proto_ops { u32 *level, bool poll); int (*transition_latency_get)(const struct scmi_protocol_handle *ph, u32 domain); + int (*rate_limit_get)(const struct scmi_protocol_handle *ph, + u32 domain, u32 *rate_limit); int (*device_opps_add)(const struct scmi_protocol_handle *ph, struct device *dev, u32 domain); int (*freq_set)(const struct scmi_protocol_handle *ph, u32 domain, -- 2.25.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 1/3] firmware: arm_scmi: Populate perf commands rate_limit @ 2024-02-22 13:56 ` Pierre Gondois 0 siblings, 0 replies; 24+ messages in thread From: Pierre Gondois @ 2024-02-22 13:56 UTC (permalink / raw) To: linux-kernel Cc: Cristian Marussi, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Pierre Gondois, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm Arm SCMI spec. v3.2, s4.5.3.4 PERFORMANCE_DOMAIN_ATTRIBUTES defines a per-domain rate_limit for performance requests: """ Rate Limit in microseconds, indicating the minimum time required between successive requests. A value of 0 indicates that this field is not supported by the platform. This field does not apply to FastChannels. """" The field is first defined in SCMI v1.0. Add support to fetch this value and advertise it through a rate_limit_get() callback. Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> --- drivers/firmware/arm_scmi/perf.c | 21 +++++++++++++++++++++ include/linux/scmi_protocol.h | 4 ++++ 2 files changed, 25 insertions(+) diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index 211e8e0aef2c..37c80376bd0a 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -153,6 +153,7 @@ struct perf_dom_info { bool perf_fastchannels; bool level_indexing_mode; u32 opp_count; + u32 rate_limit_us; u32 sustained_freq_khz; u32 sustained_perf_level; unsigned long mult_factor; @@ -266,6 +267,8 @@ scmi_perf_domain_attributes_get(const struct scmi_protocol_handle *ph, if (PROTOCOL_REV_MAJOR(version) >= 0x4) dom_info->level_indexing_mode = SUPPORTS_LEVEL_INDEXING(flags); + dom_info->rate_limit_us = le32_to_cpu(attr->rate_limit_us) & + GENMASK(19, 0); dom_info->sustained_freq_khz = le32_to_cpu(attr->sustained_freq_khz); dom_info->sustained_perf_level = @@ -855,6 +858,23 @@ scmi_dvfs_transition_latency_get(const struct scmi_protocol_handle *ph, return dom->opp[dom->opp_count - 1].trans_latency_us * 1000; } +static int +scmi_dvfs_rate_limit_get(const struct scmi_protocol_handle *ph, + u32 domain, u32 *rate_limit) +{ + struct perf_dom_info *dom; + + if (!rate_limit) + return -EINVAL; + + dom = scmi_perf_domain_lookup(ph, domain); + if (IS_ERR(dom)) + return PTR_ERR(dom); + + *rate_limit = dom->rate_limit_us; + return 0; +} + static int scmi_dvfs_freq_set(const struct scmi_protocol_handle *ph, u32 domain, unsigned long freq, bool poll) { @@ -970,6 +990,7 @@ static const struct scmi_perf_proto_ops perf_proto_ops = { .level_set = scmi_perf_level_set, .level_get = scmi_perf_level_get, .transition_latency_get = scmi_dvfs_transition_latency_get, + .rate_limit_get = scmi_dvfs_rate_limit_get, .device_opps_add = scmi_dvfs_device_opps_add, .freq_set = scmi_dvfs_freq_set, .freq_get = scmi_dvfs_freq_get, diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index f2f05fb42d28..acd956ffcb84 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -128,6 +128,8 @@ struct scmi_perf_domain_info { * @level_set: sets the performance level of a domain * @level_get: gets the performance level of a domain * @transition_latency_get: gets the DVFS transition latency for a given device + * @rate_limit_get: gets the minimum time (us) required between successive + * requests * @device_opps_add: adds all the OPPs for a given device * @freq_set: sets the frequency for a given device using sustained frequency * to sustained performance level mapping @@ -154,6 +156,8 @@ struct scmi_perf_proto_ops { u32 *level, bool poll); int (*transition_latency_get)(const struct scmi_protocol_handle *ph, u32 domain); + int (*rate_limit_get)(const struct scmi_protocol_handle *ph, + u32 domain, u32 *rate_limit); int (*device_opps_add)(const struct scmi_protocol_handle *ph, struct device *dev, u32 domain); int (*freq_set)(const struct scmi_protocol_handle *ph, u32 domain, -- 2.25.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] firmware: arm_scmi: Populate perf commands rate_limit 2024-02-22 13:56 ` Pierre Gondois @ 2024-03-04 7:55 ` Cristian Marussi -1 siblings, 0 replies; 24+ messages in thread From: Cristian Marussi @ 2024-03-04 7:55 UTC (permalink / raw) To: Pierre Gondois Cc: linux-kernel, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm On Thu, Feb 22, 2024 at 02:56:59PM +0100, Pierre Gondois wrote: > Arm SCMI spec. v3.2, s4.5.3.4 PERFORMANCE_DOMAIN_ATTRIBUTES > defines a per-domain rate_limit for performance requests: > """ > Rate Limit in microseconds, indicating the minimum time > required between successive requests. A value of 0 > indicates that this field is not supported by the > platform. This field does not apply to FastChannels. > """" > The field is first defined in SCMI v1.0. > > Add support to fetch this value and advertise it through > a rate_limit_get() callback. > Hi, LGTM. Reviewed-by: Cristian Marussi <cristian.marussi@arm.com> Thanks, Cristian > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> > --- > drivers/firmware/arm_scmi/perf.c | 21 +++++++++++++++++++++ > include/linux/scmi_protocol.h | 4 ++++ > 2 files changed, 25 insertions(+) > > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c > index 211e8e0aef2c..37c80376bd0a 100644 > --- a/drivers/firmware/arm_scmi/perf.c > +++ b/drivers/firmware/arm_scmi/perf.c > @@ -153,6 +153,7 @@ struct perf_dom_info { > bool perf_fastchannels; > bool level_indexing_mode; > u32 opp_count; > + u32 rate_limit_us; > u32 sustained_freq_khz; > u32 sustained_perf_level; > unsigned long mult_factor; > @@ -266,6 +267,8 @@ scmi_perf_domain_attributes_get(const struct scmi_protocol_handle *ph, > if (PROTOCOL_REV_MAJOR(version) >= 0x4) > dom_info->level_indexing_mode = > SUPPORTS_LEVEL_INDEXING(flags); > + dom_info->rate_limit_us = le32_to_cpu(attr->rate_limit_us) & > + GENMASK(19, 0); > dom_info->sustained_freq_khz = > le32_to_cpu(attr->sustained_freq_khz); > dom_info->sustained_perf_level = > @@ -855,6 +858,23 @@ scmi_dvfs_transition_latency_get(const struct scmi_protocol_handle *ph, > return dom->opp[dom->opp_count - 1].trans_latency_us * 1000; > } > > +static int > +scmi_dvfs_rate_limit_get(const struct scmi_protocol_handle *ph, > + u32 domain, u32 *rate_limit) > +{ > + struct perf_dom_info *dom; > + > + if (!rate_limit) > + return -EINVAL; > + > + dom = scmi_perf_domain_lookup(ph, domain); > + if (IS_ERR(dom)) > + return PTR_ERR(dom); > + > + *rate_limit = dom->rate_limit_us; > + return 0; > +} > + > static int scmi_dvfs_freq_set(const struct scmi_protocol_handle *ph, u32 domain, > unsigned long freq, bool poll) > { > @@ -970,6 +990,7 @@ static const struct scmi_perf_proto_ops perf_proto_ops = { > .level_set = scmi_perf_level_set, > .level_get = scmi_perf_level_get, > .transition_latency_get = scmi_dvfs_transition_latency_get, > + .rate_limit_get = scmi_dvfs_rate_limit_get, > .device_opps_add = scmi_dvfs_device_opps_add, > .freq_set = scmi_dvfs_freq_set, > .freq_get = scmi_dvfs_freq_get, > diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h > index f2f05fb42d28..acd956ffcb84 100644 > --- a/include/linux/scmi_protocol.h > +++ b/include/linux/scmi_protocol.h > @@ -128,6 +128,8 @@ struct scmi_perf_domain_info { > * @level_set: sets the performance level of a domain > * @level_get: gets the performance level of a domain > * @transition_latency_get: gets the DVFS transition latency for a given device > + * @rate_limit_get: gets the minimum time (us) required between successive > + * requests > * @device_opps_add: adds all the OPPs for a given device > * @freq_set: sets the frequency for a given device using sustained frequency > * to sustained performance level mapping > @@ -154,6 +156,8 @@ struct scmi_perf_proto_ops { > u32 *level, bool poll); > int (*transition_latency_get)(const struct scmi_protocol_handle *ph, > u32 domain); > + int (*rate_limit_get)(const struct scmi_protocol_handle *ph, > + u32 domain, u32 *rate_limit); > int (*device_opps_add)(const struct scmi_protocol_handle *ph, > struct device *dev, u32 domain); > int (*freq_set)(const struct scmi_protocol_handle *ph, u32 domain, > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] firmware: arm_scmi: Populate perf commands rate_limit @ 2024-03-04 7:55 ` Cristian Marussi 0 siblings, 0 replies; 24+ messages in thread From: Cristian Marussi @ 2024-03-04 7:55 UTC (permalink / raw) To: Pierre Gondois Cc: linux-kernel, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm On Thu, Feb 22, 2024 at 02:56:59PM +0100, Pierre Gondois wrote: > Arm SCMI spec. v3.2, s4.5.3.4 PERFORMANCE_DOMAIN_ATTRIBUTES > defines a per-domain rate_limit for performance requests: > """ > Rate Limit in microseconds, indicating the minimum time > required between successive requests. A value of 0 > indicates that this field is not supported by the > platform. This field does not apply to FastChannels. > """" > The field is first defined in SCMI v1.0. > > Add support to fetch this value and advertise it through > a rate_limit_get() callback. > Hi, LGTM. Reviewed-by: Cristian Marussi <cristian.marussi@arm.com> Thanks, Cristian > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> > --- > drivers/firmware/arm_scmi/perf.c | 21 +++++++++++++++++++++ > include/linux/scmi_protocol.h | 4 ++++ > 2 files changed, 25 insertions(+) > > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c > index 211e8e0aef2c..37c80376bd0a 100644 > --- a/drivers/firmware/arm_scmi/perf.c > +++ b/drivers/firmware/arm_scmi/perf.c > @@ -153,6 +153,7 @@ struct perf_dom_info { > bool perf_fastchannels; > bool level_indexing_mode; > u32 opp_count; > + u32 rate_limit_us; > u32 sustained_freq_khz; > u32 sustained_perf_level; > unsigned long mult_factor; > @@ -266,6 +267,8 @@ scmi_perf_domain_attributes_get(const struct scmi_protocol_handle *ph, > if (PROTOCOL_REV_MAJOR(version) >= 0x4) > dom_info->level_indexing_mode = > SUPPORTS_LEVEL_INDEXING(flags); > + dom_info->rate_limit_us = le32_to_cpu(attr->rate_limit_us) & > + GENMASK(19, 0); > dom_info->sustained_freq_khz = > le32_to_cpu(attr->sustained_freq_khz); > dom_info->sustained_perf_level = > @@ -855,6 +858,23 @@ scmi_dvfs_transition_latency_get(const struct scmi_protocol_handle *ph, > return dom->opp[dom->opp_count - 1].trans_latency_us * 1000; > } > > +static int > +scmi_dvfs_rate_limit_get(const struct scmi_protocol_handle *ph, > + u32 domain, u32 *rate_limit) > +{ > + struct perf_dom_info *dom; > + > + if (!rate_limit) > + return -EINVAL; > + > + dom = scmi_perf_domain_lookup(ph, domain); > + if (IS_ERR(dom)) > + return PTR_ERR(dom); > + > + *rate_limit = dom->rate_limit_us; > + return 0; > +} > + > static int scmi_dvfs_freq_set(const struct scmi_protocol_handle *ph, u32 domain, > unsigned long freq, bool poll) > { > @@ -970,6 +990,7 @@ static const struct scmi_perf_proto_ops perf_proto_ops = { > .level_set = scmi_perf_level_set, > .level_get = scmi_perf_level_get, > .transition_latency_get = scmi_dvfs_transition_latency_get, > + .rate_limit_get = scmi_dvfs_rate_limit_get, > .device_opps_add = scmi_dvfs_device_opps_add, > .freq_set = scmi_dvfs_freq_set, > .freq_get = scmi_dvfs_freq_get, > diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h > index f2f05fb42d28..acd956ffcb84 100644 > --- a/include/linux/scmi_protocol.h > +++ b/include/linux/scmi_protocol.h > @@ -128,6 +128,8 @@ struct scmi_perf_domain_info { > * @level_set: sets the performance level of a domain > * @level_get: gets the performance level of a domain > * @transition_latency_get: gets the DVFS transition latency for a given device > + * @rate_limit_get: gets the minimum time (us) required between successive > + * requests > * @device_opps_add: adds all the OPPs for a given device > * @freq_set: sets the frequency for a given device using sustained frequency > * to sustained performance level mapping > @@ -154,6 +156,8 @@ struct scmi_perf_proto_ops { > u32 *level, bool poll); > int (*transition_latency_get)(const struct scmi_protocol_handle *ph, > u32 domain); > + int (*rate_limit_get)(const struct scmi_protocol_handle *ph, > + u32 domain, u32 *rate_limit); > int (*device_opps_add)(const struct scmi_protocol_handle *ph, > struct device *dev, u32 domain); > int (*freq_set)(const struct scmi_protocol_handle *ph, u32 domain, > -- > 2.25.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] firmware: arm_scmi: Populate perf commands rate_limit 2024-02-22 13:56 ` Pierre Gondois @ 2024-03-05 11:47 ` Sudeep Holla -1 siblings, 0 replies; 24+ messages in thread From: Sudeep Holla @ 2024-03-05 11:47 UTC (permalink / raw) To: Pierre Gondois Cc: linux-kernel, Sudeep Holla, Cristian Marussi, Christian Loehle, Ionela Voinescu, Dietmar Eggemann, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm On Thu, Feb 22, 2024 at 02:56:59PM +0100, Pierre Gondois wrote: > Arm SCMI spec. v3.2, s4.5.3.4 PERFORMANCE_DOMAIN_ATTRIBUTES > defines a per-domain rate_limit for performance requests: > """ > Rate Limit in microseconds, indicating the minimum time > required between successive requests. A value of 0 > indicates that this field is not supported by the > platform. This field does not apply to FastChannels. > """" > The field is first defined in SCMI v1.0. > > Add support to fetch this value and advertise it through > a rate_limit_get() callback. Similarly, Acked-by: Sudeep Holla <sudeep.holla@arm.com> -- Regards, Sudeep ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] firmware: arm_scmi: Populate perf commands rate_limit @ 2024-03-05 11:47 ` Sudeep Holla 0 siblings, 0 replies; 24+ messages in thread From: Sudeep Holla @ 2024-03-05 11:47 UTC (permalink / raw) To: Pierre Gondois Cc: linux-kernel, Sudeep Holla, Cristian Marussi, Christian Loehle, Ionela Voinescu, Dietmar Eggemann, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm On Thu, Feb 22, 2024 at 02:56:59PM +0100, Pierre Gondois wrote: > Arm SCMI spec. v3.2, s4.5.3.4 PERFORMANCE_DOMAIN_ATTRIBUTES > defines a per-domain rate_limit for performance requests: > """ > Rate Limit in microseconds, indicating the minimum time > required between successive requests. A value of 0 > indicates that this field is not supported by the > platform. This field does not apply to FastChannels. > """" > The field is first defined in SCMI v1.0. > > Add support to fetch this value and advertise it through > a rate_limit_get() callback. Similarly, Acked-by: Sudeep Holla <sudeep.holla@arm.com> -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/3] firmware: arm_scmi: Populate fast channel rate_limit 2024-02-22 13:56 ` Pierre Gondois @ 2024-02-22 13:57 ` Pierre Gondois -1 siblings, 0 replies; 24+ messages in thread From: Pierre Gondois @ 2024-02-22 13:57 UTC (permalink / raw) To: linux-kernel Cc: Cristian Marussi, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Pierre Gondois, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm Arm SCMI spec. v3.2, s4.5.3.12 PERFORMANCE_DESCRIBE_FASTCHANNEL defines a per-domain rate_limit for performance requests: """ Rate Limit in microseconds, indicating the minimum time required between successive requests. A value of 0 indicates that this field is not applicable or supported on the platform. """" The field is first defined in SCMI v2.0. Add support to fetch this value and advertise it through a fast_switch_rate_limit() callback. Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> --- drivers/firmware/arm_scmi/driver.c | 5 ++++- drivers/firmware/arm_scmi/perf.c | 32 +++++++++++++++++++++++---- drivers/firmware/arm_scmi/powercap.c | 12 ++++++---- drivers/firmware/arm_scmi/protocols.h | 4 +++- include/linux/scmi_protocol.h | 4 ++++ 5 files changed, 47 insertions(+), 10 deletions(-) diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 3ea64b22cf0d..1d38ecfafc59 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -1617,7 +1617,7 @@ static void scmi_common_fastchannel_init(const struct scmi_protocol_handle *ph, u8 describe_id, u32 message_id, u32 valid_size, u32 domain, void __iomem **p_addr, - struct scmi_fc_db_info **p_db) + struct scmi_fc_db_info **p_db, u32 *rate_limit) { int ret; u32 flags; @@ -1661,6 +1661,9 @@ scmi_common_fastchannel_init(const struct scmi_protocol_handle *ph, goto err_xfer; } + if (rate_limit) + *rate_limit = le32_to_cpu(resp->rate_limit) & GENMASK(19, 0); + phys_addr = le32_to_cpu(resp->chan_addr_low); phys_addr |= (u64)le32_to_cpu(resp->chan_addr_high) << 32; addr = devm_ioremap(ph->dev, phys_addr, size); diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index 37c80376bd0a..fbcbd703198a 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -789,23 +789,27 @@ static void scmi_perf_domain_init_fc(const struct scmi_protocol_handle *ph, ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, PERF_LEVEL_GET, 4, dom->id, - &fc[PERF_FC_LEVEL].get_addr, NULL); + &fc[PERF_FC_LEVEL].get_addr, NULL, + &fc[PERF_FC_LEVEL].rate_limit); ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, PERF_LIMITS_GET, 8, dom->id, - &fc[PERF_FC_LIMIT].get_addr, NULL); + &fc[PERF_FC_LIMIT].get_addr, NULL, + &fc[PERF_FC_LIMIT].rate_limit); if (dom->info.set_perf) ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, PERF_LEVEL_SET, 4, dom->id, &fc[PERF_FC_LEVEL].set_addr, - &fc[PERF_FC_LEVEL].set_db); + &fc[PERF_FC_LEVEL].set_db, + &fc[PERF_FC_LEVEL].rate_limit); if (dom->set_limits) ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, PERF_LIMITS_SET, 8, dom->id, &fc[PERF_FC_LIMIT].set_addr, - &fc[PERF_FC_LIMIT].set_db); + &fc[PERF_FC_LIMIT].set_db, + &fc[PERF_FC_LIMIT].rate_limit); dom->fc_info = fc; } @@ -974,6 +978,25 @@ static bool scmi_fast_switch_possible(const struct scmi_protocol_handle *ph, return dom->fc_info && dom->fc_info[PERF_FC_LEVEL].set_addr; } +static int scmi_fast_switch_rate_limit(const struct scmi_protocol_handle *ph, + u32 domain, u32 *rate_limit) +{ + struct perf_dom_info *dom; + + if (!rate_limit) + return -EINVAL; + + dom = scmi_perf_domain_lookup(ph, domain); + if (IS_ERR(dom)) + return PTR_ERR(dom); + + if (!dom->fc_info) + return -EINVAL; + + *rate_limit = dom->fc_info[PERF_FC_LEVEL].rate_limit; + return 0; +} + static enum scmi_power_scale scmi_power_scale_get(const struct scmi_protocol_handle *ph) { @@ -996,6 +1019,7 @@ static const struct scmi_perf_proto_ops perf_proto_ops = { .freq_get = scmi_dvfs_freq_get, .est_power_get = scmi_dvfs_est_power_get, .fast_switch_possible = scmi_fast_switch_possible, + .fast_switch_rate_limit = scmi_fast_switch_rate_limit, .power_scale_get = scmi_power_scale_get, }; diff --git a/drivers/firmware/arm_scmi/powercap.c b/drivers/firmware/arm_scmi/powercap.c index a4c6cd4716fe..604184c044ff 100644 --- a/drivers/firmware/arm_scmi/powercap.c +++ b/drivers/firmware/arm_scmi/powercap.c @@ -703,20 +703,24 @@ static void scmi_powercap_domain_init_fc(const struct scmi_protocol_handle *ph, ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, POWERCAP_CAP_SET, 4, domain, &fc[POWERCAP_FC_CAP].set_addr, - &fc[POWERCAP_FC_CAP].set_db); + &fc[POWERCAP_FC_CAP].set_db, + &fc[POWERCAP_FC_CAP].rate_limit); ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, POWERCAP_CAP_GET, 4, domain, - &fc[POWERCAP_FC_CAP].get_addr, NULL); + &fc[POWERCAP_FC_CAP].get_addr, NULL, + &fc[POWERCAP_FC_CAP].rate_limit); ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, POWERCAP_PAI_SET, 4, domain, &fc[POWERCAP_FC_PAI].set_addr, - &fc[POWERCAP_FC_PAI].set_db); + &fc[POWERCAP_FC_PAI].set_db, + &fc[POWERCAP_FC_PAI].rate_limit); ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, POWERCAP_PAI_GET, 4, domain, - &fc[POWERCAP_FC_PAI].get_addr, NULL); + &fc[POWERCAP_FC_PAI].get_addr, NULL, + &fc[POWERCAP_PAI_GET].rate_limit); *p_fc = fc; } diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h index e683c26f24eb..8b5d9ce4a33a 100644 --- a/drivers/firmware/arm_scmi/protocols.h +++ b/drivers/firmware/arm_scmi/protocols.h @@ -234,6 +234,7 @@ struct scmi_fc_info { void __iomem *set_addr; void __iomem *get_addr; struct scmi_fc_db_info *set_db; + u32 rate_limit; }; /** @@ -268,7 +269,8 @@ struct scmi_proto_helpers_ops { u8 describe_id, u32 message_id, u32 valid_size, u32 domain, void __iomem **p_addr, - struct scmi_fc_db_info **p_db); + struct scmi_fc_db_info **p_db, + u32 *rate_limit); void (*fastchannel_db_ring)(struct scmi_fc_db_info *db); }; diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index acd956ffcb84..fafedb3b6604 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -139,6 +139,8 @@ struct scmi_perf_domain_info { * at a given frequency * @fast_switch_possible: indicates if fast DVFS switching is possible or not * for a given device + * @fast_switch_rate_limit: gets the minimum time (us) required between + * successive fast_switching requests * @power_scale_mw_get: indicates if the power values provided are in milliWatts * or in some other (abstract) scale */ @@ -168,6 +170,8 @@ struct scmi_perf_proto_ops { unsigned long *rate, unsigned long *power); bool (*fast_switch_possible)(const struct scmi_protocol_handle *ph, u32 domain); + int (*fast_switch_rate_limit)(const struct scmi_protocol_handle *ph, + u32 domain, u32 *rate_limit); enum scmi_power_scale (*power_scale_get)(const struct scmi_protocol_handle *ph); }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/3] firmware: arm_scmi: Populate fast channel rate_limit @ 2024-02-22 13:57 ` Pierre Gondois 0 siblings, 0 replies; 24+ messages in thread From: Pierre Gondois @ 2024-02-22 13:57 UTC (permalink / raw) To: linux-kernel Cc: Cristian Marussi, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Pierre Gondois, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm Arm SCMI spec. v3.2, s4.5.3.12 PERFORMANCE_DESCRIBE_FASTCHANNEL defines a per-domain rate_limit for performance requests: """ Rate Limit in microseconds, indicating the minimum time required between successive requests. A value of 0 indicates that this field is not applicable or supported on the platform. """" The field is first defined in SCMI v2.0. Add support to fetch this value and advertise it through a fast_switch_rate_limit() callback. Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> --- drivers/firmware/arm_scmi/driver.c | 5 ++++- drivers/firmware/arm_scmi/perf.c | 32 +++++++++++++++++++++++---- drivers/firmware/arm_scmi/powercap.c | 12 ++++++---- drivers/firmware/arm_scmi/protocols.h | 4 +++- include/linux/scmi_protocol.h | 4 ++++ 5 files changed, 47 insertions(+), 10 deletions(-) diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 3ea64b22cf0d..1d38ecfafc59 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -1617,7 +1617,7 @@ static void scmi_common_fastchannel_init(const struct scmi_protocol_handle *ph, u8 describe_id, u32 message_id, u32 valid_size, u32 domain, void __iomem **p_addr, - struct scmi_fc_db_info **p_db) + struct scmi_fc_db_info **p_db, u32 *rate_limit) { int ret; u32 flags; @@ -1661,6 +1661,9 @@ scmi_common_fastchannel_init(const struct scmi_protocol_handle *ph, goto err_xfer; } + if (rate_limit) + *rate_limit = le32_to_cpu(resp->rate_limit) & GENMASK(19, 0); + phys_addr = le32_to_cpu(resp->chan_addr_low); phys_addr |= (u64)le32_to_cpu(resp->chan_addr_high) << 32; addr = devm_ioremap(ph->dev, phys_addr, size); diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index 37c80376bd0a..fbcbd703198a 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -789,23 +789,27 @@ static void scmi_perf_domain_init_fc(const struct scmi_protocol_handle *ph, ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, PERF_LEVEL_GET, 4, dom->id, - &fc[PERF_FC_LEVEL].get_addr, NULL); + &fc[PERF_FC_LEVEL].get_addr, NULL, + &fc[PERF_FC_LEVEL].rate_limit); ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, PERF_LIMITS_GET, 8, dom->id, - &fc[PERF_FC_LIMIT].get_addr, NULL); + &fc[PERF_FC_LIMIT].get_addr, NULL, + &fc[PERF_FC_LIMIT].rate_limit); if (dom->info.set_perf) ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, PERF_LEVEL_SET, 4, dom->id, &fc[PERF_FC_LEVEL].set_addr, - &fc[PERF_FC_LEVEL].set_db); + &fc[PERF_FC_LEVEL].set_db, + &fc[PERF_FC_LEVEL].rate_limit); if (dom->set_limits) ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, PERF_LIMITS_SET, 8, dom->id, &fc[PERF_FC_LIMIT].set_addr, - &fc[PERF_FC_LIMIT].set_db); + &fc[PERF_FC_LIMIT].set_db, + &fc[PERF_FC_LIMIT].rate_limit); dom->fc_info = fc; } @@ -974,6 +978,25 @@ static bool scmi_fast_switch_possible(const struct scmi_protocol_handle *ph, return dom->fc_info && dom->fc_info[PERF_FC_LEVEL].set_addr; } +static int scmi_fast_switch_rate_limit(const struct scmi_protocol_handle *ph, + u32 domain, u32 *rate_limit) +{ + struct perf_dom_info *dom; + + if (!rate_limit) + return -EINVAL; + + dom = scmi_perf_domain_lookup(ph, domain); + if (IS_ERR(dom)) + return PTR_ERR(dom); + + if (!dom->fc_info) + return -EINVAL; + + *rate_limit = dom->fc_info[PERF_FC_LEVEL].rate_limit; + return 0; +} + static enum scmi_power_scale scmi_power_scale_get(const struct scmi_protocol_handle *ph) { @@ -996,6 +1019,7 @@ static const struct scmi_perf_proto_ops perf_proto_ops = { .freq_get = scmi_dvfs_freq_get, .est_power_get = scmi_dvfs_est_power_get, .fast_switch_possible = scmi_fast_switch_possible, + .fast_switch_rate_limit = scmi_fast_switch_rate_limit, .power_scale_get = scmi_power_scale_get, }; diff --git a/drivers/firmware/arm_scmi/powercap.c b/drivers/firmware/arm_scmi/powercap.c index a4c6cd4716fe..604184c044ff 100644 --- a/drivers/firmware/arm_scmi/powercap.c +++ b/drivers/firmware/arm_scmi/powercap.c @@ -703,20 +703,24 @@ static void scmi_powercap_domain_init_fc(const struct scmi_protocol_handle *ph, ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, POWERCAP_CAP_SET, 4, domain, &fc[POWERCAP_FC_CAP].set_addr, - &fc[POWERCAP_FC_CAP].set_db); + &fc[POWERCAP_FC_CAP].set_db, + &fc[POWERCAP_FC_CAP].rate_limit); ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, POWERCAP_CAP_GET, 4, domain, - &fc[POWERCAP_FC_CAP].get_addr, NULL); + &fc[POWERCAP_FC_CAP].get_addr, NULL, + &fc[POWERCAP_FC_CAP].rate_limit); ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, POWERCAP_PAI_SET, 4, domain, &fc[POWERCAP_FC_PAI].set_addr, - &fc[POWERCAP_FC_PAI].set_db); + &fc[POWERCAP_FC_PAI].set_db, + &fc[POWERCAP_FC_PAI].rate_limit); ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, POWERCAP_PAI_GET, 4, domain, - &fc[POWERCAP_FC_PAI].get_addr, NULL); + &fc[POWERCAP_FC_PAI].get_addr, NULL, + &fc[POWERCAP_PAI_GET].rate_limit); *p_fc = fc; } diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h index e683c26f24eb..8b5d9ce4a33a 100644 --- a/drivers/firmware/arm_scmi/protocols.h +++ b/drivers/firmware/arm_scmi/protocols.h @@ -234,6 +234,7 @@ struct scmi_fc_info { void __iomem *set_addr; void __iomem *get_addr; struct scmi_fc_db_info *set_db; + u32 rate_limit; }; /** @@ -268,7 +269,8 @@ struct scmi_proto_helpers_ops { u8 describe_id, u32 message_id, u32 valid_size, u32 domain, void __iomem **p_addr, - struct scmi_fc_db_info **p_db); + struct scmi_fc_db_info **p_db, + u32 *rate_limit); void (*fastchannel_db_ring)(struct scmi_fc_db_info *db); }; diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index acd956ffcb84..fafedb3b6604 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -139,6 +139,8 @@ struct scmi_perf_domain_info { * at a given frequency * @fast_switch_possible: indicates if fast DVFS switching is possible or not * for a given device + * @fast_switch_rate_limit: gets the minimum time (us) required between + * successive fast_switching requests * @power_scale_mw_get: indicates if the power values provided are in milliWatts * or in some other (abstract) scale */ @@ -168,6 +170,8 @@ struct scmi_perf_proto_ops { unsigned long *rate, unsigned long *power); bool (*fast_switch_possible)(const struct scmi_protocol_handle *ph, u32 domain); + int (*fast_switch_rate_limit)(const struct scmi_protocol_handle *ph, + u32 domain, u32 *rate_limit); enum scmi_power_scale (*power_scale_get)(const struct scmi_protocol_handle *ph); }; -- 2.25.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] firmware: arm_scmi: Populate fast channel rate_limit 2024-02-22 13:57 ` Pierre Gondois @ 2024-03-04 8:00 ` Cristian Marussi -1 siblings, 0 replies; 24+ messages in thread From: Cristian Marussi @ 2024-03-04 8:00 UTC (permalink / raw) To: Pierre Gondois Cc: linux-kernel, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm On Thu, Feb 22, 2024 at 02:57:00PM +0100, Pierre Gondois wrote: > Arm SCMI spec. v3.2, s4.5.3.12 PERFORMANCE_DESCRIBE_FASTCHANNEL > defines a per-domain rate_limit for performance requests: > """ > Rate Limit in microseconds, indicating the minimum time > required between successive requests. A value of 0 > indicates that this field is not applicable or supported > on the platform. > """" > The field is first defined in SCMI v2.0. > > Add support to fetch this value and advertise it through > a fast_switch_rate_limit() callback. > > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> Hi, LGTM. Reviewed-by: Cristian Marussi <cristian.marussi@arm.com> Thanks, Cristian > --- > drivers/firmware/arm_scmi/driver.c | 5 ++++- > drivers/firmware/arm_scmi/perf.c | 32 +++++++++++++++++++++++---- > drivers/firmware/arm_scmi/powercap.c | 12 ++++++---- > drivers/firmware/arm_scmi/protocols.h | 4 +++- > include/linux/scmi_protocol.h | 4 ++++ > 5 files changed, 47 insertions(+), 10 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c > index 3ea64b22cf0d..1d38ecfafc59 100644 > --- a/drivers/firmware/arm_scmi/driver.c > +++ b/drivers/firmware/arm_scmi/driver.c > @@ -1617,7 +1617,7 @@ static void > scmi_common_fastchannel_init(const struct scmi_protocol_handle *ph, > u8 describe_id, u32 message_id, u32 valid_size, > u32 domain, void __iomem **p_addr, > - struct scmi_fc_db_info **p_db) > + struct scmi_fc_db_info **p_db, u32 *rate_limit) > { > int ret; > u32 flags; > @@ -1661,6 +1661,9 @@ scmi_common_fastchannel_init(const struct scmi_protocol_handle *ph, > goto err_xfer; > } > > + if (rate_limit) > + *rate_limit = le32_to_cpu(resp->rate_limit) & GENMASK(19, 0); > + > phys_addr = le32_to_cpu(resp->chan_addr_low); > phys_addr |= (u64)le32_to_cpu(resp->chan_addr_high) << 32; > addr = devm_ioremap(ph->dev, phys_addr, size); > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c > index 37c80376bd0a..fbcbd703198a 100644 > --- a/drivers/firmware/arm_scmi/perf.c > +++ b/drivers/firmware/arm_scmi/perf.c > @@ -789,23 +789,27 @@ static void scmi_perf_domain_init_fc(const struct scmi_protocol_handle *ph, > > ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, > PERF_LEVEL_GET, 4, dom->id, > - &fc[PERF_FC_LEVEL].get_addr, NULL); > + &fc[PERF_FC_LEVEL].get_addr, NULL, > + &fc[PERF_FC_LEVEL].rate_limit); > > ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, > PERF_LIMITS_GET, 8, dom->id, > - &fc[PERF_FC_LIMIT].get_addr, NULL); > + &fc[PERF_FC_LIMIT].get_addr, NULL, > + &fc[PERF_FC_LIMIT].rate_limit); > > if (dom->info.set_perf) > ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, > PERF_LEVEL_SET, 4, dom->id, > &fc[PERF_FC_LEVEL].set_addr, > - &fc[PERF_FC_LEVEL].set_db); > + &fc[PERF_FC_LEVEL].set_db, > + &fc[PERF_FC_LEVEL].rate_limit); > > if (dom->set_limits) > ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, > PERF_LIMITS_SET, 8, dom->id, > &fc[PERF_FC_LIMIT].set_addr, > - &fc[PERF_FC_LIMIT].set_db); > + &fc[PERF_FC_LIMIT].set_db, > + &fc[PERF_FC_LIMIT].rate_limit); > > dom->fc_info = fc; > } > @@ -974,6 +978,25 @@ static bool scmi_fast_switch_possible(const struct scmi_protocol_handle *ph, > return dom->fc_info && dom->fc_info[PERF_FC_LEVEL].set_addr; > } > > +static int scmi_fast_switch_rate_limit(const struct scmi_protocol_handle *ph, > + u32 domain, u32 *rate_limit) > +{ > + struct perf_dom_info *dom; > + > + if (!rate_limit) > + return -EINVAL; > + > + dom = scmi_perf_domain_lookup(ph, domain); > + if (IS_ERR(dom)) > + return PTR_ERR(dom); > + > + if (!dom->fc_info) > + return -EINVAL; > + > + *rate_limit = dom->fc_info[PERF_FC_LEVEL].rate_limit; > + return 0; > +} > + > static enum scmi_power_scale > scmi_power_scale_get(const struct scmi_protocol_handle *ph) > { > @@ -996,6 +1019,7 @@ static const struct scmi_perf_proto_ops perf_proto_ops = { > .freq_get = scmi_dvfs_freq_get, > .est_power_get = scmi_dvfs_est_power_get, > .fast_switch_possible = scmi_fast_switch_possible, > + .fast_switch_rate_limit = scmi_fast_switch_rate_limit, > .power_scale_get = scmi_power_scale_get, > }; > > diff --git a/drivers/firmware/arm_scmi/powercap.c b/drivers/firmware/arm_scmi/powercap.c > index a4c6cd4716fe..604184c044ff 100644 > --- a/drivers/firmware/arm_scmi/powercap.c > +++ b/drivers/firmware/arm_scmi/powercap.c > @@ -703,20 +703,24 @@ static void scmi_powercap_domain_init_fc(const struct scmi_protocol_handle *ph, > ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, > POWERCAP_CAP_SET, 4, domain, > &fc[POWERCAP_FC_CAP].set_addr, > - &fc[POWERCAP_FC_CAP].set_db); > + &fc[POWERCAP_FC_CAP].set_db, > + &fc[POWERCAP_FC_CAP].rate_limit); > > ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, > POWERCAP_CAP_GET, 4, domain, > - &fc[POWERCAP_FC_CAP].get_addr, NULL); > + &fc[POWERCAP_FC_CAP].get_addr, NULL, > + &fc[POWERCAP_FC_CAP].rate_limit); > > ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, > POWERCAP_PAI_SET, 4, domain, > &fc[POWERCAP_FC_PAI].set_addr, > - &fc[POWERCAP_FC_PAI].set_db); > + &fc[POWERCAP_FC_PAI].set_db, > + &fc[POWERCAP_FC_PAI].rate_limit); > > ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, > POWERCAP_PAI_GET, 4, domain, > - &fc[POWERCAP_FC_PAI].get_addr, NULL); > + &fc[POWERCAP_FC_PAI].get_addr, NULL, > + &fc[POWERCAP_PAI_GET].rate_limit); > > *p_fc = fc; > } > diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h > index e683c26f24eb..8b5d9ce4a33a 100644 > --- a/drivers/firmware/arm_scmi/protocols.h > +++ b/drivers/firmware/arm_scmi/protocols.h > @@ -234,6 +234,7 @@ struct scmi_fc_info { > void __iomem *set_addr; > void __iomem *get_addr; > struct scmi_fc_db_info *set_db; > + u32 rate_limit; > }; > > /** > @@ -268,7 +269,8 @@ struct scmi_proto_helpers_ops { > u8 describe_id, u32 message_id, > u32 valid_size, u32 domain, > void __iomem **p_addr, > - struct scmi_fc_db_info **p_db); > + struct scmi_fc_db_info **p_db, > + u32 *rate_limit); > void (*fastchannel_db_ring)(struct scmi_fc_db_info *db); > }; > > diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h > index acd956ffcb84..fafedb3b6604 100644 > --- a/include/linux/scmi_protocol.h > +++ b/include/linux/scmi_protocol.h > @@ -139,6 +139,8 @@ struct scmi_perf_domain_info { > * at a given frequency > * @fast_switch_possible: indicates if fast DVFS switching is possible or not > * for a given device > + * @fast_switch_rate_limit: gets the minimum time (us) required between > + * successive fast_switching requests > * @power_scale_mw_get: indicates if the power values provided are in milliWatts > * or in some other (abstract) scale > */ > @@ -168,6 +170,8 @@ struct scmi_perf_proto_ops { > unsigned long *rate, unsigned long *power); > bool (*fast_switch_possible)(const struct scmi_protocol_handle *ph, > u32 domain); > + int (*fast_switch_rate_limit)(const struct scmi_protocol_handle *ph, > + u32 domain, u32 *rate_limit); > enum scmi_power_scale (*power_scale_get)(const struct scmi_protocol_handle *ph); > }; > > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] firmware: arm_scmi: Populate fast channel rate_limit @ 2024-03-04 8:00 ` Cristian Marussi 0 siblings, 0 replies; 24+ messages in thread From: Cristian Marussi @ 2024-03-04 8:00 UTC (permalink / raw) To: Pierre Gondois Cc: linux-kernel, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm On Thu, Feb 22, 2024 at 02:57:00PM +0100, Pierre Gondois wrote: > Arm SCMI spec. v3.2, s4.5.3.12 PERFORMANCE_DESCRIBE_FASTCHANNEL > defines a per-domain rate_limit for performance requests: > """ > Rate Limit in microseconds, indicating the minimum time > required between successive requests. A value of 0 > indicates that this field is not applicable or supported > on the platform. > """" > The field is first defined in SCMI v2.0. > > Add support to fetch this value and advertise it through > a fast_switch_rate_limit() callback. > > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> Hi, LGTM. Reviewed-by: Cristian Marussi <cristian.marussi@arm.com> Thanks, Cristian > --- > drivers/firmware/arm_scmi/driver.c | 5 ++++- > drivers/firmware/arm_scmi/perf.c | 32 +++++++++++++++++++++++---- > drivers/firmware/arm_scmi/powercap.c | 12 ++++++---- > drivers/firmware/arm_scmi/protocols.h | 4 +++- > include/linux/scmi_protocol.h | 4 ++++ > 5 files changed, 47 insertions(+), 10 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c > index 3ea64b22cf0d..1d38ecfafc59 100644 > --- a/drivers/firmware/arm_scmi/driver.c > +++ b/drivers/firmware/arm_scmi/driver.c > @@ -1617,7 +1617,7 @@ static void > scmi_common_fastchannel_init(const struct scmi_protocol_handle *ph, > u8 describe_id, u32 message_id, u32 valid_size, > u32 domain, void __iomem **p_addr, > - struct scmi_fc_db_info **p_db) > + struct scmi_fc_db_info **p_db, u32 *rate_limit) > { > int ret; > u32 flags; > @@ -1661,6 +1661,9 @@ scmi_common_fastchannel_init(const struct scmi_protocol_handle *ph, > goto err_xfer; > } > > + if (rate_limit) > + *rate_limit = le32_to_cpu(resp->rate_limit) & GENMASK(19, 0); > + > phys_addr = le32_to_cpu(resp->chan_addr_low); > phys_addr |= (u64)le32_to_cpu(resp->chan_addr_high) << 32; > addr = devm_ioremap(ph->dev, phys_addr, size); > diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c > index 37c80376bd0a..fbcbd703198a 100644 > --- a/drivers/firmware/arm_scmi/perf.c > +++ b/drivers/firmware/arm_scmi/perf.c > @@ -789,23 +789,27 @@ static void scmi_perf_domain_init_fc(const struct scmi_protocol_handle *ph, > > ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, > PERF_LEVEL_GET, 4, dom->id, > - &fc[PERF_FC_LEVEL].get_addr, NULL); > + &fc[PERF_FC_LEVEL].get_addr, NULL, > + &fc[PERF_FC_LEVEL].rate_limit); > > ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, > PERF_LIMITS_GET, 8, dom->id, > - &fc[PERF_FC_LIMIT].get_addr, NULL); > + &fc[PERF_FC_LIMIT].get_addr, NULL, > + &fc[PERF_FC_LIMIT].rate_limit); > > if (dom->info.set_perf) > ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, > PERF_LEVEL_SET, 4, dom->id, > &fc[PERF_FC_LEVEL].set_addr, > - &fc[PERF_FC_LEVEL].set_db); > + &fc[PERF_FC_LEVEL].set_db, > + &fc[PERF_FC_LEVEL].rate_limit); > > if (dom->set_limits) > ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL, > PERF_LIMITS_SET, 8, dom->id, > &fc[PERF_FC_LIMIT].set_addr, > - &fc[PERF_FC_LIMIT].set_db); > + &fc[PERF_FC_LIMIT].set_db, > + &fc[PERF_FC_LIMIT].rate_limit); > > dom->fc_info = fc; > } > @@ -974,6 +978,25 @@ static bool scmi_fast_switch_possible(const struct scmi_protocol_handle *ph, > return dom->fc_info && dom->fc_info[PERF_FC_LEVEL].set_addr; > } > > +static int scmi_fast_switch_rate_limit(const struct scmi_protocol_handle *ph, > + u32 domain, u32 *rate_limit) > +{ > + struct perf_dom_info *dom; > + > + if (!rate_limit) > + return -EINVAL; > + > + dom = scmi_perf_domain_lookup(ph, domain); > + if (IS_ERR(dom)) > + return PTR_ERR(dom); > + > + if (!dom->fc_info) > + return -EINVAL; > + > + *rate_limit = dom->fc_info[PERF_FC_LEVEL].rate_limit; > + return 0; > +} > + > static enum scmi_power_scale > scmi_power_scale_get(const struct scmi_protocol_handle *ph) > { > @@ -996,6 +1019,7 @@ static const struct scmi_perf_proto_ops perf_proto_ops = { > .freq_get = scmi_dvfs_freq_get, > .est_power_get = scmi_dvfs_est_power_get, > .fast_switch_possible = scmi_fast_switch_possible, > + .fast_switch_rate_limit = scmi_fast_switch_rate_limit, > .power_scale_get = scmi_power_scale_get, > }; > > diff --git a/drivers/firmware/arm_scmi/powercap.c b/drivers/firmware/arm_scmi/powercap.c > index a4c6cd4716fe..604184c044ff 100644 > --- a/drivers/firmware/arm_scmi/powercap.c > +++ b/drivers/firmware/arm_scmi/powercap.c > @@ -703,20 +703,24 @@ static void scmi_powercap_domain_init_fc(const struct scmi_protocol_handle *ph, > ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, > POWERCAP_CAP_SET, 4, domain, > &fc[POWERCAP_FC_CAP].set_addr, > - &fc[POWERCAP_FC_CAP].set_db); > + &fc[POWERCAP_FC_CAP].set_db, > + &fc[POWERCAP_FC_CAP].rate_limit); > > ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, > POWERCAP_CAP_GET, 4, domain, > - &fc[POWERCAP_FC_CAP].get_addr, NULL); > + &fc[POWERCAP_FC_CAP].get_addr, NULL, > + &fc[POWERCAP_FC_CAP].rate_limit); > > ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, > POWERCAP_PAI_SET, 4, domain, > &fc[POWERCAP_FC_PAI].set_addr, > - &fc[POWERCAP_FC_PAI].set_db); > + &fc[POWERCAP_FC_PAI].set_db, > + &fc[POWERCAP_FC_PAI].rate_limit); > > ph->hops->fastchannel_init(ph, POWERCAP_DESCRIBE_FASTCHANNEL, > POWERCAP_PAI_GET, 4, domain, > - &fc[POWERCAP_FC_PAI].get_addr, NULL); > + &fc[POWERCAP_FC_PAI].get_addr, NULL, > + &fc[POWERCAP_PAI_GET].rate_limit); > > *p_fc = fc; > } > diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h > index e683c26f24eb..8b5d9ce4a33a 100644 > --- a/drivers/firmware/arm_scmi/protocols.h > +++ b/drivers/firmware/arm_scmi/protocols.h > @@ -234,6 +234,7 @@ struct scmi_fc_info { > void __iomem *set_addr; > void __iomem *get_addr; > struct scmi_fc_db_info *set_db; > + u32 rate_limit; > }; > > /** > @@ -268,7 +269,8 @@ struct scmi_proto_helpers_ops { > u8 describe_id, u32 message_id, > u32 valid_size, u32 domain, > void __iomem **p_addr, > - struct scmi_fc_db_info **p_db); > + struct scmi_fc_db_info **p_db, > + u32 *rate_limit); > void (*fastchannel_db_ring)(struct scmi_fc_db_info *db); > }; > > diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h > index acd956ffcb84..fafedb3b6604 100644 > --- a/include/linux/scmi_protocol.h > +++ b/include/linux/scmi_protocol.h > @@ -139,6 +139,8 @@ struct scmi_perf_domain_info { > * at a given frequency > * @fast_switch_possible: indicates if fast DVFS switching is possible or not > * for a given device > + * @fast_switch_rate_limit: gets the minimum time (us) required between > + * successive fast_switching requests > * @power_scale_mw_get: indicates if the power values provided are in milliWatts > * or in some other (abstract) scale > */ > @@ -168,6 +170,8 @@ struct scmi_perf_proto_ops { > unsigned long *rate, unsigned long *power); > bool (*fast_switch_possible)(const struct scmi_protocol_handle *ph, > u32 domain); > + int (*fast_switch_rate_limit)(const struct scmi_protocol_handle *ph, > + u32 domain, u32 *rate_limit); > enum scmi_power_scale (*power_scale_get)(const struct scmi_protocol_handle *ph); > }; > > -- > 2.25.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] firmware: arm_scmi: Populate fast channel rate_limit 2024-02-22 13:57 ` Pierre Gondois @ 2024-03-05 11:46 ` Sudeep Holla -1 siblings, 0 replies; 24+ messages in thread From: Sudeep Holla @ 2024-03-05 11:46 UTC (permalink / raw) To: Pierre Gondois Cc: linux-kernel, Sudeep Holla, Cristian Marussi, Christian Loehle, Ionela Voinescu, Dietmar Eggemann, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm On Thu, Feb 22, 2024 at 02:57:00PM +0100, Pierre Gondois wrote: > Arm SCMI spec. v3.2, s4.5.3.12 PERFORMANCE_DESCRIBE_FASTCHANNEL > defines a per-domain rate_limit for performance requests: > """ > Rate Limit in microseconds, indicating the minimum time > required between successive requests. A value of 0 > indicates that this field is not applicable or supported > on the platform. > """" > The field is first defined in SCMI v2.0. > > Add support to fetch this value and advertise it through > a fast_switch_rate_limit() callback. > I did a quick check and these changes doesn't conflict with -next SCMI content. So if Viresh is happy to pick them up for v6.9, Acked-by: Sudeep Holla <sudeep.holla@arm.com> -- Regards, Sudeep ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] firmware: arm_scmi: Populate fast channel rate_limit @ 2024-03-05 11:46 ` Sudeep Holla 0 siblings, 0 replies; 24+ messages in thread From: Sudeep Holla @ 2024-03-05 11:46 UTC (permalink / raw) To: Pierre Gondois Cc: linux-kernel, Sudeep Holla, Cristian Marussi, Christian Loehle, Ionela Voinescu, Dietmar Eggemann, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm On Thu, Feb 22, 2024 at 02:57:00PM +0100, Pierre Gondois wrote: > Arm SCMI spec. v3.2, s4.5.3.12 PERFORMANCE_DESCRIBE_FASTCHANNEL > defines a per-domain rate_limit for performance requests: > """ > Rate Limit in microseconds, indicating the minimum time > required between successive requests. A value of 0 > indicates that this field is not applicable or supported > on the platform. > """" > The field is first defined in SCMI v2.0. > > Add support to fetch this value and advertise it through > a fast_switch_rate_limit() callback. > I did a quick check and these changes doesn't conflict with -next SCMI content. So if Viresh is happy to pick them up for v6.9, Acked-by: Sudeep Holla <sudeep.holla@arm.com> -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/3] cpufreq: scmi: Set transition_delay_us 2024-02-22 13:56 ` Pierre Gondois @ 2024-02-22 13:57 ` Pierre Gondois -1 siblings, 0 replies; 24+ messages in thread From: Pierre Gondois @ 2024-02-22 13:57 UTC (permalink / raw) To: linux-kernel Cc: Cristian Marussi, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Pierre Gondois, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm Make use of the newly added callbacks: - rate_limit_get() - fast_switch_rate_limit() to populate policies's `transition_delay_us`, defined as the 'Preferred average time interval between consecutive invocations of the driver to set the frequency for this policy.' Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> --- drivers/cpufreq/scmi-cpufreq.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c index 4ee23f4ebf4a..0b483bd0d3ca 100644 --- a/drivers/cpufreq/scmi-cpufreq.c +++ b/drivers/cpufreq/scmi-cpufreq.c @@ -144,6 +144,29 @@ scmi_get_cpu_power(struct device *cpu_dev, unsigned long *power, return 0; } +static int +scmi_get_rate_limit(u32 domain, bool has_fast_switch) +{ + int ret, rate_limit; + + if (has_fast_switch) { + /* + * Fast channels are used whenever available, + * so use their rate_limit value if populated. + */ + ret = perf_ops->fast_switch_rate_limit(ph, domain, + &rate_limit); + if (!ret && rate_limit) + return rate_limit; + } + + ret = perf_ops->rate_limit_get(ph, domain, &rate_limit); + if (ret) + return 0; + + return rate_limit; +} + static int scmi_cpufreq_init(struct cpufreq_policy *policy) { int ret, nr_opp, domain; @@ -250,6 +273,9 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy) policy->fast_switch_possible = perf_ops->fast_switch_possible(ph, domain); + policy->transition_delay_us = + scmi_get_rate_limit(domain, policy->fast_switch_possible); + return 0; out_free_opp: -- 2.25.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/3] cpufreq: scmi: Set transition_delay_us @ 2024-02-22 13:57 ` Pierre Gondois 0 siblings, 0 replies; 24+ messages in thread From: Pierre Gondois @ 2024-02-22 13:57 UTC (permalink / raw) To: linux-kernel Cc: Cristian Marussi, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Pierre Gondois, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm Make use of the newly added callbacks: - rate_limit_get() - fast_switch_rate_limit() to populate policies's `transition_delay_us`, defined as the 'Preferred average time interval between consecutive invocations of the driver to set the frequency for this policy.' Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> --- drivers/cpufreq/scmi-cpufreq.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c index 4ee23f4ebf4a..0b483bd0d3ca 100644 --- a/drivers/cpufreq/scmi-cpufreq.c +++ b/drivers/cpufreq/scmi-cpufreq.c @@ -144,6 +144,29 @@ scmi_get_cpu_power(struct device *cpu_dev, unsigned long *power, return 0; } +static int +scmi_get_rate_limit(u32 domain, bool has_fast_switch) +{ + int ret, rate_limit; + + if (has_fast_switch) { + /* + * Fast channels are used whenever available, + * so use their rate_limit value if populated. + */ + ret = perf_ops->fast_switch_rate_limit(ph, domain, + &rate_limit); + if (!ret && rate_limit) + return rate_limit; + } + + ret = perf_ops->rate_limit_get(ph, domain, &rate_limit); + if (ret) + return 0; + + return rate_limit; +} + static int scmi_cpufreq_init(struct cpufreq_policy *policy) { int ret, nr_opp, domain; @@ -250,6 +273,9 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy) policy->fast_switch_possible = perf_ops->fast_switch_possible(ph, domain); + policy->transition_delay_us = + scmi_get_rate_limit(domain, policy->fast_switch_possible); + return 0; out_free_opp: -- 2.25.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] cpufreq: scmi: Set transition_delay_us 2024-02-22 13:57 ` Pierre Gondois @ 2024-03-04 8:15 ` Cristian Marussi -1 siblings, 0 replies; 24+ messages in thread From: Cristian Marussi @ 2024-03-04 8:15 UTC (permalink / raw) To: Pierre Gondois Cc: linux-kernel, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm On Thu, Feb 22, 2024 at 02:57:01PM +0100, Pierre Gondois wrote: > Make use of the newly added callbacks: > - rate_limit_get() > - fast_switch_rate_limit() > to populate policies's `transition_delay_us`, defined as the > 'Preferred average time interval between consecutive > invocations of the driver to set the frequency for this policy.' > > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> > --- > drivers/cpufreq/scmi-cpufreq.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > Hi, > diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c > index 4ee23f4ebf4a..0b483bd0d3ca 100644 > --- a/drivers/cpufreq/scmi-cpufreq.c > +++ b/drivers/cpufreq/scmi-cpufreq.c > @@ -144,6 +144,29 @@ scmi_get_cpu_power(struct device *cpu_dev, unsigned long *power, > return 0; > } > > +static int > +scmi_get_rate_limit(u32 domain, bool has_fast_switch) > +{ > + int ret, rate_limit; > + > + if (has_fast_switch) { > + /* > + * Fast channels are used whenever available, > + * so use their rate_limit value if populated. > + */ > + ret = perf_ops->fast_switch_rate_limit(ph, domain, > + &rate_limit); > + if (!ret && rate_limit) > + return rate_limit; > + } > + > + ret = perf_ops->rate_limit_get(ph, domain, &rate_limit); > + if (ret) > + return 0; > + > + return rate_limit; > +} > + > static int scmi_cpufreq_init(struct cpufreq_policy *policy) > { > int ret, nr_opp, domain; > @@ -250,6 +273,9 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy) > policy->fast_switch_possible = > perf_ops->fast_switch_possible(ph, domain); > > + policy->transition_delay_us = > + scmi_get_rate_limit(domain, policy->fast_switch_possible); > + > return 0; > As a second thought, I have just realized that now we have 2 ops to get the rate_limit for a domain, one used in case of FCs and another in case of std messaging w/out FCs, BUT given that we always use FCs when available, AND we do not indeed have any way from perf_ops to explicitly request a set/get ops NOT to use FCs when available, does it even make sense to expose such 2 functions ? Do we need such flexibility ? Shouldn't we just expose one single rate_limit perf_ops and let the SCMI core decide what to return depending on the presence or not of the FCs for that domain ? Maybe @Sudeep thinks differently. Thanks, Cristian ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] cpufreq: scmi: Set transition_delay_us @ 2024-03-04 8:15 ` Cristian Marussi 0 siblings, 0 replies; 24+ messages in thread From: Cristian Marussi @ 2024-03-04 8:15 UTC (permalink / raw) To: Pierre Gondois Cc: linux-kernel, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Rafael J. Wysocki, Viresh Kumar, linux-arm-kernel, linux-pm On Thu, Feb 22, 2024 at 02:57:01PM +0100, Pierre Gondois wrote: > Make use of the newly added callbacks: > - rate_limit_get() > - fast_switch_rate_limit() > to populate policies's `transition_delay_us`, defined as the > 'Preferred average time interval between consecutive > invocations of the driver to set the frequency for this policy.' > > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> > --- > drivers/cpufreq/scmi-cpufreq.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > Hi, > diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c > index 4ee23f4ebf4a..0b483bd0d3ca 100644 > --- a/drivers/cpufreq/scmi-cpufreq.c > +++ b/drivers/cpufreq/scmi-cpufreq.c > @@ -144,6 +144,29 @@ scmi_get_cpu_power(struct device *cpu_dev, unsigned long *power, > return 0; > } > > +static int > +scmi_get_rate_limit(u32 domain, bool has_fast_switch) > +{ > + int ret, rate_limit; > + > + if (has_fast_switch) { > + /* > + * Fast channels are used whenever available, > + * so use their rate_limit value if populated. > + */ > + ret = perf_ops->fast_switch_rate_limit(ph, domain, > + &rate_limit); > + if (!ret && rate_limit) > + return rate_limit; > + } > + > + ret = perf_ops->rate_limit_get(ph, domain, &rate_limit); > + if (ret) > + return 0; > + > + return rate_limit; > +} > + > static int scmi_cpufreq_init(struct cpufreq_policy *policy) > { > int ret, nr_opp, domain; > @@ -250,6 +273,9 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy) > policy->fast_switch_possible = > perf_ops->fast_switch_possible(ph, domain); > > + policy->transition_delay_us = > + scmi_get_rate_limit(domain, policy->fast_switch_possible); > + > return 0; > As a second thought, I have just realized that now we have 2 ops to get the rate_limit for a domain, one used in case of FCs and another in case of std messaging w/out FCs, BUT given that we always use FCs when available, AND we do not indeed have any way from perf_ops to explicitly request a set/get ops NOT to use FCs when available, does it even make sense to expose such 2 functions ? Do we need such flexibility ? Shouldn't we just expose one single rate_limit perf_ops and let the SCMI core decide what to return depending on the presence or not of the FCs for that domain ? Maybe @Sudeep thinks differently. Thanks, Cristian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/3] scmi-cpufreq: Set transition_delay_us 2024-02-22 13:56 ` Pierre Gondois @ 2024-03-04 7:00 ` Viresh Kumar -1 siblings, 0 replies; 24+ messages in thread From: Viresh Kumar @ 2024-03-04 7:00 UTC (permalink / raw) To: Pierre Gondois Cc: linux-kernel, Cristian Marussi, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Rafael J. Wysocki, linux-arm-kernel, linux-pm On 22-02-24, 14:56, Pierre Gondois wrote: > policy's fields definitions: > `transition_delay_us`: > The minimum amount of time between two consecutive freq. requests > for one policy. > `transition_latency`: > Delta between freq. change request and effective freq. change on > the hardware. > > cpufreq_policy_transition_delay_us() uses the `transition_delay_us` > value if available. Otherwise a value is induced from the policy's > `transition_latency`. > > The scmi-cpufreq driver doesn't populate the `transition_delay_us`. > Values matching the definition are available through the SCMI > specification. > Add support to fetch these values and use them in the scmi-cpufreq > driver. How do we merge this series ? I can only pick the last commit. -- viresh ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/3] scmi-cpufreq: Set transition_delay_us @ 2024-03-04 7:00 ` Viresh Kumar 0 siblings, 0 replies; 24+ messages in thread From: Viresh Kumar @ 2024-03-04 7:00 UTC (permalink / raw) To: Pierre Gondois Cc: linux-kernel, Cristian Marussi, Christian Loehle, Ionela Voinescu, Sudeep Holla, Dietmar Eggemann, Rafael J. Wysocki, linux-arm-kernel, linux-pm On 22-02-24, 14:56, Pierre Gondois wrote: > policy's fields definitions: > `transition_delay_us`: > The minimum amount of time between two consecutive freq. requests > for one policy. > `transition_latency`: > Delta between freq. change request and effective freq. change on > the hardware. > > cpufreq_policy_transition_delay_us() uses the `transition_delay_us` > value if available. Otherwise a value is induced from the policy's > `transition_latency`. > > The scmi-cpufreq driver doesn't populate the `transition_delay_us`. > Values matching the definition are available through the SCMI > specification. > Add support to fetch these values and use them in the scmi-cpufreq > driver. How do we merge this series ? I can only pick the last commit. -- viresh _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/3] scmi-cpufreq: Set transition_delay_us 2024-03-04 7:00 ` Viresh Kumar @ 2024-03-04 11:42 ` Sudeep Holla -1 siblings, 0 replies; 24+ messages in thread From: Sudeep Holla @ 2024-03-04 11:42 UTC (permalink / raw) To: Viresh Kumar Cc: Pierre Gondois, linux-kernel, Cristian Marussi, Christian Loehle, Sudeep Holla, Ionela Voinescu, Dietmar Eggemann, Rafael J. Wysocki, linux-arm-kernel, linux-pm On Mon, Mar 04, 2024 at 12:30:58PM +0530, Viresh Kumar wrote: > On 22-02-24, 14:56, Pierre Gondois wrote: > > policy's fields definitions: > > `transition_delay_us`: > > The minimum amount of time between two consecutive freq. requests > > for one policy. > > `transition_latency`: > > Delta between freq. change request and effective freq. change on > > the hardware. > > > > cpufreq_policy_transition_delay_us() uses the `transition_delay_us` > > value if available. Otherwise a value is induced from the policy's > > `transition_latency`. > > > > The scmi-cpufreq driver doesn't populate the `transition_delay_us`. > > Values matching the definition are available through the SCMI > > specification. > > Add support to fetch these values and use them in the scmi-cpufreq > > driver. > > How do we merge this series ? I can only pick the last commit. I have sent my PR for v6.9 already and was deferring this to v6.10 The changes look good to me. If it doesn't conflict much with -next SCMI content, then I am happy to ack and you can take all of them together. Otherwise we can revisit strategy at -rc1. Thoughts ? -- Regards, Sudeep ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/3] scmi-cpufreq: Set transition_delay_us @ 2024-03-04 11:42 ` Sudeep Holla 0 siblings, 0 replies; 24+ messages in thread From: Sudeep Holla @ 2024-03-04 11:42 UTC (permalink / raw) To: Viresh Kumar Cc: Pierre Gondois, linux-kernel, Cristian Marussi, Christian Loehle, Sudeep Holla, Ionela Voinescu, Dietmar Eggemann, Rafael J. Wysocki, linux-arm-kernel, linux-pm On Mon, Mar 04, 2024 at 12:30:58PM +0530, Viresh Kumar wrote: > On 22-02-24, 14:56, Pierre Gondois wrote: > > policy's fields definitions: > > `transition_delay_us`: > > The minimum amount of time between two consecutive freq. requests > > for one policy. > > `transition_latency`: > > Delta between freq. change request and effective freq. change on > > the hardware. > > > > cpufreq_policy_transition_delay_us() uses the `transition_delay_us` > > value if available. Otherwise a value is induced from the policy's > > `transition_latency`. > > > > The scmi-cpufreq driver doesn't populate the `transition_delay_us`. > > Values matching the definition are available through the SCMI > > specification. > > Add support to fetch these values and use them in the scmi-cpufreq > > driver. > > How do we merge this series ? I can only pick the last commit. I have sent my PR for v6.9 already and was deferring this to v6.10 The changes look good to me. If it doesn't conflict much with -next SCMI content, then I am happy to ack and you can take all of them together. Otherwise we can revisit strategy at -rc1. Thoughts ? -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/3] scmi-cpufreq: Set transition_delay_us 2024-03-04 11:42 ` Sudeep Holla @ 2024-03-06 5:24 ` Viresh Kumar -1 siblings, 0 replies; 24+ messages in thread From: Viresh Kumar @ 2024-03-06 5:24 UTC (permalink / raw) To: Sudeep Holla Cc: Pierre Gondois, linux-kernel, Cristian Marussi, Christian Loehle, Ionela Voinescu, Dietmar Eggemann, Rafael J. Wysocki, linux-arm-kernel, linux-pm On 04-03-24, 11:42, Sudeep Holla wrote: > On Mon, Mar 04, 2024 at 12:30:58PM +0530, Viresh Kumar wrote: > > On 22-02-24, 14:56, Pierre Gondois wrote: > > > policy's fields definitions: > > > `transition_delay_us`: > > > The minimum amount of time between two consecutive freq. requests > > > for one policy. > > > `transition_latency`: > > > Delta between freq. change request and effective freq. change on > > > the hardware. > > > > > > cpufreq_policy_transition_delay_us() uses the `transition_delay_us` > > > value if available. Otherwise a value is induced from the policy's > > > `transition_latency`. > > > > > > The scmi-cpufreq driver doesn't populate the `transition_delay_us`. > > > Values matching the definition are available through the SCMI > > > specification. > > > Add support to fetch these values and use them in the scmi-cpufreq > > > driver. > > > > How do we merge this series ? I can only pick the last commit. > > I have sent my PR for v6.9 already and was deferring this to v6.10 > The changes look good to me. If it doesn't conflict much with -next > SCMI content, then I am happy to ack and you can take all of them > together. Otherwise we can revisit strategy at -rc1. Thoughts ? Applied. Thanks. -- viresh ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/3] scmi-cpufreq: Set transition_delay_us @ 2024-03-06 5:24 ` Viresh Kumar 0 siblings, 0 replies; 24+ messages in thread From: Viresh Kumar @ 2024-03-06 5:24 UTC (permalink / raw) To: Sudeep Holla Cc: Pierre Gondois, linux-kernel, Cristian Marussi, Christian Loehle, Ionela Voinescu, Dietmar Eggemann, Rafael J. Wysocki, linux-arm-kernel, linux-pm On 04-03-24, 11:42, Sudeep Holla wrote: > On Mon, Mar 04, 2024 at 12:30:58PM +0530, Viresh Kumar wrote: > > On 22-02-24, 14:56, Pierre Gondois wrote: > > > policy's fields definitions: > > > `transition_delay_us`: > > > The minimum amount of time between two consecutive freq. requests > > > for one policy. > > > `transition_latency`: > > > Delta between freq. change request and effective freq. change on > > > the hardware. > > > > > > cpufreq_policy_transition_delay_us() uses the `transition_delay_us` > > > value if available. Otherwise a value is induced from the policy's > > > `transition_latency`. > > > > > > The scmi-cpufreq driver doesn't populate the `transition_delay_us`. > > > Values matching the definition are available through the SCMI > > > specification. > > > Add support to fetch these values and use them in the scmi-cpufreq > > > driver. > > > > How do we merge this series ? I can only pick the last commit. > > I have sent my PR for v6.9 already and was deferring this to v6.10 > The changes look good to me. If it doesn't conflict much with -next > SCMI content, then I am happy to ack and you can take all of them > together. Otherwise we can revisit strategy at -rc1. Thoughts ? Applied. Thanks. -- viresh _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2024-03-06 5:25 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-02-22 13:56 [PATCH 0/3] scmi-cpufreq: Set transition_delay_us Pierre Gondois 2024-02-22 13:56 ` Pierre Gondois 2024-02-22 13:56 ` [PATCH 1/3] firmware: arm_scmi: Populate perf commands rate_limit Pierre Gondois 2024-02-22 13:56 ` Pierre Gondois 2024-03-04 7:55 ` Cristian Marussi 2024-03-04 7:55 ` Cristian Marussi 2024-03-05 11:47 ` Sudeep Holla 2024-03-05 11:47 ` Sudeep Holla 2024-02-22 13:57 ` [PATCH 2/3] firmware: arm_scmi: Populate fast channel rate_limit Pierre Gondois 2024-02-22 13:57 ` Pierre Gondois 2024-03-04 8:00 ` Cristian Marussi 2024-03-04 8:00 ` Cristian Marussi 2024-03-05 11:46 ` Sudeep Holla 2024-03-05 11:46 ` Sudeep Holla 2024-02-22 13:57 ` [PATCH 3/3] cpufreq: scmi: Set transition_delay_us Pierre Gondois 2024-02-22 13:57 ` Pierre Gondois 2024-03-04 8:15 ` Cristian Marussi 2024-03-04 8:15 ` Cristian Marussi 2024-03-04 7:00 ` [PATCH 0/3] scmi-cpufreq: " Viresh Kumar 2024-03-04 7:00 ` Viresh Kumar 2024-03-04 11:42 ` Sudeep Holla 2024-03-04 11:42 ` Sudeep Holla 2024-03-06 5:24 ` Viresh Kumar 2024-03-06 5:24 ` Viresh Kumar
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.