[PATCH 1/2] OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
From: Tero Kristo 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 [p...@pwsan.com: wrote commit message] Signed-off-by: Paul Walmsley --- arch/arm/mach-omap2/cpuidle34xx.c | 10 -- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c index 0335cd8..d1b7789 100644 --- a/arch/arm/mach-omap2/cpuidle34xx.c +++ b/arch/arm/mach-omap2/cpuidle34xx.c @@ -9,8 +9,9 @@ * Copyright (C) 2007 Texas Instruments, Inc. * Karthik Dasu * - * Copyright (C) 2006 Nokia Corporation + * Copyright (C) 2006, 2011 Nokia Corporation * Tony Lindgren + * Tero Kristo * * Copyright (C) 2005 Texas Instruments, Inc. * Richard Woodruff @@ -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 || -- 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 1/2] OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
Paul Walmsley writes: > From: Tero Kristo > > 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 > [p...@pwsan.com: wrote commit message] > Signed-off-by: Paul Walmsley This isn't quite ready for merge, and needs a little more testing with current DSS driver and mainline. > --- > arch/arm/mach-omap2/cpuidle34xx.c | 10 -- > 1 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-omap2/cpuidle34xx.c > b/arch/arm/mach-omap2/cpuidle34xx.c > index 0335cd8..d1b7789 100644 > --- a/arch/arm/mach-omap2/cpuidle34xx.c > +++ b/arch/arm/mach-omap2/cpuidle34xx.c > @@ -9,8 +9,9 @@ > * Copyright (C) 2007 Texas Instruments, Inc. > * Karthik Dasu > * > - * Copyright (C) 2006 Nokia Corporation > + * Copyright (C) 2006, 2011 Nokia Corporation > * Tony Lindgren > + * Tero Kristo > * > * Copyright (C) 2005 Texas Instruments, Inc. > * Richard Woodruff > @@ -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 -- 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 1/2] OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
Hi Kevin, >-Original Message- >From: ext Kevin Hilman [mailto:khil...@ti.com] >Sent: 04 March, 2011 01:25 >To: Paul Walmsley >Cc: linux-omap@vger.kernel.org; linux-arm-ker...@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 on > >Paul Walmsley writes: > >> From: Tero Kristo >> >> 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 >> [p...@pwsan.com: wrote commit message] >> Signed-off-by: Paul Walmsley > >This isn't quite ready for merge, and needs a little more testing with >current DSS driver and mainline. > >> --- >> arch/arm/mach-omap2/cpuidle34xx.c | 10 -- >> 1 files changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach- >omap2/cpuidle34xx.c >> index 0335cd8..d1b7789 100644 >> --- a/arch/arm/mach-omap2/cpuidle34xx.c >> +++ b/arch/arm/mach-omap2/cpuidle34xx.c >> @@ -9,8 +9,9 @@ >> * Copyright (C) 2007 Texas Instruments, Inc. >> * Karthik Dasu >> * >> - * Copyright (C) 2006 Nokia Corporation >> + * Copyright (C) 2006, 2011 Nokia Corporation >> * Tony Lindgren >> + * Tero Kristo >> * >> * Copyright (C) 2005 Texas Instruments, Inc. >> * Richard Woodruff >> @@ -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. 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. > >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 -- 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 1/2] OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
Hi Tero, writes: [...] >>> >>> + /* 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. 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 -- 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 1/2] OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
>-Original Message- >From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- >ow...@vger.kernel.org] On Behalf Of ext Kevin Hilman >Sent: 04 March, 2011 18:56 >To: Kristo Tero (Nokia-MS/Tampere) >Cc: p...@pwsan.com; linux-omap@vger.kernel.org; linux-arm- >ker...@lists.infradead.org >Subject: Re: [PATCH 1/2] OMAP3: cpuidle: prevent CORE power domain from >going to RET or OFF when DSS is on > >Hi Tero, > > writes: > >[...] > >>>> >>>> + /* 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. > >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 -- 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 1/2] OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
writes: [...] > > + /* 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. >> >>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? Paul will have to answer that one. Kevin -- 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 1/2] OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
Hi, On Fri, 4 Mar 2011, tero.kri...@nokia.com wrote: > 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 -- 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 1/2] OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
Hi Tero, On Mon, 7 Mar 2011, tero.kri...@nokia.com wrote: > 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 -- 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
