Bug 88121

Created attachment 157411 [details]
alsa-info for unpatched driver before suspend

Created attachment 157421 [details]
alsa-info for unpatched driver after suspend

Created attachment 157431 [details]
alsa-info for patched driver

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 #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.

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 #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.

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.

(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 #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?

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.

(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 :)

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.

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?