| Message ID | cover.1550748118.git.viresh.kumar@linaro.org |
|---|---|
| Headers | show |
| Series | cpufreq: Use QoS layer to manage freq-constraints | expand |
On 25-02-19, 12:14, Qais Yousef wrote: > On 02/25/19 14:39, Viresh Kumar wrote: > > On 25-02-19, 08:58, Qais Yousef wrote: > > > On 02/25/19 10:01, Viresh Kumar wrote: > > > > > > + min = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MIN_FREQUENCY); > > > > > > + max = dev_pm_qos_read_value(cpu_dev, DEV_PM_QOS_MAX_FREQUENCY); > > > > > > + > > > > > > + if (min > new_policy->min) > > > > > > + new_policy->min = min; > > > > > > + if (max < new_policy->max) > > > > > > + new_policy->max = max; > > > > > And this is why we need to check here if the PM QoS value doesn't conflict with > > > the current min/max, right? Until the current notifier code is removed they > > > could trip over each others. > > > > No. The above if/else block is already removed as part of patch 5/5. It was > > required because of conflict between userspace specific min/max and qos min/max, > > which are migrated to use qos by patc 5/5. > > > > The cpufreq notifier mechanism already lets users play with min/max and that is > > already safe from conflicts. > > > > > > > It would be nice to add a comment here about PM QoS managing and remembering > > > values > > > > I am not sure if that would add any value. Some documentation update may be > > useful for people looking for details though, that I shall do after all the > > changes get in and things become a bit stable. > > > > Up to you. But not everyone is familiar with the code and a one line comment > that points to where aggregation is happening would be helpful for someone > scanning this code IMHO. Okay, will add something then. > > > and that we need to be careful that both mechanisms don't trip over > > > each others until this transient period is over. > > > > The second mechanism will die very very soon once this is merged, migrating them > > shouldn't be a big challenge AFAICT. I didn't attempt that because I didn't > > wanted to waste time updating things in case this version also doesn't make > > sense to others. > > > > > I have a nit too. It would be nice to explicitly state this is > > > CPU_{MIN,MAX}_FREQUENCY. I can see someone else adding {MIN,MAX}_FREQUENCY for > > > something elsee (memory maybe?) > > > > This is not CPU specific, but any device. The same interface shall be used by > > devfreq as well, who wanted to use freq-constraints initially. > > > > I don't get that to be honest. I probably have to read more. > > Is what you're saying that when applying a MIN_FREQUENCY constraint the same > value will be applied to both cpufreq and devfreq? Isn't this too coarse? Oh no. A constraint with QoS is added like this: dev_pm_qos_add_request(dev, req, DEV_PM_QOS_MIN_FREQUENCY, min); Now dev here can be any device struct, CPU's or GPU's or anything else. All the MIN freq requests are stored/processed per device and for a CPU in cpufreq all we will see is MIN requests for the CPUs. And so the macro is required to be a bit generic and shouldn't have CPU word within it. Hope I was able to clarify your doubt a bit. Thanks. -- viresh
On 21-02-19, 12:30, Rafael J. Wysocki wrote: > On Thursday, February 21, 2019 12:29:26 PM CET Viresh Kumar wrote: > > Hello, > > > > This patchset attempts to manage CPU frequency constraints using the PM > > QoS framework. It only does the basic stuff right now and moves the > > userspace constraints to use the QoS infrastructure. > > > > Todo: > > - Migrate all users to the QoS framework and get rid of cpufreq specific > > notifiers. > > - Make PM QoS learn about the relation of CPUs in a policy, so a single > > list of constraints is managed for all of them instead of per-cpu > > constraints. > > > > V1->V2: > > - The previous version introduced a completely new framework, this one > > moves to PM QoS instead. > > - Lots of changes because of this. > > Well, thanks for working on this, but I'm rather unlikely to look at it in > detail before 5.1-rc1 is released. Hi Rafael, This series was posted almost 3 months back. Any comments on this would be very helpful. -- viresh
Hello, This patchset attempts to manage CPU frequency constraints using the PM QoS framework. It only does the basic stuff right now and moves the userspace constraints to use the QoS infrastructure. Todo: - Migrate all users to the QoS framework and get rid of cpufreq specific notifiers. - Make PM QoS learn about the relation of CPUs in a policy, so a single list of constraints is managed for all of them instead of per-cpu constraints. V1->V2: - The previous version introduced a completely new framework, this one moves to PM QoS instead. - Lots of changes because of this. -- viresh Viresh Kumar (5): PM / QOS: Pass request type to dev_pm_qos_{add|remove}_notifier() PM / QOS: Pass request type to dev_pm_qos_read_value() PM / QoS: Add support for MIN/MAX frequency constraints cpufreq: Register notifiers with the PM QoS framework cpufreq: Add QoS requests for userspace constraints Documentation/power/pm_qos_interface.txt | 12 +- drivers/base/power/domain.c | 8 +- drivers/base/power/domain_governor.c | 4 +- drivers/base/power/qos.c | 115 +++++++++++-- drivers/base/power/runtime.c | 2 +- drivers/cpufreq/cpufreq.c | 202 ++++++++++++++++------- drivers/cpuidle/governor.c | 2 +- include/linux/cpufreq.h | 12 +- include/linux/pm_qos.h | 71 ++++++-- 9 files changed, 323 insertions(+), 105 deletions(-) -- 2.21.0.rc0.269.g1a574e7a288b