Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[ALSA hda 3.4-rc] suspend2ram regression in snd_hda_intel

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


khlebnikov at openvz

May 4, 2012, 12:21 AM

Post #1 of 7 (138 views)
Permalink
[ALSA hda 3.4-rc] suspend2ram regression in snd_hda_intel

Mainline commit 785f857d1cb0856b612b46a0545b74aa2596e44a
("ALSA: hda - Set codec to D3 forcibly even if not used")
breaks sound for my notebook (thinkpad x220),
looks like codec cannot wake from power-save state after resume.
After reverting looks like all works fine.

how to reproduce:
* unplug the power cord (to enable hda power safe mode)
* play something and stop
* wait until the background noise disappears (codec went into power-save mode)
* suspend
* resume
* play something again and hear the silence
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


khlebnikov at openvz

May 6, 2012, 1:32 AM

Post #2 of 7 (132 views)
Permalink
Re: [ALSA hda 3.4-rc] suspend2ram regression in snd_hda_intel [In reply to]

Konstantin Khlebnikov wrote:
> Mainline commit 785f857d1cb0856b612b46a0545b74aa2596e44a
> ("ALSA: hda - Set codec to D3 forcibly even if not used")
> breaks sound for my notebook (thinkpad x220),
> looks like codec cannot wake from power-save state after resume.
> After reverting looks like all works fine.
>
> how to reproduce:
> * unplug the power cord (to enable hda power safe mode)
> * play something and stop
> * wait until the background noise disappears (codec went into power-save mode)
> * suspend
> * resume
> * play something again and hear the silence
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo [at] vger
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

Bump

BTW, I don't see any locks around hda power-save code, for example hda_power_work() reads
codec->power_on and codec->power_count without any locks. This seems racy.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


tiwai at suse

May 6, 2012, 3:37 AM

Post #3 of 7 (129 views)
Permalink
Re: [ALSA hda 3.4-rc] suspend2ram regression in snd_hda_intel [In reply to]

At Sun, 06 May 2012 12:32:14 +0400,
Konstantin Khlebnikov wrote:
>
> Konstantin Khlebnikov wrote:
> > Mainline commit 785f857d1cb0856b612b46a0545b74aa2596e44a
> > ("ALSA: hda - Set codec to D3 forcibly even if not used")
> > breaks sound for my notebook (thinkpad x220),
> > looks like codec cannot wake from power-save state after resume.
> > After reverting looks like all works fine.
> >
> > how to reproduce:
> > * unplug the power cord (to enable hda power safe mode)
> > * play something and stop
> > * wait until the background noise disappears (codec went into power-save mode)
> > * suspend
> > * resume
> > * play something again and hear the silence
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo [at] vger
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>
> Bump

It's a weekend, so don't expect quick responces from any normal person
:)

Which 3.4-rc did you try? There was a known problem with this codec
regarding the power-up sequence, but this should have been fixed in
the very recent version.

If the latest Linus tree still shows the problem, try sound git tree
master or for-next branch.
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git

Then attach alsa-info.sh output (run with --no-upload option) before
and after S3 (i.e. working and non-working states).


> BTW, I don't see any locks around hda power-save code, for example hda_power_work() reads
> codec->power_on and codec->power_count without any locks. This seems racy.

Yeah, we may add an extra spinlock. But it's not too critical for the
whole operation, so I didn't care much so far. At most, you'll miss one
power-save cycle. Maybe a good time to fix it now for 3.5.


thanks,

Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


khlebnikov at openvz

May 8, 2012, 12:13 AM

Post #4 of 7 (130 views)
Permalink
Re: [ALSA hda 3.4-rc] suspend2ram regression in snd_hda_intel [In reply to]

Takashi Iwai wrote:
> At Sun, 06 May 2012 12:32:14 +0400,
> Konstantin Khlebnikov wrote:
>>
>> Konstantin Khlebnikov wrote:
>>> Mainline commit 785f857d1cb0856b612b46a0545b74aa2596e44a
>>> ("ALSA: hda - Set codec to D3 forcibly even if not used")
>>> breaks sound for my notebook (thinkpad x220),
>>> looks like codec cannot wake from power-save state after resume.
>>> After reverting looks like all works fine.
>>>
>>> how to reproduce:
>>> * unplug the power cord (to enable hda power safe mode)
>>> * play something and stop
>>> * wait until the background noise disappears (codec went into power-save mode)
>>> * suspend
>>> * resume
>>> * play something again and hear the silence
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majordomo [at] vger
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>>
>> Bump
>
> It's a weekend, so don't expect quick responces from any normal person
> :)

Yeah, sorry. In this country a mess with holidays and weekends.

>
> Which 3.4-rc did you try? There was a known problem with this codec
> regarding the power-up sequence, but this should have been fixed in
> the very recent version.

