| Message ID | 20170817230122.30655-4-tony@atomide.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/17/2017 06:01 PM, Tony Lindgren wrote: > With the idle code in place needed for supporting off mode for cpus, > let's enable it. This seems to save about 0.2W of power compared to > CPU retention states based on quick measurement on omap5-uevm. That makes sense since the Silicon you probably have is pre-production silicon. > > Some parts of the code is based on an earlier patch done by Santosh > Shilimkar <santosh.shilimkar@oracle.com> in TI Linux kernel tree as > commit 7e3035cf0e9b ("ARM: OMAP4+: CPUidle: Add OMAP5 idle driver > support") > unfortunately, you have been looking at some preproduction code which was being developed prior to the silicon going into production (also the reason why I have'nt upstreamed those changes). > diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c > --- a/arch/arm/mach-omap2/pm44xx.c > +++ b/arch/arm/mach-omap2/pm44xx.c > @@ -225,7 +225,7 @@ int __init omap4_pm_init_early(void) > if (cpu_is_omap446x()) > pm44xx_errata |= PM_OMAP4_ROM_SMP_BOOT_ERRATUM_GICD; > > - if (soc_is_omap54xx() || soc_is_dra7xx()) > + if (soc_is_dra7xx()) > pm44xx_errata |= PM_OMAP4_CPU_OSWR_DISABLE; > > return 0; > diff --git a/arch/arm/mach-omap2/powerdomains54xx_data.c b/arch/arm/mach-omap2/powerdomains54xx_data.c > --- a/arch/arm/mach-omap2/powerdomains54xx_data.c > +++ b/arch/arm/mach-omap2/powerdomains54xx_data.c > @@ -107,8 +107,8 @@ static struct powerdomain cpu0_54xx_pwrdm = { > .voltdm = { .name = "mpu" }, > .prcm_offs = OMAP54XX_PRCM_MPU_PRM_C0_INST, > .prcm_partition = OMAP54XX_PRCM_MPU_PARTITION, > - .pwrsts = PWRSTS_RET_ON, > - .pwrsts_logic_ret = PWRSTS_RET, > + .pwrsts = PWRSTS_OFF_RET_ON, > + .pwrsts_logic_ret = PWRSTS_OFF_RET, > .banks = 1, > .pwrsts_mem_ret = { > [0] = PWRSTS_OFF_RET, /* cpu0_l1 */ > @@ -124,8 +124,8 @@ static struct powerdomain cpu1_54xx_pwrdm = { > .voltdm = { .name = "mpu" }, > .prcm_offs = OMAP54XX_PRCM_MPU_PRM_C1_INST, > .prcm_partition = OMAP54XX_PRCM_MPU_PARTITION, > - .pwrsts = PWRSTS_RET_ON, > - .pwrsts_logic_ret = PWRSTS_RET, > + .pwrsts = PWRSTS_OFF_RET_ON, > + .pwrsts_logic_ret = PWRSTS_OFF_RET, > .banks = 1, > .pwrsts_mem_ret = { > [0] = PWRSTS_OFF_RET, /* cpu1_l1 */ > @@ -158,7 +158,7 @@ static struct powerdomain mpu_54xx_pwrdm = { > .prcm_offs = OMAP54XX_PRM_MPU_INST, > .prcm_partition = OMAP54XX_PRM_PARTITION, > .pwrsts = PWRSTS_RET_ON, > - .pwrsts_logic_ret = PWRSTS_RET, > + .pwrsts_logic_ret = PWRSTS_OFF_RET, > .banks = 2, > .pwrsts_mem_ret = { > [0] = PWRSTS_OFF_RET, /* mpu_l2 */ > unfortunately, I have to NAK this patch. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9f5dc91b691cf296c49aedf0a671fd659a70f737 as per Technical Reference Manual SWPU282AF–May 2012–Revised August 2016, PM_MPU_PWRSTCTRL can only support: ON INA, RET. (CSWR only). Same applies to CPUs as well. which was the reason in the first place for me to send the patch upstream.
* Nishanth Menon <nm@ti.com> [170817 18:30]: > On 08/17/2017 06:01 PM, Tony Lindgren wrote: > > With the idle code in place needed for supporting off mode for cpus, > > let's enable it. This seems to save about 0.2W of power compared to > > CPU retention states based on quick measurement on omap5-uevm. > > That makes sense since the Silicon you probably have is pre-production > silicon. It seems to be es2.0, the measurement was just based on a glance of the power supply after rmmod of ehci-omap and ohci-platform modules. > unfortunately, you have been looking at some preproduction code which was > being developed prior to the silicon going into production (also the reason > why I have'nt upstreamed those changes). Oh I did not know that, I was just looking at the old ti-linux-3-8-y-kernel that the igepv5 kernel tree is based on. > unfortunately, I have to NAK this patch. > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9f5dc91b691cf296c49aedf0a671fd659a70f737 > as per Technical Reference Manual SWPU282AF–May 2012–Revised August 2016, > PM_MPU_PWRSTCTRL can only support: ON INA, RET. (CSWR only). > > Same applies to CPUs as well. which was the reason in the first place for me > to send the patch upstream. OK, is there some hardware errata issued on that? I also noticed these patches won't work when booted with LPAE enabled kernel for some reason. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/18/2017 10:26 AM, Tony Lindgren wrote: > * Nishanth Menon <nm@ti.com> [170817 18:30]: >> On 08/17/2017 06:01 PM, Tony Lindgren wrote: >>> With the idle code in place needed for supporting off mode for cpus, >>> let's enable it. This seems to save about 0.2W of power compared to >>> CPU retention states based on quick measurement on omap5-uevm. >> >> That makes sense since the Silicon you probably have is pre-production >> silicon. > > It seems to be es2.0, the measurement was just based on a glance > of the power supply after rmmod of ehci-omap and ohci-platform > modules. yeah -> ES2.0 did have two stages prior to being approved for production. > >> unfortunately, you have been looking at some preproduction code which was >> being developed prior to the silicon going into production (also the reason >> why I have'nt upstreamed those changes). > > Oh I did not know that, I was just looking at the old > ti-linux-3-8-y-kernel that the igepv5 kernel tree is > based on. > unfortunately so. >> unfortunately, I have to NAK this patch. >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9f5dc91b691cf296c49aedf0a671fd659a70f737 >> as per Technical Reference Manual SWPU282AF–May 2012–Revised August 2016, >> PM_MPU_PWRSTCTRL can only support: ON INA, RET. (CSWR only). >> >> Same applies to CPUs as well. which was the reason in the first place for me >> to send the patch upstream. > > OK, is there some hardware errata issued on that? To my knowledge, errata is only when TRM is not updated (feature that is supposed to work, but does'nt). TRM's version history indicates descope. At least to my information all production customers should have been informed about this as well. > > I also noticed these patches won't work when booted with LPAE > enabled kernel for some reason. > Obviously, I dont think I am interested in chasing any further down this road. SoC descope is a severe decision that has been taken after a lot of internal and customer considerations. there is no point in trying to enable a descoped feature when even if it worked on 10 boards means nothing from a production device perspective, and there are devices out there in production with OMAP5.
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c --- a/arch/arm/mach-omap2/cpuidle44xx.c +++ b/arch/arm/mach-omap2/cpuidle44xx.c @@ -63,6 +63,11 @@ static struct idle_statedata omap5_idle_data[] = { .mpu_state = PWRDM_POWER_RET, .mpu_logic_state = PWRDM_POWER_RET, }, + { + .cpu_state = PWRDM_POWER_OFF, + .mpu_state = PWRDM_POWER_RET, + .mpu_logic_state = PWRDM_POWER_OFF, + }, }; static struct idle_statedata dra7_idle_data[] = { @@ -296,6 +301,15 @@ static struct cpuidle_driver omap5_idle_driver = { .name = "C2", .desc = "CPUx CSWR, MPUSS CSWR", }, + { + /* C3 - CPU0 OFF + CPU1 OFF + MPU OSWR */ + .exit_latency = 460 + 518, + .target_residency = 1100, + .flags = CPUIDLE_FLAG_TIMER_STOP | CPUIDLE_FLAG_COUPLED, + .enter = omap_enter_idle_coupled, + .name = "C3", + .desc = "CPUx OFF, MPUSS OSWR", + }, }, .state_count = ARRAY_SIZE(omap5_idle_data), .safe_state_index = 0, diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c --- a/arch/arm/mach-omap2/pm44xx.c +++ b/arch/arm/mach-omap2/pm44xx.c @@ -225,7 +225,7 @@ int __init omap4_pm_init_early(void) if (cpu_is_omap446x()) pm44xx_errata |= PM_OMAP4_ROM_SMP_BOOT_ERRATUM_GICD; - if (soc_is_omap54xx() || soc_is_dra7xx()) + if (soc_is_dra7xx()) pm44xx_errata |= PM_OMAP4_CPU_OSWR_DISABLE; return 0; diff --git a/arch/arm/mach-omap2/powerdomains54xx_data.c b/arch/arm/mach-omap2/powerdomains54xx_data.c --- a/arch/arm/mach-omap2/powerdomains54xx_data.c +++ b/arch/arm/mach-omap2/powerdomains54xx_data.c @@ -107,8 +107,8 @@ static struct powerdomain cpu0_54xx_pwrdm = { .voltdm = { .name = "mpu" }, .prcm_offs = OMAP54XX_PRCM_MPU_PRM_C0_INST, .prcm_partition = OMAP54XX_PRCM_MPU_PARTITION, - .pwrsts = PWRSTS_RET_ON, - .pwrsts_logic_ret = PWRSTS_RET, + .pwrsts = PWRSTS_OFF_RET_ON, + .pwrsts_logic_ret = PWRSTS_OFF_RET, .banks = 1, .pwrsts_mem_ret = { [0] = PWRSTS_OFF_RET, /* cpu0_l1 */ @@ -124,8 +124,8 @@ static struct powerdomain cpu1_54xx_pwrdm = { .voltdm = { .name = "mpu" }, .prcm_offs = OMAP54XX_PRCM_MPU_PRM_C1_INST, .prcm_partition = OMAP54XX_PRCM_MPU_PARTITION, - .pwrsts = PWRSTS_RET_ON, - .pwrsts_logic_ret = PWRSTS_RET, + .pwrsts = PWRSTS_OFF_RET_ON, + .pwrsts_logic_ret = PWRSTS_OFF_RET, .banks = 1, .pwrsts_mem_ret = { [0] = PWRSTS_OFF_RET, /* cpu1_l1 */ @@ -158,7 +158,7 @@ static struct powerdomain mpu_54xx_pwrdm = { .prcm_offs = OMAP54XX_PRM_MPU_INST, .prcm_partition = OMAP54XX_PRM_PARTITION, .pwrsts = PWRSTS_RET_ON, - .pwrsts_logic_ret = PWRSTS_RET, + .pwrsts_logic_ret = PWRSTS_OFF_RET, .banks = 2, .pwrsts_mem_ret = { [0] = PWRSTS_OFF_RET, /* mpu_l2 */
With the idle code in place needed for supporting off mode for cpus, let's enable it. This seems to save about 0.2W of power compared to CPU retention states based on quick measurement on omap5-uevm. Some parts of the code is based on an earlier patch done by Santosh Shilimkar <santosh.shilimkar@oracle.com> in TI Linux kernel tree as commit 7e3035cf0e9b ("ARM: OMAP4+: CPUidle: Add OMAP5 idle driver support") Cc: Dave Gerlach <d-gerlach@ti.com> Cc: Nishanth Menon <nm@ti.com> Cc: Santosh Shilimkar <santosh.shilimkar@oracle.com> Cc: Tero Kristo <t-kristo@ti.com> Signed-off-by: Tony Lindgren <tony@atomide.com> --- arch/arm/mach-omap2/cpuidle44xx.c | 14 ++++++++++++++ arch/arm/mach-omap2/pm44xx.c | 2 +- arch/arm/mach-omap2/powerdomains54xx_data.c | 10 +++++----- 3 files changed, 20 insertions(+), 6 deletions(-)