RE: [PATCH 01/17] PM: fix suspend control for IVA2

Tero.Kristo
2009-10-19T09:23:07Z

>-----Original Message-----
>From: ext Girish S G [mailto:girishsg@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-owner@vger.kernel.org
>[mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Tero
>> Kristo
>>
>> From: Tero Kristo <tero.kristo@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.kristo@nokia.com>
>> Acked-by: Ameya Palande <ameya.palande@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