Recent changes to genpd prevents those PM domains being powered-on during
initialization from being powered-off during the boot sequence. Based upon
whether CONFIG_PM_CONFIG_PM_GENERIC_DOMAINS_OF is set of not, genpd relies
on the sync_state mechanism or the genpd_power_off_unused() (which is a
late_initcall_sync), to understand when it's okay to allow these PM domains
to be powered-off.

This new behaviour in genpd has lead to problems on different platforms [1].

In this series, I am therefore suggesting to restore the behavior of
genpd_power_off_unused() along with introducing a new genpd config flag,
GENPD_FLAG_NO_STAY_ON, to allow genpd OF providers to opt-out from the new
behaviour.

Kind regards
Ulf Hansson

[1]
https://lore.kernel.org/all/20250701114733.636510-1-ulf.hansson@linaro.org/
https://lore.kernel.org/all/20250902-rk3576-lockup-regression-v1-1-c4a0c9daeb00@collabora.com/

Ulf Hansson (5):
  pmdomain: core: Restore behaviour for disabling unused PM domains
  pmdomain: rockchip: Fix regulator dependency with
    GENPD_FLAG_NO_STAY_ON
  pmdomain: renesas: rcar-sysc: Don't keep unused PM domains powered-on
  pmdomain: renesas: rcar-gen4-sysc: Don't keep unused PM domains
    powered-on
  pmdomain: renesas: rmobile-sysc: Don't keep unused PM domains
    powered-on

 drivers/pmdomain/core.c                   | 20 ++++++++++++++------
 drivers/pmdomain/renesas/rcar-gen4-sysc.c |  1 +
 drivers/pmdomain/renesas/rcar-sysc.c      |  1 +
 drivers/pmdomain/renesas/rmobile-sysc.c   |  3 ++-
 drivers/pmdomain/rockchip/pm-domains.c    |  2 +-
 include/linux/pm_domain.h                 |  7 +++++++
 6 files changed, 26 insertions(+), 8 deletions(-)

-- 
2.43.0

On Tue, Sep 9, 2025 at 4:11 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> Recent changes to genpd prevents those PM domains being powered-on during
> initialization from being powered-off during the boot sequence. Based upon
> whether CONFIG_PM_CONFIG_PM_GENERIC_DOMAINS_OF is set of not, genpd relies
> on the sync_state mechanism or the genpd_power_off_unused() (which is a
> late_initcall_sync), to understand when it's okay to allow these PM domains
> to be powered-off.
>
> This new behaviour in genpd has lead to problems on different platforms [1].
>
> In this series, I am therefore suggesting to restore the behavior of
> genpd_power_off_unused() along with introducing a new genpd config flag,
> GENPD_FLAG_NO_STAY_ON, to allow genpd OF providers to opt-out from the new
> behaviour.
>
> Kind regards
> Ulf Hansson
>
> [1]
> https://lore.kernel.org/all/20250701114733.636510-1-ulf.hansson@linaro.org/
> https://lore.kernel.org/all/20250902-rk3576-lockup-regression-v1-1-c4a0c9daeb00@collabora.com/

I'm surprised this is happening. The default behavior of sync_state()
is to never turn off a power domain until all the consumers have
probed.

Is there a consumer that's not captured correctly? If so, can we add them?

Also, there are already sync_state related kernel configs, command
line options and sysfs files that should allow people to power off
devices (at different levels of granularity) even if its consumers
haven't probed.

Thanks,
Saravana

>
> Ulf Hansson (5):
>   pmdomain: core: Restore behaviour for disabling unused PM domains
>   pmdomain: rockchip: Fix regulator dependency with
>     GENPD_FLAG_NO_STAY_ON
>   pmdomain: renesas: rcar-sysc: Don't keep unused PM domains powered-on
>   pmdomain: renesas: rcar-gen4-sysc: Don't keep unused PM domains
>     powered-on
>   pmdomain: renesas: rmobile-sysc: Don't keep unused PM domains
>     powered-on
>
>  drivers/pmdomain/core.c                   | 20 ++++++++++++++------
>  drivers/pmdomain/renesas/rcar-gen4-sysc.c |  1 +
>  drivers/pmdomain/renesas/rcar-sysc.c      |  1 +
>  drivers/pmdomain/renesas/rmobile-sysc.c   |  3 ++-
>  drivers/pmdomain/rockchip/pm-domains.c    |  2 +-
>  include/linux/pm_domain.h                 |  7 +++++++
>  6 files changed, 26 insertions(+), 8 deletions(-)
>
> --
> 2.43.0
>

