public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Zhang Heng <zhangheng@kylinos.cn>
Cc: perex@perex.cz, tiwai@suse.com, linux-sound@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ALSA: hda/generic: Add mic autoswitch support for dyn_adc_switch mode
Date: Mon, 04 May 2026 18:25:17 +0200	[thread overview]
Message-ID: <87jytjw39u.wl-tiwai@suse.de> (raw)
In-Reply-To: <20260503114512.331805-1-zhangheng@kylinos.cn>

On Sun, 03 May 2026 13:45:12 +0200,
Zhang Heng wrote:
> 
> When auto_mic is not available but dyn_adc_switch mode is enabled
> (e.g., on laptops with both front and rear mic jacks), this patch
> enables automatic microphone switching based on jack detection.
> 
> The patch includes three changes:
> 
> 1. In check_dyn_adc_switch(): Register jack detect callbacks for
>    all input pins when dyn_adc_switch is enabled and auto_mic is
>    not available.
> 
> 2. In call_mic_autoswitch(): Add handling for dyn_adc_switch mode
>    to check imux_pins[] for jack presence, searching from back to
>    front (last inserted wins).
> 
> 3. In mux_select(): Notify Capture Source controls after switching
>    to sync with user-space (PulseAudio/PipeWire).
> 
> Problem description:
> On Ubuntu 20.04 (with older PulseAudio/PipeWire):
> - Front mic is unplugged
> - Plug in rear mic
> - Jack event is reported correctly
> - Volume control (PulseAudio/PipeWire) recognizes the rear mic
> - But alsamixer does NOT switch to rear mic
> - Codec also does NOT perform the switch
> 
> The root cause is that in dyn_adc_switch mode without auto_mic,
> the jack detect callback was not properly set up to trigger the
> mic autoswitch. The call_mic_autoswitch() function only calls
> mic_autoswitch_hook or snd_hda_gen_mic_autoswitch(), which rely
> on auto_mic being enabled.
> 
> Additionally, after mux_select() performs the switch, user-space
> (PulseAudio/PipeWire) may not be aware of the path change,
> causing Capture Switch to show 'off'.
> 
> This patch fixes both issues by:
> 1. Registering jack detect callbacks for all input pins in
>    dyn_adc_switch mode
> 2. Notifying Capture Source controls after switching
> 
> Tested on SN6186 codec with Ubuntu 20.04 and 25.10.
> 
> Question to the community: Is this approach correct? Should additional
> changes be made to handle mute state preservation, or is this purely
> a user-space issue that requires updating PulseAudio/PipeWire?
> 
> Testing and feedback are welcome.

I think the basic idea is OK.  We can treat the auto-mic with dynamic
ADC switches, too.

But it's not clear what's the actual intent in your code changes:


> Signed-off-by: Zhang Heng <zhangheng@kylinos.cn>
> ---
>  sound/hda/codecs/generic.c | 82 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 79 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/hda/codecs/generic.c b/sound/hda/codecs/generic.c
> index 660a9f2c0ded..c536de10d8b8 100644
> --- a/sound/hda/codecs/generic.c
> +++ b/sound/hda/codecs/generic.c
> @@ -27,6 +27,10 @@
>  #include "hda_beep.h"
>  #include "generic.h"
>  
> +/* Forward declaration */
> +static void call_mic_autoswitch(struct hda_codec *codec,
> +				struct hda_jack_callback *jack);
> +
>  
>  /**
>   * snd_hda_gen_spec_init - initialize hda_gen_spec struct
> @@ -3238,6 +3242,19 @@ static int check_dyn_adc_switch(struct hda_codec *codec)
>  	if (!spec->dyn_adc_switch && spec->multi_cap_vol)
>  		spec->num_adc_nids = 1;
>  
> +	/* Enable mic autoswitch for dyn_adc_switch mode when auto_mic is not available */
> +	if (!spec->auto_mic && imux->num_items > 1) {
> +		int i;
> +		for (i = 0; i < imux->num_items; i++) {
> +			hda_nid_t pin = spec->imux_pins[i];
> +			if (!is_jack_detectable(codec, pin))
> +				continue;
> +			snd_hda_jack_detect_enable_callback(codec, pin,
> +							    call_mic_autoswitch);
> +		}
> +		codec_dbg(codec, "Enable mic autoswitch for input sources\n");
> +	}

IMO, this should be rather put at auto_mic_check_imux() instead,
something like:

@@ -4789,13 +4789,15 @@ static bool auto_mic_check_imux(struct hda_codec *codec)
 	const struct hda_input_mux *imux;
 	int i;
 
