Lets not expect domains marked as `RPM_ALWAYS_ON`/`ALWAYS_ON` to be ON at boot

Lets not expect domains marked as `RPM_ALWAYS_ON`/`ALWAYS_ON` to be ON at boot

[Ajay Agarwal]

Hello Linux PM experts

For a given power domain, if the `genpd->flags` has the GENPD_FLAG_RPM_ALWAYS_ON
flag set, then pm_genpd_init expects that domain to be ON:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/power/domain.c?h=v6.7-rc3#n2102

I have a usecase where the power domain is not ON by default on kernel boot,
but I also want it to stay ON during RPM. But the current logic fails the
`pm_genpd_init` for me.

IMO, the kernel should not force a PD which needs to stay ON during RPM to be ON
at boot also. It is quite possible that the PD will be turned ON for the very
first time by a client driver in the kernel itself.

Additionally, I think the kernel as well should not enforce this for PDs which 
do not want to turn OFF ever, i.e., the ones which have `GENPD_FLAG_ALWAYS_ON`
flag set. Let the kernel turn ON these PDs when it wants and only then prevent
the RPM or system suspend from turning these domains OFF.

Proposed patch in pm_genpd_init:
```
-       /* Always-on domains must be powered on at initialization. */
-       if ((genpd_is_always_on(genpd) || genpd_is_rpm_always_on(genpd)) &&
-                       !genpd_status_on(genpd)) {
-               pr_err("always-on PM domain %s is not on\n", genpd->name);
-               return -EINVAL;
-       }
-
```

Looking for your views here.

Thanks
Ajay

Re: Lets not expect domains marked as `RPM_ALWAYS_ON`/`ALWAYS_ON` to be ON at boot

[Ulf Hansson]

On Fri, 1 Dec 2023 at 07:06, Ajay Agarwal <ajayagarwal@google.com> wrote:
>
> Hello Linux PM experts
>
> For a given power domain, if the `genpd->flags` has the GENPD_FLAG_RPM_ALWAYS_ON
> flag set, then pm_genpd_init expects that domain to be ON:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/power/domain.c?h=v6.7-rc3#n2102
>
> I have a usecase where the power domain is not ON by default on kernel boot,
> but I also want it to stay ON during RPM. But the current logic fails the
> `pm_genpd_init` for me.
>
> IMO, the kernel should not force a PD which needs to stay ON during RPM to be ON
> at boot also. It is quite possible that the PD will be turned ON for the very
> first time by a client driver in the kernel itself.

Right, you have a point. However, there is a problem with allowing this.

A consumer driver for an "irq_safe" device that is attached to a
"non-irq-safe" genpd, may call pm_runtime_get_sync() to turn on its
device/PM domain. This wouldn't work unless the PM domain has already
been powered-on by genpd.

This problem is only present when a device has multiple PM domains to
attach to, as it's only in this case when genpd leaves the PM domain
in it's current state (which could be powered-off). In the single PM
domain case, we are always powering-on the genpd, upfront, when
attaching.

The simple solution would be to always always power-on the PM domain
during attach. Perhaps the upsides of doing this are greater than the
downsides. For example, the consumer driver will get consistent
behaviour, no matter whether its device is attached to a single or
multiple PM domain(s).

What do you think?

>
> Additionally, I think the kernel as well should not enforce this for PDs which
> do not want to turn OFF ever, i.e., the ones which have `GENPD_FLAG_ALWAYS_ON`
> flag set. Let the kernel turn ON these PDs when it wants and only then prevent
> the RPM or system suspend from turning these domains OFF.

No, I don't agree with this.

The whole purpose of GENPD_FLAG_ALWAYS_ON is to allow representing PM
domains that are really supposed to be powered-on always. Like always
on power-islands/rails.

Moreover genpd makes assumptions about these PM domains during
initializations and in other paths too, which would then break.

>
> Proposed patch in pm_genpd_init:
> ```
> -       /* Always-on domains must be powered on at initialization. */
> -       if ((genpd_is_always_on(genpd) || genpd_is_rpm_always_on(genpd)) &&
> -                       !genpd_status_on(genpd)) {
> -               pr_err("always-on PM domain %s is not on\n", genpd->name);
> -               return -EINVAL;
> -       }
> -
> ```
>
> Looking for your views here.
>
> Thanks
> Ajay

Kind regards
Uffe