Hi,

On Tue Sep 9, 2025 at 1:11 PM CEST, Ulf Hansson wrote:
> Recent changes to genpd prevents those PM domains being powered-on during
> initialization from being powered-off during the boot sequence. Based upon
> whether CONFIG_PM_CONFIG_PM_GENERIC_DOMAINS_OF is set of not, genpd relies
> on the sync_state mechanism or the genpd_power_off_unused() (which is a
> late_initcall_sync), to understand when it's okay to allow these PM domains
> to be powered-off.
>
> This new behaviour in genpd has lead to problems on different platforms [1].
>
> In this series, I am therefore suggesting to restore the behavior of
> genpd_power_off_unused() along with introducing a new genpd config flag,
> GENPD_FLAG_NO_STAY_ON, to allow genpd OF providers to opt-out from the new
> behaviour.

Is it expected that I'm still seeing this on a Rock64 (rk3328), just
like before [1]?

  [   17.124202] rockchip-pm-domain ff100000.syscon:power-controller: sync_state() pending due to ff300000.gpu
  [   17.129799] rockchip-pm-domain ff100000.syscon:power-controller: sync_state() pending due to ff350000.video-codec
  [   17.140003] rockchip-pm-domain ff100000.syscon:power-controller: sync_state() pending due to ff360000.video-codec

This is with a 6.17-rc5 kernel with this patch set applied.
And it also has this patch from Christian Hewitt added, now in v3:
https://lore.kernel.org/linux-rockchip/20250906120810.1833016-1-christianshewitt@gmail.com/

When I boot into a 6.17-rc5 kernel without any patches applied, I do get
the 2 for ff350000.video-codec and ff360000.video-codec, but not the 
ff300000.gpu one.

Interestingly:
ff300000.gpu -> power-domains = <&power RK3328_PD_GPU>;
ff350000.video-codec -> power-domains = <&power RK3328_PD_VPU>;
ff360000.video-codec -> power-domains = <&power RK3328_PD_VIDEO>;

I would be surprised if that was a coincidence.

Cheers,
  Diederik

[1] https://lore.kernel.org/all/DCK0O99SYSCF.BMBAEUV24C1G@cknow.org/

>
> Kind regards
> Ulf Hansson
>
> [1]
> https://lore.kernel.org/all/20250701114733.636510-1-ulf.hansson@linaro.org/
> https://lore.kernel.org/all/20250902-rk3576-lockup-regression-v1-1-c4a0c9daeb00@collabora.com/
>
> Ulf Hansson (5):
>   pmdomain: core: Restore behaviour for disabling unused PM domains
>   pmdomain: rockchip: Fix regulator dependency with
>     GENPD_FLAG_NO_STAY_ON
>   pmdomain: renesas: rcar-sysc: Don't keep unused PM domains powered-on
>   pmdomain: renesas: rcar-gen4-sysc: Don't keep unused PM domains
>     powered-on
>   pmdomain: renesas: rmobile-sysc: Don't keep unused PM domains
>     powered-on
>
>  drivers/pmdomain/core.c                   | 20 ++++++++++++++------
>  drivers/pmdomain/renesas/rcar-gen4-sysc.c |  1 +
>  drivers/pmdomain/renesas/rcar-sysc.c      |  1 +
>  drivers/pmdomain/renesas/rmobile-sysc.c   |  3 ++-
>  drivers/pmdomain/rockchip/pm-domains.c    |  2 +-
>  include/linux/pm_domain.h                 |  7 +++++++
>  6 files changed, 26 insertions(+), 8 deletions(-)

Hi Diederik,

On Wed, 10 Sept 2025 at 21:33, Diederik de Haas <didi.debian@cknow.org> wrote:
> On Tue Sep 9, 2025 at 1:11 PM CEST, Ulf Hansson wrote:
> > Recent changes to genpd prevents those PM domains being powered-on during
> > initialization from being powered-off during the boot sequence. Based upon
> > whether CONFIG_PM_CONFIG_PM_GENERIC_DOMAINS_OF is set of not, genpd relies
> > on the sync_state mechanism or the genpd_power_off_unused() (which is a
> > late_initcall_sync), to understand when it's okay to allow these PM domains
> > to be powered-off.
> >
> > This new behaviour in genpd has lead to problems on different platforms [1].
> >
> > In this series, I am therefore suggesting to restore the behavior of
> > genpd_power_off_unused() along with introducing a new genpd config flag,
> > GENPD_FLAG_NO_STAY_ON, to allow genpd OF providers to opt-out from the new
> > behaviour.
>
> Is it expected that I'm still seeing this on a Rock64 (rk3328), just
> like before [1]?
>
>   [   17.124202] rockchip-pm-domain ff100000.syscon:power-controller: sync_state() pending due to ff300000.gpu
>   [   17.129799] rockchip-pm-domain ff100000.syscon:power-controller: sync_state() pending due to ff350000.video-codec
>   [   17.140003] rockchip-pm-domain ff100000.syscon:power-controller: sync_state() pending due to ff360000.video-codec

