2021-03-24 17:19:00

by Kai-Heng Feng

[permalink] [raw]
Subject: [PATCH 2/2] ALSA: usb-audio: Check connector value on resume

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.

Signed-off-by: Kai-Heng Feng <[email protected]>
---
sound/usb/mixer.c | 18 ++++++++++++++++++
sound/usb/mixer.h | 1 +
2 files changed, 19 insertions(+)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 98f5417a70e4..6a553d891b0f 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -3631,11 +3631,28 @@ static int restore_mixer_value(struct usb_mixer_elem_list *list)
return 0;
}

+static int resume_connector(struct usb_mixer_elem_list *list)
+{
+ struct usb_mixer_elem_info *cval = mixer_elem_list_to_info(list);
+
+ if (cval->val_type != USB_MIXER_BOOLEAN || cval->channels != 1)
+ return 0;
+
+ return get_connector_value(cval, NULL, NULL);
+}
+
int snd_usb_mixer_resume(struct usb_mixer_interface *mixer, bool reset_resume)
{
struct usb_mixer_elem_list *list;
int id, err;

+ for (id = 0; id < MAX_ID_ELEMS; id++) {
+ for_each_mixer_elem(list, mixer, id) {
+ if (list->resume_connector)
+ list->resume_connector(list);
+ }
+ }
+
if (reset_resume) {
/* restore cached mixer values */
for (id = 0; id < MAX_ID_ELEMS; id++) {
@@ -3664,5 +3681,6 @@ void snd_usb_mixer_elem_init_std(struct usb_mixer_elem_list *list,
list->dump = snd_usb_mixer_dump_cval;
#ifdef CONFIG_PM
list->resume = restore_mixer_value;
+ list->resume_connector = resume_connector;
#endif
}
diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
index c29e27ac43a7..843ccff0eea3 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 resume_connector;
};

/* iterate over mixer element list of the given unit id */
--
2.30.2


2021-03-25 07:21:30

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH 2/2] ALSA: usb-audio: Check connector value on resume

On Wed, 24 Mar 2021 18:14:08 +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.
>
> Signed-off-by: Kai-Heng Feng <[email protected]>

Just to be sure: this workaround is always needed no matter whether
reset_resume is set or not? If so, it's better to change the resume
callback to take reset_resume argument and call it always. The
resume_connector() can be folded into there.


thanks,

Takashi

> ---
> sound/usb/mixer.c | 18 ++++++++++++++++++
> sound/usb/mixer.h | 1 +
> 2 files changed, 19 insertions(+)
>
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index 98f5417a70e4..6a553d891b0f 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -3631,11 +3631,28 @@ static int restore_mixer_value(struct usb_mixer_elem_list *list)
> return 0;
> }
>
> +static int resume_connector(struct usb_mixer_elem_list *list)
> +{
> + struct usb_mixer_elem_info *cval = mixer_elem_list_to_info(list);
> +
> + if (cval->val_type != USB_MIXER_BOOLEAN || cval->channels != 1)
> + return 0;
> +
> + return get_connector_value(cval, NULL, NULL);
> +}
> +
> int snd_usb_mixer_resume(struct usb_mixer_interface *mixer, bool reset_resume)
> {
> struct usb_mixer_elem_list *list;
> int id, err;
>
> + for (id = 0; id < MAX_ID_ELEMS; id++) {
> + for_each_mixer_elem(list, mixer, id) {
> + if (list->resume_connector)
> + list->resume_connector(list);
> + }
> + }
> +
> if (reset_resume) {
> /* restore cached mixer values */
> for (id = 0; id < MAX_ID_ELEMS; id++) {
> @@ -3664,5 +3681,6 @@ void snd_usb_mixer_elem_init_std(struct usb_mixer_elem_list *list,
> list->dump = snd_usb_mixer_dump_cval;
> #ifdef CONFIG_PM
> list->resume = restore_mixer_value;
> + list->resume_connector = resume_connector;
> #endif
> }
> diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
> index c29e27ac43a7..843ccff0eea3 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 resume_connector;
> };
>
> /* iterate over mixer element list of the given unit id */
> --
> 2.30.2
>

2021-03-25 07:28:08

by Kai-Heng Feng

[permalink] [raw]
Subject: Re: [PATCH 2/2] ALSA: usb-audio: Check connector value on resume

On Thu, Mar 25, 2021 at 3:19 PM Takashi Iwai <[email protected]> wrote:
>
> On Wed, 24 Mar 2021 18:14:08 +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.
> >
> > Signed-off-by: Kai-Heng Feng <[email protected]>
>
> Just to be sure: this workaround is always needed no matter whether
> reset_resume is set or not?

Yes, reset_resume is irrelevant for this issue. Getting connector
status is the key.

> If so, it's better to change the resume
> callback to take reset_resume argument and call it always. The
> resume_connector() can be folded into there.

OK, will send V2.

Kai-Heng

>
>
> thanks,
>
> Takashi
>
> > ---
> > sound/usb/mixer.c | 18 ++++++++++++++++++
> > sound/usb/mixer.h | 1 +
> > 2 files changed, 19 insertions(+)
> >
> > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> > index 98f5417a70e4..6a553d891b0f 100644
> > --- a/sound/usb/mixer.c
> > +++ b/sound/usb/mixer.c
> > @@ -3631,11 +3631,28 @@ static int restore_mixer_value(struct usb_mixer_elem_list *list)
> > return 0;
> > }
> >
> > +static int resume_connector(struct usb_mixer_elem_list *list)
> > +{
> > + struct usb_mixer_elem_info *cval = mixer_elem_list_to_info(list);
> > +
> > + if (cval->val_type != USB_MIXER_BOOLEAN || cval->channels != 1)
> > + return 0;
> > +
> > + return get_connector_value(cval, NULL, NULL);
> > +}
> > +
> > int snd_usb_mixer_resume(struct usb_mixer_interface *mixer, bool reset_resume)
> > {
> > struct usb_mixer_elem_list *list;
> > int id, err;
> >
> > + for (id = 0; id < MAX_ID_ELEMS; id++) {
> > + for_each_mixer_elem(list, mixer, id) {
> > + if (list->resume_connector)
> > + list->resume_connector(list);
> > + }
> > + }
> > +
> > if (reset_resume) {
> > /* restore cached mixer values */
> > for (id = 0; id < MAX_ID_ELEMS; id++) {
> > @@ -3664,5 +3681,6 @@ void snd_usb_mixer_elem_init_std(struct usb_mixer_elem_list *list,
> > list->dump = snd_usb_mixer_dump_cval;
> > #ifdef CONFIG_PM
> > list->resume = restore_mixer_value;
> > + list->resume_connector = resume_connector;
> > #endif
> > }
> > diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h
> > index c29e27ac43a7..843ccff0eea3 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 resume_connector;
> > };
> >
> > /* iterate over mixer element list of the given unit id */
> > --
> > 2.30.2
> >