On 24 August 2018 at 12:38, Lorenzo Pieralisi [off-list ref] wrote:

On Fri, Aug 24, 2018 at 11:26:19AM +0200, Ulf Hansson wrote:

[...]

quoted

quoted

That's a good question and it maybe gives a path towards a solution.

AFAICS the genPD governor only selects the idle state parameter that
determines the idle state at, say, GenPD cpumask level it does not touch
the CPUidle decision, that works on a subset of idle states (at cpu
level).

That's my understanding, which can be wrong so please correct me
if that's the case because that's a bit confusing.

Let's imagine that we flattened out the list of idle states and feed
CPUidle with it (all of them - cpu, cluster, package, system - as it is
in the mainline _now_). Then the GenPD governor can run-through the
CPUidle selection and _demote_ the idle state if necessary since it
understands that some CPUs in the GenPD will wake up shortly and break
the target residency hyphothesis the CPUidle governor is expecting.

The whole idea about this series is improving CPUidle decision when
the target idle state is _shared_ among groups of cpus (again, please
do correct me if I am wrong).

Absolutely, this is one of the main reason for the series!

quoted

It is obvious that a GenPD governor must only demote - never promote a
CPU idle state selection given that hierarchy implies more power
savings and higher target residencies required.

Absolutely. I apologize if I have been using the word "promote"
wrongly, I realize it may be a bit confusing.

quoted

This whole series would become more generic and won't depend on
PSCI OSI at all - actually that would become a hierarchical
CPUidle governor.

Well, to me we need a first user of the new infrastructure code in
genpd and PSCI is probably the easiest one to start with. An option
would be to start with an old ARM32 platform, but it seems a bit silly
to me.

If the code can be structured as described above as a hierarchical
(possibly optional through a Kconfig entry or sysfs tuning) idle
decision you can apply it to _any_ PSCI based platform out there,
provided that the new governor improves power savings.

quoted

In regards to OS-initiated mode vs platform coordinated mode, let's
discuss that in details in the other email thread instead.

I think that's crystal clear by now that IMHO PSCI OS-initiated mode is
a red-herring, it has nothing to do with this series, it is there just
because QC firmware does not support PSCI platform coordinated suspend
mode.

I fully agree that the series isn't specific to PSCI OSI mode. On the
other hand, for PSCI OSI mode, that's where I see this series to fit
naturally. And in particular for the QCOM 410c board.

When it comes to the PSCI PC mode, it may under certain circumstances
be useful to deploy this approach for that as well, and I agree that
it seems reasonable to have that configurable as opt-in, somehow.

Although, let's discuss that separately, in a next step. Or at least
let's try to keep PSCI related technical discussions to the other
thread, as that makes it easier to follow.

You can apply the concept in this series to _any_ arch provided
the power domains representation is correct (and again, I would sound
like a broken record but the series must improve power savings over
vanilla CPUidle menu governor).

I agree, but let me elaborate a bit, to hopefully add some clarity,
which I may not have been able to communicate earlier.

The goal with the series is to enable platforms to support all its
available idlestates, which are shared among a group of CPUs. This is
the case for QCOM 410c, for example.

To my knowledge, we have other ARM32 based platforms that currently
have disabled some of its cluster idle states. That's because they
can't know when it's safe to power off the cluster "coherency domain",
in cases when the platform also have other shared resources in it.

The point is, to see improved power savings, additional platform
deployment may be needed and that just takes time. For example runtime
PM support is needed in those drivers that deals with the "shared
resources", a correctly modeled PM domain topology using genpd, etc,
etc.

quoted

quoted

I still think that PSCI firmware and most certainly mwait() play the
role the GenPD governor does since they can detect in FW/HW whether
that's worthwhile to switch off a domain, the information is obviously
there and the kernel would just add latency to the idle path in that
case but let's gloss over this for the sake of this discussion.

Yep, let's discuss that separately.

That said, can I interpret your comments on the series up until this
change, that you seems rather happy with where the series is going?

It is something we have been discussing with Daniel since generic idle
was merged for Arm a long while back. I have nothing against describing
idle states with power domains but it must improve idle decisions
against the mainline. As I said before, runtime PM can also be used
to get rid of CPU PM notifiers (because with power domains we KNOW
what devices eg PMU are switched off on idle entry, we do not guess
any longer; replacing CPU PM notifiers is challenging and can be
tackled - if required - in a different series).

Yes, we have be talking about the CPU PM and CPU_CLUSTER_PM notifiers
and I fully agree. It's something that we should look into and in
future steps.

Bottom line (talk is cheap, I know and apologise about that): this
series (up until this change) adds complexity to the idle path and lots
of code; if its usage is made optional and can be switched on on systems
where it saves power that's fine by me as long as we keep PSCI
OS-initiated idle states out of the equation, that's an orthogonal
discussion as, I hope, I managed to convey.

Thanks,
Lorenzo

Lorenzo, thanks for your feedback!

Please, when you have time, could you also reply to the other thread
we started, I would like to understand how I should proceed with this
series.

Kind regards
Uffe