Yes, as the sync state is still blocked on them.
Disabling unused PM Domains is done independently of sync state.

> This is with a 6.17-rc5 kernel with this patch set applied.
> And it also has this patch from Christian Hewitt added, now in v3:
> https://lore.kernel.org/linux-rockchip/20250906120810.1833016-1-christianshewitt@gmail.com/
>
> When I boot into a 6.17-rc5 kernel without any patches applied, I do get
> the 2 for ff350000.video-codec and ff360000.video-codec, but not the
> ff300000.gpu one.
>
> Interestingly:
> ff300000.gpu -> power-domains = <&power RK3328_PD_GPU>;
> ff350000.video-codec -> power-domains = <&power RK3328_PD_VPU>;
> ff360000.video-codec -> power-domains = <&power RK3328_PD_VIDEO>;
>
> I would be surprised if that was a coincidence.

Fw_devlinks ignores the index cell (RK3328_PD_*), hence all links are
created pointing to the pmdomain controller (in case it has a platform
driver) or the first pmdomain (in case it has not). thus blocking the
sync state call and power-down for _all_ pmdomains managed by the
controller.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Hi Geert,

On Thu Sep 11, 2025 at 9:18 AM CEST, Geert Uytterhoeven wrote:
> On Wed, 10 Sept 2025 at 21:33, Diederik de Haas <didi.debian@cknow.org> wrote:
>> On Tue Sep 9, 2025 at 1:11 PM CEST, Ulf Hansson wrote:
>> > Recent changes to genpd prevents those PM domains being powered-on during
>> > initialization from being powered-off during the boot sequence. Based upon
>> > whether CONFIG_PM_CONFIG_PM_GENERIC_DOMAINS_OF is set of not, genpd relies
>> > on the sync_state mechanism or the genpd_power_off_unused() (which is a
>> > late_initcall_sync), to understand when it's okay to allow these PM domains
>> > to be powered-off.
>> >
>> > This new behaviour in genpd has lead to problems on different platforms [1].
>> >
>> > In this series, I am therefore suggesting to restore the behavior of
>> > genpd_power_off_unused() along with introducing a new genpd config flag,
>> > GENPD_FLAG_NO_STAY_ON, to allow genpd OF providers to opt-out from the new
>> > behaviour.
>>
>> Is it expected that I'm still seeing this on a Rock64 (rk3328), just
>> like before [1]?
>>
>>   [   17.124202] rockchip-pm-domain ff100000.syscon:power-controller: sync_state() pending due to ff300000.gpu
>>   [   17.129799] rockchip-pm-domain ff100000.syscon:power-controller: sync_state() pending due to ff350000.video-codec
>>   [   17.140003] rockchip-pm-domain ff100000.syscon:power-controller: sync_state() pending due to ff360000.video-codec
>
> Yes, as the sync state is still blocked on them.
> Disabling unused PM Domains is done independently of sync state.
>
>> This is with a 6.17-rc5 kernel with this patch set applied.
>> And it also has this patch from Christian Hewitt added, now in v3:
>> https://lore.kernel.org/linux-rockchip/20250906120810.1833016-1-christianshewitt@gmail.com/
>>
>> When I boot into a 6.17-rc5 kernel without any patches applied, I do get
>> the 2 for ff350000.video-codec and ff360000.video-codec, but not the
>> ff300000.gpu one.
>>
>> Interestingly:
>> ff300000.gpu -> power-domains = <&power RK3328_PD_GPU>;
>> ff350000.video-codec -> power-domains = <&power RK3328_PD_VPU>;
>> ff360000.video-codec -> power-domains = <&power RK3328_PD_VIDEO>;
>>
>> I would be surprised if that was a coincidence.
>
> Fw_devlinks ignores the index cell (RK3328_PD_*), hence all links are
> created pointing to the pmdomain controller (in case it has a platform
> driver) or the first pmdomain (in case it has not). thus blocking the
> sync state call and power-down for _all_ pmdomains managed by the
> controller.

