[PATCH] ASoC: Add power down of widget powered up for suspend

[PATCH] ASoC: Add power down of widget powered up for suspend

[Joonyoung Shim]

If the widget was powered up when the device enters to the suspend, it
remains on because the power of the widget doesn't change, but we need
to power down the widget on suspend.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 sound/soc/soc-dapm.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index d2af872..099d3b6 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -978,8 +978,11 @@ static int dapm_power_widgets(struct snd_soc_codec *codec, int event)
 				continue;
 
 			power = w->power_check(w);
-			if (power)
+			if (power) {
 				sys_power = 1;
+				if (event == SND_SOC_DAPM_STREAM_SUSPEND)
+					power = 0;
+			}
 
 			if (w->power == power)
 				continue;
-- 
1.6.3.3

Re: [PATCH] ASoC: Add power down of widget powered up for suspend

[Mark Brown]

On Mon, Nov 23, 2009 at 02:45:16PM +0900, Joonyoung Shim wrote:
> If the widget was powered up when the device enters to the suspend, it
> remains on because the power of the widget doesn't change, but we need
> to power down the widget on suspend.

Hrm, yes - good spot, this got missed in the refactoring to make the
bais maintinence less dependant on stream state.  However...

>  			power = w->power_check(w);
> -			if (power)
> +			if (power) {
>  				sys_power = 1;
> +				if (event == SND_SOC_DAPM_STREAM_SUSPEND)
> +					power = 0;
> +			}

We don't want to set sys_power here (since we want to bring the bias
down to standby in preparatio for suspend) and we can skip the power
check so something like the patch below ought to do the trick.  Could
you test and let me know, please?

That said, we probably want some more flexibility here for systems which
want to suspend with bypass paths active.  They'll want to pause streams
being directly driven by the CPU but leave any other paths active.  That
would be a new feature, though.

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index eaadb4b..0d294ef 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -977,9 +977,19 @@ static int dapm_power_widgets(struct snd_soc_codec *codec, int event)
 			if (!w->power_check)
 				continue;
 
-			power = w->power_check(w);
-			if (power)
-				sys_power = 1;
+			/* If we're suspending then pull down all the 
+			 * power. */
+			switch (event) {
+			case SND_SOC_DAPM_STREAM_SUSPEND:
+				power = 0;
+				break;
+
+			default:
+				power = w->power_check(w);
+				if (power)
+					sys_power = 1;
+				break;
+			}
 
 			if (w->power == power)
 				continue;
@@ -1003,8 +1013,12 @@ static int dapm_power_widgets(struct snd_soc_codec *codec, int event)
 		case SND_SOC_DAPM_STREAM_RESUME:
 			sys_power = 1;
 			break;
+		case SND_SOC_DAPM_STREAM_SUSPEND:
+			sys_power = 0;
+			break;
 		case SND_SOC_DAPM_STREAM_NOP:
 			sys_power = codec->bias_level != SND_SOC_BIAS_STANDBY;
+			break;
 		default:
 			break;
 		}

Re: [PATCH] ASoC: Add power down of widget powered up for suspend

[Joonyoung Shim]

On 11/23/2009 9:22 PM, Mark Brown wrote:
> On Mon, Nov 23, 2009 at 02:45:16PM +0900, Joonyoung Shim wrote:
>> If the widget was powered up when the device enters to the suspend, it
>> remains on because the power of the widget doesn't change, but we need
>> to power down the widget on suspend.
> 
> Hrm, yes - good spot, this got missed in the refactoring to make the
> bais maintinence less dependant on stream state.  However...
> 
>>  			power = w->power_check(w);
>> -			if (power)
>> +			if (power) {
>>  				sys_power = 1;
>> +				if (event == SND_SOC_DAPM_STREAM_SUSPEND)
>> +					power = 0;
>> +			}
> 
> We don't want to set sys_power here (since we want to bring the bias
> down to standby in preparatio for suspend) and we can skip the power
> check so something like the patch below ought to do the trick.  Could
> you test and let me know, please?
> 

OK, i missed about sys_power. I tested and below patch solves this issue 
too. I think your patch is better.

Thanks.

> That said, we probably want some more flexibility here for systems which
> want to suspend with bypass paths active.  They'll want to pause streams
> being directly driven by the CPU but leave any other paths active.  That
> would be a new feature, though.
> 

Then, is it possible that the bypass path is alive on suspend?

> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index eaadb4b..0d294ef 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -977,9 +977,19 @@ static int dapm_power_widgets(struct snd_soc_codec *codec, int event)
>  			if (!w->power_check)
>  				continue;
>  
> -			power = w->power_check(w);
> -			if (power)
> -				sys_power = 1;
> +			/* If we're suspending then pull down all the 
> +			 * power. */
> +			switch (event) {
> +			case SND_SOC_DAPM_STREAM_SUSPEND:
> +				power = 0;
> +				break;
> +
> +			default:
> +				power = w->power_check(w);
> +				if (power)
> +					sys_power = 1;
> +				break;
> +			}
>  
>  			if (w->power == power)
>  				continue;
> @@ -1003,8 +1013,12 @@ static int dapm_power_widgets(struct snd_soc_codec *codec, int event)
>  		case SND_SOC_DAPM_STREAM_RESUME:
>  			sys_power = 1;
>  			break;
> +		case SND_SOC_DAPM_STREAM_SUSPEND:
> +			sys_power = 0;
> +			break;
>  		case SND_SOC_DAPM_STREAM_NOP:
>  			sys_power = codec->bias_level != SND_SOC_BIAS_STANDBY;
> +			break;
>  		default:
>  			break;
>  		}
>