[alsa-devel] [PATCH 0/6] usb-audio: quirks for Dell WD15 dock and some extensions
Hi,
here is a patch set for adding quirks and some new extensions for Dell WD15 dock. The first patch is the resend of the previous post, and the rest are new.
Basically it adds a new "Keep Interface" control for the card to skip resetting the USB interface per stream manipulation. It helps for reducing the long pause before each start of stream on Dell WD15 dock. OTOH, this was confirmed to be harmless on some dock early firmware version, so we can't set it statically in the driver. So, instead, the feature is exposed as a control element so that user can flip dynamically. This feature may be useful for other USB audio devices as well.
Another extension is to allow the quirk entry to override the card longname field. This is needed to give a proper UCM profile. WD15 dock provides two streams, one for the headphone and another for the line out. It's a good example for using a UCM profile.
Takashi
===
Takashi Iwai (6): ALSA: usb-audio: Initialize Dell Dock playback volumes ALSA: usb-audio: Avoid superfluous usb_set_interface() calls ALSA: usb-audio: Add keep_iface flag ALSA: usb-audio: Add "Keep Interface" control ALSA: usb-audio: Allow to override the longname string ALSA: usb-audio: Give proper vendor/product name for Dell WD15 Dock
sound/usb/card.c | 146 ++++++++++++++++++++++++++++------------------- sound/usb/mixer.c | 47 +++++++++++++++ sound/usb/mixer_quirks.c | 34 +++++++++++ sound/usb/mixer_quirks.h | 4 ++ sound/usb/pcm.c | 28 ++++----- sound/usb/quirks-table.h | 10 ++++ sound/usb/usbaudio.h | 4 ++ 7 files changed, 200 insertions(+), 73 deletions(-)
In the early commit adcdd0d5a1cb ("ALSA: usb-audio: Skip volume controls triggers hangup on Dell USB Dock"), we add the mixer quirks for Dell dock to skip two mixer FU's for playback. This supposed that the device has always the proper initial volume, but it doesn't seem always correct.
This patch adds the explicit initialization of the volumes to the fixed 0dB at the device probe time. Also, such a fixup is needed after the resume, so a new function is hooked to the resume callback as well.
Bugzilla: http://bugzilla.suse.com/show_bug.cgi?id=1089467 Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/mixer.c | 2 ++ sound/usb/mixer_quirks.c | 34 ++++++++++++++++++++++++++++++++++ sound/usb/mixer_quirks.h | 4 ++++ 3 files changed, 40 insertions(+)
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 344d7b069d59..76fabc4b72b5 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -2948,6 +2948,8 @@ int snd_usb_mixer_resume(struct usb_mixer_interface *mixer, bool reset_resume) } }
+ snd_usb_mixer_resume_quirk(mixer); + return snd_usb_mixer_activate(mixer); } #endif diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c index 56537a156580..4377374affd3 100644 --- a/sound/usb/mixer_quirks.c +++ b/sound/usb/mixer_quirks.c @@ -1799,6 +1799,26 @@ static int snd_soundblaster_e1_switch_create(struct usb_mixer_interface *mixer) NULL); }
+static void dell_dock_init_vol(struct snd_usb_audio *chip, int ch, int id) +{ + u16 buf = 0; + + snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0), UAC_SET_CUR, + USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT, + ch, snd_usb_ctrl_intf(chip) | (id << 8), + &buf, 2); +} + +static int dell_dock_mixer_init(struct usb_mixer_interface *mixer) +{ + /* fix to 0dB playback volumes */ + dell_dock_init_vol(mixer->chip, 1, 16); + dell_dock_init_vol(mixer->chip, 2, 16); + dell_dock_init_vol(mixer->chip, 1, 19); + dell_dock_init_vol(mixer->chip, 2, 19); + return 0; +} + int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer) { int err = 0; @@ -1884,11 +1904,25 @@ int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer) case USB_ID(0x041e, 0x323b): /* Creative Sound Blaster E1 */ err = snd_soundblaster_e1_switch_create(mixer); break; + case USB_ID(0x0bda, 0x4014): /* Dell WD15 dock */ + err = dell_dock_mixer_init(mixer); + break; }
return err; }
+#ifdef CONFIG_PM +void snd_usb_mixer_resume_quirk(struct usb_mixer_interface *mixer) +{ + switch (mixer->chip->usb_id) { + case USB_ID(0x0bda, 0x4014): /* Dell WD15 dock */ + dell_dock_mixer_init(mixer); + break; + } +} +#endif + void snd_usb_mixer_rc_memory_change(struct usb_mixer_interface *mixer, int unitid) { diff --git a/sound/usb/mixer_quirks.h b/sound/usb/mixer_quirks.h index b5abd328a361..52be26db558f 100644 --- a/sound/usb/mixer_quirks.h +++ b/sound/usb/mixer_quirks.h @@ -14,5 +14,9 @@ void snd_usb_mixer_fu_apply_quirk(struct usb_mixer_interface *mixer, struct usb_mixer_elem_info *cval, int unitid, struct snd_kcontrol *kctl);
+#ifdef CONFIG_PM +void snd_usb_mixer_resume_quirk(struct usb_mixer_interface *mixer); +#endif + #endif /* SND_USB_MIXER_QUIRKS_H */
This is a preliminary change for the upcoming quirk implementation.
Currently USB-audio driver tries to call usb_set_interface() whenever the format change with interface/altset modification happens. In this patch, the check is replaced with the comparison of cur_altsetting and the targeted altsetting pointer, so that the driver may skip the unnecessary function calls.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/pcm.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index ad39b3cca247..ae7d8a0a0a0a 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -499,7 +499,7 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) iface = usb_ifnum_to_if(dev, fmt->iface); if (WARN_ON(!iface)) return -EINVAL; - alts = &iface->altsetting[fmt->altset_idx]; + alts = usb_altnum_to_altsetting(iface, fmt->altsetting); altsd = get_iface_desc(alts); if (WARN_ON(altsd->bAlternateSetting != fmt->altsetting)) return -EINVAL; @@ -521,9 +521,7 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) }
/* set interface */ - if (subs->interface != fmt->iface || - subs->altset_idx != fmt->altset_idx) { - + if (iface->cur_altsetting != alts) { err = snd_usb_select_mode_quirk(subs, fmt); if (err < 0) return -EIO; @@ -537,12 +535,11 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) } dev_dbg(&dev->dev, "setting usb interface %d:%d\n", fmt->iface, fmt->altsetting); - subs->interface = fmt->iface; - subs->altset_idx = fmt->altset_idx; - snd_usb_set_interface_quirk(dev); }
+ subs->interface = fmt->iface; + subs->altset_idx = fmt->altset_idx; subs->data_endpoint = snd_usb_add_endpoint(subs->stream->chip, alts, fmt->endpoint, subs->direction, SND_USB_ENDPOINT_TYPE_DATA);
Introduce a new flag to struct snd_usb_audio for allowing the device to skip usb_set_interface() calls at changing or closing the stream. As of this patch, the flag is nowhere set, so it's just a place holder. The dynamic switching will be added in the following patch.
A background information for this change:
Dell WD15 dock with Realtek chip gives a very long pause at each time the driver changes the altset, which eventually happens at every PCM stream open/close and parameter change. As the long pause happens in each usb_set_interface() call, there is nothing we can do as long as it's called. The workaround is to reduce calling it as much as possible, and this flag indicates that behavior.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/pcm.c | 17 ++++++++++------- sound/usb/usbaudio.h | 3 +++ 2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index ae7d8a0a0a0a..dc2dfec9effd 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -509,12 +509,14 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
/* close the old interface */ if (subs->interface >= 0 && subs->interface != fmt->iface) { - err = usb_set_interface(subs->dev, subs->interface, 0); - if (err < 0) { - dev_err(&dev->dev, - "%d:%d: return to setting 0 failed (%d)\n", - fmt->iface, fmt->altsetting, err); - return -EIO; + if (!subs->stream->chip->keep_iface) { + err = usb_set_interface(subs->dev, subs->interface, 0); + if (err < 0) { + dev_err(&dev->dev, + "%d:%d: return to setting 0 failed (%d)\n", + fmt->iface, fmt->altsetting, err); + return -EIO; + } } subs->interface = -1; subs->altset_idx = 0; @@ -1253,7 +1255,8 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream, int direction)
stop_endpoints(subs, true);
- if (subs->interface >= 0 && + if (!as->chip->keep_iface && + subs->interface >= 0 && !snd_usb_lock_shutdown(subs->stream->chip)) { usb_set_interface(subs->dev, subs->interface, 0); subs->interface = -1; diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index 4d5c89a7ba2b..32f4a5425536 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -59,6 +59,9 @@ struct snd_usb_audio {
int setup; /* from the 'device_setup' module param */ bool autoclock; /* from the 'autoclock' module param */ + bool keep_iface; /* keep interface/altset after closing + * or parameter change + */
struct usb_host_interface *ctrl_intf; /* the audio control interface */ };
This patch adds "Keep Interface" control for each USB-audio device. The control element is with SND_CTL_IFACE_CARD, so that it won't appear on any sane mixer applications. For a device that is confirmed to work well with "keep-interface" mode, user can flip the control via amixer, e.g. % amixer -c1 cset iface=CARD,name='Keep Interface' on
and save/restore the state via alsactl.
The reason to provide this via control API is that the behavior must be pretty depending on the device (and the firmware in it), so it's not ideal to apply via module option.
For a device that certainly works, we may set it statically via a quirk table entry. But a device like Dell WD15 dock behaves so differently depending on the firmware, and we can't set it statically. So leave this as a dynamic switch each user can adjust freely.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/mixer.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 76fabc4b72b5..bb203b3684fc 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -2801,6 +2801,48 @@ static int snd_usb_mixer_status_create(struct usb_mixer_interface *mixer) return 0; }
+static int keep_iface_ctl_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct usb_mixer_interface *mixer = snd_kcontrol_chip(kcontrol); + + ucontrol->value.integer.value[0] = mixer->chip->keep_iface; + return 0; +} + +static int keep_iface_ctl_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct usb_mixer_interface *mixer = snd_kcontrol_chip(kcontrol); + bool keep_iface = !!ucontrol->value.integer.value[0]; + + if (mixer->chip->keep_iface == keep_iface) + return 0; + mixer->chip->keep_iface = keep_iface; + return 1; +} + +static const struct snd_kcontrol_new keep_iface_ctl = { + .iface = SNDRV_CTL_ELEM_IFACE_CARD, + .name = "Keep Interface", + .info = snd_ctl_boolean_mono_info, + .get = keep_iface_ctl_get, + .put = keep_iface_ctl_put, +}; + +static int create_keep_iface_ctl(struct usb_mixer_interface *mixer) +{ + struct snd_kcontrol *kctl = snd_ctl_new1(&keep_iface_ctl, mixer); + + /* need only one control per card */ + if (snd_ctl_find_id(mixer->chip->card, &kctl->id)) { + snd_ctl_free_one(kctl); + return 0; + } + + return snd_ctl_add(mixer->chip->card, kctl); +} + int snd_usb_create_mixer(struct snd_usb_audio *chip, int ctrlif, int ignore_error) { @@ -2842,6 +2884,9 @@ int snd_usb_create_mixer(struct snd_usb_audio *chip, int ctrlif, if ((err = snd_usb_mixer_controls(mixer)) < 0 || (err = snd_usb_mixer_status_create(mixer)) < 0) goto _error; + err = create_keep_iface_ctl(mixer); + if (err < 0) + goto _error;
snd_usb_mixer_apply_create_quirk(mixer);
Historically USB-audio driver sets the card's longname field with the details of the device and the bus information. It's good per se, but not preferable when it's referred as the identifier for UCM profile.
This patch adds a quirk profile_name field to override the card's longname string to a pre-defined one, so that one can create a unique and consistent ID string for the specific USB device via a quirk table to be used as a UCM profile name.
The patch does a slight code refactoring to split out the functions to set shortname and longname fields as well.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/card.c | 146 ++++++++++++++++++++++++++++++--------------------- sound/usb/usbaudio.h | 1 + 2 files changed, 88 insertions(+), 59 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c index 4a1c6bb3dfa0..36c289bae169 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -349,6 +349,90 @@ static int snd_usb_audio_dev_free(struct snd_device *device) return snd_usb_audio_free(chip); }
+static void usb_audio_make_shortname(struct usb_device *dev, + struct snd_usb_audio *chip, + const struct snd_usb_audio_quirk *quirk) +{ + struct snd_card *card = chip->card; + + if (quirk && quirk->product_name && *quirk->product_name) { + strlcpy(card->shortname, quirk->product_name, + sizeof(card->shortname)); + return; + } + + /* retrieve the device string as shortname */ + if (!dev->descriptor.iProduct || + usb_string(dev, dev->descriptor.iProduct, + card->shortname, sizeof(card->shortname)) <= 0) { + /* no name available from anywhere, so use ID */ + sprintf(card->shortname, "USB Device %#04x:%#04x", + USB_ID_VENDOR(chip->usb_id), + USB_ID_PRODUCT(chip->usb_id)); + } + + strim(card->shortname); +} + +static void usb_audio_make_longname(struct usb_device *dev, + struct snd_usb_audio *chip, + const struct snd_usb_audio_quirk *quirk) +{ + struct snd_card *card = chip->card; + int len; + + /* shortcut - if any pre-defined string is given, use it */ + if (quirk && quirk->profile_name && *quirk->profile_name) { + strlcpy(card->longname, quirk->profile_name, + sizeof(card->longname)); + return; + } + + if (quirk && quirk->vendor_name && *quirk->vendor_name) { + len = strlcpy(card->longname, quirk->vendor_name, sizeof(card->longname)); + } else { + /* retrieve the vendor and device strings as longname */ + if (dev->descriptor.iManufacturer) + len = usb_string(dev, dev->descriptor.iManufacturer, + card->longname, sizeof(card->longname)); + else + len = 0; + /* we don't really care if there isn't any vendor string */ + } + if (len > 0) { + strim(card->longname); + if (*card->longname) + strlcat(card->longname, " ", sizeof(card->longname)); + } + + strlcat(card->longname, card->shortname, sizeof(card->longname)); + + len = strlcat(card->longname, " at ", sizeof(card->longname)); + + if (len < sizeof(card->longname)) + usb_make_path(dev, card->longname + len, sizeof(card->longname) - len); + + switch (snd_usb_get_speed(dev)) { + case USB_SPEED_LOW: + strlcat(card->longname, ", low speed", sizeof(card->longname)); + break; + case USB_SPEED_FULL: + strlcat(card->longname, ", full speed", sizeof(card->longname)); + break; + case USB_SPEED_HIGH: + strlcat(card->longname, ", high speed", sizeof(card->longname)); + break; + case USB_SPEED_SUPER: + strlcat(card->longname, ", super speed", sizeof(card->longname)); + break; + case USB_SPEED_SUPER_PLUS: + strlcat(card->longname, ", super speed plus", sizeof(card->longname)); + break; + default: + break; + } +} + /* * create a chip instance and set its names. */ @@ -360,7 +444,7 @@ static int snd_usb_audio_create(struct usb_interface *intf, { struct snd_card *card; struct snd_usb_audio *chip; - int err, len; + int err; char component[14]; static struct snd_device_ops ops = { .dev_free = snd_usb_audio_dev_free, @@ -422,64 +506,8 @@ static int snd_usb_audio_create(struct usb_interface *intf, USB_ID_VENDOR(chip->usb_id), USB_ID_PRODUCT(chip->usb_id)); snd_component_add(card, component);
- /* retrieve the device string as shortname */ - if (quirk && quirk->product_name && *quirk->product_name) { - strlcpy(card->shortname, quirk->product_name, sizeof(card->shortname)); - } else { - if (!dev->descriptor.iProduct || - usb_string(dev, dev->descriptor.iProduct, - card->shortname, sizeof(card->shortname)) <= 0) { - /* no name available from anywhere, so use ID */ - sprintf(card->shortname, "USB Device %#04x:%#04x", - USB_ID_VENDOR(chip->usb_id), - USB_ID_PRODUCT(chip->usb_id)); - } - } - strim(card->shortname); - - /* retrieve the vendor and device strings as longname */ - if (quirk && quirk->vendor_name && *quirk->vendor_name) { - len = strlcpy(card->longname, quirk->vendor_name, sizeof(card->longname)); - } else { - if (dev->descriptor.iManufacturer) - len = usb_string(dev, dev->descriptor.iManufacturer, - card->longname, sizeof(card->longname)); - else - len = 0; - /* we don't really care if there isn't any vendor string */ - } - if (len > 0) { - strim(card->longname); - if (*card->longname) - strlcat(card->longname, " ", sizeof(card->longname)); - } - - strlcat(card->longname, card->shortname, sizeof(card->longname)); - - len = strlcat(card->longname, " at ", sizeof(card->longname)); - - if (len < sizeof(card->longname)) - usb_make_path(dev, card->longname + len, sizeof(card->longname) - len); - - switch (snd_usb_get_speed(dev)) { - case USB_SPEED_LOW: - strlcat(card->longname, ", low speed", sizeof(card->longname)); - break; - case USB_SPEED_FULL: - strlcat(card->longname, ", full speed", sizeof(card->longname)); - break; - case USB_SPEED_HIGH: - strlcat(card->longname, ", high speed", sizeof(card->longname)); - break; - case USB_SPEED_SUPER: - strlcat(card->longname, ", super speed", sizeof(card->longname)); - break; - case USB_SPEED_SUPER_PLUS: - strlcat(card->longname, ", super speed plus", sizeof(card->longname)); - break; - default: - break; - } + usb_audio_make_shortname(dev, chip, quirk); + usb_audio_make_longname(dev, chip, quirk);
snd_usb_audio_create_proc(chip);
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index 32f4a5425536..1cb6b3e9483c 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -112,6 +112,7 @@ enum quirk_type { struct snd_usb_audio_quirk { const char *vendor_name; const char *product_name; + const char *profile_name; /* override the card->longname */ int16_t ifnum; uint16_t type; const void *data;
Dell WD15 Dock with 0bda:4014 doesn't give any useful strings for the vendor and the product names. Name them more specifically via quirk, as well as the UCM profile name.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/quirks-table.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h index 754e632a27bd..0e37e358ca97 100644 --- a/sound/usb/quirks-table.h +++ b/sound/usb/quirks-table.h @@ -3371,5 +3371,15 @@ AU0828_DEVICE(0x2040, 0x7270, "Hauppauge", "HVR-950Q"), } } }, +/* Dell WD15 Dock */ +{ + USB_DEVICE(0x0bda, 0x4014), + .driver_info = (unsigned long) & (const struct snd_usb_audio_quirk) { + .vendor_name = "Dell", + .product_name = "WD15 Dock", + .profile_name = "Dell-WD15-Dock", + .ifnum = QUIRK_NO_INTERFACE + } +},
#undef USB_DEVICE_VENDOR_SPEC
participants (1)
-
Takashi Iwai