I don't think I fully understand this (not your problem due to lack of
knowledge on my part), but you mentioning 'fw_devlink' rang a bell.

Some time ago Nicolas Frattaroli and I worked on an image for PINE64's
*Quartz* devices and that added ``fw_devlink=off`` to cmdline.
I've been using it on all my Rockchip based devices, without
understanding that parameter ... but (apparently) on my 'rock64-test'
device, where I tested it with, I had removed that parameter.
Putting that parameter back ... and those warnings are gone!

Thanks a LOT for that hint!

Groetjes,
  Diederik

On Thu, Sep 11, 2025 at 1:26 AM Diederik de Haas <didi.debian@cknow.org> wrote:
>
> Hi Geert,
>
> On Thu Sep 11, 2025 at 9:18 AM CEST, Geert Uytterhoeven wrote:
> > On Wed, 10 Sept 2025 at 21:33, Diederik de Haas <didi.debian@cknow.org> wrote:
> >> On Tue Sep 9, 2025 at 1:11 PM CEST, Ulf Hansson wrote:
> >> > Recent changes to genpd prevents those PM domains being powered-on during
> >> > initialization from being powered-off during the boot sequence. Based upon
> >> > whether CONFIG_PM_CONFIG_PM_GENERIC_DOMAINS_OF is set of not, genpd relies
> >> > on the sync_state mechanism or the genpd_power_off_unused() (which is a
> >> > late_initcall_sync), to understand when it's okay to allow these PM domains
> >> > to be powered-off.
> >> >
> >> > This new behaviour in genpd has lead to problems on different platforms [1].
> >> >
> >> > In this series, I am therefore suggesting to restore the behavior of
> >> > genpd_power_off_unused() along with introducing a new genpd config flag,
> >> > GENPD_FLAG_NO_STAY_ON, to allow genpd OF providers to opt-out from the new
> >> > behaviour.
> >>
> >> Is it expected that I'm still seeing this on a Rock64 (rk3328), just
> >> like before [1]?
> >>
> >>   [   17.124202] rockchip-pm-domain ff100000.syscon:power-controller: sync_state() pending due to ff300000.gpu
> >>   [   17.129799] rockchip-pm-domain ff100000.syscon:power-controller: sync_state() pending due to ff350000.video-codec
> >>   [   17.140003] rockchip-pm-domain ff100000.syscon:power-controller: sync_state() pending due to ff360000.video-codec
> >
> > Yes, as the sync state is still blocked on them.
> > Disabling unused PM Domains is done independently of sync state.
> >
> >> This is with a 6.17-rc5 kernel with this patch set applied.
> >> And it also has this patch from Christian Hewitt added, now in v3:
> >> https://lore.kernel.org/linux-rockchip/20250906120810.1833016-1-christianshewitt@gmail.com/
> >>
> >> When I boot into a 6.17-rc5 kernel without any patches applied, I do get
> >> the 2 for ff350000.video-codec and ff360000.video-codec, but not the
> >> ff300000.gpu one.
> >>
> >> Interestingly:
> >> ff300000.gpu -> power-domains = <&power RK3328_PD_GPU>;
> >> ff350000.video-codec -> power-domains = <&power RK3328_PD_VPU>;
> >> ff360000.video-codec -> power-domains = <&power RK3328_PD_VIDEO>;
> >>
> >> I would be surprised if that was a coincidence.
> >
> > Fw_devlinks ignores the index cell (RK3328_PD_*), hence all links are
> > created pointing to the pmdomain controller (in case it has a platform
> > driver) or the first pmdomain (in case it has not). thus blocking the
> > sync state call and power-down for _all_ pmdomains managed by the
> > controller.
>
> I don't think I fully understand this (not your problem due to lack of
> knowledge on my part), but you mentioning 'fw_devlink' rang a bell.
>
> Some time ago Nicolas Frattaroli and I worked on an image for PINE64's
> *Quartz* devices and that added ``fw_devlink=off`` to cmdline.
> I've been using it on all my Rockchip based devices, without
> understanding that parameter ... but (apparently) on my 'rock64-test'
> device, where I tested it with, I had removed that parameter.
> Putting that parameter back ... and those warnings are gone!
>
> Thanks a LOT for that hint!

Please don't just disable fw_devlink using fw_devlink=off. We want to
fix any issues you are hitting with it. I might even delete this "off"
option sometime. It was meant as an early debug option.

Thanks,
Saravana