ARM64: CPUIdle driver is not select any Idle state other then WFI

ARM64: CPUIdle driver is not select any Idle state other then WFI

[Vivek yadav]

Hi @all,

I am working on one custom SoC. Where I add one CPUIdle state for
``arm,cortex-a55`` processor.

idle-states {
      entry-method = "psci";
       cpu_ret_l: cpu-retention-l {
         compatible = "arm,idle-state";
         arm,psci-suspend-param = <0x00010001>;
         local-timer-stop;
         entry-latency-us = <55>;
          exit-latency-us = <140>;
          min-residency-us = <780>;
    };
};

I am using ``Menu governor`` with the ``psci_idle driver`` in its original form.
After booting Linux I find out that the CPUIdle core is never going
inside the ``cpu-retention`` state.
To check time spent by CPU in any state. I am using the below command.

``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/time``

OUTPUT:
0 ===>CPU0 state0 (WFI)
0 ===>CPU0 state1 (cpu-retention)

increasing some time value ===>CPU1 state0 (WFI)
0 ===>CPU1 state1 (cpu-retention)

increasing some time value
0

increasing some time value
0

What am I doing wrong? Why does ``cpu-retention`` state time not increase?
Any pointer will be helpful.

Regards,
Vivek

Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Vivek yadav]

---------- Forwarded message ---------
From: Vivek yadav <linux.ninja23@gmail.com>
Date: Fri, Oct 11, 2024 at 3:14 PM
Subject: ARM64: CPUIdle driver is not select any Idle state other then WFI
To: <linux-pm@vger.kernel.org>


Hi @all,

I am working on one custom SoC. Where I add one CPUIdle state for
``arm,cortex-a55`` processor.

idle-states {
      entry-method = "psci";
       cpu_ret_l: cpu-retention-l {
         compatible = "arm,idle-state";
         arm,psci-suspend-param = <0x00010001>;
         local-timer-stop;
         entry-latency-us = <55>;
          exit-latency-us = <140>;
          min-residency-us = <780>;
    };
};

I am using ``Menu governor`` with the ``psci_idle driver`` in its original form.
After booting Linux I find out that the CPUIdle core is never going
inside the ``cpu-retention`` state.
To check time spent by CPU in any state. I am using the below command.

``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/time``

OUTPUT:
0 ===>CPU0 state0 (WFI)
0 ===>CPU0 state1 (cpu-retention)

increasing some time value ===>CPU1 state0 (WFI)
0 ===>CPU1 state1 (cpu-retention)

increasing some time value
0

increasing some time value
0

What am I doing wrong? Why does ``cpu-retention`` state time not increase?
Any pointer will be helpful.

Regards,
Vivek

Re: Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Christian Loehle]

On 10/14/24 11:28, Vivek yadav wrote:
> ---------- Forwarded message ---------
> From: Vivek yadav <linux.ninja23@gmail.com>
> Date: Fri, Oct 11, 2024 at 3:14 PM
> Subject: ARM64: CPUIdle driver is not select any Idle state other then WFI
> To: <linux-pm@vger.kernel.org>
> 
> 
> Hi @all,
> 
> I am working on one custom SoC. Where I add one CPUIdle state for
> ``arm,cortex-a55`` processor.
> 
> idle-states {
>       entry-method = "psci";
>        cpu_ret_l: cpu-retention-l {
>          compatible = "arm,idle-state";
>          arm,psci-suspend-param = <0x00010001>;
>          local-timer-stop;
>          entry-latency-us = <55>;
>           exit-latency-us = <140>;
>           min-residency-us = <780>;
>     };
> };
> 
> I am using ``Menu governor`` with the ``psci_idle driver`` in its original form.
> After booting Linux I find out that the CPUIdle core is never going
> inside the ``cpu-retention`` state.
> To check time spent by CPU in any state. I am using the below command.
> 
> ``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/time``
> 
> OUTPUT:
> 0 ===>CPU0 state0 (WFI)
> 0 ===>CPU0 state1 (cpu-retention)
> 
> increasing some time value ===>CPU1 state0 (WFI)
> 0 ===>CPU1 state1 (cpu-retention)
> 
> increasing some time value
> 0
> 
> increasing some time value
> 0
> 
> What am I doing wrong? Why does ``cpu-retention`` state time not increase?
> Any pointer will be helpful.

What is the value of 
cat /sys/devices/system/cpu/cpu*/cpuidle/state*/usage
?
Does state1 usage increase?

What if you disable state0 on all CPUs?
echo 1 > /sys/devices/system/cpu/cpu*/cpuidle/state0/disable

Re: Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Vivek yadav]

Hi Christian,

> What is the value of
> cat /sys/devices/system/cpu/cpu*/cpuidle/state*/usage?

0 ===>CPU0 state0 (WFI)
0 ===>CPU0 state1 (cpu-retention)
408 ==> these values are increasing
0
38
0
14
0

> What if you disable state0 on all CPUs?
> echo 1 > /sys/devices/system/cpu/cpu*/cpuidle/state0/disable

After executing above command.  CPU Idle state WFI time stopped. But
no change in ``state1`` time.

``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/time``

OUTPUT:
0 ===>CPU0 state0 (WFI)
0 ===>CPU0 state1 (cpu-retention)

increasing some time value(23968) ===>CPU1 state0 (WFI)
0 ===>CPU1 state1 (cpu-retention)

increasing some time value(17512) ==> now it also not increasing
0

increasing some time value(6661)
0

``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/usage``

0 ===>CPU0 state0 (WFI)
0 ===>CPU0 state1 (cpu-retention)

408 ==> now these values also stop increasing
0

42
0

14
0

Regards
Vivek





Regards
Vivek

On Mon, Oct 14, 2024 at 6:24 PM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 10/14/24 11:28, Vivek yadav wrote:
> > ---------- Forwarded message ---------
> > From: Vivek yadav <linux.ninja23@gmail.com>
> > Date: Fri, Oct 11, 2024 at 3:14 PM
> > Subject: ARM64: CPUIdle driver is not select any Idle state other then WFI
> > To: <linux-pm@vger.kernel.org>
> >
> >
> > Hi @all,
> >
> > I am working on one custom SoC. Where I add one CPUIdle state for
> > ``arm,cortex-a55`` processor.
> >
> > idle-states {
> >       entry-method = "psci";
> >        cpu_ret_l: cpu-retention-l {
> >          compatible = "arm,idle-state";
> >          arm,psci-suspend-param = <0x00010001>;
> >          local-timer-stop;
> >          entry-latency-us = <55>;
> >           exit-latency-us = <140>;
> >           min-residency-us = <780>;
> >     };
> > };
> >
> > I am using ``Menu governor`` with the ``psci_idle driver`` in its original form.
> > After booting Linux I find out that the CPUIdle core is never going
> > inside the ``cpu-retention`` state.
> > To check time spent by CPU in any state. I am using the below command.
> >
> > ``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/time``
> >
> > OUTPUT:
> > 0 ===>CPU0 state0 (WFI)
> > 0 ===>CPU0 state1 (cpu-retention)
> >
> > increasing some time value ===>CPU1 state0 (WFI)
> > 0 ===>CPU1 state1 (cpu-retention)
> >
> > increasing some time value
> > 0
> >
> > increasing some time value
> > 0
> >
> > What am I doing wrong? Why does ``cpu-retention`` state time not increase?
> > Any pointer will be helpful.
>
> What is the value of
> cat /sys/devices/system/cpu/cpu*/cpuidle/state*/usage
> ?
> Does state1 usage increase?
>
> What if you disable state0 on all CPUs?
> echo 1 > /sys/devices/system/cpu/cpu*/cpuidle/state0/disable

