Re: [PATCH 0/5] pmdomain: Restore behaviour for disabling unused PM domains

Re: [PATCH 0/5] pmdomain: Restore behaviour for disabling unused PM domains

[Diederik de Haas]

[-- Attachment #1.1: Type: text/plain, Size: 3038 bytes --]

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(-)


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 0/5] pmdomain: Restore behaviour for disabling unused PM domains

[Geert Uytterhoeven]

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

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 0/5] pmdomain: Restore behaviour for disabling unused PM domains

[Diederik de Haas]

[-- Attachment #1.1: Type: text/plain, Size: 3167 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 0/5] pmdomain: Restore behaviour for disabling unused PM domains

[Saravana Kannan]

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

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip