Re: [PATCH 01/17] PM: fix suspend control for IVA2
Hi Tero, Ameya, Girish, On Fri, 16 Oct 2009, Tero Kristo wrote: From: Tero Kristo tero.kri...@nokia.com IVA2 controls its target power state individually, thus suspend should not touch IVA2. Without this patch DSP suspend always fails. We don't allow other device drivers to touch PRCM bits, so we should probably should remove all PRCM register accesses from the DSPBridge code, so all power control should go through the ARM. Is there a reason why the ARM code can't handle the DSP powerdomain? - 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 01/17] PM: fix suspend control for IVA2
From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- ow...@vger.kernel.org] On Behalf Of Paul Walmsley Sent: Thursday, October 22, 2009 5:24 AM IVA2 controls its target power state individually, thus suspend should not touch IVA2. Without this patch DSP suspend always fails. We don't allow other device drivers to touch PRCM bits, so we should probably should remove all PRCM register accesses from the DSPBridge code, so all power control should go through the ARM. Is there a reason why the ARM code can't handle the DSP powerdomain? Sharing with DSP is something which probably could use some improvement. Today DSP self-manages its domain. Its (bios) micro-kernel makes decisions to optimize its domain. The ARM can't really micro-manage the DSP as he doesn't even want to know at the detail level what the DSP is up to at every instant. - During idle time cpuidle should just be checking dsp status to see if its current state gets in the way of a low c-state. - bridge does register with suspend frame work so he should do the right thing when in the system. * problem is when bridge isn't there what to do. This is especially after an unload of the bridge. What has been proposed in the past is like what Kevin inputted in related thread about having maintenance hand off. But for some reason it never quite to the top of the list. - bridge does request thought clock frame work clocks especially of those which are public peripherals. - bridge could request everything but it was not projected as power efficient waking up the big arm core for something the dsp could do itself and has all control over. This is especially true if you have dsp doing the rendering for something like mp3. it gets the wake ups and streaming and only every great while wakes the arm to give it a pile more data. Waking the arm every time it runs its equivalent of cpuidle was discourged. - main other sharing conflict was with irq routing between arm and bridge. Right now its kind of init mode setup. I guess this is ok for todays usage. .. so after context current code is not requesting through pm code purposefully. The hardware has been evolving from omap1,2,3,4 to make for more of a distributed model. After all the details/constraints are understood with silicon there is some time to re-evaluate if its paying back or not. Regards, Richard W. -- 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 01/17] PM: fix suspend control for IVA2
On Thu, 2009-10-22 at 16:21 -0500, Woodruff, Richard wrote: From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- ow...@vger.kernel.org] On Behalf Of Paul Walmsley Sent: Thursday, October 22, 2009 5:24 AM IVA2 controls its target power state individually, thus suspend should not touch IVA2. Without this patch DSP suspend always fails. We don't allow other device drivers to touch PRCM bits, so we should probably should remove all PRCM register accesses from the DSPBridge code, so all power control should go through the ARM. Is there a reason why the ARM code can't handle the DSP powerdomain? Sharing with DSP is something which probably could use some improvement. Today DSP self-manages its domain. Its (bios) micro-kernel makes decisions to optimize its domain. The ARM can't really micro-manage the DSP as he doesn't even want to know at the detail level what the DSP is up to at every instant. - During idle time cpuidle should just be checking dsp status to see if its current state gets in the way of a low c-state. - bridge does register with suspend frame work so he should do the right thing when in the system. * problem is when bridge isn't there what to do. This is especially after an unload of the bridge. Just a side note reminder: bridge does not exist in the real world. [dedek...@sauron linux-omap-2.6]\$ ls drivers/dsp/bridge ls: cannot access drivers/dsp/bridge: No such file or directory -- Best Regards, Artem Bityutskiy (Артём Битюцкий) -- 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 01/17] PM: fix suspend control for IVA2
-Original Message-
From: ext Girish S G [mailto:giris...@ti.com]
Sent: 16 October, 2009 20:16
To: Kristo Tero (Nokia-D/Tampere); linux-omap@vger.kernel.org
Subject: RE: [PATCH 01/17] PM: fix suspend control for IVA2
-Original Message-
From: linux-omap-ow...@vger.kernel.org
[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Tero
Kristo
From: Tero Kristo tero.kri...@nokia.com
IVA2 controls its target power state individually, thus
suspend should not
touch IVA2. Without this patch DSP suspend always fails.
Signed-off-by: Tero Kristo tero.kri...@nokia.com
Acked-by: Ameya Palande ameya.pala...@nokia.com
---
arch/arm/mach-omap2/pm34xx.c |9 -
1 files changed, 8 insertions(+), 1 deletions(-)
static struct prm_setup_vc prm_setup = {
.clksetup = 0xff,
@@ -676,6 +676,12 @@ static int omap3_pm_suspend(void)
pwrst-saved_state =
pwrdm_read_next_pwrst(pwrst-pwrdm);
/* Set ones wanted by suspend */
list_for_each_entry(pwrst, pwrst_list, node) {
+/* Special handling for IVA2, just use current
sleep state */
+if (pwrst-pwrdm == iva2_pwrdm) {
+state = pwrdm_read_pwrst(pwrst-pwrdm);
+if (state PWRDM_POWER_ON)
+pwrst-next_state = state;
+}
Agree, IVA2 pwrdm is handled autonomously by bridge. I think
this needs some additional change to remove all the redundant
configuration of iva pwdm. There are some inconsistencies like,
- Say enable_off_mode is disabled. Before doing system
wide suspend if DSP hibernates then IVA2 will be put to OFF. In that
case we have IVA2 going to OFF and other domains in RET. This
might not be an issue, but it's bad from sytem PM framework integrity
perspective.
This is an issue with bridge driver, and I am not sure how this should be
fixed. Currently bridge driver does not care whether off mode is enabled or not.
- enable_off_mode-omap3_pm_off_mode_enable will also
touch IVA2 power domain next state. This we don't want to do if dsp
bridge is already taking care of IVA2.
IMO, we need to have some mechanism wherein if bridge PM takes
care of IVA then PM framework should not configure the IVA
powerstate. It should only do if bridge PM is disabled.
Should we have a Kconfig option for this? Like CONFIG_OMAP3_BRIDGE_PM, and
disable all iva2 controls from pm34xx.c if it is enabled? Otherwise control
IVA2 as currently done.
-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 01/17] PM: fix suspend control for IVA2
-Original Message- From: tero.kri...@nokia.com [mailto:tero.kri...@nokia.com] Sent: Monday, October 19, 2009 4:23 AM To: giris...@ti.com; linux-omap@vger.kernel.org Subject: RE: [PATCH 01/17] PM: fix suspend control for IVA2 Agree, IVA2 pwrdm is handled autonomously by bridge. I think this needs some additional change to remove all the redundant configuration of iva pwdm. There are some inconsistencies like, - Say enable_off_mode is disabled. Before doing system wide suspend if DSP hibernates then IVA2 will be put to OFF. In that case we have IVA2 going to OFF and other domains in RET. This might not be an issue, but it's bad from sytem PM framework integrity perspective. This is an issue with bridge driver, and I am not sure how this should be fixed. Currently bridge driver does not care whether off mode is enabled or not. I have seen bridge considering enable_off_mode flag in suspend/resume path. But while hibernation (idle timeout) it goes to OFF, irrespective of the OFF flag. - enable_off_mode-omap3_pm_off_mode_enable will also touch IVA2 power domain next state. This we don't want to do if dsp bridge is already taking care of IVA2. IMO, we need to have some mechanism wherein if bridge PM takes care of IVA then PM framework should not configure the IVA powerstate. It should only do if bridge PM is disabled. Should we have a Kconfig option for this? Like CONFIG_OMAP3_BRIDGE_PM, and disable all iva2 controls from pm34xx.c if it is enabled? Otherwise control IVA2 as currently done. Yes, this looks ok to me. -Girish -- 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 01/17] PM: fix suspend control for IVA2
-Original Message-
From: linux-omap-ow...@vger.kernel.org
[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Tero
Kristo
From: Tero Kristo tero.kri...@nokia.com
IVA2 controls its target power state individually, thus suspend should not
touch IVA2. Without this patch DSP suspend always fails.
Signed-off-by: Tero Kristo tero.kri...@nokia.com
Acked-by: Ameya Palande ameya.pala...@nokia.com
---
arch/arm/mach-omap2/pm34xx.c |9 -
1 files changed, 8 insertions(+), 1 deletions(-)
static struct prm_setup_vc prm_setup = {
.clksetup = 0xff,
@@ -676,6 +676,12 @@ static int omap3_pm_suspend(void)
pwrst-saved_state = pwrdm_read_next_pwrst(pwrst-pwrdm);
/* Set ones wanted by suspend */
list_for_each_entry(pwrst, pwrst_list, node) {
+ /* Special handling for IVA2, just use current sleep state */
+ if (pwrst-pwrdm == iva2_pwrdm) {
+ state = pwrdm_read_pwrst(pwrst-pwrdm);
+ if (state PWRDM_POWER_ON)
+ pwrst-next_state = state;
+ }
Agree, IVA2 pwrdm is handled autonomously by bridge. I think this needs some
additional change to remove all the redundant
configuration of iva pwdm. There are some inconsistencies like,
- Say enable_off_mode is disabled. Before doing system wide suspend if
DSP hibernates then IVA2 will be put to OFF. In that
case we have IVA2 going to OFF and other domains in RET. This might not be an
issue, but it's bad from sytem PM framework integrity
perspective.
- enable_off_mode-omap3_pm_off_mode_enable will also touch IVA2 power
domain next state. This we don't want to do if dsp
bridge is already taking care of IVA2.
IMO, we need to have some mechanism wherein if bridge PM takes care of IVA then
PM framework should not configure the IVA
powerstate. It should only do if bridge PM is disabled.
-Girish
--
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