Re: Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Christian Loehle]

On 10/14/24 16:49, Vivek yadav wrote:
> * # Be careful, this email looks suspicious; * User Impersonation: The display name on this email is very similar to an internal user but was sent from an email address not normally used by the person. *
> Hi Christian,
> 
>> What is the value of
>> cat /sys/devices/system/cpu/cpu*/cpuidle/state*/usage?
> 
> 0 ===>CPU0 state0 (WFI)
> 0 ===>CPU0 state1 (cpu-retention)
> 408 ==> these values are increasing
> 0
> 38
> 0
> 14
> 0
> 
>> What if you disable state0 on all CPUs?
>> echo 1 > /sys/devices/system/cpu/cpu*/cpuidle/state0/disable
> 
> After executing above command.  CPU Idle state WFI time stopped. But
> no change in ``state1`` time.
> 
> ``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/time``
> 
> OUTPUT:
> 0 ===>CPU0 state0 (WFI)
> 0 ===>CPU0 state1 (cpu-retention)
> 
> increasing some time value(23968) ===>CPU1 state0 (WFI)
> 0 ===>CPU1 state1 (cpu-retention)
> 
> increasing some time value(17512) ==> now it also not increasing
> 0
> 
> increasing some time value(6661)
> 0
> 
> ``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/usage``
> 
> 0 ===>CPU0 state0 (WFI)
> 0 ===>CPU0 state1 (cpu-retention)
> 
> 408 ==> now these values also stop increasing
> 0
> 
> 42
> 0
> 
> 14
> 0

My guess would be that state1 can't be entered, i.e.
entered_state = target_state->enter(dev, drv, index);
returns < 0, in that case the sysfs 'rejected' field
will be incremented instead of usage and time.

Is that the case for you?
In that case your psci description is probably wrong
and doesn't match your system.

Regards,
Christian

Re: Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Vivek yadav]

Hi Christian,
> My guess would be that state1 can't be entered, i.e.
> entered_state = target_state->enter(dev, drv, index);
> returns < 0, in that case the sysfs 'rejected' field
> will be incremented instead of usage and time.
> Is that the case for you?

Yes, I checked your guess was correct. ``enter_state`` value equals to Zero.

``#cat /sys/devices/system/cpu/cpu*/cpuidle/state*/rejected``
0 ==> state_0 CPU(0)
0 ==> state_1 CPU(0)
0 ==> state_0 rejected values CPU(1)
600117 ==> state_1 increasing values CPU(1)
0
854505
0
41756644

> In that case your psci description is probably wrong
> and doesn't match your system.

What do you mean by ``psci`` description ?

I have one node in the device  tree file.

psci {
     compatible = "arm,psci-1.0";
     method = "smc";
};

On Tue, Oct 15, 2024 at 3:12 PM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 10/14/24 16:49, Vivek yadav wrote:
> > * # Be careful, this email looks suspicious; * User Impersonation: The display name on this email is very similar to an internal user but was sent from an email address not normally used by the person. *
> > Hi Christian,
> >
> >> What is the value of
> >> cat /sys/devices/system/cpu/cpu*/cpuidle/state*/usage?
> >
> > 0 ===>CPU0 state0 (WFI)
> > 0 ===>CPU0 state1 (cpu-retention)
> > 408 ==> these values are increasing
> > 0
> > 38
> > 0
> > 14
> > 0
> >
> >> What if you disable state0 on all CPUs?
> >> echo 1 > /sys/devices/system/cpu/cpu*/cpuidle/state0/disable
> >
> > After executing above command.  CPU Idle state WFI time stopped. But
> > no change in ``state1`` time.
> >
> > ``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/time``
> >
> > OUTPUT:
> > 0 ===>CPU0 state0 (WFI)
> > 0 ===>CPU0 state1 (cpu-retention)
> >
> > increasing some time value(23968) ===>CPU1 state0 (WFI)
> > 0 ===>CPU1 state1 (cpu-retention)
> >
> > increasing some time value(17512) ==> now it also not increasing
> > 0
> >
> > increasing some time value(6661)
> > 0
> >
> > ``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/usage``
> >
> > 0 ===>CPU0 state0 (WFI)
> > 0 ===>CPU0 state1 (cpu-retention)
> >
> > 408 ==> now these values also stop increasing
> > 0
> >
> > 42
> > 0
> >
> > 14
> > 0
>
> My guess would be that state1 can't be entered, i.e.
> entered_state = target_state->enter(dev, drv, index);
> returns < 0, in that case the sysfs 'rejected' field
> will be incremented instead of usage and time.
>
> Is that the case for you?
> In that case your psci description is probably wrong
> and doesn't match your system.
>
> Regards,
> Christian
>
>

Re: Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Vivek yadav]

Hi Christian,
> My guess would be that state1 can't be entered, i.e.
> entered_state = target_state->enter(dev, drv, index);
> returns < 0, in that case the sysfs 'rejected' field
> will be incremented instead of usage and time.
> Is that the case for you?

Yes, I checked your guess was correct. ``enter_state`` value equals to Zero.

``#cat /sys/devices/system/cpu/cpu*/cpuidle/state*/rejected``
0 ==> state_0 CPU(0)
0 ==> state_1 CPU(0)
0 ==> state_0 rejected values CPU(1)
600117 ==> state_1 increasing values CPU(1)
0
854505
0
41756644

> In that case your psci description is probably wrong
> and doesn't match your system.

What do you mean by ``psci`` description ?

I have one node in the device  tree file.

psci {
     compatible = "arm,psci-1.0";
     method = "smc";
};

Please find some logs. I am not able to conclude much by seeing the below logs.

#dmesg | grep "PSCI"
psci: PSCIv1.1 detected in firmware.
psci: Using standard PSCI v0.2 function IDs

#dmesg | grep "SMC"
psci: SMC calling Convention v1.2
SMCCC: SOC_ID: ARCH_SOC_ID not implemented, skipping ....

#dmesg | grep "MIGRATE_INTO_TYPE"
psci: MIGRATE_INTO_TYPE not supported.

Regards,
Vivek

Re: Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Dhruva Gole]

Hi Vivek,

