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

[Dylan Reid]

On Wed, Nov 16, 2011 at 12:53 AM, Takashi Iwai <tiwai at suse.de> wrote:
> 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?
That is better. I tested it this morning and it works like a charm.  Can you
apply that?

Thanks Takashi,

Dylan
>
>
> 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);
>