| Message ID | 20180322213956.217933-3-achant@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 22 Mar 2018 22:39:56 +0100, Andrew Chant wrote: > > This implements UAC2 jack detection support, presenting > jack status as a boolean read-only mono mixer. > > The presence of any channel in the UAC2_TE_CONNECTOR > control for a terminal will result in the mixer saying > the jack is connected. > > Mixer naming follows the convention in sound/core/ctljack.c, > terminating the mixer with " Jack". > For additional clues as to which jack is being presented, > the name is prefixed with " - Input Jack" or " - Output Jack" > depending on if it's an input or output terminal. > > This is required because terminal names are ambiguous > between inputs and outputs and often duplicated - > Bidirectional terminal types (0x400 -> 0x4FF) > "... may be used separately for input only or output only. > These types require two Terminal descriptors. Both have the same type." > (quote from "USB Device Class Definition for Terminal Types") > > Since bidirectional terminal types are common for headphone adapters, > this distinguishes between two otherwise identically-named > jack controls. > > Tested with a UAC2 audio device with connector control capability. > > Signed-off-by: Andrew Chant <achant@google.com> > --- > sound/usb/mixer.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 98 insertions(+), 1 deletion(-) > > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c > index 3075ac50a391..6cb61307aefe 100644 > --- a/sound/usb/mixer.c > +++ b/sound/usb/mixer.c > @@ -1235,6 +1235,21 @@ static int mixer_ctl_feature_put(struct snd_kcontrol *kcontrol, > return changed; > } > > +/* get the current value from a mixer element */ > +static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + struct usb_mixer_elem_info *cval = kcontrol->private_data; > + int val, err; > + > + err = snd_usb_get_cur_mix_value(cval, 0, 0, &val); > + if (err < 0) > + return filter_error(cval, err); > + val = (val != 0); > + ucontrol->value.integer.value[0] = val; > + return 0; > +} > + > static struct snd_kcontrol_new usb_feature_unit_ctl = { > .iface = SNDRV_CTL_ELEM_IFACE_MIXER, > .name = "", /* will be filled later manually */ > @@ -1252,6 +1267,16 @@ static const struct snd_kcontrol_new usb_feature_unit_ctl_ro = { > .put = NULL, > }; > > +/* A UAC connector mixer control */ > +static struct snd_kcontrol_new usb_connector_ctl_ro = { > + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, Let's follow the other usages, and here iface is set to SNDRV_CTL_ELEM_IFACE_CARD. This was for avoiding such volatile controls appearing the mixer application. > + .name = "", /* will be filled later manually */ > + .access = SNDRV_CTL_ELEM_ACCESS_READ, Actually we should have set SNDRV_CTL_ELEM_ACCESS_VOLATILE, too. We forgot it in ctljack.c, too. The rest looks good. But one thing to confirm: the value change notification is done in snd_usb_mixer_interrupt()? Also, is the jack re-detected after suspend/resume? That is, plug off the jack while sleeping, and after the resume, is the jack status change recognized and notified? thanks, Takashi
On Fri, Mar 23, 2018 at 12:47 AM, Takashi Iwai <tiwai@suse.de> wrote: > Let's follow the other usages, and here iface is set to > SNDRV_CTL_ELEM_IFACE_CARD. This was for avoiding such volatile > controls appearing the mixer application. Will do - it looks like the "Clock Source Validity" control also needs this. I'll send a patch for that as well. >> + .access = SNDRV_CTL_ELEM_ACCESS_READ, > > Actually we should have set SNDRV_CTL_ELEM_ACCESS_VOLATILE, too. > We forgot it in ctljack.c, too. Sounds good. > > The rest looks good. But one thing to confirm: the value change > notification is done in snd_usb_mixer_interrupt()? > > Also, is the jack re-detected after suspend/resume? That is, plug off > the jack while sleeping, and after the resume, is the jack status > change recognized and notified? Yes - the USB audio device causes a remote wakeup, then after a few ms responds with the interrupt data message indicating the connector control that changed. This ends up calling snd_usb_mixer_interrupt(). On codeaurora kernels for Qualcomm SoCs, there's a patch which enables autosuspend for any USB device with an audio interface. https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/sound/usb/card.c?h=LA.HB.1.1.5.c1#n663 so suspend/resume paths are well tested when devices are used on Android. Two new questions: #1 - I'd like to report the impedance of the load connected to the jack (generally as an integer). Is a read-only control element the right way to do this? #2 - In our particular case there's a single jack which can detect presence of a microphone and playback separately. USB descriptors provide a mechanism to show that the input and output terminals are linked in this way (terminal type 4xx along with bAssocTerminal). Is there a better way of having this reported than two separate mono jack controls?
On Fri, 23 Mar 2018 23:24:55 +0100, Andrew Chant wrote: > > Two new questions: > #1 - I'd like to report the impedance of the load connected to the jack > (generally as an integer). Is a read-only control element the right way to > do this? Yes, it would be suitable. > #2 - In our particular case there's a single jack which can detect presence of > a microphone and playback separately. USB descriptors provide a mechanism to > show that the input and output terminals are linked in this way > (terminal type 4xx along with bAssocTerminal). > > Is there a better way of having this reported than two separate mono jack > controls? This has been discussed in the past. The current two bool kctls are the mapping of the former input devices. One suggestion was to use an enum for the jack control. But practically seen this would break user-space apps, so likely no-go. I guess we need to live as is -- two bools to represent one full-duplex jack. thanks, Takashi
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 3075ac50a391..6cb61307aefe 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -1235,6 +1235,21 @@ static int mixer_ctl_feature_put(struct snd_kcontrol *kcontrol, return changed; } +/* get the current value from a mixer element */ +static int mixer_ctl_connector_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct usb_mixer_elem_info *cval = kcontrol->private_data; + int val, err; + + err = snd_usb_get_cur_mix_value(cval, 0, 0, &val); + if (err < 0) + return filter_error(cval, err); + val = (val != 0); + ucontrol->value.integer.value[0] = val; + return 0; +} + static struct snd_kcontrol_new usb_feature_unit_ctl = { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = "", /* will be filled later manually */ @@ -1252,6 +1267,16 @@ static const struct snd_kcontrol_new usb_feature_unit_ctl_ro = { .put = NULL, }; +/* A UAC connector mixer control */ +static struct snd_kcontrol_new usb_connector_ctl_ro = { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "", /* will be filled later manually */ + .access = SNDRV_CTL_ELEM_ACCESS_READ, + .info = snd_ctl_boolean_mono_info, + .get = mixer_ctl_connector_get, + .put = NULL, +}; + /* * This symbol is exported in order to allow the mixer quirks to * hook up to the standard feature unit control mechanism @@ -1464,6 +1489,55 @@ static void build_feature_ctl(struct mixer_build *state, void *raw_desc, snd_usb_mixer_add_control(&cval->head, kctl); } +static void get_connector_control_name(struct mixer_build *state, + struct usb_audio_term *term, + bool is_input, char *name, int name_size) +{ + int name_len = get_term_name(state, term, name, name_size, 0); + + if (name_len == 0) + strlcpy(name, "Unknown", name_size); + + /* + * sound/core/ctljack.c has a convention of naming jack controls + * by ending in " Jack". Make it slightly more useful by + * indicating Input or Output after the terminal name. + */ + if (is_input) + strlcat(name, " - Input Jack", name_size); + else + strlcat(name, " - Output Jack", name_size); +} + +/* Build a mixer control for a UAC connector control (jack-detect) */ +static void build_connector_control(struct mixer_build *state, + struct usb_audio_term *term, bool is_input) +{ + struct snd_kcontrol *kctl; + struct usb_mixer_elem_info *cval; + + cval = kzalloc(sizeof(*cval), GFP_KERNEL); + if (!cval) + return; + snd_usb_mixer_elem_init_std(&cval->head, state->mixer, term->id); + cval->control = UAC2_TE_CONNECTOR; + cval->val_type = USB_MIXER_BOOLEAN; + cval->channels = term->channels; + cval->cmask = term->chconfig; + cval->min = 0; + cval->max = 1; + kctl = snd_ctl_new1(&usb_connector_ctl_ro, cval); + if (!kctl) { + usb_audio_err(state->chip, "cannot malloc kcontrol\n"); + kfree(cval); + return; + } + get_connector_control_name(state, term, is_input, kctl->id.name, + sizeof(kctl->id.name)); + kctl->private_free = snd_usb_mixer_elem_free; + snd_usb_mixer_add_control(&cval->head, kctl); +} + static int parse_clock_source_unit(struct mixer_build *state, int unitid, void *_ftr) { @@ -1770,6 +1844,23 @@ static void build_mixer_unit_ctl(struct mixer_build *state, snd_usb_mixer_add_control(&cval->head, kctl); } +static int parse_audio_input_terminal(struct mixer_build *state, int unitid, + void *raw_desc) +{ + struct usb_audio_term iterm; + struct uac2_input_terminal_descriptor *d = raw_desc; + + check_input_term(state, d->bTerminalID, &iterm); + if (state->mixer->protocol == UAC_VERSION_2) { + /* Check for jack detection. */ + if (uac_v2v3_control_is_readable(d->bmControls, + UAC2_TE_CONNECTOR)) { + build_connector_control(state, &iterm, true); + } + } + return 0; +} + /* * parse a mixer unit */ @@ -2320,7 +2411,7 @@ static int parse_audio_unit(struct mixer_build *state, int unitid) if (protocol == UAC_VERSION_1 || protocol == UAC_VERSION_2) { switch (p1[2]) { case UAC_INPUT_TERMINAL: - return 0; /* NOP */ + return parse_audio_input_terminal(state, unitid, p1); case UAC_MIXER_UNIT: return parse_audio_mixer_unit(state, unitid, p1); case UAC2_CLOCK_SOURCE: @@ -2463,6 +2554,12 @@ static int snd_usb_mixer_controls(struct usb_mixer_interface *mixer) err = parse_audio_unit(&state, desc->bCSourceID); if (err < 0 && err != -EINVAL) return err; + + if (uac_v2v3_control_is_readable(desc->bmControls, + UAC2_TE_CONNECTOR)) { + build_connector_control(&state, &state.oterm, + false); + } } else { /* UAC_VERSION_3 */ struct uac3_output_terminal_descriptor *desc = p;
This implements UAC2 jack detection support, presenting jack status as a boolean read-only mono mixer. The presence of any channel in the UAC2_TE_CONNECTOR control for a terminal will result in the mixer saying the jack is connected. Mixer naming follows the convention in sound/core/ctljack.c, terminating the mixer with " Jack". For additional clues as to which jack is being presented, the name is prefixed with " - Input Jack" or " - Output Jack" depending on if it's an input or output terminal. This is required because terminal names are ambiguous between inputs and outputs and often duplicated - Bidirectional terminal types (0x400 -> 0x4FF) "... may be used separately for input only or output only. These types require two Terminal descriptors. Both have the same type." (quote from "USB Device Class Definition for Terminal Types") Since bidirectional terminal types are common for headphone adapters, this distinguishes between two otherwise identically-named jack controls. Tested with a UAC2 audio device with connector control capability. Signed-off-by: Andrew Chant <achant@google.com> --- sound/usb/mixer.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 1 deletion(-)