Bug still in latest v3.4-rc6.
This is serious degradation since v3.3, so I hope it will be fixed before release.

>
> If the latest Linus tree still shows the problem, try sound git tree
> master or for-next branch.
> git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git

I don't see there anything related to this problem. Bug is still there.
It's 100% reproductible and disappears after reverting that commit.
Looks like driver missed one power-up cycle after system-resume,
after second power-save down/up cycle all work fine.

>
> Then attach g output (run with --no-upload option) before
> and after S3 (i.e. working and non-working states).

see attachments (kernel is v3.4-rc6 with changes from your branch)

start mplayer (work fine)
alsa-info.sh -> alsa-info.before
stop mplayer
suspend
resume
start mplayer (doesn't work)
alsa-info.sh -> alsa-info.after
stop mplayer
wait a second
start mplayer (work fine)

>
>
>> BTW, I don't see any locks around hda power-save code, for example hda_power_work() reads
>> codec->power_on and codec->power_count without any locks. This seems racy.
>
> Yeah, we may add an extra spinlock. But it's not too critical for the
> whole operation, so I didn't care much so far. At most, you'll miss one
> power-save cycle. Maybe a good time to fix it now for 3.5.

I'm afraid this code can easily miss one power-up cycle and stays in power-save mode.
This can be unrelated to my problem, but from my point of view this is very careless.
For example snd_hda_power_up() can easily race with hda_power_work().

>
>
> thanks,
>
> Takashi
Attachments: alsa-info.txt.after (27.6 KB)
  alsa-info.txt.before (28.0 KB)


tiwai at suse

May 8, 2012, 1:11 AM

Post #5 of 7 (129 views)
Permalink
Re: [ALSA hda 3.4-rc] suspend2ram regression in snd_hda_intel [In reply to]

At Tue, 08 May 2012 11:13:23 +0400,
Konstantin Khlebnikov wrote:
>
> Takashi Iwai wrote:
> > At Sun, 06 May 2012 12:32:14 +0400,
> > Konstantin Khlebnikov wrote:
> >>
> >> Konstantin Khlebnikov wrote:
> >>> Mainline commit 785f857d1cb0856b612b46a0545b74aa2596e44a
> >>> ("ALSA: hda - Set codec to D3 forcibly even if not used")
> >>> breaks sound for my notebook (thinkpad x220),
> >>> looks like codec cannot wake from power-save state after resume.
> >>> After reverting looks like all works fine.
> >>>
> >>> how to reproduce:
> >>> * unplug the power cord (to enable hda power safe mode)
> >>> * play something and stop
> >>> * wait until the background noise disappears (codec went into power-save mode)
> >>> * suspend
> >>> * resume
> >>> * play something again and hear the silence
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >>> the body of a message to majordomo [at] vger
> >>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>> Please read the FAQ at http://www.tux.org/lkml/
> >>
> >> Bump
> >
> > It's a weekend, so don't expect quick responces from any normal person
> > :)
>
> Yeah, sorry. In this country a mess with holidays and weekends.
>
> >
> > Which 3.4-rc did you try? There was a known problem with this codec
> > regarding the power-up sequence, but this should have been fixed in
> > the very recent version.
>
> Bug still in latest v3.4-rc6.
> This is serious degradation since v3.3, so I hope it will be fixed before release.
>
> >
> > If the latest Linus tree still shows the problem, try sound git tree
> > master or for-next branch.
> > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
>
> I don't see there anything related to this problem. Bug is still there.
> It's 100% reproductible and disappears after reverting that commit.
> Looks like driver missed one power-up cycle after system-resume,
> after second power-save down/up cycle all work fine.

OK. Could you try to pass model=auto and see whether it changes
the behavior? I've checked only the auto-parser mode recently.
It might be just the static quirk that got broken.

Note that the mixer elements will be renamed by this, so make sure
that you adjust the mixer before testing S3.


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


tiwai at suse

May 8, 2012, 1:49 AM

Post #6 of 7 (130 views)
Permalink
Re: [ALSA hda 3.4-rc] suspend2ram regression in snd_hda_intel [In reply to]

