[PATCH] ASoC: rt298: remove idle_bias_off = true

[PATCH] ASoC: rt298: remove idle_bias_off = true

[Bard Liao]

From: bardliao <bardliao@realtek.com>

Codec will not go into suspend if there is any widget forced on with
idle_bias_off true. We want codec go into suspend when the system is
suspend. Also, we don't do anything in bias level OFF case. So it is
actually no different in bias level STANDBY or OFF case.

Signed-off-by: Bard Liao <bardliao@realtek.com>
---
 sound/soc/codecs/rt298.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/codecs/rt298.c b/sound/soc/codecs/rt298.c
index f80cfe4..d403c20 100644
--- a/sound/soc/codecs/rt298.c
+++ b/sound/soc/codecs/rt298.c
@@ -1094,7 +1094,6 @@ static struct snd_soc_codec_driver soc_codec_dev_rt298 = {
 	.suspend = rt298_suspend,
 	.resume = rt298_resume,
 	.set_bias_level = rt298_set_bias_level,
-	.idle_bias_off = true,
 	.controls = rt298_snd_controls,
 	.num_controls = ARRAY_SIZE(rt298_snd_controls),
 	.dapm_widgets = rt298_dapm_widgets,
-- 
1.8.1.1.439.g50a6b54

Re: [PATCH] ASoC: rt298: remove idle_bias_off = true

[Vinod Koul]

On Thu, Jun 02, 2016 at 02:51:52PM +0800, Bard Liao wrote:
> From: bardliao <bardliao@realtek.com>
> 
> Codec will not go into suspend if there is any widget forced on with
> idle_bias_off true. We want codec go into suspend when the system is
> suspend. Also, we don't do anything in bias level OFF case. So it is
> actually no different in bias level STANDBY or OFF case.

Tested-by: Senthilnathan Veppur <senthilnathanx.veppur@intel.com>
Reveiwed-by: Vinod Koul <vinod.koul@intel.com>

-- 
~Vinod

Re: [PATCH] ASoC: rt298: remove idle_bias_off = true

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 987 bytes --]

On Thu, Jun 02, 2016 at 02:51:52PM +0800, Bard Liao wrote:

> Codec will not go into suspend if there is any widget forced on with
> idle_bias_off true. We want codec go into suspend when the system is
> suspend. Also, we don't do anything in bias level OFF case. So it is
> actually no different in bias level STANDBY or OFF case.

Hrm.  This feels like something that the framework might be doing wrong
here.  We should either suspend the CODEC all the time even if there are
widgets enabled or we should never suspend the CODEC even if there are
widgets enabled but having it depend on idle_bias_off seems like the
wrong thing here.  My expectation would be that we would not suspend
with widgets on since the widgets being on implies the CODEC being on
and we'd expect suspend to do things like cutting the device level
power.  

Could you talk through the use case in more detail - what's the widget
and why is it still OK for suspend to run even with widgets on?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: rt298: remove idle_bias_off = true

[Bard Liao]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Friday, June 03, 2016 7:40 AM
> To: Bard Liao
> Cc: lgirdwood@gmail.com; alsa-devel@alsa-project.org;
> lars@metafoo.de; Flove; Oder Chiou; senthilnathanx.veppur@intel.com;
> ramesh.babu@intel.com
> Subject: Re: [PATCH] ASoC: rt298: remove idle_bias_off = true
> 
> On Thu, Jun 02, 2016 at 02:51:52PM +0800, Bard Liao wrote:
> 
> > Codec will not go into suspend if there is any widget forced on with
> > idle_bias_off true. We want codec go into suspend when the system is
> > suspend. Also, we don't do anything in bias level OFF case. So it is
> > actually no different in bias level STANDBY or OFF case.
> 
> Hrm.  This feels like something that the framework might be doing
> wrong here.  We should either suspend the CODEC all the time even if
> there are widgets enabled or we should never suspend the CODEC even if
> there are widgets enabled but having it depend on idle_bias_off seems
> like the wrong thing here.  My expectation would be that we would not
> suspend with widgets on since the widgets being on implies the CODEC
> being on and we'd expect suspend to do things like cutting the device
> level power.
> 
> Could you talk through the use case in more detail - what's the widget
> and why is it still OK for suspend to run even with widgets on?

We need to force on "LDO1" widget when a jack is plugged in. That
is for push button detection(not implement yet). Also, all jack related
functions such as headphone playback, headset capture need "LDO1"
on. However, we don't need these functions in suspend. Also, it seems
the external power will be cut down in suspend even codec suspend
function is not called. In that case, codec will lost its register setting.
I.e. Codec registers will be cleaned. And we need to sync with cache.

> 
> ------Please consider the environment before printing this e-mail.

Re: [PATCH] ASoC: rt298: remove idle_bias_off = true

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1040 bytes --]

On Fri, Jun 03, 2016 at 03:09:32AM +0000, Bard Liao wrote:

> > Could you talk through the use case in more detail - what's the widget
> > and why is it still OK for suspend to run even with widgets on?

> We need to force on "LDO1" widget when a jack is plugged in. That
> is for push button detection(not implement yet). Also, all jack related
> functions such as headphone playback, headset capture need "LDO1"
> on. However, we don't need these functions in suspend. Also, it seems
> the external power will be cut down in suspend even codec suspend
> function is not called. In that case, codec will lost its register setting.
> I.e. Codec registers will be cleaned. And we need to sync with cache.

In a situation like that I'd expect the machine driver to be disabling
jack detection over suspend - it should only be being left enabled if
the jack detection is expected to continue functioning over suspend (a
lot of Android systems do this so things like starting playback from the
button on a plugged in headset continue to work).

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]