Re: [RFT][PATCH 0/3] cpufreq / PM: QoS: Introduce frequency QoS and use it in cpufreq

["Rafael J. Wysocki" <rafael@kernel.org>]

On Wed, Oct 23, 2019 at 12:06 AM Leonard Crestez
<leonard.crestez@nxp.com> wrote:
>
> Hello,
>
> I've been working on a series which add DEV_PM_QOS support to devfreq,
> now at v9:
>
>         https://patchwork.kernel.org/cover/11171807/
>
> Your third patch removes DEV_PM_QOS_FREQUENCY_MIN/MAX that my series
> depends upon. I found the email on patchwork, hopefully the in-reply-to
> header is OK?
>
> As far as I can tell the replacement ("frequency qos") needs constraints
> to be managed outside the device infrastructure and it's not obviously
> usable a generic mechanism for making "min_freq/max_freq" requests to a
> specific device.

You can add a struct freq_constrants pointer to struct dev_pm_info and
use it just fine.  It doesn't have to be bolted into struct
dev_pm_qos.

> I've read a bit through your emails and it seems the problem is that
> you're dealing with dev_pm_qos on per-policy basis but each "struct
> cpufreq_policy" can cover multiple CPU devices.
>
> An alternative solution which follows dev_pm_qos would be to add
> notifiers for each CPU inside cpufreq_online and cpufreq_offline. This
> makes quite a bit of sense because each CPU is a separate "device" with
> a possibly distinct list of qos requests.

But combining the lists of requests for all the CPUs in a policy
defeats the idea of automatic aggregation of requests which really is
what PM QoS is about.

There have to be two lists of requests per policy, one for the max and
one for the min frequency.

> If cpufreq needs a group of CPUs to run at the same frequency then it
> should deal with this by doing dev_pm_qos_read_frequency on each CPU
> device and picking a frequency that attempts to satisfy all constraints.

No, that would be combining the requests by hand.

> Handling sysfs min/max_freq through dev_pm_qos would be of dubious
> value, though I guess you could register identical requests for each CPU.
>
> I'm not familiar with what you're trying to accomplish with PM_QOS other
> than replace the sysfs min_freq/max_freq files:

QoS-based management of the frequency limits is not really needed for
that.  The real motivation for adding it were things like thermal and
platform firmware induced limits that all have their own values to
combine with the ones provided by user space.

> What I want to do is add
> a driver using the interconnect driver which translates requests for
> "bandwidth-on-a-path" into "frequency-on-a-device". More specifically a
> display driver could request bandwidth to RAM and this would be
> translated into min frequency for NoC and the DDR controller, both of
> which implement scaling via devfreq:
>
>         https://patchwork.kernel.org/cover/11104113/
>         https://patchwork.kernel.org/cover/11111865/
>
> This is part of an effort to upstream an out-of-tree "busfreq" feature
> which allows device device to make "min frequency requests" through an
> entirely out-of-tree mechanism. It would also allow finer-grained
> scaling that what IMX tree currently support.
>
> If you're making cpufreq qos constrains be "per-cpufreq-policy" then
> it's not clear how you would handle in-kernel constraints from other
> subsystems. Would users have to get a pointer to struct cpufreq_policy
> and struct freq_constraints?

Yes.

> That would make object lifetime a nightmare!

Why really?  It is not much different from the device PM QoS case.

Actually, https://patchwork.kernel.org/patch/11193019/ is a simple
one-for-one replacement of the former.  As it turns out, all of its
users have access to a policy object anyway already.

> But dev_pm_qos solves this by tying to struct device.

Well, the cpufreq sysfs is per-policy and not per-CPU and we really
need a per-policy min and max frequency in cpufreq, for governors etc.

> And if you don't care about in-kernel requests then what's the purpose
> of involving PM QoS? The old min/max_freq sysfs implementation worked.

See above.

Thanks!