On Oct 14, 2024 at 16:06:34 +0530, Vivek yadav wrote:
> ---------- Forwarded message ---------
> From: Vivek yadav <linux.ninja23@gmail.com>
> Date: Fri, Oct 11, 2024 at 3:14 PM
> Subject: ARM64: CPUIdle driver is not select any Idle state other then WFI
> To: <linux-pm@vger.kernel.org>

+ Kevin, Vignesh and few colleagues at TI who have been working on this as
well.

> 
> 
> Hi @all,
> 
> I am working on one custom SoC. Where I add one CPUIdle state for
> ``arm,cortex-a55`` processor.

Any further luck on this?

I have also been working on something similar[1] but on an A53 core on
TI-K3 AM62x processor.

> 
> idle-states {
>       entry-method = "psci";
>        cpu_ret_l: cpu-retention-l {
>          compatible = "arm,idle-state";
>          arm,psci-suspend-param = <0x00010001>;
>          local-timer-stop;
>          entry-latency-us = <55>;
>           exit-latency-us = <140>;
>           min-residency-us = <780>;
>     };
> };
> 
> I am using ``Menu governor`` with the ``psci_idle driver`` in its original form.
> After booting Linux I find out that the CPUIdle core is never going
> inside the ``cpu-retention`` state.
> To check time spent by CPU in any state. I am using the below command.
> 
> ``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/time``

What I was seeing is in a multi core system (2 or more) all cores don't
enter the idle-state simultaneously. There's something keeping atleast 1
core always busy. However I could definitely see entry into TF-A from 1
core at a time.
I then switched to a single core system to see if we were atall able to
enter TF-A when only 1 core was available for linux, it turned out that
with the "local-timer-stop" property that we have, this is never
possible.

See this chunk in the kernel cpuidle driver:
	if (broadcast && tick_broadcast_enter()) {

When I dug deeper into tick_broadcast_enter it always returns something
non zero and hence in my case it was entering the if block and tried to
find a deepest state. Then the deepest state would always return WFI and
not the idle-state I had added.

What we found out was on our kernel we end up using 

kernel/time/tick-broadcast-hrtimer.c 

This always seems to be keeping atleast 1 CPU busy and prevents idle.
If we remove the local-timer-stop it was helping us, but we still need
to dig into the full impact of what that entails and I am still
interested in finding out how so many other users of similar idle-state
implementation are able to do so without trouble.

Arm64 recommends to use arch_timer instead of external timers. Once we
enter el3, timer interrupts to el1 is blocked and hence it's equivalent
to local-timer-stop, so it does make sense to keep this property, but
then how are others able to enter idle-states for all plugged CPUs at
the same time?

> 
> OUTPUT:
> 0 ===>CPU0 state0 (WFI)
> 0 ===>CPU0 state1 (cpu-retention)
> 
> increasing some time value ===>CPU1 state0 (WFI)
> 0 ===>CPU1 state1 (cpu-retention)
> 
> increasing some time value
> 0
> 
> increasing some time value
> 0
> 
> What am I doing wrong? Why does ``cpu-retention`` state time not increase?
> Any pointer will be helpful.

I had asked a similar qn. on IRC [2], but didn't get much response. I
am still interested in finding out what the right thing to do here is.

[1] https://github.com/DhruvaG2000/v-linux/commit/0fd088d624276a2e72b8dc6660d261ab6d194f4b#diff-34369928f669d14776f8f5bdbe3fc3d75306196a2ac28b1a4d7e17402b9c3995R160
[2] https://libera.irclog.whitequark.org/armlinux/2024-08-23 

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated

Re: Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Sudeep Holla]

On Wed, Dec 11, 2024 at 11:20:52AM +0530, Dhruva Gole wrote:
> Hi Vivek,
>
> On Oct 14, 2024 at 16:06:34 +0530, Vivek yadav wrote:
> > ---------- Forwarded message ---------
> > From: Vivek yadav <linux.ninja23@gmail.com>
> > Date: Fri, Oct 11, 2024 at 3:14 PM
> > Subject: ARM64: CPUIdle driver is not select any Idle state other then WFI
> > To: <linux-pm@vger.kernel.org>
>
> + Kevin, Vignesh and few colleagues at TI who have been working on this as
> well.
>
> >
> >
> > Hi @all,
> >
> > I am working on one custom SoC. Where I add one CPUIdle state for
> > ``arm,cortex-a55`` processor.
>
> Any further luck on this?
>
> I have also been working on something similar[1] but on an A53 core on
> TI-K3 AM62x processor.

Does upstream DTS have support for this platform to understand it better ?
Even reference to any complete DT file for the platform will help.

> >
> > idle-states {
> >       entry-method = "psci";
> >        cpu_ret_l: cpu-retention-l {
> >          compatible = "arm,idle-state";
> >          arm,psci-suspend-param = <0x00010001>;
> >          local-timer-stop;
> >          entry-latency-us = <55>;
> >           exit-latency-us = <140>;
> >           min-residency-us = <780>;
> >     };
> > };
> >
> > I am using ``Menu governor`` with the ``psci_idle driver`` in its original form.
> > After booting Linux I find out that the CPUIdle core is never going
> > inside the ``cpu-retention`` state.
> > To check time spent by CPU in any state. I am using the below command.
> >
> > ``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/time``
>
> What I was seeing is in a multi core system (2 or more) all cores don't
> enter the idle-state simultaneously. There's something keeping atleast 1
> core always busy. However I could definitely see entry into TF-A from 1
> core at a time.

Does the platform have system timers ? What are the deeper idle states ?
If it is retention state with local timers on, I doubt if my suspicion of
CPU acting as broadcast timer in absence of a better/system timer.

> I then switched to a single core system to see if we were atall able to
> enter TF-A when only 1 core was available for linux, it turned out that
> with the "local-timer-stop" property that we have, this is never
> possible.
>

Yes my suspicion seems correct now but I can't confirm unless I understand
the platform completely.

> See this chunk in the kernel cpuidle driver:
> 	if (broadcast && tick_broadcast_enter()) {
>
> When I dug deeper into tick_broadcast_enter it always returns something
> non zero and hence in my case it was entering the if block and tried to
> find a deepest state. Then the deepest state would always return WFI and
> not the idle-state I had added.
>
> What we found out was on our kernel we end up using
>
> kernel/time/tick-broadcast-hrtimer.c
>
> This always seems to be keeping atleast 1 CPU busy and prevents idle.
> If we remove the local-timer-stop it was helping us, but we still need
> to dig into the full impact of what that entails and I am still
> interested in finding out how so many other users of similar idle-state
> implementation are able to do so without trouble.
>

Interesting. So if the platform is functional removing local-timer-stop,
I am bit confused. Either there is something else that is getting it out
from the idle state so, it should be fine and it could be just some
misconfiguration.

> Arm64 recommends to use arch_timer instead of external timers. Once we
> enter el3, timer interrupts to el1 is blocked and hence it's equivalent
> to local-timer-stop, so it does make sense to keep this property, but
> then how are others able to enter idle-states for all plugged CPUs at
> the same time?
>

Some systems have system timer that can take over as broadcast timer when
CPUs enter deeper idle states where the local timers are stopped.

--
Regards,
Sudeep

Re: Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Dhruva Gole]