At Tue, 08 May 2012 10:11:06 +0200,
Takashi Iwai wrote:
>
> At Tue, 08 May 2012 11:13:23 +0400,
> Konstantin Khlebnikov wrote:
> >
> > Takashi Iwai wrote:
> > > At Sun, 06 May 2012 12:32:14 +0400,
> > > Konstantin Khlebnikov wrote:
> > >>
> > >> Konstantin Khlebnikov wrote:
> > >>> Mainline commit 785f857d1cb0856b612b46a0545b74aa2596e44a
> > >>> ("ALSA: hda - Set codec to D3 forcibly even if not used")
> > >>> breaks sound for my notebook (thinkpad x220),
> > >>> looks like codec cannot wake from power-save state after resume.
> > >>> After reverting looks like all works fine.
> > >>>
> > >>> how to reproduce:
> > >>> * unplug the power cord (to enable hda power safe mode)
> > >>> * play something and stop
> > >>> * wait until the background noise disappears (codec went into power-save mode)
> > >>> * suspend
> > >>> * resume
> > >>> * play something again and hear the silence
> > >>> --
> > >>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > >>> the body of a message to majordomo [at] vger
> > >>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >>> Please read the FAQ at http://www.tux.org/lkml/
> > >>
> > >> Bump
> > >
> > > It's a weekend, so don't expect quick responces from any normal person
> > > :)
> >
> > Yeah, sorry. In this country a mess with holidays and weekends.
> >
> > >
> > > Which 3.4-rc did you try? There was a known problem with this codec
> > > regarding the power-up sequence, but this should have been fixed in
> > > the very recent version.
> >
> > Bug still in latest v3.4-rc6.
> > This is serious degradation since v3.3, so I hope it will be fixed before release.
> >
> > >
> > > If the latest Linus tree still shows the problem, try sound git tree
> > > master or for-next branch.
> > > git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> >
> > I don't see there anything related to this problem. Bug is still there.
> > It's 100% reproductible and disappears after reverting that commit.
> > Looks like driver missed one power-up cycle after system-resume,
> > after second power-save down/up cycle all work fine.
>
> OK. Could you try to pass model=auto and see whether it changes
> the behavior? I've checked only the auto-parser mode recently.
> It might be just the static quirk that got broken.
>
> Note that the mixer elements will be renamed by this, so make sure
> that you adjust the mixer before testing S3.

FWIW, the below is an ad hoc fix. If this works for you, I'll queue
it up for 3.4 kernel.


thanks,

Takashi

---
From: Takashi Iwai <tiwai [at] suse>
Subject: [PATCH] ALSA: hda - Fix S3 regression on Thinkpad X220

The commit [.785f857d: ALSA: hda - Set codec to D3 forcibly even if not
used] caused an S3 regression on Lenovo Thinkpad X220. The codec
stays in D3 even after the resume by some reason.

As a simple workaround, just avoid the new behavior by that commit,
i.e. going to D3 forcibly, for these Conexant codec chips.

