Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1330381 > unrolled thread

Re: Nokia N900: Proper C-states

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: Nokia N900: Proper C-states Tony Lindgren <tony@atomide.com> - 2016-02-09 17:00 +0100
    Re: Nokia N900: Proper C-states Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-02-09 18:10 +0100
      Re: Nokia N900: Proper C-states Pali Rohár <pali.rohar@gmail.com> - 2016-02-11 16:10 +0100
        Re: Nokia N900: Proper C-states Tony Lindgren <tony@atomide.com> - 2016-02-11 18:50 +0100
          Re: Nokia N900: Proper C-states Pali Rohár <pali.rohar@gmail.com> - 2016-02-15 11:20 +0100
            Re: Nokia N900: Proper C-states Tony Lindgren <tony@atomide.com> - 2016-02-15 16:30 +0100
              [PATCH] omap3: Add cpuidle parameters table for omap3430 Pali Rohár <pali.rohar@gmail.com> - 2016-02-15 19:10 +0100
                Re: [PATCH] omap3: Add cpuidle parameters table for omap3430 Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-02-16 11:10 +0100
                  Re: [PATCH] omap3: Add cpuidle parameters table for omap3430 Pali Rohár <pali.rohar@gmail.com> - 2016-02-16 11:10 +0100
                    Re: [PATCH] omap3: Add cpuidle parameters table for omap3430 Tony Lindgren <tony@atomide.com> - 2016-02-16 17:10 +0100
                      Re: [PATCH] omap3: Add cpuidle parameters table for omap3430 Tony Lindgren <tony@atomide.com> - 2016-02-19 19:40 +0100
        Re: Nokia N900: Proper C-states Pali Rohár <pali.rohar@gmail.com> - 2016-02-15 11:20 +0100

#1330381 — Re: Nokia N900: Proper C-states

* Daniel Lezcano <daniel.lezcano@linaro.org> [160102 14:01]:
> On 01/02/2016 03:26 PM, Pali Rohár wrote:
> >Hello,
> >
> >due to this Daniel Lezcano commit (ARM: OMAP3: cpuidle - remove rx51
> >cpuidle parameters table)
> >
> >https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=231900afba52d6faddfb480cde4132d4edc089bc
> >
> >we need patch cpuidle34xx.c code to fix commit for Nokia N900. See:
> >
> >https://github.com/pali/linux-n900/commit/e147fd4b678f1f3d7a5235287910960bd41e04dc
> >
> >As Nokia N900 code is converting from legacy board code to DST, I would
> >like to know how to patch correctly omap3_idle_driver in DTS with
> >correct values measured for Nokia N900. Thanks!

I'm almost certain those values are not n900 specific but should be
used for omap3430 in general.

> the conversion to the DT based arm cpuidle driver could be a bit complex
> with one issue (index 0 != cpu_do_idle()) and one performance regression
> (cpu_pm_enter/exit will be called in retention mode, even if this is not
> needed).
> 
> It will result in a PM code only on one side and on the other side, the
> generic cpuidle-arm.c driver will be used instead with the DT definition. It
> is worth to the conversion because the result will be nice IMO.
> 
> Added Lorenzo who is initially author of the arm generic driver. We already
> discussed in the past about those two issues above and I think this is
> something we should improve.

While this conversion process is going on, we could just add
omap3430_idle_driver and set that based on cpu_is_omap3430()
in omap3_idle_init().

This is assuming it really is 3430 specific. The only thing that
could potentially make it n900 specific is the fact that Nokia had
their own bootrom. But AFAIK these latencies are hardware specific
and should not be related to the bootrom.

Regards,

Tony

[toc] | [next] | [standalone]

#1330482