On Dec 11, 2024 at 12:18:25 +0000, Sudeep Holla wrote:
> On Wed, Dec 11, 2024 at 11:20:52AM +0530, Dhruva Gole wrote:
[...]
> > >
> > >
> > > Hi @all,
> > >
> > > I am working on one custom SoC. Where I add one CPUIdle state for
> > > ``arm,cortex-a55`` processor.
> >
> > Any further luck on this?
> >
> > I have also been working on something similar[1] but on an A53 core on
> > TI-K3 AM62x processor.
> 
> Does upstream DTS have support for this platform to understand it better ?
> Even reference to any complete DT file for the platform will help.

Yes, you can ref to the AM625 (CPU layout) DT here:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/ti/k3-am625.dtsi

The board/starter kit DT is:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/ti/k3-am625-sk.dts

The patches for idle state are not upstream, and only exist in this
patch of mine here:
https://github.com/DhruvaG2000/v-linux/commit/0fd088d624276a2e72b8dc6660d261ab6d194f4b

[...]
> > See this chunk in the kernel cpuidle driver:
> > 	if (broadcast && tick_broadcast_enter()) {
> >
> > When I dug deeper into tick_broadcast_enter it always returns something
> > non zero and hence in my case it was entering the if block and tried to
> > find a deepest state. Then the deepest state would always return WFI and
> > not the idle-state I had added.
> >
> > What we found out was on our kernel we end up using
> >
> > kernel/time/tick-broadcast-hrtimer.c
> >
> > This always seems to be keeping atleast 1 CPU busy and prevents idle.
> > If we remove the local-timer-stop it was helping us, but we still need
> > to dig into the full impact of what that entails and I am still
> > interested in finding out how so many other users of similar idle-state
> > implementation are able to do so without trouble.
> >
> 
> Interesting. So if the platform is functional removing local-timer-stop,
> I am bit confused. Either there is something else that is getting it out

Yes it was interesting to us too, as to how the RCU didn't kick in and
system continued to function as though nothing was wrong.

> from the idle state so, it should be fine and it could be just some

It's probably UART keypresses or some userspace processes that get
scheduled that bring the CPUs back out of TF-A's cpu_standby.
Is it possible that EL1 interrupts can bring EL3 out of WFI? Is yes then
it explains the behaviour. The arch timer could also be continuing to
tick and bringing the CPUs out of ATF WFI.

> misconfiguration.
> 
> > Arm64 recommends to use arch_timer instead of external timers. Once we
> > enter el3, timer interrupts to el1 is blocked and hence it's equivalent
> > to local-timer-stop, so it does make sense to keep this property, but
> > then how are others able to enter idle-states for all plugged CPUs at
> > the same time?
> >
> 
> Some systems have system timer that can take over as broadcast timer when
> CPUs enter deeper idle states where the local timers are stopped.

In CPUIdle we're not really clock gating anything so the timer does keep
ticking. So in this particular case it might make sense to remove the
local-timer-stop property from the idle-state.

However we're looking into taking this further and putting interconnect
and few other PLLs in bypass which could cause arch timer for eg. to
tick slower. In this case would it still make sense to omit the
property? We may even have some usecases planned where we may turn OFF
the CPU once it is in TF-A cpu_standby/ WFI. What would be the right
approach in such scenarios?

Could you provide any examples where the local-timer-stop property is
being used and an alternative timer can be configured once we enter the
idle-state where CPU CTX maybe lost or clocks maybe bypass? It would be
great if you could share some example implementation if you're aware.

I took a look at QCom / NXP DT's but I couldn't exactly figure out how
they were setting up alternate timers once CPU hit idle-state.

PS. If it helps atall here's also my hack TF-A code where we enter just
CPU RET state for now:
https://github.com/DhruvaG2000/tfa-dev/commit/2d4b441d2f4c6d9ee0d6a62d93920ee8ab77dd42#diff-4fa4f4f6a5faa221390928a8079f76392536559f66e420a0182dba8e5966f4c6R232

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated

Re: Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Sudeep Holla]

On Wed, Dec 11, 2024 at 08:04:28PM +0530, Dhruva Gole wrote:
> On Dec 11, 2024 at 12:18:25 +0000, Sudeep Holla wrote:
> > On Wed, Dec 11, 2024 at 11:20:52AM +0530, Dhruva Gole wrote:
> [...]
> > > >
> > > >
> > > > Hi @all,
> > > >
> > > > I am working on one custom SoC. Where I add one CPUIdle state for
> > > > ``arm,cortex-a55`` processor.
> > >
> > > Any further luck on this?
> > >
> > > I have also been working on something similar[1] but on an A53 core on
> > > TI-K3 AM62x processor.
> > 
> > Does upstream DTS have support for this platform to understand it better ?
> > Even reference to any complete DT file for the platform will help.
> 
> Yes, you can ref to the AM625 (CPU layout) DT here:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/ti/k3-am625.dtsi
> 
> The board/starter kit DT is:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/ti/k3-am625-sk.dts
> 
> The patches for idle state are not upstream, and only exist in this
> patch of mine here:
> https://github.com/DhruvaG2000/v-linux/commit/0fd088d624276a2e72b8dc6660d261ab6d194f4b
>

"arm,psci-suspend-param" indicate that this idle state doesn't loose the
cpu context which means timer doesn't stop. So adding "local-timer-stop"
sound completely wrong to me.

