OMAP3: cpuidle: prevent CORE pwrdm from entering low-power state when DSS active
Subsystems (3)
cover OMAP3: cpuidle: prevent CORE pwrdm from entering low-power state when DSS active
This series prevents the OMAP3 CPUIdle code from programming the CORE powerdomain to enter any power state lower than ON when the DSS power domain is ON. (The assumption is that in this case, the DSS is active, DMA'ing lines from RAM and displaying them somewhere.)
Tero, please don't hesitate to comment on the patch from you if I got the commit message wrong.
- Paul
---
Tero Kristo (1):
OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
Paul Walmsley (1):
OMAP3: cpuidle: add more details to the DSS-related CORE power domain state restriction
arch/arm/mach-omap2/cpuidle34xx.c | 36 ++++++++++++++++++++++++++++++++++-- 1 files changed, 34 insertions(+), 2 deletions(-)
PATCH 1/2 OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
From: Tero Kristo <[email protected]>
Prevent the CORE power domain from entering RETENTION or OFF when DSS is on. Otherwise, the display FIFO(s) may underflow due to the time needed for the CORE to wake back up, causing tearing and unnecessary interrupts.
--- arch/arm/mach-omap2/cpuidle34xx.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/cpuidle34xx.c +8 -2
@@ -9,8 +9,9 @@* Copyright (C) 2007 Texas Instruments, Inc. * Karthik Dasu <[email protected]> * -* Copyright (C) 2006 Nokia Corporation +* Copyright (C) 2006, 2011 Nokia Corporation * Tony Lindgren <[email protected]> +* Tero Kristo <[email protected]> * * Copyright (C) 2005 Texas Instruments, Inc. * Richard Woodruff <[email protected]>
@@ -268,6 +269,12 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev, goto select_state; } +/* If DSS is active, prevent CORE RET/OFF */ + dss_state =pwrdm_read_pwrst (dss_pd); +if (dss_state ==PWRDM_POWER_ON && + core_next_state !=PWRDM_POWER_ON ) + core_next_state =PWRDM_POWER_INACTIVE ; +/* * Prevent PER off if CORE is not in retention or off as this * would disable PER wakeups completely.
@@ -288,7 +295,6 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev, iva2_state =pwrdm_read_pwrst (iva2_pd); sgx_state =pwrdm_read_pwrst (sgx_pd); usb_state =pwrdm_read_pwrst (usb_pd); - dss_state =pwrdm_read_pwrst (dss_pd); if (cam_state > PWRDM_POWER_OFF || dss_state > PWRDM_POWER_OFF ||
PATCH 1/2 OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
Quoted 10 lines · Paul Walmsley
From: Tero Kristo <[email protected]>
Prevent the CORE power domain from entering RETENTION or OFF when DSS is on. Otherwise, the display FIFO(s) may underflow due to the time needed for the CORE to wake back up, causing tearing and unnecessary interrupts.
Signed-off-by Tero Kristo <[email protected]>[paul at pwsan.com: wrote commit message]Signed-off-by Paul Walmsley <[email protected]>
This isn't quite ready for merge, and needs a little more testing with current DSS driver and mainline.
Quoted 20 lines
--- arch/arm/mach-omap2/cpuidle34xx.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-)arch/arm/mach-omap2/cpuidle34xx.c +2
-1@@ -9,8 +9,9 @@* Copyright (C) 2007 Texas Instruments, Inc. * Karthik Dasu <[email protected]> * -* Copyright (C) 2006 Nokia Corporation +* Copyright (C) 2006, 2011 Nokia Corporation * Tony Lindgren <[email protected]> +* Tero Kristo <[email protected]> * * Copyright (C) 2005 Texas Instruments, Inc. * Richard Woodruff <[email protected]>
arch/arm/mach-omap2/cpuidle34xx.c static int omap3_enter_idle_bm(struct cpuidle_device *dev, · line 277
Quoted 9 lines
arch/arm/mach-omap2/cpuidle34xx.c +6
-0@@ -268,6 +269,12 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev, goto select_state; } +/* If DSS is active, prevent CORE RET/OFF */ + dss_state =pwrdm_read_pwrst (dss_pd); +if (dss_state ==PWRDM_POWER_ON && + core_next_state !=PWRDM_POWER_ON ) + core_next_state =PWRDM_POWER_INACTIVE ; +
Due to sleepdeps/autodeps, when this code runs, DSS powerdomain is always on. The result is that CORE is always set to INACTIVE.
A side effect of this problem is exposing a known issue with PER wakeups (UART3, GPIO). Since CORE never goes to retention/off, IO-pad wakeups are never enabled, so PER wakeups do not work.
I think we need a different way of checking for DSS activity.
Sounds like another usecase for idle notifiers.
Kevin
PATCH 1/2 OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
Hi Kevin,
Quoted 50 lines
-----Original Message-----
From: ext Kevin Hilman [mailto:khilman at ti.com]
Sent: 04 March, 2011 01:25
To: Paul Walmsley
Cc: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Kristo Tero (Nokia-MS/Tampere)
Subject: Re: [PATCH 1/2] OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is onQuoted 10 lines · Paul Walmsley
From: Tero Kristo <[email protected]>
Prevent the CORE power domain from entering RETENTION or OFF when DSS is on. Otherwise, the display FIFO(s) may underflow due to the time needed for the CORE to wake back up, causing tearing and unnecessary interrupts.
Signed-off-by Tero Kristo <[email protected]>[paul at pwsan.com: wrote commit message]Signed-off-by Paul Walmsley <[email protected]>This isn't quite ready for merge, and needs a little more testing with current DSS driver and mainline.
arch/arm/mach-
Quoted 5 lines
--- arch/arm/mach-omap2/cpuidle34xx.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-)arch/arm/mach- renamed from arch/arm/mach-omap2/cpuidle34xx.c +0
-0
omap2/cpuidle34xx.carch/arm/mach-omap2/cpuidle34xx.c static int omap3_enter_idle_bm(struct
Quoted 15 lines
index 0335cd8..d1b7789 100644
arch/arm/mach-omap2/cpuidle34xx.c +2
-1@@ -9,8 +9,9 @@* Copyright (C) 2007 Texas Instruments, Inc. * Karthik Dasu <[email protected]> * -* Copyright (C) 2006 Nokia Corporation +* Copyright (C) 2006, 2011 Nokia Corporation * Tony Lindgren <[email protected]> +* Tero Kristo <[email protected]> * * Copyright (C) 2005 Texas Instruments, Inc. * Richard Woodruff <[email protected]> @@ -268,6 +269,12 @@ static int omap3_enter_idle_bm(struct
cpuidle_device*dev,Quoted 3 lines
arch/arm/mach-omap2/cpuidle34xx.c +0
-0goto select_state; }
Quoted 9 lines
arch/arm/mach-omap2/cpuidle34xx.c static int omap3_enter_idle_bm(struct cpuidle_device *dev, · line 277
Quoted 6 lines
arch/arm/mach-omap2/cpuidle34xx.c +6
-0+/* If DSS is active, prevent CORE RET/OFF */ + dss_state =pwrdm_read_pwrst (dss_pd); +if (dss_state ==PWRDM_POWER_ON && + core_next_state !=PWRDM_POWER_ON ) + core_next_state =PWRDM_POWER_INACTIVE ; +Due to sleepdeps/autodeps, when this code runs, DSS powerdomain is always on. The result is that CORE is always set to INACTIVE.
Now I recall that someone was asking about a patch similar to this earlier, and had the same issue with DSS sleepdep collision.
What is the reason for having the sleepdep for DSS powerdomain anyway? At least I can't see any reason why the sleepdep for DSS should be set. In my opinion it should be perfectly okay for DSS domain to idle independently of MPU/CORE, as this is going to be better for power consumption also.
Quoted 10 lines
A side effect of this problem is exposing a known issue with PER wakeups (UART3, GPIO). Since CORE never goes to retention/off, IO-pad wakeups are never enabled, so PER wakeups do not work.
I think we need a different way of checking for DSS activity.
Sounds like another usecase for idle notifiers.
Kevin
PATCH 1/2 OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
Hi Tero,
<[email protected]> writes:
[...]
Quoted 15 lines
Quoted 10 lines
arch/arm/mach-omap2/cpuidle34xx.c static int omap3_enter_idle_bm(struct cpuidle_device *dev, · line 277
Quoted 7 lines
arch/arm/mach-omap2/cpuidle34xx.c +6
-0+/* If DSS is active, prevent CORE RET/OFF */ + dss_state =pwrdm_read_pwrst (dss_pd); +if (dss_state ==PWRDM_POWER_ON && + core_next_state !=PWRDM_POWER_ON ) + core_next_state =PWRDM_POWER_INACTIVE ; +Due to sleepdeps/autodeps, when this code runs, DSS powerdomain is always on. The result is that CORE is always set to INACTIVE.
Now I recall that someone was asking about a patch similar to this earlier, and had the same issue with DSS sleepdep collision.
Quoted 5 lines
What is the reason for having the sleepdep for DSS powerdomain anyway? At least I can't see any reason why the sleepdep for DSS should be set. In my opinion it should be perfectly okay for DSS domain to idle independently of MPU/CORE, as this is going to be better for power consumption also.
Agreed, but currently the sleepdeps with MPU are automatically managed (by clkdm autodeps and hwmod initiator deps.) Until we have merged a solution to more selectively enable sleepdeps (or remove them) $SUBJECT patch cannot be merged.
Kevin
PATCH 1/2 OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
Quoted 3 lines
-----Original Message-----
From: [email protected] [mailto:linux-omap-
Quoted 35 lines
owner at vger.kernel.org] On Behalf Of ext Kevin Hilman
Sent: 04 March, 2011 18:56
To: Kristo Tero (Nokia-MS/Tampere)
Cc: paul at pwsan.com; linux-omap at vger.kernel.org; linux-arm- kernel at lists.infradead.org
Subject: Re: [PATCH 1/2] OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is onHi Tero,
<[email protected]> writes:
[...]
Quoted 15 lines
Quoted 10 lines
arch/arm/mach-omap2/cpuidle34xx.c static int omap3_enter_idle_bm(struct cpuidle_device *dev, · line 277
Quoted 7 lines
arch/arm/mach-omap2/cpuidle34xx.c +6
-0+/* If DSS is active, prevent CORE RET/OFF */ + dss_state =pwrdm_read_pwrst (dss_pd); +if (dss_state ==PWRDM_POWER_ON && + core_next_state !=PWRDM_POWER_ON ) + core_next_state =PWRDM_POWER_INACTIVE ; +Due to sleepdeps/autodeps, when this code runs, DSS powerdomain is always on. The result is that CORE is always set to INACTIVE.
Now I recall that someone was asking about a patch similar to this earlier, and had the same issue with DSS sleepdep collision.
Quoted 5 lines
What is the reason for having the sleepdep for DSS powerdomain anyway? At least I can't see any reason why the sleepdep for DSS should be set. In my opinion it should be perfectly okay for DSS domain to idle independently of MPU/CORE, as this is going to be better for power consumption also.
Quoted 4 lines
Agreed, but currently the sleepdeps with MPU are automatically managed (by clkdm autodeps and hwmod initiator deps.) Until we have merged a solution to more selectively enable sleepdeps (or remove them) $SUBJECT patch cannot be merged.
Ok I thought this is the case... it would be possible to implement a temporary/permanent solution that uses idle status check instead of pwrdm state check, and prevent core idle if dss is not going to idle. What is the current status with those idlest patches anyway?
-Tero
PATCH 1/2 OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
<[email protected]> writes:
[...]
Quoted 26 lines
Quoted 21 lines
Quoted 15 lines
Quoted 10 lines
arch/arm/mach-omap2/cpuidle34xx.c static int omap3_enter_idle_bm(struct cpuidle_device *dev, · line 277
Quoted 7 lines
arch/arm/mach-omap2/cpuidle34xx.c +6
-0+/* If DSS is active, prevent CORE RET/OFF */ + dss_state =pwrdm_read_pwrst (dss_pd); +if (dss_state ==PWRDM_POWER_ON && + core_next_state !=PWRDM_POWER_ON ) + core_next_state =PWRDM_POWER_INACTIVE ; +Due to sleepdeps/autodeps, when this code runs, DSS powerdomain is always on. The result is that CORE is always set to INACTIVE.
Now I recall that someone was asking about a patch similar to this earlier, and had the same issue with DSS sleepdep collision.
Quoted 5 lines
What is the reason for having the sleepdep for DSS powerdomain anyway? At least I can't see any reason why the sleepdep for DSS should be set. In my opinion it should be perfectly okay for DSS domain to idle independently of MPU/CORE, as this is going to be better for power consumption also.
Quoted 4 lines
Agreed, but currently the sleepdeps with MPU are automatically managed (by clkdm autodeps and hwmod initiator deps.) Until we have merged a solution to more selectively enable sleepdeps (or remove them) $SUBJECT patch cannot be merged.
Quoted 4 lines
Ok I thought this is the case... it would be possible to implement a temporary/permanent solution that uses idle status check instead of pwrdm state check, and prevent core idle if dss is not going to idle. What is the current status with those idlest patches anyway?
Paul will have to answer that one.
Kevin
PATCH 1/2 OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
In reply to Tero.Kristo at nokia.comHi Tero,
Quoted 1 line · Tero.Kristo at nokia.com
What is the current status with those idlest patches anyway?
They aren't completed yet, so they won't make it into 2.6.39. I hope to get them in early in 2.6.40.
- Paul
PATCH 1/2 OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
In reply to Tero.Kristo at nokia.comHi,
Quoted 5 lines · Tero.Kristo at nokia.com
What is the reason for having the sleepdep for DSS powerdomain anyway? At least I can't see any reason why the sleepdep for DSS should be set. In my opinion it should be perfectly okay for DSS domain to idle independently of MPU/CORE, as this is going to be better for power consumption also.
Those autodeps are only there because one of the old CDP trees had them in there. Talking with some people at TI, as I understand it, they are only useful for working around hardware bugs. As far as I know, as long as a target module has its main clock and interface clock enabled, the PRCM should not assert its SIdleReq line to the module, and so the module should never enter idle.
But no one seems to have an authoritative idea of which autodeps are still needed, and which are not. Perhaps you, or someone else, might like to try seeing which ones can be safely removed? After applying this patch here:
http://marc.info/?l=linux-omap&m=129962861605157&w=2
it should be possible to simply set a CLKDM_NO_AUTODEPS flag on a struct clockdomain, and then no autodep will be added.
regards
- Paul
PATCH 2/2 OMAP3: cpuidle: add more details to the DSS-related CORE power domain state restriction
In reply to Paul WalmsleyProvide some more details about the rationale behind the CORE power domain state restriction while DSS is enabled -- hopefully, enough to enable others to implement an approach based on the DSS's required wakeup latency, which can vary based on quite a few parameters.
--- arch/arm/mach-omap2/cpuidle34xx.c | 28 +++++++++++++++++++++++++++- 1 files changed, 27 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/cpuidle34xx.c +27 -1
@@ -269,7 +269,33 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev, goto select_state; } -/* If DSS is active, prevent CORE RET/OFF */ +/* +* If DSS is active, prevent any CORE power domain transition +* to RETENTION or lower. This is because we don't currently +* have the infrastructure in the DSS driver or kernel to +* enter CORE RET without causing DSS FIFO underflows. When +* the DSS FIFO underflows, the screen image can be distorted, +* because the CORE may not be able to come back on-line +* quickly enough to service DSS requests to the SDRAM +* framebuffer. The DSS may also interrupt the MPU when the +* FIFO underflows, potentially wasting power. +* +* The approach below is easy to implement, but potentially +* wastes power, especially with screens that don't require a +* backlight. Probably the best long-term way to fix this is +* for the DSS driver to constrain the CORE and SDRAM minimum +* power state with omap_pm_set_max_dev_wakeup_lat(), based on +* the DSS FIFO size(s), watermark levels, drain rate, and +* refill time. Based on that constraint, the OMAP PM code +* should adjust the CORE maximum power state, CORE DPLL +* autoidle mode, CORE_CLK rate, and voltage scaling to ensure +* the required wakeup latency at the lowest power +* consumption. +* +* (More information on the DSS low-power refresh mode can be +* found in _Using Display Low-Power Refresh on the OMAP3430 +* Device_, TI Application Report, SWPA158 - October 2008.) +*/ dss_state =pwrdm_read_pwrst (dss_pd);if (dss_state ==PWRDM_POWER_ON && core_next_state !=PWRDM_POWER_ON )
cover OMAP3: cpuidle: prevent CORE pwrdm from entering low-power state when DSS active
In reply to Paul WalmsleyQuoted 16 lines
-----Original Message-----
From: ext Paul Walmsley [mailto:paul at pwsan.com]
Sent: 02 March, 2011 00:49
To: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Kristo Tero (Nokia-MS/Tampere)
Subject: [PATCH 0/2] OMAP3: cpuidle: prevent CORE pwrdm from entering low-power state when DSS activeThis series prevents the OMAP3 CPUIdle code from programming the CORE powerdomain to enter any power state lower than ON when the DSS power domain
is ON. (The assumption is that in this case, the DSS is active, DMA'ing lines from RAM and displaying them somewhere.)
Quoted 3 lines
Tero, please don't hesitate to comment on the patch from you if I got the
commit message wrong.
Hi Paul,
It looks good to me.
Quoted 18 lines
- Paul
---
Tero Kristo (1):
OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF
when DSS is onPaul Walmsley (1):
OMAP3: cpuidle: add more details to the DSS-related CORE power
domain state restriction
arch/arm/mach-omap2/cpuidle34xx.c| 36
++++++++++++++++++++++++++++++++++--1 files changed, 34 insertions(+), 2 deletions(-)