On 02/09/2016 04:56 PM, Tony Lindgren wrote:
> * Daniel Lezcano <daniel.lezcano@linaro.org> [160102 14:01]:
>> On 01/02/2016 03:26 PM, Pali Rohár wrote:
>>> Hello,
>>>
>>> due to this Daniel Lezcano commit (ARM: OMAP3: cpuidle - remove rx51
>>> cpuidle parameters table)
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=231900afba52d6faddfb480cde4132d4edc089bc
>>>
>>> we need patch cpuidle34xx.c code to fix commit for Nokia N900. See:
>>>
>>> https://github.com/pali/linux-n900/commit/e147fd4b678f1f3d7a5235287910960bd41e04dc
>>>
>>> As Nokia N900 code is converting from legacy board code to DST, I would
>>> like to know how to patch correctly omap3_idle_driver in DTS with
>>> correct values measured for Nokia N900. Thanks!
>
> I'm almost certain those values are not n900 specific but should be
> used for omap3430 in general.
>
>> the conversion to the DT based arm cpuidle driver could be a bit complex
>> with one issue (index 0 != cpu_do_idle()) and one performance regression
>> (cpu_pm_enter/exit will be called in retention mode, even if this is not
>> needed).
>>
>> It will result in a PM code only on one side and on the other side, the
>> generic cpuidle-arm.c driver will be used instead with the DT definition. It
>> is worth to the conversion because the result will be nice IMO.
>>
>> Added Lorenzo who is initially author of the arm generic driver. We already
>> discussed in the past about those two issues above and I think this is
>> something we should improve.
>
> While this conversion process is going on, we could just add
> omap3430_idle_driver and set that based on cpu_is_omap3430()
> in omap3_idle_init().

Yes, that could be a temporary alternative.




-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[toc] | [prev] | [next] | [standalone]

#1332130

On Tuesday 09 February 2016 18:06:26 Daniel Lezcano wrote:
> On 02/09/2016 04:56 PM, Tony Lindgren wrote:
> >* Daniel Lezcano <daniel.lezcano@linaro.org> [160102 14:01]:
> >>On 01/02/2016 03:26 PM, Pali Rohár wrote:
> >>>Hello,
> >>>
> >>>due to this Daniel Lezcano commit (ARM: OMAP3: cpuidle - remove rx51
> >>>cpuidle parameters table)
> >>>
> >>>https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=231900afba52d6faddfb480cde4132d4edc089bc
> >>>
> >>>we need patch cpuidle34xx.c code to fix commit for Nokia N900. See:
> >>>
> >>>https://github.com/pali/linux-n900/commit/e147fd4b678f1f3d7a5235287910960bd41e04dc
> >>>
> >>>As Nokia N900 code is converting from legacy board code to DST, I would
> >>>like to know how to patch correctly omap3_idle_driver in DTS with
> >>>correct values measured for Nokia N900. Thanks!
> >
> >I'm almost certain those values are not n900 specific but should be
> >used for omap3430 in general.
> >

cpuidle parameters for Nokia N900 were added by this commit:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5a1b1d3a9efad6bd53d01ff02e86626d1a51d697

which says:

Numbers based on measurements made in October 2009 for PM optimized
kernel with CPU freq enabled.

Tony, do you have any other omap3430 device?

> >>the conversion to the DT based arm cpuidle driver could be a bit complex
> >>with one issue (index 0 != cpu_do_idle()) and one performance regression
> >>(cpu_pm_enter/exit will be called in retention mode, even if this is not
> >>needed).
> >>
> >>It will result in a PM code only on one side and on the other side, the
> >>generic cpuidle-arm.c driver will be used instead with the DT definition. It
> >>is worth to the conversion because the result will be nice IMO.
> >>
> >>Added Lorenzo who is initially author of the arm generic driver. We already
> >>discussed in the past about those two issues above and I think this is
> >>something we should improve.
> >
> >While this conversion process is going on, we could just add
> >omap3430_idle_driver and set that based on cpu_is_omap3430()
> >in omap3_idle_init().
> 
> Yes, that could be a temporary alternative.

I would be happy with any solution that allows me to drop my hack patch
from linux-n900 tree.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1332269

* Pali Rohár <pali.rohar@gmail.com> [160211 07:03]:
> > On 02/09/2016 04:56 PM, Tony Lindgren wrote:
> > >I'm almost certain those values are not n900 specific but should be
> > >used for omap3430 in general.
> 
> cpuidle parameters for Nokia N900 were added by this commit:
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5a1b1d3a9efad6bd53d01ff02e86626d1a51d697
> 
> which says:
> 
> Numbers based on measurements made in October 2009 for PM optimized
> kernel with CPU freq enabled.
> 
> Tony, do you have any other omap3430 device?

Yes I can make sure it works for omap3-ldp for a minimal case.
That's a 3430 ES 3.0 that's a GP SoC.

