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 willnot work correctly
* when booting with headset plugged. So skip setting it for thecodec 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 anyloopback 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.