diff mbox

[12/13] DSPBRIDGE: fix resuming problem with dsp hibernation

Message ID 1247669795-23895-13-git-send-email-ameya.palande@nokia.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Ameya Palande July 15, 2009, 2:56 p.m. UTC
From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>

The constraints(opp >= 2) should be set whenever returning from DSP
hibernation.

This fixes the problem at the following steps:
1) wait DSP hibernate
2) echo mem > /sys/power/state
3) type key to wakeup

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---
 drivers/dsp/bridge/wmd/tiomap_sm.c |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

Comments

Guzman Lugo, Fernando July 15, 2009, 11:10 p.m. UTC | #1
Hi,

> -----Original Message-----
> From: Ameya Palande [mailto:ameya.palande@nokia.com]
> Sent: Wednesday, July 15, 2009 9:57 AM
> To: linux-omap@vger.kernel.org
> Cc: Ramirez Luna, Omar; Guzman Lugo, Fernando; Menon, Nishanth;
> hiroshi.doyu@nokia.com
> Subject: [PATCH 12/13] DSPBRIDGE: fix resuming problem with dsp
> hibernation
> 
> From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> 
> The constraints(opp >= 2) should be set whenever returning from DSP
> hibernation.
> 
> This fixes the problem at the following steps:
> 1) wait DSP hibernate
> 2) echo mem > /sys/power/state
> 3) type key to wakeup
> 

The patch looks good, you are removing the check for OPP1 because it is suppose that if the DSP is in Hibernation we have a DSP constrain in OPP1, so I think the "if" is always true. However I don't see the issue the mention could you please tell me what was the problem you were seeing?


> Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> ---
>  drivers/dsp/bridge/wmd/tiomap_sm.c |   16 +++++-----------
>  1 files changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c
> b/drivers/dsp/bridge/wmd/tiomap_sm.c
> index 02e2675..69f6fb3 100644
> --- a/drivers/dsp/bridge/wmd/tiomap_sm.c
> +++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
> @@ -98,11 +98,6 @@ DSP_STATUS CHNLSM_DisableInterrupt(struct
> WMD_DEV_CONTEXT *pDevContext)
>  DSP_STATUS CHNLSM_InterruptDSP2(struct WMD_DEV_CONTEXT *pDevContext,
>  				u16 wMbVal)
>  {
> -#ifdef CONFIG_BRIDGE_DVFS
> -	struct dspbridge_platform_data *pdata =
> -		omap_dspbridge_dev->dev.platform_data;
> -	u32 opplevel = 0;
> -#endif
>  	struct CFG_HOSTRES resources;
>  	DSP_STATUS status = DSP_SOK;
>  	unsigned long timeout;
> @@ -116,12 +111,11 @@ DSP_STATUS CHNLSM_InterruptDSP2(struct
> WMD_DEV_CONTEXT *pDevContext,
>  	if (pDevContext->dwBrdState == BRD_DSP_HIBERNATION ||
>  	    pDevContext->dwBrdState == BRD_HIBERNATION) {
>  #ifdef CONFIG_BRIDGE_DVFS
> -		if (pdata->dsp_get_opp)
> -			opplevel = (*pdata->dsp_get_opp)();
> -		if (opplevel == VDD1_OPP1) {
> -			if (pdata->dsp_set_min_opp)
> -				(*pdata->dsp_set_min_opp)(VDD1_OPP2);
> -		}
> +		struct dspbridge_platform_data *pdata =
> +			omap_dspbridge_dev->dev.platform_data;
> +
> +		if (pdata->dsp_set_min_opp)
> +			(*pdata->dsp_set_min_opp)(VDD1_OPP2);
>  #endif
>  		/* Restart the peripheral clocks */
>  		DSP_PeripheralClocks_Enable(pDevContext, NULL);
> --
> 1.6.2.4
> 

Regards,
Fernando.
--
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
Ameya Palande July 16, 2009, 7:12 a.m. UTC | #2
Hi Fernando,

ext Guzman Lugo, Fernando wrote:
> 
> Hi,
> 
>> -----Original Message-----
>> From: Ameya Palande [mailto:ameya.palande@nokia.com]
>> Sent: Wednesday, July 15, 2009 9:57 AM
>> To: linux-omap@vger.kernel.org
>> Cc: Ramirez Luna, Omar; Guzman Lugo, Fernando; Menon, Nishanth;
>> hiroshi.doyu@nokia.com
>> Subject: [PATCH 12/13] DSPBRIDGE: fix resuming problem with dsp
>> hibernation
>>
>> From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
>>
>> The constraints(opp >= 2) should be set whenever returning from DSP
>> hibernation.
>>
>> This fixes the problem at the following steps:
>> 1) wait DSP hibernate
>> 2) echo mem > /sys/power/state
>> 3) type key to wakeup
>>
> 
> The patch looks good, you are removing the check for OPP1 because it is suppose
> that if the DSP is in Hibernation we have a DSP constrain in OPP1,
> so I think the "if" is always true. However I don't see the issue the mention
> could you please tell me what was the problem you were seeing?

While waking up DSP if VDD1 OPP is greater that 1 then without this patch,
constraint won't be set. This results in a case where PM layer later decides to
switch to OPP1 while DSP is active, which results in a crash. To avoid this we
are setting VDD1 to OPP2 unconditionally while waking up from hibernation :)

Cheers,
Ameya.
--
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
diff mbox

Patch

diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c b/drivers/dsp/bridge/wmd/tiomap_sm.c
index 02e2675..69f6fb3 100644
--- a/drivers/dsp/bridge/wmd/tiomap_sm.c
+++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
@@ -98,11 +98,6 @@  DSP_STATUS CHNLSM_DisableInterrupt(struct WMD_DEV_CONTEXT *pDevContext)
 DSP_STATUS CHNLSM_InterruptDSP2(struct WMD_DEV_CONTEXT *pDevContext,
 				u16 wMbVal)
 {
-#ifdef CONFIG_BRIDGE_DVFS
-	struct dspbridge_platform_data *pdata =
-		omap_dspbridge_dev->dev.platform_data;
-	u32 opplevel = 0;
-#endif
 	struct CFG_HOSTRES resources;
 	DSP_STATUS status = DSP_SOK;
 	unsigned long timeout;
@@ -116,12 +111,11 @@  DSP_STATUS CHNLSM_InterruptDSP2(struct WMD_DEV_CONTEXT *pDevContext,
 	if (pDevContext->dwBrdState == BRD_DSP_HIBERNATION ||
 	    pDevContext->dwBrdState == BRD_HIBERNATION) {
 #ifdef CONFIG_BRIDGE_DVFS
-		if (pdata->dsp_get_opp)
-			opplevel = (*pdata->dsp_get_opp)();
-		if (opplevel == VDD1_OPP1) {
-			if (pdata->dsp_set_min_opp)
-				(*pdata->dsp_set_min_opp)(VDD1_OPP2);
-		}
+		struct dspbridge_platform_data *pdata =
+			omap_dspbridge_dev->dev.platform_data;
+
+		if (pdata->dsp_set_min_opp)
+			(*pdata->dsp_set_min_opp)(VDD1_OPP2);
 #endif
 		/* Restart the peripheral clocks */
 		DSP_PeripheralClocks_Enable(pDevContext, NULL);