Regards,

Tony

[toc] | [prev] | [next] | [standalone]

#1334347

On Thursday 11 February 2016 09:47:50 Tony Lindgren wrote:
> * Pali Rohár <pali.rohar@gmail.com> [160211 07:03]:
> > > On 02/09/2016 04:56 PM, Tony Lindgren wrote:
> > > >I'm almost certain those values are not n900 specific but should be
> > > >used for omap3430 in general.
> > 
> > cpuidle parameters for Nokia N900 were added by this commit:
> > 
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5a1b1d3a9efad6bd53d01ff02e86626d1a51d697
> > 
> > which says:
> > 
> > Numbers based on measurements made in October 2009 for PM optimized
> > kernel with CPU freq enabled.
> > 
> > Tony, do you have any other omap3430 device?
> 
> Yes I can make sure it works for omap3-ldp for a minimal case.
> That's a 3430 ES 3.0 that's a GP SoC.
> 
> Regards,
> 
> Tony

Ok. If it works and you think those values are not Nokia N900 specific,
but omap3430, we can add them into kernel.

Do you want me to prepare patch? Or will you do it?

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1334528

* Pali Rohár <pali.rohar@gmail.com> [160215 02:12]:
> On Thursday 11 February 2016 09:47:50 Tony Lindgren wrote:
> > * Pali Rohár <pali.rohar@gmail.com> [160211 07:03]:
> > > > On 02/09/2016 04:56 PM, Tony Lindgren wrote:
> > > > >I'm almost certain those values are not n900 specific but should be
> > > > >used for omap3430 in general.
> > > 
> > > cpuidle parameters for Nokia N900 were added by this commit:
> > > 
> > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5a1b1d3a9efad6bd53d01ff02e86626d1a51d697
> > > 
> > > which says:
> > > 
> > > Numbers based on measurements made in October 2009 for PM optimized
> > > kernel with CPU freq enabled.
> > > 
> > > Tony, do you have any other omap3430 device?
> > 
> > Yes I can make sure it works for omap3-ldp for a minimal case.
> > That's a 3430 ES 3.0 that's a GP SoC.
> > 
> > Regards,
> > 
> > Tony
> 
> Ok. If it works and you think those values are not Nokia N900 specific,
> but omap3430, we can add them into kernel.
> 
> Do you want me to prepare patch? Or will you do it?

Yes please do if you can, I have my hands full with other
regressions in Linux next.

Regards,

Tony

[toc] | [prev] | [next] | [standalone]

#1334678 — [PATCH] omap3: Add cpuidle parameters table for omap3430

