Skip to contentSkip to conversation

Nokia N900: Proper C-states

PATCH In review Pali Rohár 3 participants 14 messages Jan 2, 2016 – Feb 19, 2016

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

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!

Signature · 2 lines
Pali Rohár
[email protected]
#1 lore raw
Quoted 7 lines · Pali Rohár

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

Quoted 7 lines

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!

Hi Pali,

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.

Signature · 5 lines
 <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
maintainer
#2 lore raw
Quoted 8 lines · Daniel Lezcano
Quoted 7 lines · Pali Rohár

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

Quoted 7 lines
Quoted 7 lines

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.

Quoted 5 lines

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

Quoted 7 lines

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

#3 lore raw
Quoted 29 lines · Tony Lindgren
Quoted 8 lines · Daniel Lezcano
Quoted 7 lines · Pali Rohár

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

Quoted 7 lines
Quoted 7 lines

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.

Quoted 9 lines

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.

Quoted 7 lines
Quoted 3 lines

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.

Signature · 5 lines
 <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
#4 lore raw
Quoted 14 lines · On Tuesday 09 February 2016 18:06:26 Daniel Lezcano wrote:
Quoted 13 lines · Tony Lindgren
Quoted 12 lines · Daniel Lezcano
Quoted 11 lines · Pali Rohár

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

Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
Quoted 3 lines

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?

Quoted 13 lines
Quoted 13 lines
Quoted 12 lines

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.

Quoted 5 lines
Quoted 3 lines

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.

Signature · 2 lines
Pali Rohár
[email protected]
maintainer
#5 lore raw
Quoted 6 lines · Pali Rohár
Quoted 3 lines · Tony Lindgren
Quoted 2 lines

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:

Quoted 8 lines

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

#6 lore raw
Quoted 14 lines · On Thursday 11 February 2016 09:47:50 Tony Lindgren wrote:
Quoted 13 lines · Pali Rohár
Quoted 3 lines · Tony Lindgren
Quoted 2 lines

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.

Quoted 8 lines
Quoted 1 line

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?

Signature · 2 lines
Pali Rohár
[email protected]
maintainer
#7 lore raw
Quoted 20 lines · Pali Rohár
Quoted 19 lines · On Thursday 11 February 2016 09:47:50 Tony Lindgren wrote:
Quoted 6 lines · Pali Rohár
Quoted 3 lines · Tony Lindgren
Quoted 2 lines

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:

Quoted 8 lines

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.

Quoted 8 lines
Quoted 3 lines

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

#8 lore raw

PATCH 1/1 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.

Fixes 231900afba52d6faddfb480cde4132d4edc089bc
---
 arch/arm/mach-omap2/cpuidle34xx.c |   69 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)
arch/arm/mach-omap2/cpuidle34xx.c +68 -1
index aa7b379..2a3db0b 100644
@@ -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);
 }
Signature · 1 line
1.7.9.5
Acked-by on v1 1/1
#9 lore raw

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

Quoted 6 lines · Pali Rohár

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.

Quoted 6 lines

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

Fixes 231900afba52d6faddfb480cde4132d4edc089bc

---

Signature · 5 lines
 <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
#10 lore raw

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

Quoted 7 lines · On Tuesday 16 February 2016 11:01:43 Daniel Lezcano wrote:
Quoted 6 lines · Pali Rohár

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.

Quoted 8 lines
Quoted 6 lines

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

Fixes 231900afba52d6faddfb480cde4132d4edc089bc

---

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

Signature · 2 lines
Pali Rohár
[email protected]
maintainer
#11 lore raw

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

Quoted 11 lines · Pali Rohár
Quoted 10 lines · On Tuesday 16 February 2016 11:01:43 Daniel Lezcano wrote:
Quoted 9 lines · Pali Rohár

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.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Fixes 231900afba52d6faddfb480cde4132d4edc089bc

---

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

maintainer
#12 lore raw

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

Quoted 16 lines · Tony Lindgren
Quoted 15 lines · Pali Rohár
Quoted 14 lines · On Tuesday 16 February 2016 11:01:43 Daniel Lezcano wrote:
Quoted 12 lines · Pali Rohár

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.

Fixes 231900afba52d6faddfb480cde4132d4edc089bc

---

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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

#13 lore raw
In reply to Pali Rohár
Quoted 23 lines · On Thursday 11 February 2016 16:02:21 Pali Rohár wrote:
Quoted 14 lines · On Tuesday 09 February 2016 18:06:26 Daniel Lezcano wrote:
Quoted 13 lines · Tony Lindgren
Quoted 12 lines · Daniel Lezcano
Quoted 11 lines · Pali Rohár

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

Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
Quoted 3 lines

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.

Quoted 8 lines

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?

Signature · 2 lines
Pali Rohár
[email protected]