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