Based on CPU type choose generic omap3 or omap3430 specific cpuidle
parameters. Parameters for omap3430 were measured on Nokia N900 device and
added by commit 5a1b1d3a9efa ("OMAP3: RX-51: Pass cpu idle parameters")
which were later removed by commit 231900afba52 ("ARM: OMAP3: cpuidle -
remove rx51 cpuidle parameters table") due to huge code complexity.

This patch brings cpuidle parameters for omap3430 devices again, but uses
simple condition based on CPU type.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Fixes: 231900afba52d6faddfb480cde4132d4edc089bc
---
 arch/arm/mach-omap2/cpuidle34xx.c |   69 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index aa7b379..2a3db0b 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -34,6 +34,7 @@
 #include "pm.h"
 #include "control.h"
 #include "common.h"
+#include "soc.h"
 
 /* Mach specific information to be recorded in the C-state driver_data */
 struct omap3_idle_statedata {
@@ -315,6 +316,69 @@ static struct cpuidle_driver omap3_idle_driver = {
 	.safe_state_index = 0,
 };
 
+/*
+ * Numbers based on measurements made in October 2009 for PM optimized kernel
+ * with CPU freq enabled on device Nokia N900. Assumes OPP2 (main idle OPP,
+ * and worst case latencies).
+ */
+static struct cpuidle_driver omap3430_idle_driver = {
+	.name             = "omap3430_idle",
+	.owner            = THIS_MODULE,
+	.states = {
+		{
+			.enter		  = omap3_enter_idle_bm,
+			.exit_latency	  = 110 + 162,
+			.target_residency = 5,
+			.name		  = "C1",
+			.desc		  = "MPU ON + CORE ON",
+		},
+		{
+			.enter		  = omap3_enter_idle_bm,
+			.exit_latency	  = 106 + 180,
+			.target_residency = 309,
+			.name		  = "C2",
+			.desc		  = "MPU ON + CORE ON",
+		},
+		{
+			.enter		  = omap3_enter_idle_bm,
+			.exit_latency	  = 107 + 410,
+			.target_residency = 46057,
+			.name		  = "C3",
+			.desc		  = "MPU RET + CORE ON",
+		},
+		{
+			.enter		  = omap3_enter_idle_bm,
+			.exit_latency	  = 121 + 3374,
+			.target_residency = 46057,
+			.name		  = "C4",
+			.desc		  = "MPU OFF + CORE ON",
+		},
+		{
+			.enter		  = omap3_enter_idle_bm,
+			.exit_latency	  = 855 + 1146,
+			.target_residency = 46057,
+			.name		  = "C5",
+			.desc		  = "MPU RET + CORE RET",
+		},
+		{
+			.enter		  = omap3_enter_idle_bm,
+			.exit_latency	  = 7580 + 4134,
+			.target_residency = 484329,
+			.name		  = "C6",
+			.desc		  = "MPU OFF + CORE RET",
+		},
+		{
+			.enter		  = omap3_enter_idle_bm,
+			.exit_latency	  = 7505 + 15274,
+			.target_residency = 484329,
+			.name		  = "C7",
+			.desc		  = "MPU OFF + CORE OFF",
+		},
+	},
+	.state_count = ARRAY_SIZE(omap3_idle_data),
+	.safe_state_index = 0,
+};
+
 /* Public functions */
 
 /**
@@ -333,5 +397,8 @@ int __init omap3_idle_init(void)
 	if (!mpu_pd || !core_pd || !per_pd || !cam_pd)
 		return -ENODEV;
 
-	return cpuidle_register(&omap3_idle_driver, NULL);
+	if (cpu_is_omap3430())
+		return cpuidle_register(&omap3430_idle_driver, NULL);
+	else
+		return cpuidle_register(&omap3_idle_driver, NULL);
 }
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1335229 — Re: [PATCH] omap3: Add cpuidle parameters table for omap3430

On 02/15/2016 07:02 PM, Pali Rohár wrote:
> Based on CPU type choose generic omap3 or omap3430 specific cpuidle
> parameters. Parameters for omap3430 were measured on Nokia N900 device and
> added by commit 5a1b1d3a9efa ("OMAP3: RX-51: Pass cpu idle parameters")
> which were later removed by commit 231900afba52 ("ARM: OMAP3: cpuidle -
> remove rx51 cpuidle parameters table") due to huge code complexity.
>
> This patch brings cpuidle parameters for omap3430 devices again, but uses
> simple condition based on CPU type.
>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> Fixes: 231900afba52d6faddfb480cde4132d4edc089bc
> ---

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[toc] | [prev] | [next] | [standalone]

#1335237 — Re: [PATCH] omap3: Add cpuidle parameters table for omap3430

On Tuesday 16 February 2016 11:01:43 Daniel Lezcano wrote:
> On 02/15/2016 07:02 PM, Pali Rohár wrote:
> >Based on CPU type choose generic omap3 or omap3430 specific cpuidle
> >parameters. Parameters for omap3430 were measured on Nokia N900 device and
> >added by commit 5a1b1d3a9efa ("OMAP3: RX-51: Pass cpu idle parameters")
> >which were later removed by commit 231900afba52 ("ARM: OMAP3: cpuidle -
> >remove rx51 cpuidle parameters table") due to huge code complexity.
> >
> >This patch brings cpuidle parameters for omap3430 devices again, but uses
> >simple condition based on CPU type.
> >
> >Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> >Fixes: 231900afba52d6faddfb480cde4132d4edc089bc
> >---
> 
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Tony, Nishanth (or somebody else...) can you test this patch on more
omap3 devices to check if it does not break existing cpuidle code?

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1335585 — Re: [PATCH] omap3: Add cpuidle parameters table for omap3430

* Pali Rohár <pali.rohar@gmail.com> [160216 02:09]:
> On Tuesday 16 February 2016 11:01:43 Daniel Lezcano wrote:
> > On 02/15/2016 07:02 PM, Pali Rohár wrote:
> > >Based on CPU type choose generic omap3 or omap3430 specific cpuidle
> > >parameters. Parameters for omap3430 were measured on Nokia N900 device and
> > >added by commit 5a1b1d3a9efa ("OMAP3: RX-51: Pass cpu idle parameters")
> > >which were later removed by commit 231900afba52 ("ARM: OMAP3: cpuidle -
> > >remove rx51 cpuidle parameters table") due to huge code complexity.
> > >
> > >This patch brings cpuidle parameters for omap3430 devices again, but uses
> > >simple condition based on CPU type.
> > >
> > >Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > >Fixes: 231900afba52d6faddfb480cde4132d4edc089bc
> > >---
> > 
> > Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> 
> Tony, Nishanth (or somebody else...) can you test this patch on more
> omap3 devices to check if it does not break existing cpuidle code?

Seems to work for me on a ldp, which is GP 3430 ES3.0.

Regards,

Tony

[toc] | [prev] | [next] | [standalone]

#1338406 — Re: [PATCH] omap3: Add cpuidle parameters table for omap3430

* Tony Lindgren <tony@atomide.com> [160216 08:11]:
> * Pali Rohár <pali.rohar@gmail.com> [160216 02:09]:
> > On Tuesday 16 February 2016 11:01:43 Daniel Lezcano wrote:
> > > On 02/15/2016 07:02 PM, Pali Rohár wrote:
> > > >Based on CPU type choose generic omap3 or omap3430 specific cpuidle
> > > >parameters. Parameters for omap3430 were measured on Nokia N900 device and
> > > >added by commit 5a1b1d3a9efa ("OMAP3: RX-51: Pass cpu idle parameters")
> > > >which were later removed by commit 231900afba52 ("ARM: OMAP3: cpuidle -
> > > >remove rx51 cpuidle parameters table") due to huge code complexity.
> > > >
> > > >This patch brings cpuidle parameters for omap3430 devices again, but uses
> > > >simple condition based on CPU type.
> > > >
> > > >Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > >Fixes: 231900afba52d6faddfb480cde4132d4edc089bc
> > > >---
> > > 
> > > Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > 
> > Tony, Nishanth (or somebody else...) can you test this patch on more
> > omap3 devices to check if it does not break existing cpuidle code?
> 
> Seems to work for me on a ldp, which is GP 3430 ES3.0.

OK applying this into omap-for-v4.6/fixes-not-urgent considering
we've already lived with it for years.

Regards,

Tony

[toc] | [prev] | [next] | [standalone]

#1334344

On Thursday 11 February 2016 16:02:21 Pali Rohár wrote:
> On Tuesday 09 February 2016 18:06:26 Daniel Lezcano wrote:
> > On 02/09/2016 04:56 PM, Tony Lindgren wrote:
> > >* Daniel Lezcano <daniel.lezcano@linaro.org> [160102 14:01]:
> > >>On 01/02/2016 03:26 PM, Pali Rohár wrote:
> > >>>Hello,
> > >>>
> > >>>due to this Daniel Lezcano commit (ARM: OMAP3: cpuidle - remove rx51
> > >>>cpuidle parameters table)
> > >>>
> > >>>https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=231900afba52d6faddfb480cde4132d4edc089bc
> > >>>
> > >>>we need patch cpuidle34xx.c code to fix commit for Nokia N900. See:
> > >>>
> > >>>https://github.com/pali/linux-n900/commit/e147fd4b678f1f3d7a5235287910960bd41e04dc
> > >>>
> > >>>As Nokia N900 code is converting from legacy board code to DST, I would
> > >>>like to know how to patch correctly omap3_idle_driver in DTS with
> > >>>correct values measured for Nokia N900. Thanks!
> > >
> > >I'm almost certain those values are not n900 specific but should be
> > >used for omap3430 in general.
> > >
> 
> cpuidle parameters for Nokia N900 were added by this commit:
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5a1b1d3a9efad6bd53d01ff02e86626d1a51d697
> 
> which says:
> 
> Numbers based on measurements made in October 2009 for PM optimized
> kernel with CPU freq enabled.

Adding Kalle Jokiniemi, author of that commit which added cpuidle
parameters for Nokia N900.

Kalle, do you know if cpuidle values measured are Nokia N900 specific or
general omap3430?

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web