Re: [PATCH] ALSA: usb-audio: Apply implicit feedback mode for BOSS devices

[Takashi Iwai <tiwai@suse.de>]

On Tue, 20 Apr 2021 17:42:39 +0200,
Lucas wrote:
> 
> Sorry, in my haste, I mistakenly wrote:
> 
> > For me, silence due to the wrong line takes the form of unheard output
> > playback, while still working for input capture, so I'm not sure this has
> > any bearing on your issue.
> >
> Output playback silence occurred in one device (Roland INTEGRA-7 on higher
> sample rates) which had the line IMPLICIT_FB_FIXED_DEV and needed
> IMPLICIT_FB_BOTH_DEV.  So it's the opposite situation.  Capture issues,
> such as yours, with arecord were exactly what I saw when a device used
> IMPLICIT_FB_BOTH_DEV while needing IMPLICIT_FB_FIXED_DEV instead.
> 
> Here's one of my previous examples of a device not compatible with
> IMPLICIT_FB_BOTH_DEV:
> 
> > Roland VG-99 doesn't capture, but plays well:
> > arecord -D hw:VG99 -f S24_3LE -r 44100 -c 2 ./file.wav
> > Recording WAVE './file.wav' : Signed 24 bit Little Endian in 3bytes, Rate
> > 44100 Hz, Stereo
> > arecord: xrun:1672: read/write error, state = PREPARED
> >
> > aplay -D hw:VG99 -f S24_3LE -r 44100 -c 2 ./other-file.wav
> > Playing WAVE './other-file.wav' : Signed 24 bit Little Endian in 3bytes,
> > Rate 44100 Hz, Stereo
> >
> 
> I'm sorry if that caused confusion.  It does seem your issue could be the
> very same as the VG-99's above.  That is, a device without "Asynchronous"
> "Synch Type" for either of its "IN" or "OUT" endpoints.  In that case,
> IMPLICIT_FB_FIXED_DEV should work.

I checked the patch again, and found possibly incorrect setups in the
quirk entries.  Namely, the added entries have all EP=0x0d iface=0x01,
and those don't match with the actual device configurations, judging
from the lsusb outputs you pasted in
  https://bugzilla.kernel.org/show_bug.cgi?id=212519

So, the incorrect hook was applied to the capture stream, I suppose.

Could you try the patch below on top of the latest for-next branch?
This should apply the implicit feedback mode for the playback if both
streams are with ASYNC, while it applies the capture quirk as long as
it matches with the Roland ID and the descriptor pattern (playback,
then capture interface).


thanks,

Takashi

---
--- a/sound/usb/implicit.c
+++ b/sound/usb/implicit.c
@@ -79,6 +79,7 @@ static const struct snd_usb_implicit_fb_match playback_implicit_fb_quirks[] = {
 
 /* Implicit feedback quirk table for capture: only FIXED type */
 static const struct snd_usb_implicit_fb_match capture_implicit_fb_quirks[] = {
+#if 0
 	IMPLICIT_FB_FIXED_DEV(0x0582, 0x00a6, 0x0d, 0x01), /* Roland JUNO-G */
 	IMPLICIT_FB_FIXED_DEV(0x0582, 0x00a9, 0x0d, 0x01), /* Roland MC-808 */
 	IMPLICIT_FB_FIXED_DEV(0x0582, 0x00ad, 0x0d, 0x01), /* Roland SH-201 */
@@ -146,6 +147,7 @@ static const struct snd_usb_implicit_fb_match capture_implicit_fb_quirks[] = {
 	IMPLICIT_FB_BOTH_DEV(0x0582, 0x01fd, 0x0d, 0x01), /* Roland Boutique SH-01A */
 	IMPLICIT_FB_BOTH_DEV(0x0582, 0x01ff, 0x0d, 0x01), /* Roland Boutique D-05 */
 	IMPLICIT_FB_BOTH_DEV(0x0582, 0x0203, 0x0d, 0x01), /* BOSS AD-10 */
+#endif
 
 	{} /* terminator */
 };
@@ -223,8 +225,31 @@ static int add_roland_implicit_fb(struct snd_usb_audio *chip,
 		return 0;
 	epd = get_endpoint(alts, 0);
 	if (!usb_endpoint_is_isoc_in(epd) ||
-	    (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
-					USB_ENDPOINT_USAGE_IMPLICIT_FB)
+	    (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) != USB_ENDPOINT_SYNC_ASYNC)
+		return 0;
+	chip->playback_first = 1;
+	return add_implicit_fb_sync_ep(chip, fmt, epd->bEndpointAddress, 0,
+				       ifnum, alts);
+}
+
+static int add_roland_capture_quirk(struct snd_usb_audio *chip,
+				    struct audioformat *fmt,
+				    unsigned int ifnum,
+				    unsigned int altsetting)
+{
+	struct usb_host_interface *alts;
+	struct usb_endpoint_descriptor *epd;
+
+	alts = snd_usb_get_host_interface(chip, ifnum, altsetting);
+	if (!alts)
+		return 0;
+	if (alts->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC ||
+	    (alts->desc.bInterfaceSubClass != 2 &&
+	     alts->desc.bInterfaceProtocol != 2) ||
+	    alts->desc.bNumEndpoints < 1)
+		return 0;
+	epd = get_endpoint(alts, 0);
+	if (!usb_endpoint_is_isoc_out(epd))
 		return 0;
 	return add_implicit_fb_sync_ep(chip, fmt, epd->bEndpointAddress, 0,
 				       ifnum, alts);
@@ -404,6 +429,18 @@ static int audioformat_capture_quirk(struct snd_usb_audio *chip,
 	if (p && (p->type == IMPLICIT_FB_FIXED || p->type == IMPLICIT_FB_BOTH))
 		return add_implicit_fb_sync_ep(chip, fmt, p->ep_num, 0,
 					       p->iface, NULL);
+
+	/* Roland/BOSS need full-duplex streams */
+	if (alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
+	    alts->desc.bInterfaceProtocol == 2 &&
+	    alts->desc.bNumEndpoints == 1 &&
+	    USB_ID_VENDOR(chip->usb_id) == 0x0582 /* Roland */) {
+		if (add_roland_capture_quirk(chip, fmt,
+					     alts->desc.bInterfaceNumber - 1,
+					     alts->desc.bAlternateSetting))
+			return 1;
+	}
+
 	if (is_pioneer_implicit_fb(chip, alts))
 		return 1; /* skip the quirk, also don't handle generic sync EP */
 	return 0;