| Message ID | 1422435597-19330-1-git-send-email-bardliao@realtek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 28, 2015 at 04:59:57PM +0800, Bard Liao wrote: > We do the power down process at bias level STANDBY. And do nothing > at bias level OFF. As a result, the register setting is the same > between bias level STANDBY and OFF. So we don't need to set > idle_bias_off true. > - .idle_bias_off = true, > + .idle_bias_off = false, There is no need to set false explicitly, it's the default. It also seems better to keep the flag if it's true, even if we don't actively do anything at the minute we may in future (either in the framework or in the driver) so the information seems useful.
> -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: Wednesday, January 28, 2015 7:36 PM > To: Bard Liao > Cc: lgirdwood@gmail.com; alsa-devel@alsa-project.org; Flove; Oder > Chiou; yang.jie@intel.com > Subject: Re: [PATCH] ASoC: rt286: set idle_bias_off to false > > On Wed, Jan 28, 2015 at 04:59:57PM +0800, Bard Liao wrote: > > We do the power down process at bias level STANDBY. And do nothing at > > bias level OFF. As a result, the register setting is the same between > > bias level STANDBY and OFF. So we don't need to set idle_bias_off > > true. > > > - .idle_bias_off = true, > > + .idle_bias_off = false, > > There is no need to set false explicitly, it's the default. It also seems > better to keep the flag if it's true, even if we don't actively do anything at > the minute we may in future (either in the framework or in the driver) so > the information seems useful. Actually, we meet an issue. We use force enable in rt286_jack_detect. If a headset is inserted, the bias_level will not be OFF due to some widgets are ON. If idle_bias_off is true, rt286_suspend will not be invoked in suspend. And I "guess" rt286's power will be cut off in suspend. That's what we handle in rt286_suspend/resume functions. If rt286 's power is cut off and don't re-sync the cache, it will cause some unexpected issues. Is there any good way to fix such issues? Thanks. > > ------Please consider the environment before printing this e-mail.
On Wed, Jan 28, 2015 at 12:45:13PM +0000, Bard Liao wrote: > > > - .idle_bias_off = true, > > > + .idle_bias_off = false, > > There is no need to set false explicitly, it's the default. It also seems > > better to keep the flag if it's true, even if we don't actively do anything at > > the minute we may in future (either in the framework or in the driver) so > > the information seems useful. > Actually, we meet an issue. We use force enable in rt286_jack_detect. If So, this is one of the reasons why people ask for good, clear changelogs... > a headset is inserted, the bias_level will not be OFF due to some widgets > are ON. If idle_bias_off is true, rt286_suspend will not be invoked in > suspend. And I "guess" rt286's power will be cut off in suspend. That's > what we handle in rt286_suspend/resume functions. If rt286 's power is > cut off and don't re-sync the cache, it will cause some unexpected issues. > Is there any good way to fix such issues? If jack detection should be disabled over suspend the machine driver should do that (eg, by setting a NULL jack). It's actually common for systems to leave jack detection up over suspend as in systems like phones the jack can be a wake source, the hook switch is commonly expected to always be operational.
~Keyon > -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: Thursday, January 29, 2015 4:10 AM > To: Bard Liao > Cc: lgirdwood@gmail.com; alsa-devel@alsa-project.org; Flove; Oder Chiou; Jie, > Yang > Subject: Re: [PATCH] ASoC: rt286: set idle_bias_off to false > > On Wed, Jan 28, 2015 at 12:45:13PM +0000, Bard Liao wrote: > > > > > - .idle_bias_off = true, > > > > + .idle_bias_off = false, > > > > There is no need to set false explicitly, it's the default. It also > > > seems better to keep the flag if it's true, even if we don't > > > actively do anything at the minute we may in future (either in the > > > framework or in the driver) so the information seems useful. > > > Actually, we meet an issue. We use force enable in rt286_jack_detect. > > If > > So, this is one of the reasons why people ask for good, clear changelogs... > > > a headset is inserted, the bias_level will not be OFF due to some > > widgets are ON. If idle_bias_off is true, rt286_suspend will not be > > invoked in suspend. And I "guess" rt286's power will be cut off in > > suspend. That's what we handle in rt286_suspend/resume functions. If > > rt286 's power is cut off and don't re-sync the cache, it will cause some > unexpected issues. > > > Is there any good way to fix such issues? > > If jack detection should be disabled over suspend the machine driver should do > that (eg, by setting a NULL jack). It's actually common for systems to leave > jack detection up over suspend as in systems like phones the jack can be a wake [Keyon] as my understanding, setting idle_bias_off to false won't disable jack detection over suspend, only the action that poweroff the codec will disable it, Bard, is it correct? setting idle_bias_off to false is the easiest solution for us, it is also reasonable here, because rt286 don't need soc core to set BIAS OFF specially when in idle status, (BIAS_OFF and BIAS_STANDBY are same for it), and seems no side effect here. > source, the hook switch is commonly expected to always be operational.
On Thu, Jan 29, 2015 at 09:07:08AM +0000, Jie, Yang wrote: Please fix your mailer to word wrap within paragraphs, your mails are very hard to read. > > If jack detection should be disabled over suspend the machine driver should do > > that (eg, by setting a NULL jack). It's actually common for systems to leave > > jack detection up over suspend as in systems like phones the jack can be a wake > [Keyon] as my understanding, setting idle_bias_off to false won't disable jack detection > over suspend, only the action that poweroff the codec will disable it, Bard, is it correct? Right, you've got it the wrong way round - the goal is turn off the widgets that are holding the device on. > setting idle_bias_off to false is the easiest solution for us, it is also reasonable here, > because rt286 don't need soc core to set BIAS OFF specially when in idle status, (BIAS_OFF > and BIAS_STANDBY are same for it), and seems no side effect here. Currently. And to be honest I would have expected that the behaviour would be the same with widgets on regardless of the bias level so we probably want to fix that which would cause the same problem resurface even if it were bodged around for now.
> -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: Thursday, January 29, 2015 6:52 PM > To: Jie, Yang > Cc: Bard Liao; lgirdwood@gmail.com; alsa-devel@alsa-project.org; Flove; Oder > Chiou > Subject: Re: [PATCH] ASoC: rt286: set idle_bias_off to false > > On Thu, Jan 29, 2015 at 09:07:08AM +0000, Jie, Yang wrote: > > Please fix your mailer to word wrap within paragraphs, your mails are very hard > to read. [Keyon] sorry for that, should fix it this time. > > > > If jack detection should be disabled over suspend the machine driver > > > should do that (eg, by setting a NULL jack). It's actually common > > > for systems to leave jack detection up over suspend as in systems > > > like phones the jack can be a wake > > > [Keyon] as my understanding, setting idle_bias_off to false won't > > disable jack detection over suspend, only the action that poweroff the codec > will disable it, Bard, is it correct? > > Right, you've got it the wrong way round - the goal is turn off the widgets that > are holding the device on. > > > setting idle_bias_off to false is the easiest solution for us, it is > > also reasonable here, because rt286 don't need soc core to set BIAS > > OFF specially when in idle status, (BIAS_OFF and BIAS_STANDBY are same for > it), and seems no side effect here. > > Currently. And to be honest I would have expected that the behaviour would > be the same with widgets on regardless of the bias level so we probably want > to fix that which would cause the same problem resurface even if it were > bodged around for now. [Keyon] OK, then I will try disable jack detection in the machine driver before calling snd_soc_suspend(), can you share example about how to set a NULL jack?
On Thu, Jan 29, 2015 at 01:18:18PM +0000, Jie, Yang wrote: > > > setting idle_bias_off to false is the easiest solution for us, it is > > > also reasonable here, because rt286 don't need soc core to set BIAS > > > OFF specially when in idle status, (BIAS_OFF and BIAS_STANDBY are same for > > it), and seems no side effect here. > > Currently. And to be honest I would have expected that the behaviour would > > be the same with widgets on regardless of the bias level so we probably want > > to fix that which would cause the same problem resurface even if it were > > bodged around for now. > [Keyon] OK, then I will try disable jack detection in the machine driver before calling snd_soc_suspend(), can you share example about how to set a NULL jack? Several of the Wolfson drivers support setting a NULL jack to disable detection, I can't think of any boards that currently use it due to the need to use detection in suspend on platforms. Looks like word wrapping is still not working in your mailer BTW.
diff --git a/sound/soc/codecs/rt286.c b/sound/soc/codecs/rt286.c index 847cc4b..81bdd276 100644 --- a/sound/soc/codecs/rt286.c +++ b/sound/soc/codecs/rt286.c @@ -1144,7 +1144,7 @@ static struct snd_soc_codec_driver soc_codec_dev_rt286 = { .suspend = rt286_suspend, .resume = rt286_resume, .set_bias_level = rt286_set_bias_level, - .idle_bias_off = true, + .idle_bias_off = false, .controls = rt286_snd_controls, .num_controls = ARRAY_SIZE(rt286_snd_controls), .dapm_widgets = rt286_dapm_widgets,