[nrf fromlist] soc: nordic: nrf54h20: Add LRC around idle

[adamkondraciuk]

Add LRC domain power management around idle. The Fast Clock Domain needs to be forced-on during CPU idle. Also the force needs to be disabled on idle-exit.

[hubertmis]

The resource usage conflict with clock_control seems to be still in place. Have a look at these functions:
https://github.com/nrfconnect/sdk-zephyr/blob/main/drivers/clock_control/clock_control_nrf2_common.c#L169-L198

[hubertmis]

Just please align the sub-states ids with the design. Other than that it looks good!

[adamkondraciuk]

Just please align the sub-states ids with the design. Other than that it looks good!

How about overlays here ? Maybe those states could be defined in DTS instead?

[hubertmis]

How about overlays here ? Maybe those states could be defined in DTS instead?

Yes, all power states should be defined in the .soc device tree files. Not in the sample overlays.

[karstenkoenig]

How about overlays here ? Maybe those states could be defined in DTS instead?

Yes, all power states should be defined in the .soc device tree files. Not in the sample overlays.

@nordic-segl did that here and has his name forever tied to nrf54h20 power management ❤️
zephyrproject-rtos/zephyr#79098

The way the idle state is defined there will mean that any k_sleep() with a wakeup under 100ms will not go via the pm defined idle state but handled by the idle thread itself via a direct call to k_cpu_idle().

[hubertmis]

The way the idle state is defined there will mean that any k_sleep() with a wakeup under 100ms will not go via the pm defined idle state but handled by the idle thread itself via a direct call to k_cpu_idle().

Then I would consider modifying this a little:

power-states {
	active: active {
		compatible = "zephyr,power-state";
		power-state-name = "active";
		min-residency-us = <0>;
	};
	idle: idle {
		compatible = "zephyr,power-state";
		power-state-name = "suspend-to-idle";
		substate-id = <0>;
		min-residency-us = <100>; // Guessing. TBD
	};
	// substate-id = <1>; is reserved for "idle-cache-retained"
	idle-cache-disabled: idle-cache-disabled {
		compatible = "zephyr,power-state";
		power-state-name = "suspend-to-idle";
		substate-id = <2>;
		min-residency-us = <100000>; // Guessing. TBD
	};
};

This way the power management decisions are explicit and clear. You don't need to get expertise "What happens when the current residency is lower than defined for any power state?"

[adamkondraciuk]

The way the idle state is defined there will mean that any k_sleep() with a wakeup under 100ms will not go via the pm defined idle state but handled by the idle thread itself via a direct call to k_cpu_idle().

Then I would consider modifying this a little:

power-states {
	active: active {
		compatible = "zephyr,power-state";
		power-state-name = "active";
		min-residency-us = <0>;
	};
	idle: idle {
		compatible = "zephyr,power-state";
		power-state-name = "suspend-to-idle";
		substate-id = <0>;
		min-residency-us = <100>; // Guessing. TBD
	};
	// substate-id = <1>; is reserved for "idle-cache-retained"
	idle-cache-disabled: idle-cache-disabled {
		compatible = "zephyr,power-state";
		power-state-name = "suspend-to-idle";
		substate-id = <2>;
		min-residency-us = <100000>; // Guessing. TBD
	};
};

This way the power management decisions are explicit and clear. You don't need to get expertise "What happens when the current residency is lower than defined for any power state?"

Zephyr doc describes such behavior very clearly here . Does it make sense to redefine those states which would behave exactly as described in Zephyr doc? That is a bit confusing. Also it introduces additional overhead for the simplest power states. IMO even idle:idle is too much here

[hubertmis]

Zephyr doc describes such behavior very clearly here . Does it make sense to redefine those states which would behave exactly as described in Zephyr doc? That is a bit confusing. Also it introduces additional overhead for the simplest power states. IMO even idle:idle is too much here

Ok, I wasn't aware it is a rule in Zephyr. Then I'm fine with keeping the power states set limited to idle-with-cache-disabled and s2ram. But let's add substates' placeholders for lighter sleep states in case we need to add active in the future:

power-states {
	// substate-id = <0>; is reserved for "idle", cache powered on
	// substate-id = <1>; is reserved for "idle-cache-retained"
	idle-cache-disabled: idle-cache-disabled {
		compatible = "zephyr,power-state";
		power-state-name = "suspend-to-idle";
		substate-id = <2>;
		min-residency-us = <100000>; // Guessing. TBD
	};
	s2ram: s2ram {
		compatible = "zephyr,power-state";
		power-state-name = "suspend-to-ram";
		substate-id = <0>;
		min-residency-us = <...>;
	};
};

[nordic-piks]

Duplicated power states definition needs to be remove from https://github.com/nrfconnect/sdk-nrf/tree/main/tests/benchmarks/multicore
at PR which updates into sdk-nrf.