Reported-by: Konstantin Khlebnikov <khlebnikov [at] openvz>
Signed-off-by: Takashi Iwai <tiwai [at] suse>
---
sound/pci/hda/hda_codec.c | 3 ++-
sound/pci/hda/hda_codec.h | 1 +
sound/pci/hda/patch_conexant.c | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 7a8fcc4..f38c4a8 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -5444,7 +5444,8 @@ int snd_hda_suspend(struct hda_bus *bus)
list_for_each_entry(codec, &bus->codec_list, list) {
if (hda_codec_is_power_on(codec))
hda_call_codec_suspend(codec);
- else /* forcibly change the power to D3 even if not used */
+ else if (!codec->no_force_to_d3)
+ /* forcibly change the power to D3 even if not used */
hda_set_power_state(codec,
codec->afg ? codec->afg : codec->mfg,
AC_PWRST_D3);
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 56b4f74..2b3f646 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -854,6 +854,7 @@ struct hda_codec {
unsigned int single_adc_amp:1; /* adc in-amp takes no index
* (e.g. CX20549 codec)
*/
+ unsigned int no_force_to_d3:1; /* don't go to D3 forcibly at sleep */
unsigned int no_sticky_stream:1; /* no sticky-PCM stream assignment */
unsigned int pins_shutup:1; /* pins are shut up */
unsigned int no_trigger_sense:1; /* don't trigger at pin-sensing */
diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
index d906c5b..6b680c2 100644
--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -3018,6 +3018,7 @@ static int patch_cxt5066(struct hda_codec *codec)
if (!spec)
return -ENOMEM;
codec->spec = spec;
+ codec->no_force_to_d3 = 1;

codec->patch_ops = conexant_patch_ops;
codec->patch_ops.init = conexant_init;
--
1.7.9.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


khlebnikov at openvz

May 8, 2012, 6:05 AM

Post #7 of 7 (128 views)
Permalink
Re: [ALSA hda 3.4-rc] suspend2ram regression in snd_hda_intel [In reply to]

Takashi Iwai wrote:
> At Tue, 08 May 2012 10:11:06 +0200,
> Takashi Iwai wrote:
>>
>> At Tue, 08 May 2012 11:13:23 +0400,
>> Konstantin Khlebnikov wrote:
>>>
>>> Takashi Iwai wrote:
>>>> At Sun, 06 May 2012 12:32:14 +0400,
>>>> Konstantin Khlebnikov wrote:
>>>>>
>>>>> Konstantin Khlebnikov wrote:
>>>>>> Mainline commit 785f857d1cb0856b612b46a0545b74aa2596e44a
>>>>>> ("ALSA: hda - Set codec to D3 forcibly even if not used")
>>>>>> breaks sound for my notebook (thinkpad x220),
>>>>>> looks like codec cannot wake from power-save state after resume.
>>>>>> After reverting looks like all works fine.
>>>>>>
>>>>>> how to reproduce:
>>>>>> * unplug the power cord (to enable hda power safe mode)
>>>>>> * play something and stop
>>>>>> * wait until the background noise disappears (codec went into power-save mode)
>>>>>> * suspend
>>>>>> * resume
>>>>>> * play something again and hear the silence
>>>>>> --
>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>>>>> the body of a message to majordomo [at] vger
>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>>> Please read the FAQ at http://www.tux.org/lkml/
>>>>>
>>>>> Bump
>>>>
>>>> It's a weekend, so don't expect quick responces from any normal person
>>>> :)
>>>
>>> Yeah, sorry. In this country a mess with holidays and weekends.
>>>
>>>>
>>>> Which 3.4-rc did you try? There was a known problem with this codec
>>>> regarding the power-up sequence, but this should have been fixed in
>>>> the very recent version.
>>>
>>> Bug still in latest v3.4-rc6.
>>> This is serious degradation since v3.3, so I hope it will be fixed before release.
>>>
>>>>
>>>> If the latest Linus tree still shows the problem, try sound git tree
>>>> master or for-next branch.
>>>> git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
>>>
>>> I don't see there anything related to this problem. Bug is still there.
>>> It's 100% reproductible and disappears after reverting that commit.
>>> Looks like driver missed one power-up cycle after system-resume,
>>> after second power-save down/up cycle all work fine.
>>
>> OK. Could you try to pass model=auto and see whether it changes
>> the behavior? I've checked only the auto-parser mode recently.
>> It might be just the static quirk that got broken.
>>
>> Note that the mixer elements will be renamed by this, so make sure
>> that you adjust the mixer before testing S3.
>
> FWIW, the below is an ad hoc fix. If this works for you, I'll queue
> it up for 3.4 kernel.

Of course it helps, but this isn't "fix" at all!
Right fix follows...

>
>
> thanks,
>
> Takashi
>
> ---
> From: Takashi Iwai<tiwai [at] suse>
> Subject: [PATCH] ALSA: hda - Fix S3 regression on Thinkpad X220
>
> The commit [.785f857d: ALSA: hda - Set codec to D3 forcibly even if not
> used] caused an S3 regression on Lenovo Thinkpad X220. The codec
> stays in D3 even after the resume by some reason.
>
> As a simple workaround, just avoid the new behavior by that commit,
> i.e. going to D3 forcibly, for these Conexant codec chips.
>
> Reported-by: Konstantin Khlebnikov<khlebnikov [at] openvz>
> Signed-off-by: Takashi Iwai<tiwai [at] suse>
> ---
> sound/pci/hda/hda_codec.c | 3 ++-
> sound/pci/hda/hda_codec.h | 1 +
> sound/pci/hda/patch_conexant.c | 1 +
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> index 7a8fcc4..f38c4a8 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -5444,7 +5444,8 @@ int snd_hda_suspend(struct hda_bus *bus)
> list_for_each_entry(codec,&bus->codec_list, list) {
> if (hda_codec_is_power_on(codec))
> hda_call_codec_suspend(codec);
> - else /* forcibly change the power to D3 even if not used */
> + else if (!codec->no_force_to_d3)
> + /* forcibly change the power to D3 even if not used */
> hda_set_power_state(codec,
> codec->afg ? codec->afg : codec->mfg,
> AC_PWRST_D3);
> diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> index 56b4f74..2b3f646 100644
> --- a/sound/pci/hda/hda_codec.h
> +++ b/sound/pci/hda/hda_codec.h
> @@ -854,6 +854,7 @@ struct hda_codec {
> unsigned int single_adc_amp:1; /* adc in-amp takes no index
> * (e.g. CX20549 codec)
> */
> + unsigned int no_force_to_d3:1; /* don't go to D3 forcibly at sleep */
> unsigned int no_sticky_stream:1; /* no sticky-PCM stream assignment */
> unsigned int pins_shutup:1; /* pins are shut up */
> unsigned int no_trigger_sense:1; /* don't trigger at pin-sensing */
> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> index d906c5b..6b680c2 100644
> --- a/sound/pci/hda/patch_conexant.c
> +++ b/sound/pci/hda/patch_conexant.c
> @@ -3018,6 +3018,7 @@ static int patch_cxt5066(struct hda_codec *codec)
> if (!spec)
> return -ENOMEM;
> codec->spec = spec;
> + codec->no_force_to_d3 = 1;
>
> codec->patch_ops = conexant_patch_ops;
> codec->patch_ops.init = conexant_init;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.