[PATCH 0/6] usb-audio: quirks for Dell WD15 dock and some extensions

[PATCH 0/6] usb-audio: quirks for Dell WD15 dock and some extensions

[Takashi Iwai]

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(-)

-- 
2.16.3

[PATCH 1/6] ALSA: usb-audio: Initialize Dell Dock playback volumes

[Takashi Iwai]

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 */
 
-- 
2.16.3

[PATCH 2/6] ALSA: usb-audio: Avoid superfluous usb_set_interface() calls

[Takashi Iwai]

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

[PATCH 3/6] ALSA: usb-audio: Add keep_iface flag

[Takashi Iwai]

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 */
 };
-- 
2.16.3

[PATCH 4/6] ALSA: usb-audio: Add "Keep Interface" control

[Takashi Iwai]

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

[PATCH 5/6] ALSA: usb-audio: Allow to override the longname string

[Takashi Iwai]

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;
-- 
2.16.3

[PATCH 6/6] ALSA: usb-audio: Give proper vendor/product name for Dell WD15 Dock

[Takashi Iwai]

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
-- 
2.16.3