RE: [PATCH] omap3: Add basic support for 720MHz part
-Original Message-
From: Varadarajan, Charulatha
Sent: Tuesday, January 18, 2011 2:44 PM
To: Premi, Sanjeev
Cc: linux-omap@vger.kernel.org
Subject: Re: [PATCH] omap3: Add basic support for 720MHz part
Couple of minor comments.
[snip]...[snip]
--- a/arch/arm/mach-omap2/control.h
+++ b/arch/arm/mach-omap2/control.h
@@ -365,6 +365,13 @@
#define FEAT_NEON 0
#define FEAT_NEON_NONE 1
+/*
+ * Product ID register
+ */
Do not use multiline comment style for single line comments
[sp] Was only following the multi-line comment stye few lines
above...
+#define OMAP3_PRODID 0x020C
+
+#define OMAP3_SKUID_MASK 0x0f
+#define OMAP3_SKUID_720MHZ 0x08
#ifndef __ASSEMBLY__
#ifdef CONFIG_ARCH_OMAP2PLUS
diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 5f9086c..53fbe01 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -195,6 +195,15 @@ static void __init omap3_check_features(void)
* TODO: Get additional info (where applicable)
* e.g. Size of L2 cache.
*/
+
+ /*
+ * Does it support 720MHz?
+ */
Ditto
[sp] Same here.
But since the code is quite evident, I will remove the comment
altogether.
+ status = (OMAP3_SKUID_MASK read_tap_reg(OMAP3_PRODID));
+
+ if (status OMAP3_SKUID_720MHZ) {
+ omap3_features |= OMAP3_HAS_720MHZ;
+ }
No need of { } as there is only one line statement inside the
if condition.
[sp] missed this.
~sanjeev
[snip]...[snip]--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3: Add basic support for 720MHz part
On Thu, Jan 20, 2011 at 16:28, Premi, Sanjeev pr...@ti.com wrote: -Original Message- From: Varadarajan, Charulatha Sent: Tuesday, January 18, 2011 2:44 PM To: Premi, Sanjeev Cc: linux-omap@vger.kernel.org Subject: Re: [PATCH] omap3: Add basic support for 720MHz part Also do not miss the version number while posting patches. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3: Add basic support for 720MHz part
On Thu, Jan 20, 2011 at 04:28:11PM +0530, Premi, Sanjeev wrote:
-Original Message-
From: Varadarajan, Charulatha
Sent: Tuesday, January 18, 2011 2:44 PM
To: Premi, Sanjeev
Cc: linux-omap@vger.kernel.org
Subject: Re: [PATCH] omap3: Add basic support for 720MHz part
Couple of minor comments.
[snip]...[snip]
--- a/arch/arm/mach-omap2/control.h
+++ b/arch/arm/mach-omap2/control.h
@@ -365,6 +365,13 @@
#define FEAT_NEON 0
#define FEAT_NEON_NONE 1
+/*
+ * Product ID register
+ */
Do not use multiline comment style for single line comments
[sp] Was only following the multi-line comment stye few lines
above...
+#define OMAP3_PRODID 0x020C
+
+#define OMAP3_SKUID_MASK 0x0f
+#define OMAP3_SKUID_720MHZ 0x08
#ifndef __ASSEMBLY__
#ifdef CONFIG_ARCH_OMAP2PLUS
diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 5f9086c..53fbe01 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -195,6 +195,15 @@ static void __init omap3_check_features(void)
* TODO: Get additional info (where applicable)
* e.g. Size of L2 cache.
*/
+
+ /*
+ * Does it support 720MHz?
+ */
Ditto
[sp] Same here.
But since the code is quite evident, I will remove the comment
altogether.
+ status = (OMAP3_SKUID_MASK read_tap_reg(OMAP3_PRODID));
+
+ if (status OMAP3_SKUID_720MHZ) {
+ omap3_features |= OMAP3_HAS_720MHZ;
+ }
No need of { } as there is only one line statement inside the
if condition.
[sp] missed this.
Looks fine. Feel free to add after the above changes:
Reviewed-by: G, Manjunath Kondaiah manj...@ti.com
-Manjunath
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] omap3: Add basic support for 720MHz part
-Original Message- From: G, Manjunath Kondaiah [mailto:manj...@ti.com] Sent: Thursday, January 20, 2011 4:51 PM To: Premi, Sanjeev Cc: Varadarajan, Charulatha; linux-omap@vger.kernel.org Subject: Re: [PATCH] omap3: Add basic support for 720MHz part [snip] Looks fine. Feel free to add after the above changes: Reviewed-by: G, Manjunath Kondaiah manj...@ti.com Sorry, noticed this mail after sending the patch from linux box :( -Manjunath -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] omap3: Add basic support for 720MHz part
Kooi, -Original Message- From: Koen Kooi [mailto:k...@dominion.thruhere.net] Sent: Tuesday, January 18, 2011 11:47 PM To: Vishwanath Sripathy Cc: Nishanth Menon; Sanjeev Premi; l-o List; Thara Gopinath Subject: Re: [PATCH] omap3: Add basic support for 720MHz part Op 18 jan 2011, om 18:18 heeft Vishwanath Sripathy het volgende geschreven: Kooi, -Original Message- From: Nishanth Menon [mailto:n...@ti.com] Sent: Tuesday, January 18, 2011 10:19 PM To: Koen Kooi Cc: Sanjeev Premi; l-o List; th...@ti.com; Vishwanath Sripathy Subject: Re: [PATCH] omap3: Add basic support for 720MHz part Koen Kooi wrote, on 01/18/2011 05:38 PM: Op 18 jan 2011, om 08:49 heeft Sanjeev Premi het volgende geschreven: This patch adds support for new speed enhanced parts with ARM and IVA running at 720MHz and 520MHz respectively. These parts can be probed at run-time by reading PRODID.SKUID[3:0] at 0x4830A20C [1]. This patch specifically does following: * Detect devices capable of 720MHz. * Add new OPP * Ensure that OPP is conditionally enabled. * Check for presence of IVA before attempting to enable the corresponding OPP. Thanks for the updated patch! I'm still having problem using this together with DVFS, the kernel won't scale beyond 600MHz because 600MHz and 720MHz share the same voltage. Thara, Nishant, do you have any suggestions on how to convince the kernel that 2 frequencies can share the same voltage settings? + Vishwa Yes, this is a limitation with the current set of DVFS patches. With the next version of DVFS patches (will be out soon) this feature is going to be supported. That means you can have 2 opps with the same voltage but with different frequency. The device will be configured based on user requested frequency. Just to have a crude hack, would something like this work?: /* MPU OPP6 */ - OPP_INITIALIZER(mpu, false, 72000, 135), + OPP_INITIALIZER(mpu, false, 72000, 1350001), Or will I fall into a (post)divider trap where the kernel panics because it can't do that exact voltage? The TRM[1] chickens out with the OPP definitions: --8-- -- --- Six generic processor OPPs can be defined as: 1. OPP6 (VDD1 = v4, (MPU_CLK = fmpu5, IVA2_CLK = fiva5)) 2. OPP5 (VDD1 = v4, (MPU_CLK = fmpu4, IVA2_CLK = fiva4)) 3. OPP4 (VDD1 = v3, (MPU_CLK = fmpu3, IVA2_CLK = fiva3)) 4. OPP3 (VDD1 = v2 , (MPU_CLK = fmpu2, IVA2_CLK = fiva2)) 5. OPP2 (VDD1 = v1 , (MPU_CLK = fmpu1, IVA2_CLK = fiva1)) 6. OPP1 (VDD1 = v0 , (MPU_CLK = fmpu0, IVA2_CLK = fiva0)) where v4 v3 v2 v1 v0, --8-- -- --- My limited sample size of 3 board (2x beagle C4, 1x overo tide) shows 600MHz@1.3V works as well, but I don't know how much slack that has. And I haven't check if the hardware can do 1.3V exactly or has rounded up. So, what's the quickest way with the current code to get 720MHz working? I've voided my warranty on everything already, so that isn't a problem :) If you want a quick hack, you can modify the voltage for 700M marginally (like you have done above). But it needs to be done in multiple places. You need to modify IVA OPP6 entry in opp table and OPP6 voltage value in voltage.h as well besides above change. This should work. Vishwa regards, Koen [1] spruf98m.pdf page 355 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3: Add basic support for 720MHz part
Vishwanath Sripathy wrote, on 01/19/2011 10:56 AM: If you want a quick hack, you can modify the voltage for 700M marginally (like you have done above). But it needs to be done in multiple places. You need to modify IVA OPP6 entry in opp table and OPP6 voltage value in voltage.h as well besides above change. This should work. we may want to be sure about keeping the additional voltages in 1 step increment (12.5mV - to ensure twl conversion routines sane)- i think we may be pretty much maxed out for the tps chip used, no? -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] omap3: Add basic support for 720MHz part
-Original Message- From: Koen Kooi [mailto:k...@dominion.thruhere.net] Sent: Tuesday, January 18, 2011 9:08 PM To: Premi, Sanjeev Cc: l-o List; Menon, Nishanth; Gopinath, Thara Subject: Re: [PATCH] omap3: Add basic support for 720MHz part [snip]...[snip] Thanks for the updated patch! I'm still having problem using this together with DVFS, the kernel won't scale beyond 600MHz because 600MHz and 720MHz share the same voltage. Thara, Nishant, do you have any suggestions on how to convince the kernel that 2 frequencies can share the same voltage settings? Koen, I still haven't spent time testing with DVFS. There are few issue that need to be resolved with the DVFS infra before DVFS can be functional. Would like this patch to be de-coupled from DVFS discussions. It was not the intent of the patch... Am working to list all the issues and possible solutions for the DVFS... getting delayed due to other activities. ~sanjeev [snip]...[snip] -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3: Add basic support for 720MHz part
Op 19 jan 2011, om 10:06 heeft Nishanth Menon het volgende geschreven:
Vishwanath Sripathy wrote, on 01/19/2011 10:56 AM:
If you want a quick hack, you can modify the voltage for 700M marginally
(like you have done above). But it needs to be done in multiple places.
You need to modify IVA OPP6 entry in opp table and OPP6 voltage value in
voltage.h as well besides above change. This should work.
we may want to be sure about keeping the additional voltages in 1 step
increment (12.5mV - to ensure twl conversion routines sane)- i think we may
be pretty much maxed out for the tps chip used, no?
Not using the 12.5mV steps gives me:
[3.076873] calc_dep_vdd_volt: Not able to find a matching volt forvdd_core
corresponding to vdd_mpu 1350001 volt
[3.095306] omap_voltage_scale: Error in calculating dependent vdd
voltagesfor vdd_mpu
It does seem to work, though:
root@usrp-e1xx:~# grep MIPS /proc/cpuinfo
BogoMIPS: 717.37
I think I'll just drop the OPP5 voltage by 12.5mV since I know what silicon my
usrp-e1xx has, but I can't really give out that patch to others, since someone
might replace the overo with another one.
This is the hack I ended up using:
diff --git a/arch/arm/mach-omap2/opp3xxx_data.c
b/arch/arm/mach-omap2/opp3xxx_data.c
index 76d26c7..c913240 100644
--- a/arch/arm/mach-omap2/opp3xxx_data.c
+++ b/arch/arm/mach-omap2/opp3xxx_data.c
@@ -37,7 +37,7 @@ static struct omap_opp_def __initdata omap34xx_opp_def_list[]
= {
/* MPU OPP5 */
OPP_INITIALIZER(mpu, true, 6, 135),
/* MPU OPP6 */
- OPP_INITIALIZER(mpu, false, 72000, 135),
+ OPP_INITIALIZER(mpu, false, 72000, 1350001),
/*
* L3 OPP1 - 41.5 MHz is disabled because: The voltage for that OPP is
@@ -64,7 +64,7 @@ static struct omap_opp_def __initdata omap34xx_opp_def_list[]
= {
/* DSP OPP5 */
OPP_INITIALIZER(iva, true, 43000, 135),
/* DSP OPP6 */
- OPP_INITIALIZER(iva, false, 52000, 135),
+ OPP_INITIALIZER(iva, false, 52000, 1350001),
};
static struct omap_opp_def __initdata omap36xx_opp_def_list[] = {
diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
index d10cb1b..2b7ff8f 100644
--- a/arch/arm/mach-omap2/voltage.c
+++ b/arch/arm/mach-omap2/voltage.c
@@ -324,6 +324,7 @@ static struct omap_volt_data omap34xx_vddmpu_volt_data[] = {
VOLT_DATA_DEFINE(OMAP3430_VDD_MPU_OPP3_UV,
OMAP343X_CONTROL_FUSE_OPP3_VDD1, 0xf9, 0x18),
VOLT_DATA_DEFINE(OMAP3430_VDD_MPU_OPP4_UV,
OMAP343X_CONTROL_FUSE_OPP4_VDD1, 0xf9, 0x18),
VOLT_DATA_DEFINE(OMAP3430_VDD_MPU_OPP5_UV,
OMAP343X_CONTROL_FUSE_OPP5_VDD1, 0xf9, 0x18),
+ VOLT_DATA_DEFINE(OMAP3430_VDD_MPU_OPP6_UV,
OMAP343X_CONTROL_FUSE_OPP5_VDD1, 0xf9, 0x18),
VOLT_DATA_DEFINE(0, 0, 0, 0),
};
diff --git a/arch/arm/plat-omap/include/plat/voltage.h
b/arch/arm/plat-omap/include/plat/voltage.h
index 6782c5e..42b89db 100644
--- a/arch/arm/plat-omap/include/plat/voltage.h
+++ b/arch/arm/plat-omap/include/plat/voltage.h
@@ -31,6 +31,7 @@
#define OMAP3430_VDD_MPU_OPP3_UV 120
#define OMAP3430_VDD_MPU_OPP4_UV 127
#define OMAP3430_VDD_MPU_OPP5_UV 135
+#define OMAP3430_VDD_MPU_OPP6_UV 1350001
#define OMAP3430_VDD_CORE_OPP1_UV 975000
#define OMAP3430_VDD_CORE_OPP2_UV 105
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3: Add basic support for 720MHz part
Op 19 jan 2011, om 10:49 heeft Premi, Sanjeev het volgende geschreven: -Original Message- From: Koen Kooi [mailto:k...@dominion.thruhere.net] Sent: Tuesday, January 18, 2011 9:08 PM To: Premi, Sanjeev Cc: l-o List; Menon, Nishanth; Gopinath, Thara Subject: Re: [PATCH] omap3: Add basic support for 720MHz part [snip]...[snip] Thanks for the updated patch! I'm still having problem using this together with DVFS, the kernel won't scale beyond 600MHz because 600MHz and 720MHz share the same voltage. Thara, Nishant, do you have any suggestions on how to convince the kernel that 2 frequencies can share the same voltage settings? Koen, I still haven't spent time testing with DVFS. There are few issue that need to be resolved with the DVFS infra before DVFS can be functional. Would like this patch to be de-coupled from DVFS discussions. It was not the intent of the patch... It was not my intent to couple it to DVFS discussions, I just used it as an example to highlight the bugs I'm hitting with DVFS and 720MHz operation :) I don't want this patch to be held back by bugs in the DVFS code!-- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] omap3: Add basic support for 720MHz part
-Original Message- From: Koen Kooi [mailto:k...@dominion.thruhere.net] Sent: Tuesday, January 18, 2011 11:47 PM To: Sripathy, Vishwanath Cc: Menon, Nishanth; Premi, Sanjeev; l-o List; Gopinath, Thara Subject: Re: [PATCH] omap3: Add basic support for 720MHz part [snip]...[snip] The TRM[1] chickens out with the OPP definitions: Refer to page 122 of the OMAP3503 datasheet for the OPP definitions: http://www.ti.com/litv/pdf/sprs505f The nominal voltage for both (VDD1) OPP5 and OPP6 is 1.35V. ~sanjeev -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3: Add basic support for 720MHz part
Couple of minor comments.
On Tue, Jan 18, 2011 at 13:19, Sanjeev Premi pr...@ti.com wrote:
This patch adds support for new speed enhanced parts with ARM
and IVA running at 720MHz and 520MHz respectively. These parts
can be probed at run-time by reading PRODID.SKUID[3:0] at
0x4830A20C [1].
This patch specifically does following:
* Detect devices capable of 720MHz.
* Add new OPP
* Ensure that OPP is conditionally enabled.
* Check for presence of IVA before attempting to enable
the corresponding OPP.
[1] http://focus.ti.com/lit/ug/spruff1d/spruff1d.pdf
Signed-off-by: Sanjeev Premi pr...@ti.com
---
Since v2:
1) pr_xxx() - dev_xxx() functions - suggested by
Manjunath (manj...@ti.com)
2) Add check for presense of IVA - earlier planned to
be in a separate patch; but we multiple discussions
on optimizations.
3) Do look-up for hwmod corresponding for iva only if
iva is present. Should save multiple strcmp()s in
_lookup().
Since v1:
1) Using opp_enable() to enable the OPP after the OPP
table has been initialized.
2) Starting at 3 levels of indent, the statements had
be broken into multiple lines for most of the code.
So, opted to create a new static that enables the
OPPs corresponding to 720MHz.
3) I have only build tested this patch - will be able
to confirm working tomorrow. With any further change,
if needed. (However, functionally nothing has changed.)
arch/arm/mach-omap2/control.h | 7
arch/arm/mach-omap2/id.c | 10 +
arch/arm/mach-omap2/opp3xxx_data.c | 63
-
arch/arm/plat-omap/include/plat/cpu.h | 2 +
4 files changed, 81 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h
index f0629ae..eebc045 100644
--- a/arch/arm/mach-omap2/control.h
+++ b/arch/arm/mach-omap2/control.h
@@ -365,6 +365,13 @@
#define FEAT_NEON 0
#define FEAT_NEON_NONE 1
+/*
+ * Product ID register
+ */
Do not use multiline comment style for single line comments
+#define OMAP3_PRODID 0x020C
+
+#define OMAP3_SKUID_MASK 0x0f
+#define OMAP3_SKUID_720MHZ 0x08
#ifndef __ASSEMBLY__
#ifdef CONFIG_ARCH_OMAP2PLUS
diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 5f9086c..53fbe01 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -195,6 +195,15 @@ static void __init omap3_check_features(void)
* TODO: Get additional info (where applicable)
* e.g. Size of L2 cache.
*/
+
+ /*
+ * Does it support 720MHz?
+ */
Ditto
+ status = (OMAP3_SKUID_MASK read_tap_reg(OMAP3_PRODID));
+
+ if (status OMAP3_SKUID_720MHZ) {
+ omap3_features |= OMAP3_HAS_720MHZ;
+ }
No need of { } as there is only one line statement inside the if condition.
}
static void __init omap3_check_revision(void)
@@ -445,6 +454,7 @@ static void __init omap3_cpuinfo(void)
OMAP3_SHOW_FEATURE(neon);
OMAP3_SHOW_FEATURE(isp);
OMAP3_SHOW_FEATURE(192mhz_clk);
+ OMAP3_SHOW_FEATURE(720mhz);
printk()\n);
}
snip
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3: Add basic support for 720MHz part
Op 18 jan 2011, om 08:49 heeft Sanjeev Premi het volgende geschreven: This patch adds support for new speed enhanced parts with ARM and IVA running at 720MHz and 520MHz respectively. These parts can be probed at run-time by reading PRODID.SKUID[3:0] at 0x4830A20C [1]. This patch specifically does following: * Detect devices capable of 720MHz. * Add new OPP * Ensure that OPP is conditionally enabled. * Check for presence of IVA before attempting to enable the corresponding OPP. Thanks for the updated patch! I'm still having problem using this together with DVFS, the kernel won't scale beyond 600MHz because 600MHz and 720MHz share the same voltage. Thara, Nishant, do you have any suggestions on how to convince the kernel that 2 frequencies can share the same voltage settings? If that isn't possible, should we lower the voltages for the lower opps when detecting a 720MHz capable SoC? It would make sense that if 720MHz works at X volt that 600MHz would work at less than X volt. (I don't have a PDF reader at hand to check the TRM at the moment). Complete tree at http://dominion.thruhere.net/git/cgit.cgi/linux-omap/log/?h=koen/beagle-next but this can be reproduced as well by merging Thara's dvfs branch and applying Sanjeevs patch. regards, Koen-- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3: Add basic support for 720MHz part
Koen Kooi wrote, on 01/18/2011 05:38 PM: Op 18 jan 2011, om 08:49 heeft Sanjeev Premi het volgende geschreven: This patch adds support for new speed enhanced parts with ARM and IVA running at 720MHz and 520MHz respectively. These parts can be probed at run-time by reading PRODID.SKUID[3:0] at 0x4830A20C [1]. This patch specifically does following: * Detect devices capable of 720MHz. * Add new OPP * Ensure that OPP is conditionally enabled. * Check for presence of IVA before attempting to enable the corresponding OPP. Thanks for the updated patch! I'm still having problem using this together with DVFS, the kernel won't scale beyond 600MHz because 600MHz and 720MHz share the same voltage. Thara, Nishant, do you have any suggestions on how to convince the kernel that 2 frequencies can share the same voltage settings? + Vishwa If that isn't possible, should we lower the voltages for the lower opps when detecting a 720MHz capable SoC? It would make sense that if 720MHz works at X volt that 600MHz would work at less than X volt. (I don't have a PDF reader at hand to check the TRM at the moment). Complete tree at http://dominion.thruhere.net/git/cgit.cgi/linux-omap/log/?h=koen/beagle-next but this can be reproduced as well by merging Thara's dvfs branch and applying Sanjeevs patch. I suspect the problem lies here: http://dominion.thruhere.net/git/cgit.cgi/linux-omap/tree/arch/arm/mach-omap2/voltage.c?h=koen/beagle-next#n1847 if two OPP frequencies have same voltage, the api introduced in http://dominion.thruhere.net/git/cgit.cgi/linux-omap/commit/?h=koen/beagle-nextid=fd2991c20a8f770c7b8f5a29b3cbb17b9fca4768 will return back the first OPP entry which matches, which would be the lower frequency as OPPs are organized in the OPP layer in increasing order. -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] omap3: Add basic support for 720MHz part
Kooi, -Original Message- From: Nishanth Menon [mailto:n...@ti.com] Sent: Tuesday, January 18, 2011 10:19 PM To: Koen Kooi Cc: Sanjeev Premi; l-o List; th...@ti.com; Vishwanath Sripathy Subject: Re: [PATCH] omap3: Add basic support for 720MHz part Koen Kooi wrote, on 01/18/2011 05:38 PM: Op 18 jan 2011, om 08:49 heeft Sanjeev Premi het volgende geschreven: This patch adds support for new speed enhanced parts with ARM and IVA running at 720MHz and 520MHz respectively. These parts can be probed at run-time by reading PRODID.SKUID[3:0] at 0x4830A20C [1]. This patch specifically does following: * Detect devices capable of 720MHz. * Add new OPP * Ensure that OPP is conditionally enabled. * Check for presence of IVA before attempting to enable the corresponding OPP. Thanks for the updated patch! I'm still having problem using this together with DVFS, the kernel won't scale beyond 600MHz because 600MHz and 720MHz share the same voltage. Thara, Nishant, do you have any suggestions on how to convince the kernel that 2 frequencies can share the same voltage settings? + Vishwa Yes, this is a limitation with the current set of DVFS patches. With the next version of DVFS patches (will be out soon) this feature is going to be supported. That means you can have 2 opps with the same voltage but with different frequency. The device will be configured based on user requested frequency. Vishwa If that isn't possible, should we lower the voltages for the lower opps when detecting a 720MHz capable SoC? It would make sense that if 720MHz works at X volt that 600MHz would work at less than X volt. (I don't have a PDF reader at hand to check the TRM at the moment). Complete tree at http://dominion.thruhere.net/git/cgit.cgi/linux- omap/log/?h=koen/beagle-next but this can be reproduced as well by merging Thara's dvfs branch and applying Sanjeevs patch. I suspect the problem lies here: http://dominion.thruhere.net/git/cgit.cgi/linux- omap/tree/arch/arm/mach-omap2/voltage.c?h=koen/beagle- next#n1847 if two OPP frequencies have same voltage, the api introduced in http://dominion.thruhere.net/git/cgit.cgi/linux- omap/commit/?h=koen/beagle- nextid=fd2991c20a8f770c7b8f5a29b3cbb17b9fca4768 will return back the first OPP entry which matches, which would be the lower frequency as OPPs are organized in the OPP layer in increasing order. -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3: Add basic support for 720MHz part
Op 18 jan 2011, om 18:18 heeft Vishwanath Sripathy het volgende geschreven: Kooi, -Original Message- From: Nishanth Menon [mailto:n...@ti.com] Sent: Tuesday, January 18, 2011 10:19 PM To: Koen Kooi Cc: Sanjeev Premi; l-o List; th...@ti.com; Vishwanath Sripathy Subject: Re: [PATCH] omap3: Add basic support for 720MHz part Koen Kooi wrote, on 01/18/2011 05:38 PM: Op 18 jan 2011, om 08:49 heeft Sanjeev Premi het volgende geschreven: This patch adds support for new speed enhanced parts with ARM and IVA running at 720MHz and 520MHz respectively. These parts can be probed at run-time by reading PRODID.SKUID[3:0] at 0x4830A20C [1]. This patch specifically does following: * Detect devices capable of 720MHz. * Add new OPP * Ensure that OPP is conditionally enabled. * Check for presence of IVA before attempting to enable the corresponding OPP. Thanks for the updated patch! I'm still having problem using this together with DVFS, the kernel won't scale beyond 600MHz because 600MHz and 720MHz share the same voltage. Thara, Nishant, do you have any suggestions on how to convince the kernel that 2 frequencies can share the same voltage settings? + Vishwa Yes, this is a limitation with the current set of DVFS patches. With the next version of DVFS patches (will be out soon) this feature is going to be supported. That means you can have 2 opps with the same voltage but with different frequency. The device will be configured based on user requested frequency. Just to have a crude hack, would something like this work?: /* MPU OPP6 */ - OPP_INITIALIZER(mpu, false, 72000, 135), + OPP_INITIALIZER(mpu, false, 72000, 1350001), Or will I fall into a (post)divider trap where the kernel panics because it can't do that exact voltage? The TRM[1] chickens out with the OPP definitions: --8--- Six generic processor OPPs can be defined as: 1. OPP6 (VDD1 = v4, (MPU_CLK = fmpu5, IVA2_CLK = fiva5)) 2. OPP5 (VDD1 = v4, (MPU_CLK = fmpu4, IVA2_CLK = fiva4)) 3. OPP4 (VDD1 = v3, (MPU_CLK = fmpu3, IVA2_CLK = fiva3)) 4. OPP3 (VDD1 = v2 , (MPU_CLK = fmpu2, IVA2_CLK = fiva2)) 5. OPP2 (VDD1 = v1 , (MPU_CLK = fmpu1, IVA2_CLK = fiva1)) 6. OPP1 (VDD1 = v0 , (MPU_CLK = fmpu0, IVA2_CLK = fiva0)) where v4 v3 v2 v1 v0, --8--- My limited sample size of 3 board (2x beagle C4, 1x overo tide) shows 600MHz@1.3V works as well, but I don't know how much slack that has. And I haven't check if the hardware can do 1.3V exactly or has rounded up. So, what's the quickest way with the current code to get 720MHz working? I've voided my warranty on everything already, so that isn't a problem :) regards, Koen [1] spruf98m.pdf page 355-- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3: Add basic support for 720MHz part
Sanjeev Premi wrote, on 01/07/2011 07:07 AM:
+ if (omap3_has_720mhz()) {
+ pr_info(Enabled OPP corresponding to 720MHz\n);
+
+ omap34xx_opp_def_list[INDEX_MPU_720MHZ]
+ .default_available = true;
+ omap34xx_opp_def_list[INDEX_IVA_720MHZ]
+ .default_available = true;
for many reasons, I dont like this indexing - I am ok with most part of
the patch otherwise - how about opp_enable(dev, freq) instead?
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] omap3: Add basic support for 720MHz part
-Original Message-
From: Menon, Nishanth
Sent: Friday, January 07, 2011 7:04 PM
To: Premi, Sanjeev
Cc: linux-omap@vger.kernel.org
Subject: Re: [PATCH] omap3: Add basic support for 720MHz part
Sanjeev Premi wrote, on 01/07/2011 07:07 AM:
+ if (omap3_has_720mhz()) {
+ pr_info(Enabled OPP corresponding to
720MHz\n);
+
+ omap34xx_opp_def_list[INDEX_MPU_720MHZ]
+ .default_available = true;
+ omap34xx_opp_def_list[INDEX_IVA_720MHZ]
+ .default_available = true;
for many reasons, I dont like this indexing - I am ok with
most part of
the patch otherwise - how about opp_enable(dev, freq) instead?
I had thought about it, but opp_enable would have to be done after
omap_init_opp_table(). Two factors led me to current implementation:
1) Numer of lines of code required to get same thing done one the
opp table has been initialized.
2) The index definition and usage are localized in same file.
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3: Add basic support for 720MHz part
Premi, Sanjeev wrote, on 01/07/2011 07:50 AM:
-Original Message-
From: Menon, Nishanth
Sent: Friday, January 07, 2011 7:04 PM
To: Premi, Sanjeev
Cc: linux-omap@vger.kernel.org
Subject: Re: [PATCH] omap3: Add basic support for 720MHz part
Sanjeev Premi wrote, on 01/07/2011 07:07 AM:
+ if (omap3_has_720mhz()) {
+ pr_info(Enabled OPP corresponding to
720MHz\n);
+
+ omap34xx_opp_def_list[INDEX_MPU_720MHZ]
+ .default_available = true;
+ omap34xx_opp_def_list[INDEX_IVA_720MHZ]
+ .default_available = true;
for many reasons, I dont like this indexing - I am ok with
most part of
the patch otherwise - how about opp_enable(dev, freq) instead?
I had thought about it, but opp_enable would have to be done after
omap_init_opp_table(). Two factors led me to current implementation:
1) Numer of lines of code required to get same thing done one the
opp table has been initialized.
2) The index definition and usage are localized in same file.
right - i will leave it to kevin to comment - IMHO both will work,
though (1) might be a little more elgant and resistant to future changes
to the table (I am not sure if there would be any, but what the heck.)
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap3: Add basic support for 720MHz part
Sanjeev Premi pr...@ti.com writes:
This patch adds support for new speed enhanced parts with ARM
and IVA running at 720MHz and 520MHz respectively. These parts
can be probed at run-time by reading PRODID.SKUID[3:0] at
0x4830A20C [1].
Please expand this a little to describe exactly which parts have this
feature. All OMAP3? 34xx? 35xx? what about 36xx/37xx? ISTR the
runtime probing for this feature is available on 35xx but not on 34xx,
but a summary of this should be here.
This patch specifically does following:
* Detect devices capable of 720MHz.
* Add new OPP
* Ensure that OPP is conditionally enabled.
The patch was tested on OMAP3EVM.
nit: Unrelated to this patch, but OMAP3EVM ethernet is still broken due to
missing GPIO configuration for the network reset. This has been broken
since the GPIO hwmod merge and pointed out but nobody seems to be fixing
it.
On 720MHz capable device:
# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
72
On other devices:
# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
60
[1] http://focus.ti.com/lit/ug/spruff1d/spruff1d.pdf
Signed-off-by: Sanjeev Premi pr...@ti.com
---
arch/arm/mach-omap2/control.h |7 +++
arch/arm/mach-omap2/id.c | 10 ++
arch/arm/mach-omap2/opp3xxx_data.c| 19 ++-
arch/arm/plat-omap/include/plat/cpu.h |2 ++
4 files changed, 37 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h
index f0629ae..eebc045 100644
--- a/arch/arm/mach-omap2/control.h
+++ b/arch/arm/mach-omap2/control.h
@@ -365,6 +365,13 @@
#define FEAT_NEON 0
#define FEAT_NEON_NONE 1
+/*
+ * Product ID register
+ */
+#define OMAP3_PRODID 0x020C
+
+#define OMAP3_SKUID_MASK 0x0f
+#define OMAP3_SKUID_720MHZ 0x08
#ifndef __ASSEMBLY__
#ifdef CONFIG_ARCH_OMAP2PLUS
diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 5f9086c..53fbe01 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -195,6 +195,15 @@ static void __init omap3_check_features(void)
* TODO: Get additional info (where applicable)
* e.g. Size of L2 cache.
*/
+
+ /*
+ * Does it support 720MHz?
+ */
+ status = (OMAP3_SKUID_MASK read_tap_reg(OMAP3_PRODID));
+
+ if (status OMAP3_SKUID_720MHZ) {
+ omap3_features |= OMAP3_HAS_720MHZ;
+ }
}
static void __init omap3_check_revision(void)
@@ -445,6 +454,7 @@ static void __init omap3_cpuinfo(void)
OMAP3_SHOW_FEATURE(neon);
OMAP3_SHOW_FEATURE(isp);
OMAP3_SHOW_FEATURE(192mhz_clk);
+ OMAP3_SHOW_FEATURE(720mhz);
printk()\n);
}
diff --git a/arch/arm/mach-omap2/opp3xxx_data.c
b/arch/arm/mach-omap2/opp3xxx_data.c
index 0486fce..01582b7 100644
--- a/arch/arm/mach-omap2/opp3xxx_data.c
+++ b/arch/arm/mach-omap2/opp3xxx_data.c
@@ -22,6 +22,9 @@
#include omap_opp_data.h
+#define INDEX_MPU_720MHZ 5
+#define INDEX_IVA_720MHZ 14
+
static struct omap_opp_def __initdata omap34xx_opp_def_list[] = {
/* MPU OPP1 */
OPP_INITIALIZER(mpu, true, 12500, 975000),
@@ -33,6 +36,8 @@ static struct omap_opp_def __initdata
omap34xx_opp_def_list[] = {
OPP_INITIALIZER(mpu, true, 55000, 127),
/* MPU OPP5 */
OPP_INITIALIZER(mpu, true, 6, 135),
+ /* MPU OPP6 */
+ OPP_INITIALIZER(mpu, false, 72000, 135),
/*
* L3 OPP1 - 41.5 MHz is disabled because: The voltage for that OPP is
@@ -58,6 +63,8 @@ static struct omap_opp_def __initdata
omap34xx_opp_def_list[] = {
OPP_INITIALIZER(iva, true, 4, 127),
/* DSP OPP5 */
OPP_INITIALIZER(iva, true, 43000, 135),
+ /* DSP OPP6 */
+ OPP_INITIALIZER(iva, false, 52000, 135),
};
static struct omap_opp_def __initdata omap36xx_opp_def_list[] = {
@@ -98,9 +105,19 @@ static int __init omap3_opp_init(void)
if (cpu_is_omap3630())
r = omap_init_opp_table(omap36xx_opp_def_list,
ARRAY_SIZE(omap36xx_opp_def_list));
- else
+ else {
+ if (omap3_has_720mhz()) {
+ pr_info(Enabled OPP corresponding to 720MHz\n);
+
+ omap34xx_opp_def_list[INDEX_MPU_720MHZ]
+ .default_available = true;
+ omap34xx_opp_def_list[INDEX_IVA_720MHZ]
+ .default_available = true;
+ }
+
I'm with Nishanth here. Please stick to the OPP API for enabling OPPs.
First, it is future proof to internal table changes, but it also sets a
better precedent for others wanting to fiddle with OPPs.
Thanks,
Kevin
r = omap_init_opp_table(omap34xx_opp_def_list,
RE: [PATCH] omap3: Add basic support for 720MHz part
-Original Message- From: Kevin Hilman [mailto:khil...@ti.com] Sent: Friday, January 07, 2011 10:18 PM To: Premi, Sanjeev Cc: linux-omap@vger.kernel.org Subject: Re: [PATCH] omap3: Add basic support for 720MHz part Sanjeev Premi pr...@ti.com writes: This patch adds support for new speed enhanced parts with ARM and IVA running at 720MHz and 520MHz respectively. These parts can be probed at run-time by reading PRODID.SKUID[3:0] at 0x4830A20C [1]. Please expand this a little to describe exactly which parts have this feature. All OMAP3? 34xx? 35xx? what about 36xx/37xx? ISTR the runtime probing for this feature is available on 35xx but not on 34xx, but a summary of this should be here. [sp] Will update... [snip]...[snip] nit: Unrelated to this patch, but OMAP3EVM ethernet is still broken due to missing GPIO configuration for the network reset. This has been broken since the GPIO hwmod merge and pointed out but nobody seems to be fixing it. [sp] It is currently in works. [snip]...[snip] + omap34xx_opp_def_list[INDEX_MPU_720MHZ] + .default_available = true; + omap34xx_opp_def_list[INDEX_IVA_720MHZ] + .default_available = true; + } + I'm with Nishanth here. Please stick to the OPP API for enabling OPPs. First, it is future proof to internal table changes, but it also sets a better precedent for others wanting to fiddle with OPPs. [sp] Okay. I will have to revert the a patch on my repo to to get there. Will send an update patch on MON. Thanks, Kevin [snip]...[snip] -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
