On Thu, Mar 25, 2021 at 9:41 PM Takashi Iwai <tiwai@xxxxxxx> wrote:
>
> On Thu, 25 Mar 2021 13:12:48 +0100,
> Kai-Heng Feng wrote:
> >
> > Rear Mic on Lenovo P620 cannot record after S3, despite that there's no
> > error and the other two functions of the USB audio, Line In and Line
> > Out, work just fine.
> >
> > The mic starts to work again after running userspace app like "alsactl
> > store". Following the lead, the evidence shows that as soon as connector
> > status is queried, the mic can work again.
> >
> > So also check connector value on resume to "wake up" the USB audio to
> > make it functional.
> >
> > This can be device specific, however I think this generic approach may
> > benefit more than one device.
> >
> > While at it, also remove reset-resume path to consolidate mixer resume
> > path.
> >
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
> > ---
> > v2:
> > - Remove reset-resume.
> > - Fold the connector checking to the mixer resume callback.
>
> That's not what I meant exactly... I meant to put both into the
> single resume callback, but handle each part conditionally depending
> on reset_resume argument.
OK, I get what you mean now.
>
> But this turned out to need more changes in mixer_quirks.c
> unnecessarily. Maybe adding the two resume functions is a better
> approach in the end, but not for the specific connection thing but
> generically both resume and reset_resume callbacks. Something like
> below.
This approach looks good. Let me send another one.
Kai-Heng
>
>
> thanks,
>
> Takashi
>
>
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index b004b2e63a5d..1dab281bb269 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -3615,20 +3615,43 @@ static int restore_mixer_value(struct usb_mixer_elem_list *list)
> return 0;
> }
>
> +static int default_mixer_resume(struct usb_mixer_elem_list *list)
> +{
> + struct usb_mixer_elem_info *cval = mixer_elem_list_to_info(list);
> +
> + /* get connector value to "wake up" the USB audio */
> + if (cval->val_type == USB_MIXER_BOOLEAN && cval->channels == 1)
> + get_connector_value(cval, NULL, NULL);
> +
> + return 0;
> +}
> +
> +static int default_mixer_reset_resume(struct usb_mixer_elem_list *list)
> +{
> + int err = default_mixer_resume(list);
> +
> + if (err < 0)
> + return err;
> + return restore_mixer_value(list);
> +}
> +
> int snd_usb_mixer_resume(struct usb_mixer_interface *mixer, bool reset_resume)
> {
> struct usb_mixer_elem_list *list;
> + usb_mixer_elem_resume_func_t f;
> int id, err;
>
> - if (reset_resume) {
> - /* restore cached mixer values */
> - for (id = 0; id < MAX_ID_ELEMS; id++) {
> - for_each_mixer_elem(list, mixer, id) {
> - if (list->resume) {
> - err = list->resume(list);
> - if (err < 0)
> - return err;
> - }
> + /* restore cached mixer values */
> + for (id = 0; id < MAX_ID_ELEMS; id++) {
> + for_each_mixer_elem(list, mixer, id) {
> + if (reset_resume)
> + f = list->reset_resume;
> + else
> + f = list->resume;
> + if (f) {
> + err = list->resume(list);
> + if (err < 0)
> + return err;
> }
> }
> }
> @@ -3647,6 +3670,7 @@ void snd_usb_mixer_elem_init_std(struct usb_mixer_elem_list *list,
> list->id = unitid;
> list->dump = snd_usb_mixer_dump_cval;
> #ifdef CONFIG_PM
> - list->resume = restore_mixer_value;
> + list->resume = default_mixer_resume;
> + list->reset_resume = default_mixer_reset_resume;
> #endif
> }
> diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
> index c29e27ac43a7..e5a01f17bf3c 100644
> --- a/sound/usb/mixer.h
> +++ b/sound/usb/mixer.h
> @@ -69,6 +69,7 @@ struct usb_mixer_elem_list {
> bool is_std_info;
> usb_mixer_elem_dump_func_t dump;
> usb_mixer_elem_resume_func_t resume;
> + usb_mixer_elem_resume_func_t reset_resume;
> };
>
> /* iterate over mixer element list of the given unit id */
> diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
> index ffd922327ae4..b7f9c2fded05 100644
> --- a/sound/usb/mixer_quirks.c
> +++ b/sound/usb/mixer_quirks.c
> @@ -151,7 +151,7 @@ static int add_single_ctl_with_resume(struct usb_mixer_interface *mixer,
> *listp = list;
> list->mixer = mixer;
> list->id = id;
> - list->resume = resume;
> + list->reset_resume = resume;
> kctl = snd_ctl_new1(knew, list);
> if (!kctl) {
> kfree(list);
