Bug 88121 - lenovo ideapad s210: suspend breaks internal mic boost
Summary: lenovo ideapad s210: suspend breaks internal mic boost
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Sound(ALSA) (show other bugs)
Hardware: Intel Linux
: P1 normal
Assignee: Jaroslav Kysela
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-12 22:23 UTC by Roman Kagan
Modified: 2014-12-09 08:20 UTC (History)
1 user (show)

See Also:
Kernel Version: 3.17.2 (earlier versions too)
Tree: Fedora
Regression: No


Attachments
alsa-info for unpatched driver before suspend (35.60 KB, text/plain)
2014-11-12 22:24 UTC, Roman Kagan
Details
alsa-info for unpatched driver after suspend (35.86 KB, text/plain)
2014-11-12 22:25 UTC, Roman Kagan
Details
alsa-info for patched driver (36.75 KB, text/plain)
2014-11-12 22:26 UTC, Roman Kagan
Details
attachment-24791-0.html (197 bytes, text/html)
2014-12-09 08:20 UTC, Roman Kagan
Details

Description Roman Kagan 2014-11-12 22:23:52 UTC
In Lenovo IdeaPad S210 upon suspend the "Internal Mic Boost" control almost mutes the mic at any level other than 0 (and at level 0 it's insufficient).

The working state isn't recovered even after system poweroff; only after running Windows it comes back.

The following patch (found semi-intuitively) makes the driver fully operational:

--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -5091,6 +5091,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
        SND_PCI_QUIRK(0x17aa, 0x2212, "Thinkpad", ALC269_FIXUP_LIMIT_INT_MIC_BOOST),
        SND_PCI_QUIRK(0x17aa, 0x2214, "Thinkpad", ALC269_FIXUP_LIMIT_INT_MIC_BOOST),
        SND_PCI_QUIRK(0x17aa, 0x2215, "Thinkpad", ALC269_FIXUP_LIMIT_INT_MIC_BOOST),
+       SND_PCI_QUIRK(0x17aa, 0x3977, "IdeaPad S210", ALC283_FIXUP_INT_MIC),
        SND_PCI_QUIRK(0x17aa, 0x3978, "IdeaPad Y410P", ALC269_FIXUP_NO_SHUTUP),
        SND_PCI_QUIRK(0x17aa, 0x5013, "Thinkpad", ALC269_FIXUP_LIMIT_INT_MIC_BOOST),
        SND_PCI_QUIRK(0x17aa, 0x501a, "Thinkpad", ALC283_FIXUP_INT_MIC),

It cuts the number of levels in "Internal Mic Boost" from 4 to 2, but that's enough, and the system resumes correctly from suspend and recovers from the state left by the unpatched driver.

I've been using it against fedora kernels since 3.13 series, even though I can't explain why and how it works, and I'd be interested in merging it (or an alternative fix) into the kernel proper.
Comment 1 Roman Kagan 2014-11-12 22:24:50 UTC
Created attachment 157411 [details]
alsa-info for unpatched driver before suspend
Comment 2 Roman Kagan 2014-11-12 22:25:20 UTC
Created attachment 157421 [details]
alsa-info for unpatched driver after suspend
Comment 3 Roman Kagan 2014-11-12 22:26:28 UTC
Created attachment 157431 [details]
alsa-info for patched driver
Comment 4 Takashi Iwai 2014-11-13 06:23:55 UTC
Could you check 3.18-rc4 kernel?  A patch restoring the COEF for ALC283 was already included.

BTW, is capping the mic boost volume preferred, i.e. the higher mic boost results in too noises?  If so, it makes sense to apply a fixup in anyway.
Comment 5 Roman Kagan 2014-11-13 06:47:12 UTC
> --- Comment #4 from Takashi Iwai <tiwai@suse.de> ---
> Could you check 3.18-rc4 kernel?  A patch restoring the COEF for ALC283 was
> already included.

Do you mean this one:

commit 56779864f135b309c11dd18156784f83ddc1571e
Author: Kailang Yang <kailang@realtek.com>
Date:   Fri Oct 24 16:17:51 2014 +0800

    ALSA: hda/realtek - Update restore default value for ALC283

    Update two records for ALC283 for restore default value.

    [The update doesn't seem to have high impact on the existing machines,
     but it fixes possible issues, especially expected in BIOS changes on
     new machines, according to Realtek -- tiwai]

    Signed-off-by: Kailang Yang <kailang@realtek.com>
    Signed-off-by: Takashi Iwai <tiwai@suse.de>


I'll backport it to 3.17 and try.

> BTW, is capping the mic boost volume preferred, i.e. the higher mic boost
> results in too noises?  If so, it makes sense to apply a fixup in anyway.

I didn't observe any problems with the 4-level mic boost before
suspend.  So I'd say no, this isn't necessary.
Comment 6 Takashi Iwai 2014-11-13 06:57:58 UTC
Looking at the changes more closely, this doesn't touch the COEF your suggested fix touches.  So, it's likely irrelevant.  But still it'd be helpful if you can build 3.18-rc4 kernel for testing.

So, I'm going to apply your fix as is.  Could you provide a proper patch tha is applicable to git tree (with your sign-off and patch description)?  Or I can write up a patch if you don't mind.
Comment 7 Roman Kagan 2014-11-13 12:16:22 UTC
> --- Comment #6 from Takashi Iwai <tiwai@suse.de> ---
> Looking at the changes more closely, this doesn't touch the COEF your
> suggested
> fix touches.  So, it's likely irrelevant.  But still it'd be helpful if you
> can
> build 3.18-rc4 kernel for testing.

OK I'll do and we'll see if it helps.

> So, I'm going to apply your fix as is.  Could you provide a proper patch tha
> is
> applicable to git tree (with your sign-off and patch description)?  Or I can
> write up a patch if you don't mind.

No prob either way.  But I suggest to wait until I test with the latest kernel.
Comment 8 Roman Kagan 2014-11-15 09:28:59 UTC
So I tested it, and indeed, 3.18-rc4 makes no difference WRT suspend.

Looking closer at the fixup I think it may make more sense to
generically set this coef on ALC283, like this:

--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -2803,6 +2803,7 @@ static struct coef_fw alc283_coefs[] = {
        UPDATE_COEF(0x40, 0xf800, 0x9800), /* Class D DC enable */
        UPDATE_COEF(0x42, 0xf000, 0x2000), /* DC offset */
        WRITE_COEF(0x37, 0xfc06), /* Class D amp control */
+       WRITE_COEF(0x1a, 0x11), /* internal mic ??? */
        UPDATE_COEF(0x1b, 0x8000, 0), /* HP JD control */
        {}
 };

Note that this coef is unconditionally adjusted on ALC283 in
alc_headset_mode_*().

I tested this in 3.18-rc4 and 3.17.2.  Want me to submit this as
patch?  Would be nice to get some feedback from Realtek engineers,
too.
Comment 9 Takashi Iwai 2014-11-16 08:37:25 UTC
(In reply to Roman Kagan from comment #8)
> So I tested it, and indeed, 3.18-rc4 makes no difference WRT suspend.
> 
> Looking closer at the fixup I think it may make more sense to
> generically set this coef on ALC283, like this:
> 
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -2803,6 +2803,7 @@ static struct coef_fw alc283_coefs[] = {
>         UPDATE_COEF(0x40, 0xf800, 0x9800), /* Class D DC enable */
>         UPDATE_COEF(0x42, 0xf000, 0x2000), /* DC offset */
>         WRITE_COEF(0x37, 0xfc06), /* Class D amp control */
> +       WRITE_COEF(0x1a, 0x11), /* internal mic ??? */
>         UPDATE_COEF(0x1b, 0x8000, 0), /* HP JD control */
>         {}
>  };
> 
> Note that this coef is unconditionally adjusted on ALC283 in
> alc_headset_mode_*().

... only for certain models.  But alc283_coefs[] itself is applied to all machines, so we shouldn't put it in a generic place.
 
> I tested this in 3.18-rc4 and 3.17.2.  Want me to submit this as
> patch?  Would be nice to get some feedback from Realtek engineers,
> too.

Yes, but the best option at this moment is your first patch, just adding the fixup entry.  Could you give a proper patch for merge?
Comment 10 Roman Kagan 2014-11-17 09:43:27 UTC
> --- Comment #9 from Takashi Iwai <tiwai@suse.de> ---
>> Note that this coef is unconditionally adjusted on ALC283 in
>> alc_headset_mode_*().
>
> ... only for certain models.  But alc283_coefs[] itself is applied to all
> machines, so we shouldn't put it in a generic place.

Indeed, as long as we have no knowledge of what is affected by that control.

> Yes, but the best option at this moment is your first patch, just adding the
> fixup entry.  Could you give a proper patch for merge?

Sure.  I'm a bit concerned that the fixup it enables,
ALC283_FIXUP_INT_MIC, is chained with
ALC269_FIXUP_LIMIT_INT_MIC_BOOST, which in turn is chained with
ALC269_FIXUP_THINKPAD_ACPI.  Neither is necessary on my machine.

Maybe I should better add another fixup that only adjusts the coef and
isn't chained with anything else?
Comment 11 Roman Kagan 2014-11-17 09:48:31 UTC
I forgot to note that I logged the value of that coef and indeed, it's
changed from 0x11 to 0x01 upon suspend/resume.

So that's likely a BIOS issue (the BIOS in this machine is a complete
disaster, it does nasty things on resume, including de-synchronizing
TSC between CPU cores).  I'll have a look if there's a BIOS update for
that.
Comment 12 Takashi Iwai 2014-11-17 14:43:26 UTC
(In reply to Roman Kagan from comment #10)
> > Yes, but the best option at this moment is your first patch, just adding
> the
> > fixup entry.  Could you give a proper patch for merge?
> 
> Sure.  I'm a bit concerned that the fixup it enables,
> ALC283_FIXUP_INT_MIC, is chained with
> ALC269_FIXUP_LIMIT_INT_MIC_BOOST, which in turn is chained with
> ALC269_FIXUP_THINKPAD_ACPI.  Neither is necessary on my machine.
> 
> Maybe I should better add another fixup that only adjusts the coef and
> isn't chained with anything else?

I don't mind either way as long as it works reasonably :)
Comment 13 Takashi Iwai 2014-12-05 17:16:24 UTC
I merged your first patch as it's the simplest fix.
If you mind the boost limit with this fixup, feel free to split and add more fixups in another patch on top of it.

The merge slipped from 3.18-final pull request, so it'll be merged at first in 3.19-rc1, I guess.
Comment 14 Roman Kagan 2014-12-09 08:20:11 UTC
Created attachment 160231 [details]
attachment-24791-0.html

Sorry I've been too busy lately to try different patches and test, so
thanks a lot for taking care of it!

Are you planning to submit it to linux-stable, too?

Note You need to log in before you can comment on or make changes to this bug.