> [...]
> > > See this chunk in the kernel cpuidle driver:
> > > 	if (broadcast && tick_broadcast_enter()) {
> > >
> > > When I dug deeper into tick_broadcast_enter it always returns something
> > > non zero and hence in my case it was entering the if block and tried to
> > > find a deepest state. Then the deepest state would always return WFI and
> > > not the idle-state I had added.
> > >

It depends. If this is the last CPU and since you have marked the state with
"local-timer-stop" and the system doesn't have any other timers to use as
source of broadcast, it prevents one of the CPU entering that state. So you
could be matching all the above conditions on your platform and hence you
are observing the above.

> > > What we found out was on our kernel we end up using
> > >
> > > kernel/time/tick-broadcast-hrtimer.c
> > >
> > > This always seems to be keeping atleast 1 CPU busy and prevents idle.
> > > If we remove the local-timer-stop it was helping us, but we still need
> > > to dig into the full impact of what that entails and I am still
> > > interested in finding out how so many other users of similar idle-state
> > > implementation are able to do so without trouble.
> > >
> >

As mentioned about adding "local-timer-stop" for a retention state seems
pure wrong in my opinion as it contradicts to the fact that context is
retained.

> > Interesting. So if the platform is functional removing local-timer-stop,
> > I am bit confused. Either there is something else that is getting it out
>
> Yes it was interesting to us too, as to how the RCU didn't kick in and
> system continued to function as though nothing was wrong.
>

It worked as if it was a state with context lost. So there might be some
impact on the latency though it as the kernel assumed context lost and
re-entered/resumed through resume entry point rather than where it called
cpu_suspend() similar to wfi(). I mean only on the CPUs it was able to
enter this state as one of the CPU will never enter this if there are no
system timers to act as broadcast timer.

Does you system not have Arch timers memory mapped interface enabled and
interrupt wired to GIC(other than PPIs) ? Look at Juno R2 as example.

> > from the idle state so, it should be fine and it could be just some
> 
> It's probably UART keypresses or some userspace processes that get
> scheduled that bring the CPUs back out of TF-A's cpu_standby.

I doubt the CPU resume from suspend is based on some userspace event.

> Is it possible that EL1 interrupts can bring EL3 out of WFI? Is yes then
> it explains the behaviour. The arch timer could also be continuing to
> tick and bringing the CPUs out of ATF WFI.
>

Yes but that doesn't explain the behaviour. It could be just the timer
event from the broadcast timer.

> > misconfiguration.
> > 
> > > Arm64 recommends to use arch_timer instead of external timers. Once we
> > > enter el3, timer interrupts to el1 is blocked and hence it's equivalent
> > > to local-timer-stop, so it does make sense to keep this property, but
> > > then how are others able to enter idle-states for all plugged CPUs at
> > > the same time?
> > >
> > 
> > Some systems have system timer that can take over as broadcast timer when
> > CPUs enter deeper idle states where the local timers are stopped.
> 
> In CPUIdle we're not really clock gating anything so the timer does keep
> ticking. So in this particular case it might make sense to remove the
> local-timer-stop property from the idle-state.
>

Correct in your case it is retention state and hence local CPU timers
keep ticking and you can safely drop that property. However if you add
deeper idle states like CPU OFF with the power rail cut off, then you need
some system timer to act as backup/broadcast timer so that all the CPUs
can enter the state concurrently and wake up successfully.

> However we're looking into taking this further and putting interconnect
> and few other PLLs in bypass which could cause arch timer for eg. to
> tick slower.

I assume it will be present as another timer with the rate set appropriately.

> In this case would it still make sense to omit the property? 

No, you should mark it as stopped even if it is running at slower rate
as I am not sure if the local CPU timer support can handle rate change.

> We may even have some usecases planned where we may turn OFF
> the CPU once it is in TF-A cpu_standby/ WFI. What would be the right
> approach in such scenarios?
>

As mentioned above, this will be separate state and all CPUs can use this
if there is another system broadcast timer.

> Could you provide any examples where the local-timer-stop property is
> being used and an alternative timer can be configured once we enter the
> idle-state where CPU CTX maybe lost or clocks maybe bypass?
> great if you could share some example implementation if you're aware.

As I mentioned, Juno R2 is an example. It was broken on R0 with some SoC
errata(can't recall all the details as I looked at it almost a decade ago)

-- 
Regards,
Sudeep

Re: Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Vivek yadav]

Hi @Dhruva Gole,

Q.1. Does your CA-53 properly go into CPUIdle state and come out of
sleep state ?
Q.2. Can you provide a snapshot of command
cat /sys/devices/system/cpu/cpu*/cpuidle/state*/usage  ?
Q.3. How frequently CPUs are going into custom state1 (other than
standard WFI state) ?

> Any further luck on this?

I am still facing some issues. This issue is not closed yet.

>
> idle-states {
>       entry-method = "psci";
>        cpu_ret_l: cpu-retention-l {
>          compatible = "arm,idle-state";
>          arm,psci-suspend-param = <0x00010001>;
>          local-timer-stop;
>          entry-latency-us = <55>;
>           exit-latency-us = <140>;
>           min-residency-us = <780>;
>     };
> };
>
> I am using ``Menu governor`` with the ``psci_idle driver`` in its original form.
> After booting Linux I find out that the CPUIdle core is never going
> inside the ``cpu-retention`` state.
> To check time spent by CPU in any state. I am using the below command.
>
> ``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/time``

As of now I made some changes in the DT node. After making changes in
latency (which is mentioned below).

 idle-states {
       entry-method = "psci";
        cpu_ret_l: cpu-retention-l {
          compatible = "arm,idle-state";
          arm,psci-suspend-param = <0x00000000>;
          local-timer-stop;
          entry-latency-us = <300000>; # 300ms
           exit-latency-us = <300000>; # 300ms
           min-residency-us = <1000000>; # 1 sec
     };
 };

I can see that  CA-55 went into a sleep state (state1) using command
``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/time``.
As you mention earlier in a multicore system (2 or more) at least one
core keeps working and does not go into sleep state. It should happen
as per theory and other developers' case.

In my case, after some time, both CPUs (CPU0 and CPU1) go into sleep
state (state1). Hence the system console hangs.

My expectations are,
If I type anything on keyboard. UART interrupt should take out CPUs
from sleep state and execute commands. OR some periodic timer should
take the CPU out of sleep. Which is not happening as of now.
As you said  we can safely remove`` local-timer-stop``. It means local
timers are working for the CPUs and triggering interrupts ?

Any discussion on this topic will definitely help me.

Regards,
Vivek Yadav

On Thu, Dec 12, 2024 at 6:16 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Wed, Dec 11, 2024 at 08:04:28PM +0530, Dhruva Gole wrote:
> > On Dec 11, 2024 at 12:18:25 +0000, Sudeep Holla wrote:
> > > On Wed, Dec 11, 2024 at 11:20:52AM +0530, Dhruva Gole wrote:
> > [...]
> > > > >
> > > > >
> > > > > Hi @all,
> > > > >
> > > > > I am working on one custom SoC. Where I add one CPUIdle state for
> > > > > ``arm,cortex-a55`` processor.
> > > >
> > > > Any further luck on this?
> > > >
> > > > I have also been working on something similar[1] but on an A53 core on
> > > > TI-K3 AM62x processor.
> > >
> > > Does upstream DTS have support for this platform to understand it better ?
> > > Even reference to any complete DT file for the platform will help.
> >
> > Yes, you can ref to the AM625 (CPU layout) DT here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/ti/k3-am625.dtsi
> >
> > The board/starter kit DT is:
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/ti/k3-am625-sk.dts
> >
> > The patches for idle state are not upstream, and only exist in this
> > patch of mine here:
> > https://github.com/DhruvaG2000/v-linux/commit/0fd088d624276a2e72b8dc6660d261ab6d194f4b
> >
>
> "arm,psci-suspend-param" indicate that this idle state doesn't loose the
> cpu context which means timer doesn't stop. So adding "local-timer-stop"
> sound completely wrong to me.
>
> > [...]
> > > > See this chunk in the kernel cpuidle driver:
> > > >   if (broadcast && tick_broadcast_enter()) {
> > > >
> > > > When I dug deeper into tick_broadcast_enter it always returns something
> > > > non zero and hence in my case it was entering the if block and tried to
> > > > find a deepest state. Then the deepest state would always return WFI and
> > > > not the idle-state I had added.
> > > >
>
> It depends. If this is the last CPU and since you have marked the state with
> "local-timer-stop" and the system doesn't have any other timers to use as
> source of broadcast, it prevents one of the CPU entering that state. So you
> could be matching all the above conditions on your platform and hence you
> are observing the above.
>
> > > > What we found out was on our kernel we end up using
> > > >
> > > > kernel/time/tick-broadcast-hrtimer.c
> > > >
> > > > This always seems to be keeping atleast 1 CPU busy and prevents idle.
> > > > If we remove the local-timer-stop it was helping us, but we still need
> > > > to dig into the full impact of what that entails and I am still
> > > > interested in finding out how so many other users of similar idle-state
> > > > implementation are able to do so without trouble.
> > > >
> > >
>
> As mentioned about adding "local-timer-stop" for a retention state seems
> pure wrong in my opinion as it contradicts to the fact that context is
> retained.
>
> > > Interesting. So if the platform is functional removing local-timer-stop,
> > > I am bit confused. Either there is something else that is getting it out
> >
> > Yes it was interesting to us too, as to how the RCU didn't kick in and
> > system continued to function as though nothing was wrong.
> >
>
> It worked as if it was a state with context lost. So there might be some
> impact on the latency though it as the kernel assumed context lost and
> re-entered/resumed through resume entry point rather than where it called
> cpu_suspend() similar to wfi(). I mean only on the CPUs it was able to
> enter this state as one of the CPU will never enter this if there are no
> system timers to act as broadcast timer.
>
> Does you system not have Arch timers memory mapped interface enabled and
> interrupt wired to GIC(other than PPIs) ? Look at Juno R2 as example.
>
> > > from the idle state so, it should be fine and it could be just some
> >
> > It's probably UART keypresses or some userspace processes that get
> > scheduled that bring the CPUs back out of TF-A's cpu_standby.
>
> I doubt the CPU resume from suspend is based on some userspace event.
>
> > Is it possible that EL1 interrupts can bring EL3 out of WFI? Is yes then
> > it explains the behaviour. The arch timer could also be continuing to
> > tick and bringing the CPUs out of ATF WFI.
> >
>
> Yes but that doesn't explain the behaviour. It could be just the timer
> event from the broadcast timer.
>
> > > misconfiguration.
> > >
> > > > Arm64 recommends to use arch_timer instead of external timers. Once we
> > > > enter el3, timer interrupts to el1 is blocked and hence it's equivalent
> > > > to local-timer-stop, so it does make sense to keep this property, but
> > > > then how are others able to enter idle-states for all plugged CPUs at
> > > > the same time?
> > > >
> > >
> > > Some systems have system timer that can take over as broadcast timer when
> > > CPUs enter deeper idle states where the local timers are stopped.
> >
> > In CPUIdle we're not really clock gating anything so the timer does keep
> > ticking. So in this particular case it might make sense to remove the
> > local-timer-stop property from the idle-state.
> >
>
> Correct in your case it is retention state and hence local CPU timers
> keep ticking and you can safely drop that property. However if you add
> deeper idle states like CPU OFF with the power rail cut off, then you need
> some system timer to act as backup/broadcast timer so that all the CPUs
> can enter the state concurrently and wake up successfully.
>
> > However we're looking into taking this further and putting interconnect
> > and few other PLLs in bypass which could cause arch timer for eg. to
> > tick slower.
>
> I assume it will be present as another timer with the rate set appropriately.
>
> > In this case would it still make sense to omit the property?
>
> No, you should mark it as stopped even if it is running at slower rate
> as I am not sure if the local CPU timer support can handle rate change.
>
> > We may even have some usecases planned where we may turn OFF
> > the CPU once it is in TF-A cpu_standby/ WFI. What would be the right
> > approach in such scenarios?
> >
>
> As mentioned above, this will be separate state and all CPUs can use this
> if there is another system broadcast timer.
>
> > Could you provide any examples where the local-timer-stop property is
> > being used and an alternative timer can be configured once we enter the
> > idle-state where CPU CTX maybe lost or clocks maybe bypass?
> > great if you could share some example implementation if you're aware.
>
> As I mentioned, Juno R2 is an example. It was broken on R0 with some SoC
> errata(can't recall all the details as I looked at it almost a decade ago)
>
> --
> Regards,
> Sudeep

Re: Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Sudeep Holla]

On Mon, Jan 27, 2025 at 10:47:28PM +0530, Vivek yadav wrote:
> Hi @Dhruva Gole,
> 
> Q.1. Does your CA-53 properly go into CPUIdle state and come out of
> sleep state ?

Yes, well tested on other SoCs. Seems like system integration issue.
> As of now I made some changes in the DT node. After making changes in
> latency (which is mentioned below).
> 
>  idle-states {
>        entry-method = "psci";
>         cpu_ret_l: cpu-retention-l {
>           compatible = "arm,idle-state";
>           arm,psci-suspend-param = <0x00000000>;
>           local-timer-stop;
>           entry-latency-us = <300000>; # 300ms
>            exit-latency-us = <300000>; # 300ms
>            min-residency-us = <1000000>; # 1 sec
>      };
>  };
>

Does these align with expectation of PSCI implementation in the firmware ?


> I can see that  CA-55 went into a sleep state (state1) using command
> ``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/time``.
> As you mention earlier in a multicore system (2 or more) at least one
> core keeps working and does not go into sleep state. It should happen
> as per theory and other developers' case.
> 
> In my case, after some time, both CPUs (CPU0 and CPU1) go into sleep
> state (state1). Hence the system console hangs.
> 
> My expectations are,
> If I type anything on keyboard. UART interrupt should take out CPUs
> from sleep state and execute commands. OR some periodic timer should
> take the CPU out of sleep. Which is not happening as of now.
> As you said  we can safely remove`` local-timer-stop``. It means local
> timers are working for the CPUs and triggering interrupts ?
> 

Please go the thread and understand when and why you need local-timer-stop and
how it is related to the arm,psci-suspend-param value(especially the state
context loss bit)

I have not got response to my questions. You can just play with DT and get
things working here if the firmware expectation, hardware functionality
and DT properties don't align.

-- 
Regards,
Sudeep

Re: Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Dhruva Gole]

Hi Sudeep and Vivek,

On Jan 28, 2025 at 09:47:20 +0000, Sudeep Holla wrote:
> On Mon, Jan 27, 2025 at 10:47:28PM +0530, Vivek yadav wrote:
> > Hi @Dhruva Gole,
> > 
> > Q.1. Does your CA-53 properly go into CPUIdle state and come out of
> > sleep state ?
> 
> Yes, well tested on other SoCs. Seems like system integration issue.

Yes, with the local-timer-stop property removed, all A53 cores do enter
idle in TF-A at the same time.

> > As of now I made some changes in the DT node. After making changes in
> > latency (which is mentioned below).
> > 
> >  idle-states {
> >        entry-method = "psci";
> >         cpu_ret_l: cpu-retention-l {
> >           compatible = "arm,idle-state";
> >           arm,psci-suspend-param = <0x00000000>;
> >           local-timer-stop;
> >           entry-latency-us = <300000>; # 300ms
> >            exit-latency-us = <300000>; # 300ms
> >            min-residency-us = <1000000>; # 1 sec
> >      };
> >  };
> >
> 
> Does these align with expectation of PSCI implementation in the firmware ?

Just to add here, value of that parameter has some encoded
meaning and is given in the PSCI standard:
Table 7 power_state parameter bit fields in Original format
https://developer.arm.com/documentation/den0022/fb/?lang=en

> 
> 
> > I can see that  CA-55 went into a sleep state (state1) using command
> > ``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/time``.
> > As you mention earlier in a multicore system (2 or more) at least one
> > core keeps working and does not go into sleep state. It should happen
> > as per theory and other developers' case.
> > 
> > In my case, after some time, both CPUs (CPU0 and CPU1) go into sleep
> > state (state1). Hence the system console hangs.
> > 
> > My expectations are,
> > If I type anything on keyboard. UART interrupt should take out CPUs
> > from sleep state and execute commands. OR some periodic timer should
> > take the CPU out of sleep. Which is not happening as of now.
> > As you said  we can safely remove`` local-timer-stop``. It means local
> > timers are working for the CPUs and triggering interrupts ?
> > 
> 
> Please go the thread and understand when and why you need local-timer-stop and
> how it is related to the arm,psci-suspend-param value(especially the state
> context loss bit)

Yes this is the important bit, if you know that on your platform the
A53s are just not going to power off or stop timers upon entering idle
then you must remove the local-timer-stop property from your DT
cpu_ret_l.
However, if you do have a scenario where the timer would be getting
stopped or modified in idle scenario, then linux needs to be able to use
another timer that is routed to the GIC and is unaffected while the
system is in idle.

This is what my understanding is so far, I am yet to do experiments with
local-timer-stop + different timer in the case of idle.

> 
> I have not got response to my questions. You can just play with DT and get
> things working here if the firmware expectation, hardware functionality
> and DT properties don't align.

I have responded to the thread now, sorry for not getting back earlier!

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated

Re: Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Sudeep Holla]