-	imux = &spec->input_mux;
-	for (i = 0; i < spec->am_num_entries; i++) {
-		spec->am_entry[i].idx =
-			find_idx_in_nid_list(spec->am_entry[i].pin,
-					     spec->imux_pins, imux->num_items);
-		if (spec->am_entry[i].idx < 0)
-			return false; /* no corresponding imux */
+	if (!spec->dyn_adc_switch) {
+		imux = &spec->input_mux;
+		for (i = 0; i < spec->am_num_entries; i++) {
+			spec->am_entry[i].idx =
+				find_idx_in_nid_list(spec->am_entry[i].pin,
+						     spec->imux_pins, imux->num_items);
+			if (spec->am_entry[i].idx < 0)
+				return false; /* no corresponding imux */
+		}
 	}
 
 	/* we don't need the jack detection for the first pin */


>  	return 0;
>  }
>  
> @@ -4107,9 +4124,38 @@ static int mux_select(struct hda_codec *codec, unsigned int adc_idx,
>  	path = get_input_path(codec, adc_idx, idx);
>  	if (!path)
>  		return 0;
> -	if (path->active)
> -		return 0;
> -	snd_hda_activate_path(codec, path, true, false);
> +	if (path->active) {
> +		codec_err(codec, "[MUX] mux_select: path already active, skip activate but still notify\n");
> +	} else {
> +		snd_hda_activate_path(codec, path, true, false);
> +	}
> +
> +	/* Notify Input Source / Capture Source controls that the path has changed */
> +	if (!spec->auto_mic && spec->input_mux.num_items > 1) {
> +		struct snd_card *card = codec->card;
> +		struct snd_kcontrol *kctl;
> +		struct snd_ctl_elem_id id;
> +		int notified = 0;
> +
> +		codec_err(codec, "[MUX] mux_select: notifying controls, num_items=%d\n", spec->input_mux.num_items);
> +
> +		if (card) {
> +			list_for_each_entry(kctl, &card->controls, list) {
> +				if (kctl->id.iface != SNDRV_CTL_ELEM_IFACE_MIXER)
> +					continue;
> +				codec_err(codec, "[MUX] mux_select: found control '%s'\n", kctl->id.name);
> +				if (strncmp(kctl->id.name, "Capture Source", 14) == 0) {
> +					id = kctl->id;
> +					snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, &id);
> +					codec_err(codec, "[MUX] mux_select: notified '%s'\n", kctl->id.name);
> +					notified = 1;
> +				}
> +			}
> +		}
> +		if (!notified)
> +			codec_err(codec, "[MUX] mux_select: no Input/Capture Source control found!\n");
> +	}
> +

Hmm, I can't follow the logic here.  What does this code change try to
achieve...?

>  	if (spec->cap_sync_hook)
>  		spec->cap_sync_hook(codec, NULL, NULL);
>  	path_power_down_sync(codec, old_path);
> @@ -4602,6 +4648,36 @@ static void call_mic_autoswitch(struct hda_codec *codec,
>  				struct hda_jack_callback *jack)
>  {
>  	struct hda_gen_spec *spec = codec->spec;
> +	const struct hda_input_mux *imux;
> +	int i;
> +
> +	/* Handle dyn_adc_switch mode - use imux_pins[] */
> +	if (!spec->auto_mic && spec->dyn_adc_switch) {
> +		imux = &spec->input_mux;
> +		if (imux->num_items <= 1)
> +			goto fallback;
> +
> +		codec_err(codec, "[AUTO_MIC] call_mic_autoswitch: dyn_adc_switch mode, checking %d inputs\n", imux->num_items);
> +
> +		/* Search from back to front (last inserted wins) */
> +		for (i = imux->num_items - 1; i >= 0; i--) {
> +			hda_nid_t pin = spec->imux_pins[i];
> +			int det = is_jack_detectable(codec, pin);
> +			int state = snd_hda_jack_detect_state(codec, pin);
> +			codec_err(codec, "[AUTO_MIC] call_mic_autoswitch:   imux[%d] pin=0x%02x, det=%d, state=%s\n",
> +				  i, pin, det, state == HDA_JACK_PRESENT ? "PRESENT" : "ABSENT");
> +			if (det && state == HDA_JACK_PRESENT) {
> +				codec_err(codec, "[AUTO_MIC] call_mic_autoswitch: switching to imux %d\n", i);
> +				mux_select(codec, 0, i);
> +				return;
> +			}
> +		}
> +		/* No jack present, keep current selection (don't switch to imux 0) */
> +		codec_err(codec, "[AUTO_MIC] call_mic_autoswitch: no jack present, keeping current\n");
> +		return;
> +	}
> +
> +fallback:
>  	if (spec->mic_autoswitch_hook)
>  		spec->mic_autoswitch_hook(codec, jack);
>  	else

Ditto.  Something too complex here.
mux_select() should handle the dynamic ADC switching, per se.

And, with the auto-mic mode, the driver shouldn't expose the Capture
Source mixer control.  That said, enabling the auto-mic would have a
clear downside for the user of PA/PW, too.


thanks,

Takashi

      reply	other threads:[~2026-05-04 16:25 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-03 11:45 [PATCH] ALSA: hda/generic: Add mic autoswitch support for dyn_adc_switch mode Zhang Heng
2026-05-04 16:25 ` Takashi Iwai [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87jytjw39u.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    --cc=zhangheng@kylinos.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox