[alsa-devel] [PATCH] ALSA: hda - Send CONNECT_SEL on capture start

[Takashi Iwai]

At Wed, 16 Nov 2011 00:25:49 -0800,
Dylan Reid wrote:
> 
> cur_adc is set to zero in cs_capture_pcm_prepare.

Not really.  It's set to zero until cs_capture_pcm_preare() is called.

>  This causes the
> SET_CONNECT_SEL verb to be sent to node zero when doing automic after
> recording.  Delay sending the command until we are about to start
> capture and cur_adc is set to the correct NID.
> 
> Signed-off-by: Dylan Reid <dgreid at chromium.org>
> ---
>  sound/pci/hda/patch_cirrus.c |   16 ++++++++++------
>  1 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
> index 2a2d864..30f2f6a 100644
> --- a/sound/pci/hda/patch_cirrus.c
> +++ b/sound/pci/hda/patch_cirrus.c
> @@ -250,6 +250,9 @@ static int cs_capture_pcm_prepare(struct hda_pcm_stream *hinfo,
>  	spec->cur_adc = spec->adc_nid[spec->cur_input];
>  	spec->cur_adc_stream_tag = stream_tag;
>  	spec->cur_adc_format = format;
> +	snd_hda_codec_write(codec, spec->cur_adc, 0,
> +			    AC_VERB_SET_CONNECT_SEL,
> +			    spec->adc_idx[spec->cur_input]);
>  	snd_hda_codec_setup_stream(codec, spec->cur_adc, stream_tag, 0, format);
>  	return 0;
>  }
> @@ -685,13 +688,13 @@ static int change_cur_input(struct hda_codec *codec, unsigned int idx,
>  		/* stream is running, let's swap the current ADC */
>  		__snd_hda_codec_cleanup_stream(codec, spec->cur_adc, 1);
>  		spec->cur_adc = spec->adc_nid[idx];
> +		snd_hda_codec_write(codec, spec->cur_adc, 0,
> +				    AC_VERB_SET_CONNECT_SEL,
> +				    spec->adc_idx[idx]);
>  		snd_hda_codec_setup_stream(codec, spec->cur_adc,
>  					   spec->cur_adc_stream_tag, 0,
>  					   spec->cur_adc_format);
>  	}
> -	snd_hda_codec_write(codec, spec->cur_adc, 0,
> -			    AC_VERB_SET_CONNECT_SEL,
> -			    spec->adc_idx[idx]);

This breaks the case when the input-source is changed dynamically
during the recording with CS420x.


>  	spec->cur_input = idx;
>  	return 1;
>  }
> @@ -974,9 +977,10 @@ static void cs_automic(struct hda_codec *codec)
>  			spec->cur_input = spec->last_input;
>  		}
>  
> -		snd_hda_codec_write_cache(codec, spec->cur_adc, 0,
> -					AC_VERB_SET_CONNECT_SEL,
> -					spec->adc_idx[spec->cur_input]);
> +		if (spec->cur_adc)
> +			snd_hda_codec_write_cache(codec, spec->cur_adc, 0,
> +				AC_VERB_SET_CONNECT_SEL,
> +				spec->adc_idx[spec->cur_input]);

Well, it'd be better to create a function instead of scattering the
same verb call allover.

How about the (untested) patch below instead?


Takashi

---
diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index 2a2d864..2fbab8e 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -237,6 +237,15 @@ static int cs_dig_playback_pcm_cleanup(struct hda_pcm_stream *hinfo,
 	return snd_hda_multi_out_dig_cleanup(codec, &spec->multiout);
 }
 
+static void cs_update_input_select(struct hda_codec *codec)
+{
+	struct cs_spec *spec = codec->spec;
+	if (spec->cur_adc)
+		snd_hda_codec_write(codec, spec->cur_adc, 0,
+				    AC_VERB_SET_CONNECT_SEL,
+				    spec->adc_idx[spec->cur_input]);
+}
+
 /*
  * Analog capture
  */
@@ -250,6 +259,7 @@ static int cs_capture_pcm_prepare(struct hda_pcm_stream *hinfo,
 	spec->cur_adc = spec->adc_nid[spec->cur_input];
 	spec->cur_adc_stream_tag = stream_tag;
 	spec->cur_adc_format = format;
+	cs_update_input_select(codec);
 	snd_hda_codec_setup_stream(codec, spec->cur_adc, stream_tag, 0, format);
 	return 0;
 }
@@ -689,10 +699,8 @@ static int change_cur_input(struct hda_codec *codec, unsigned int idx,
 					   spec->cur_adc_stream_tag, 0,
 					   spec->cur_adc_format);
 	}
-	snd_hda_codec_write(codec, spec->cur_adc, 0,
-			    AC_VERB_SET_CONNECT_SEL,
-			    spec->adc_idx[idx]);
 	spec->cur_input = idx;
+	cs_update_input_select(codec);
 	return 1;
 }
 
@@ -973,10 +981,7 @@ static void cs_automic(struct hda_codec *codec)
 		} else  {
 			spec->cur_input = spec->last_input;
 		}
-
-		snd_hda_codec_write_cache(codec, spec->cur_adc, 0,
-					AC_VERB_SET_CONNECT_SEL,
-					spec->adc_idx[spec->cur_input]);
+		cs_update_input_select(codec);
 	} else {
 		if (present)
 			change_cur_input(codec, spec->automic_idx, 0);
@@ -1073,9 +1078,7 @@ static void init_input(struct hda_codec *codec)
 			cs_automic(codec);
 		else  {
 			spec->cur_adc = spec->adc_nid[spec->cur_input];
-			snd_hda_codec_write(codec, spec->cur_adc, 0,
-					AC_VERB_SET_CONNECT_SEL,
-					spec->adc_idx[spec->cur_input]);
+			cs_update_input_select(codec);
 		}
 	} else {
 		change_cur_input(codec, spec->cur_input, 1);