On Tue, Jan 28, 2025 at 04:44:07PM +0530, Dhruva Gole wrote:
> Hi Sudeep and Vivek,
>
> On Jan 28, 2025 at 09:47:20 +0000, Sudeep Holla wrote:
> > On Mon, Jan 27, 2025 at 10:47:28PM +0530, Vivek yadav wrote:
> > > Hi @Dhruva Gole,
> > >
> > > Q.1. Does your CA-53 properly go into CPUIdle state and come out of
> > > sleep state ?
> >
> > Yes, well tested on other SoCs. Seems like system integration issue.
>
> Yes, with the local-timer-stop property removed, all A53 cores do enter
> idle in TF-A at the same time.
>

Cool.

> > > As of now I made some changes in the DT node. After making changes in
> > > latency (which is mentioned below).
> > >
> > >  idle-states {
> > >        entry-method = "psci";
> > >         cpu_ret_l: cpu-retention-l {
> > >           compatible = "arm,idle-state";
> > >           arm,psci-suspend-param = <0x00000000>;
> > >           local-timer-stop;
> > >           entry-latency-us = <300000>; # 300ms
> > >            exit-latency-us = <300000>; # 300ms
> > >            min-residency-us = <1000000>; # 1 sec
> > >      };
> > >  };
> > >
> >
> > Does these align with expectation of PSCI implementation in the firmware ?
>
> Just to add here, value of that parameter has some encoded
> meaning and is given in the PSCI standard:
> Table 7 power_state parameter bit fields in Original format
> https://developer.arm.com/documentation/den0022/fb/?lang=en
>

Excellent! I just wanted to be sure that we are not just playing with these in
the DT without realising what that means to the PSCI implementation.

> >
> >
> > > I can see that  CA-55 went into a sleep state (state1) using command
> > > ``cat /sys/devices/system/cpu/cpu*/cpuidle/state*/time``.
> > > As you mention earlier in a multicore system (2 or more) at least one
> > > core keeps working and does not go into sleep state. It should happen
> > > as per theory and other developers' case.
> > >
> > > In my case, after some time, both CPUs (CPU0 and CPU1) go into sleep
> > > state (state1). Hence the system console hangs.
> > >
> > > My expectations are,
> > > If I type anything on keyboard. UART interrupt should take out CPUs
> > > from sleep state and execute commands. OR some periodic timer should
> > > take the CPU out of sleep. Which is not happening as of now.
> > > As you said  we can safely remove`` local-timer-stop``. It means local
> > > timers are working for the CPUs and triggering interrupts ?
> > >
> >
> > Please go the thread and understand when and why you need local-timer-stop and
> > how it is related to the arm,psci-suspend-param value(especially the state
> > context loss bit)
>
> Yes this is the important bit, if you know that on your platform the
> A53s are just not going to power off or stop timers upon entering idle
> then you must remove the local-timer-stop property from your DT
> cpu_ret_l.
> However, if you do have a scenario where the timer would be getting
> stopped or modified in idle scenario, then linux needs to be able to use
> another timer that is routed to the GIC and is unaffected while the
> system is in idle.
>

Looks like we are in sync with our understanding now, that's progress.

> This is what my understanding is so far, I am yet to do experiments with
> local-timer-stop + different timer in the case of idle.
>

Yes, that would be interesting.

> >
> > I have not got response to my questions. You can just play with DT and get
> > things working here if the firmware expectation, hardware functionality
> > and DT properties don't align.
>
> I have responded to the thread now, sorry for not getting back earlier!
>

No worries, just wanted to make sure those queries didn't slip through the
cracks.

--
Regards,
Sudeep

Re: Fwd: ARM64: CPUIdle driver is not select any Idle state other then WFI

[Dhruva Gole]

Hi Sudeep,

On Dec 12, 2024 at 12:46:37 +0000, Sudeep Holla wrote:
> On Wed, Dec 11, 2024 at 08:04:28PM +0530, Dhruva Gole wrote:
> > On Dec 11, 2024 at 12:18:25 +0000, Sudeep Holla wrote:
> > > On Wed, Dec 11, 2024 at 11:20:52AM +0530, Dhruva Gole wrote:
> > [...]
> > > > >
> > > > >
> > > > > Hi @all,
> > > > >
> > > > > I am working on one custom SoC. Where I add one CPUIdle state for
> > > > > ``arm,cortex-a55`` processor.
> > > >
> > > > Any further luck on this?
> > > >
> > > > I have also been working on something similar[1] but on an A53 core on
> > > > TI-K3 AM62x processor.
> > > 
> > > Does upstream DTS have support for this platform to understand it better ?
> > > Even reference to any complete DT file for the platform will help.
> > 
> > Yes, you can ref to the AM625 (CPU layout) DT here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/ti/k3-am625.dtsi
> > 
> > The board/starter kit DT is:
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/ti/k3-am625-sk.dts
> > 
> > The patches for idle state are not upstream, and only exist in this
> > patch of mine here:
> > https://github.com/DhruvaG2000/v-linux/commit/0fd088d624276a2e72b8dc6660d261ab6d194f4b
> >
> 
> "arm,psci-suspend-param" indicate that this idle state doesn't loose the
> cpu context which means timer doesn't stop. So adding "local-timer-stop"
> sound completely wrong to me.
> 

OK Understood.
Removing that does indeed help, and works in the case where our local
timers are not touched. System is indeed entering WFI on all 4 cores at
the same time, confirmed this via some logic I implemented in TF-A.

> > [...]
> > > > See this chunk in the kernel cpuidle driver:
> > > > 	if (broadcast && tick_broadcast_enter()) {
> > > >
> > > > When I dug deeper into tick_broadcast_enter it always returns something
> > > > non zero and hence in my case it was entering the if block and tried to
> > > > find a deepest state. Then the deepest state would always return WFI and
> > > > not the idle-state I had added.
> > > >
> 
> It depends. If this is the last CPU and since you have marked the state with
> "local-timer-stop" and the system doesn't have any other timers to use as
> source of broadcast, it prevents one of the CPU entering that state. So you
> could be matching all the above conditions on your platform and hence you
> are observing the above.

Yes, this is most likely the reason.

> 
> > > > What we found out was on our kernel we end up using
> > > >
> > > > kernel/time/tick-broadcast-hrtimer.c
> > > >
> > > > This always seems to be keeping atleast 1 CPU busy and prevents idle.
> > > > If we remove the local-timer-stop it was helping us, but we still need
> > > > to dig into the full impact of what that entails and I am still
> > > > interested in finding out how so many other users of similar idle-state
> > > > implementation are able to do so without trouble.
> > > >
> > >
> 
> As mentioned about adding "local-timer-stop" for a retention state seems
> pure wrong in my opinion as it contradicts to the fact that context is
> retained.
> 
> > > Interesting. So if the platform is functional removing local-timer-stop,
> > > I am bit confused. Either there is something else that is getting it out
> >
> > Yes it was interesting to us too, as to how the RCU didn't kick in and
> > system continued to function as though nothing was wrong.
> >
> 
> It worked as if it was a state with context lost. So there might be some
> impact on the latency though it as the kernel assumed context lost and
> re-entered/resumed through resume entry point rather than where it called
> cpu_suspend() similar to wfi(). I mean only on the CPUs it was able to
> enter this state as one of the CPU will never enter this if there are no
> system timers to act as broadcast timer.
> 
> Does you system not have Arch timers memory mapped interface enabled and
> interrupt wired to GIC(other than PPIs) ? Look at Juno R2 as example.

It has arch timers mem mapped and irq wired to GIC (I have talked more
on this at the last)
Your analysis seems right, if there is local-timer-stop case, then
our platform doesn't really provide an alternate timer today.

> 
> > > from the idle state so, it should be fine and it could be just some
> > 
> > It's probably UART keypresses or some userspace processes that get
> > scheduled that bring the CPUs back out of TF-A's cpu_standby.
> 
> I doubt the CPU resume from suspend is based on some userspace event.
> 
> > Is it possible that EL1 interrupts can bring EL3 out of WFI? Is yes then
> > it explains the behaviour. The arch timer could also be continuing to
> > tick and bringing the CPUs out of ATF WFI.
> >
> 
> Yes but that doesn't explain the behaviour. It could be just the timer
> event from the broadcast timer.
> 
> > > misconfiguration.
> > > 
> > > > Arm64 recommends to use arch_timer instead of external timers. Once we
> > > > enter el3, timer interrupts to el1 is blocked and hence it's equivalent
> > > > to local-timer-stop, so it does make sense to keep this property, but
> > > > then how are others able to enter idle-states for all plugged CPUs at
> > > > the same time?
> > > >
> > > 
> > > Some systems have system timer that can take over as broadcast timer when
> > > CPUs enter deeper idle states where the local timers are stopped.
> > 
> > In CPUIdle we're not really clock gating anything so the timer does keep
> > ticking. So in this particular case it might make sense to remove the
> > local-timer-stop property from the idle-state.
> >
> 
> Correct in your case it is retention state and hence local CPU timers
> keep ticking and you can safely drop that property. However if you add
> deeper idle states like CPU OFF with the power rail cut off, then you need
> some system timer to act as backup/broadcast timer so that all the CPUs
> can enter the state concurrently and wake up successfully.
> 
> > However we're looking into taking this further and putting interconnect
> > and few other PLLs in bypass which could cause arch timer for eg. to
> > tick slower.
> 
> I assume it will be present as another timer with the rate set appropriately.
> 
> > In this case would it still make sense to omit the property? 
> 
> No, you should mark it as stopped even if it is running at slower rate
> as I am not sure if the local CPU timer support can handle rate change.

OK, makes sense.

> 
> > We may even have some usecases planned where we may turn OFF
> > the CPU once it is in TF-A cpu_standby/ WFI. What would be the right
> > approach in such scenarios?
> >
> 
> As mentioned above, this will be separate state and all CPUs can use this
> if there is another system broadcast timer.
> 
> > Could you provide any examples where the local-timer-stop property is
> > being used and an alternative timer can be configured once we enter the
> > idle-state where CPU CTX maybe lost or clocks maybe bypass?
> > great if you could share some example implementation if you're aware.
> 
> As I mentioned, Juno R2 is an example. It was broken on R0 with some SoC
> errata(can't recall all the details as I looked at it almost a decade ago)

Sorry for the late response, Thanks for all the pointers!
I will go through juno R2 once and understand what's going on.

What I see maybe helping it is as you mentioned these timers being
routed to GIC. [1]

8<---------------------------------------------------------------------------
memtimer: timer@2a810000 {
	compatible = "arm,armv7-timer-mem";
	reg = <0x0 0x2a810000 0x0 0x10000>;
	clock-frequency = <50000000>;
	#address-cells = <1>;
	#size-cells = <1>;
	ranges = <0 0x0 0x2a820000 0x20000>;
	status = "disabled";
	frame@2a830000 {
		frame-number = <1>;
		interrupts = <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
		reg = <0x10000 0x10000>;
	};
};
--------------------------------------------------------------------------->8

I will have to go back and see how I can use some of the timers on my
SoC that lie outside the A53s to register as clocks incase of
local-timer-stop scenario.
We already have few timers [2], I will check which one would be the
right one to use, and remove it from being used as a pwm timer and
rather be used for the A53 timer. This[3] is the driver for our platform.

[1]
https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/arm/juno-base.dtsi#L10
[2]
https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/ti/k3-am62-main.dtsi#L247
[3]
https://github.com/torvalds/linux/blob/master/drivers/clocksource/timer-ti-dm-systimer.c

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated