Re: Fwd: [Bug 217440] New: ALC236 audio disappears from HP 15z-fc000 on warm boot

[Takashi Iwai <tiwai@suse.de>]

On Tue, 15 Aug 2023 11:08:19 +0200,
Takashi Iwai wrote:
> 
> On Tue, 15 Aug 2023 10:08:48 +0200,
> Kailang wrote:
> > 
> > Hi Takashi,
> > 
> > I think maybe attach patch is what you want.
> 
> Looks good.  Let's ask the reporter to test with it.

I believe this regression fix should go for 6.5-rc7.
Now applied.


Takashi

> 
> 
> Thanks!
> 
> Takashi
> 
> > 
> > > -----Original Message-----
> > > From: Joseph C. Sible <josephcsible@gmail.com>
> > > Sent: Friday, August 4, 2023 10:35 PM
> > > To: Takashi Iwai <tiwai@suse.de>
> > > Cc: Kailang <kailang@realtek.com>; Bagas Sanjaya <bagasdotme@gmail.com>;
> > > regressions@lists.linux.dev; perex@perex.cz; tiwai@suse.com;
> > > alsa-devel@alsa-project.org
> > > Subject: Re: Fwd: [Bug 217440] New: ALC236 audio disappears from HP
> > > 15z-fc000 on warm boot
> > > 
> > > 
> > > External mail.
> > > 
> > > 
> > > 
> > > On Mon, Jul 31, 2023 at 12:14 PM Takashi Iwai <tiwai@suse.de> wrote:
> > > >
> > > > ... and now we receive a regression report due to this change :-<
> > > >   https://bugzilla.kernel.org/show_bug.cgi?id=217732
> > > >
> > > > I believe the problem is that the patch disabled the 3kpull-low
> > > > behavior too much while some codecs still need it.
> > > >
> > > > The patch changes like:
> > > >
> > > > @@ -3638,8 +3640,7 @@ static void alc256_shutup(struct hda_codec
> > > *codec)
> > > >         /* If disable 3k pulldown control for alc257, the Mic detection will
> > > not work correctly
> > > >          * when booting with headset plugged. So skip setting it for the
> > > codec alc257
> > > >          */
> > > > -       if (codec->core.vendor_id != 0x10ec0236 &&
> > > > -           codec->core.vendor_id != 0x10ec0257)
> > > > +       if (spec->en_3kpull_low)
> > > >                 alc_update_coef_idx(codec, 0x46, 0, 3 << 12);
> > > >
> > > >         if (!spec->no_shutup_pins)
> > > >
> > > > ... while the only place setting spec->en_3kpull_low is:
> > > >
> > > > @@ -10682,6 +10683,8 @@ static int patch_alc269(struct hda_codec
> > > *codec)
> > > >                 spec->shutup = alc256_shutup;
> > > >                 spec->init_hook = alc256_init;
> > > >                 spec->gen.mixer_nid = 0; /* ALC256 does not have any
> > > > loopback mixer path */
> > > > +               if (codec->bus->pci->vendor == PCI_VENDOR_ID_AMD)
> > > > +                       spec->en_3kpull_low = true;
> > > >                 break;
> > > >         case 0x10ec0257:
> > > >                 spec->codec_variant = ALC269_TYPE_ALC257;
> > > >
> > > > Since spec->3n_3kpull_low is false as default, it means that, except
> > > > for ALC230/236/256 and PCI vendor AMD, the flag is never set, hence
> > > > it's no longer called.
> > > >
> > > > Shouldn't spec->en_3kpull_low be enabled rather as default?  Or which
> > > > models can set it?
> > > 
> > > In both my original bug and this new bug, the problem was that we're not
> > > calling it when we should be. Given that, wouldn't a simple fix be to call it if
> > > either the old condition or the new condition is true?
> > > E.g., something like this:
> > > 
> > > if ((codec->core.vendor_id != 0x10ec0236 && codec->core.vendor_id !=
> > > 0x10ec0257) || spec->en_3kpull_low)
> > > 
> > > Joseph C. Sible
> > > 
> > > ------Please consider the environment before printing this e-mail.
> > 
> > 
>