Re: [PATCH 2/3] pmdomain: Add support for system-suspend-only states

[Ulf Hansson]

On Fri, Oct 9, 2026 at 2:21 PM Sudeep Holla <sudeep.holla@xxxxxxxxxx> wrote:
>
> On Thu, Oct 08, 2026 at 02:21:49PM +0200, Ulf Hansson wrote:
> > On Tue, Oct 6, 2026 at 11:15 AM Sudeep Holla <sudeep.holla@xxxxxxxxxx> wrote:
> > >
> > > On Mon, Oct 05, 2026 at 08:59:43PM +0530, Maulik Shah wrote:
> > > > Some domain idle states require system-wide coordination and should not be
> > > > selected during regular CPU idle. However those states remain valid for
> > > > system-wide suspend like s2idle.
> > > >
> > > > Add a per-state system_state boolean and populate it from the system-state
> > > > property. Make the genpd governor skip these states during CPU idle. Leave
> > > > the system wide suspend path unchanged so s2idle can select them.
> > > >
> > >
> > > Instead of this I am thinking if we can QoS cpu latency setting and block
> > > system level states normally. Since s2idle is user driven, it should be
> > > controllable via user-space and we don't have to define bindings again
> > > if systems that use platform-coordinated needs this too. They may not
> > > use domain-idle-states.
> >
> > Even if we likely could make that work, it's seems not correct to rely
> > on userspace to make the kernel to pick the correct idle state, while
> > the decision should be based on the characteristics of the HW.
> >
> > In regards to PSCI PC mode, I believe we should consider adding the
> > similar DT property for the arm,idle-state binding and make a
> > corresponding change for the regular CPUIdle path/governors. Although,
> > it doesn't necessarily need to be part of the $subject series. I would
> >  be fine if that is handled later on too.
> >
>
> While I agree with this line of argument, I am bit worried that there is
> more deviation from ACPI _LPI binding(equivalent to these in DT). If other
> OS has manged to run the same firmware without adding such information
> to the firmware(ACPI in this case), why do we need that in DT. Is that
> information encoded elsewhere in a different form in ACPI and is this the
> right place(the idle state binding for DT) is what I am trying to understand
> here.

I don't have the information about ACPI at hand, so I will leave that
to Maulik to be answered. Or perhaps, someone else already has this
information and can share that with us?

In any case, I don't see any other reasonable way to describe this in
DT at this point.

>
> Also taking the example of screen on, won't that be prevented as the
> system domain is shared by the system level idle-states we are talking
> and the screen/display device. So it never votes to power down that
> and the PSCI OSI domain idle driver must not be able to choose the system
> level state.
>
> In general, there is some device that doesn't vote to power down the domain
> and hence we can't enter the state. So my question is, will there be some
> device that doesn't explicitly register vote and hence we need an alternate
> mechanism like the one proposed in this set ? If so, what are those and
> how does that work in system suspend states.

Assuming I understand the above use case, you are thinking that the
display device (for example) is a part of a cluster PM domain, right?

If that's the case, besides that the cluster PM domain topology must
be modelled accordingly, the display device should be hooked up to the
cluster PM domain it belongs to.

Beyond that, it's the corresponding display driver that becomes
responsible for what should happen during system suspend. If the
screen needs to stay on it may need to call device_set_awake_path()
for the display device - unless it's a decision for user space that
can control the behaviour for the display device via its sysfs node.
If things are managed accordingly, genpd_finish_suspend() may decide
to keep the corresponding PM domain powered on during system suspend.

The above said, $subject patch doesn't change any of this. It only
allows us to use more states (deeper), which are limited to system
suspend (s2idle).

>
> Or am I missing something more fundamental the makes all the above argument
> void.
>
> --
> Regards,
> Sudeep

Did that make sense?

Kind regards
Uffe