| Message ID | 20210414083255.9527-1-tiwai@suse.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ebe8dc5afb3912e2d4f5c62cf7c492a13143a77a |
| Headers | show |
| Series | ALSA: usb-audio: Apply implicit feedback mode for BOSS devices | expand |
Thanks for your help with this, Takashi. I have tested this patch on BOSS GT-1 and BOSS Katana, so we have a fairly high degree of confidence that it will work on the other BOSS devices. It may well help with other similar Roland devices. If your device has playback working, but with occasional pops/clicks in the audio stream, it is worth trying to add the IMPLICIT_FB_BOTH_DEV quirk for the device. Mike On Wed, Apr 14, 2021 at 1:33 AM Takashi Iwai <tiwai@suse.de> wrote: > During the recent rewrite of the implicit feedback support, we've > tested to apply the implicit fb on BOSS devices, but it failed, as the > capture stream didn't start without the playback. As the end result, > it got another type of quirk for tying both streams but starts > playback always (commit 6234fdc1cede "ALSA: usb-audio: Quirk for BOSS > GT-001"). > > Meanwhile, Mike Oliphant has tested the real implicit feedback mode > for the playback again with the latest code, and found out that it > actually works if the initial feedback sync is skipped; that is, on > those BOSS devices, the playback stream has to be started at first > without waiting for the capture URB completions. Otherwise it gets > stuck. In the rest operations after the capture stream processed, we > can take them as the implicit feedback source. > > This patch is an attempt to improve the support for BOSS devices with > the implicit feedback mode in the way described above. It adds a new > flag to snd_usb_audio, playback_first, indicating that the playback > stream starts without sync with the initial capture completion. This > flag is set in the quirk table with the new IMPLICIT_FB_BOTH type. > > Reported-and-tested-by: Mike Oliphant <oliphant@nostatic.org> > Signed-off-by: Takashi Iwai <tiwai@suse.de> > --- > sound/usb/endpoint.c | 3 ++- > sound/usb/implicit.c | 35 +++++++++++++++++++++++------------ > sound/usb/usbaudio.h | 1 + > 3 files changed, 26 insertions(+), 13 deletions(-) > > diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c > index 102d53515a76..f4c3d2b38abb 100644 > --- a/sound/usb/endpoint.c > +++ b/sound/usb/endpoint.c > @@ -1375,7 +1375,8 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint > *ep) > if (!ep_state_update(ep, EP_STATE_STOPPED, EP_STATE_RUNNING)) > goto __error; > > - if (snd_usb_endpoint_implicit_feedback_sink(ep)) { > + if (snd_usb_endpoint_implicit_feedback_sink(ep) && > + !ep->chip->playback_first) { > for (i = 0; i < ep->nurbs; i++) { > struct snd_urb_ctx *ctx = ep->urb + i; > list_add_tail(&ctx->ready_list, > &ep->ready_playback_urbs); > diff --git a/sound/usb/implicit.c b/sound/usb/implicit.c > index 19622c58b72b..4bd9c105a529 100644 > --- a/sound/usb/implicit.c > +++ b/sound/usb/implicit.c > @@ -21,6 +21,7 @@ enum { > IMPLICIT_FB_NONE, > IMPLICIT_FB_GENERIC, > IMPLICIT_FB_FIXED, > + IMPLICIT_FB_BOTH, /* generic playback + capture (for BOSS) */ > }; > > struct snd_usb_implicit_fb_match { > @@ -36,6 +37,9 @@ struct snd_usb_implicit_fb_match { > #define IMPLICIT_FB_FIXED_DEV(vend, prod, ep, ifnum) \ > { .id = USB_ID(vend, prod), .type = IMPLICIT_FB_FIXED, .ep_num = > (ep),\ > .iface = (ifnum) } > +#define IMPLICIT_FB_BOTH_DEV(vend, prod, ep, ifnum) \ > + { .id = USB_ID(vend, prod), .type = IMPLICIT_FB_BOTH, .ep_num = > (ep),\ > + .iface = (ifnum) } > #define IMPLICIT_FB_SKIP_DEV(vend, prod) \ > { .id = USB_ID(vend, prod), .type = IMPLICIT_FB_NONE } > > @@ -75,14 +79,14 @@ 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[] = { > - IMPLICIT_FB_FIXED_DEV(0x0582, 0x0130, 0x0d, 0x01), /* BOSS BR-80 */ > - IMPLICIT_FB_FIXED_DEV(0x0582, 0x0171, 0x0d, 0x01), /* BOSS RC-505 > */ > - IMPLICIT_FB_FIXED_DEV(0x0582, 0x0185, 0x0d, 0x01), /* BOSS GP-10 */ > - IMPLICIT_FB_FIXED_DEV(0x0582, 0x0189, 0x0d, 0x01), /* BOSS > GT-100v2 */ > - IMPLICIT_FB_FIXED_DEV(0x0582, 0x01d6, 0x0d, 0x01), /* BOSS GT-1 */ > - IMPLICIT_FB_FIXED_DEV(0x0582, 0x01d8, 0x0d, 0x01), /* BOSS Katana > */ > - IMPLICIT_FB_FIXED_DEV(0x0582, 0x01e5, 0x0d, 0x01), /* BOSS GT-001 > */ > - IMPLICIT_FB_FIXED_DEV(0x0582, 0x0203, 0x0d, 0x01), /* BOSS AD-10 */ > + IMPLICIT_FB_BOTH_DEV(0x0582, 0x0130, 0x0d, 0x01), /* BOSS BR-80 */ > + IMPLICIT_FB_BOTH_DEV(0x0582, 0x0171, 0x0d, 0x01), /* BOSS RC-505 */ > + IMPLICIT_FB_BOTH_DEV(0x0582, 0x0185, 0x0d, 0x01), /* BOSS GP-10 */ > + IMPLICIT_FB_BOTH_DEV(0x0582, 0x0189, 0x0d, 0x01), /* BOSS GT-100v2 > */ > + IMPLICIT_FB_BOTH_DEV(0x0582, 0x01d6, 0x0d, 0x01), /* BOSS GT-1 */ > + IMPLICIT_FB_BOTH_DEV(0x0582, 0x01d8, 0x0d, 0x01), /* BOSS Katana */ > + IMPLICIT_FB_BOTH_DEV(0x0582, 0x01e5, 0x0d, 0x01), /* BOSS GT-001 */ > + IMPLICIT_FB_BOTH_DEV(0x0582, 0x0203, 0x0d, 0x01), /* BOSS AD-10 */ > > {} /* terminator */ > }; > @@ -268,10 +272,17 @@ static int audioformat_implicit_fb_quirk(struct > snd_usb_audio *chip, > } > } > > - /* Don't apply playback quirks for the devices with capture quirk > */ > + /* Special handling for devices with capture quirks */ > p = find_implicit_fb_entry(chip, capture_implicit_fb_quirks, alts); > - if (p && p->type == IMPLICIT_FB_FIXED) > - return 0; /* no quirk */ > + if (p) { > + switch (p->type) { > + case IMPLICIT_FB_FIXED: > + return 0; /* no quirk */ > + case IMPLICIT_FB_BOTH: > + chip->playback_first = 1; > + return add_generic_implicit_fb(chip, fmt, alts); > + } > + } > > /* Generic UAC2 implicit feedback */ > if (attr == USB_ENDPOINT_SYNC_ASYNC && > @@ -321,7 +332,7 @@ static int audioformat_capture_quirk(struct > snd_usb_audio *chip, > const struct snd_usb_implicit_fb_match *p; > > p = find_implicit_fb_entry(chip, capture_implicit_fb_quirks, alts); > - if (p && p->type == IMPLICIT_FB_FIXED) > + 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); > return 0; > diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h > index a536ee33d36e..538831cbd925 100644 > --- a/sound/usb/usbaudio.h > +++ b/sound/usb/usbaudio.h > @@ -37,6 +37,7 @@ struct snd_usb_audio { > unsigned int txfr_quirk:1; /* Subframe boundaries on transfers */ > unsigned int tx_length_quirk:1; /* Put length specifier in > transfers */ > unsigned int need_delayed_register:1; /* warn for delayed > registration */ > + unsigned int playback_first:1; /* for implicit fb: don't wait for > the first capture URBs */ > int num_interfaces; > int num_suspended_intf; > int sample_rate_read_error; > -- > 2.26.2 > >
On Wednesday, 14 April 2021 16:08:54 BST Mike Oliphant wrote: > Thanks for your help with this, Takashi. > > I have tested this patch on BOSS GT-1 and BOSS Katana, so we have a fairly > high degree of confidence that it will work on the other BOSS devices. > > It may well help with other similar Roland devices. If your device has > playback working, but with occasional pops/clicks in the audio stream, it > is worth trying to add the IMPLICIT_FB_BOTH_DEV quirk for the device. > I have confirmed that it works with the GT-1 and Boss Katana as well. I have also tested with the Boss BR-80 and GT-001 and this still seems to work well for me, so no obvious issues have arisen from making this change. I did also try the Boss JS-8. This works in Ardour, but capture from command line using arecord results in a file of silence. I'll do some more digging into this one. Regards, Keith
Keith, if it's anything like the Roland devices I've tested with this long-awaited patch, could you show the JS-8's "lsusb -v"? 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. Takashi Iwai has indicated a likely connection between needing the IMPLICIT_FB_BOTH_DEVICE line if there's "Asynchronous" "Synch Type" for both input and output endpoints. Otherwise, possibly, it should be reverted to IMPLICIT_FB_FIXED_DEVICE for the JS-8. I did check my Roland devices with jack, but only after they were all working with standard arecord and aplay. Thanks, Lucas On Mon, Apr 19, 2021 at 9:13 AM Keith A. Milner <maillist@superlative.org> wrote: > On Wednesday, 14 April 2021 16:08:54 BST Mike Oliphant wrote: > > Thanks for your help with this, Takashi. > > > > I have tested this patch on BOSS GT-1 and BOSS Katana, so we have a > fairly > > high degree of confidence that it will work on the other BOSS devices. > > > > It may well help with other similar Roland devices. If your device has > > playback working, but with occasional pops/clicks in the audio stream, it > > is worth trying to add the IMPLICIT_FB_BOTH_DEV quirk for the device. > > > > I have confirmed that it works with the GT-1 and Boss Katana as well. > > I have also tested with the Boss BR-80 and GT-001 and this still seems to > work > well for me, so no obvious issues have arisen from making this change. > > I did also try the Boss JS-8. This works in Ardour, but capture from > command > line using arecord results in a file of silence. I'll do some more digging > into > this one. > > Regards, > > Keith > > > > >
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. Thanks, and sorry, Lucas
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;
First, thanks very much for trying to cover all devices through detection! I had hoped something like this could be done, but sadly, it has created a mixed result: Roland VG-99 Perfect!: 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 ^CAborted by signal Interrupt... aplay -D hw:VG99 -f S24_3LE -r 44100 -c 2 ./file.wav Playing WAVE './file.wav' : Signed 24 bit Little Endian in 3bytes, Rate 44100 Hz, Stereo Roland INTEGRA-7 doesn't capture, but plays perfectly (only 96 kHz mode tested): arecord -D hw:INTEGRA7 -f S32_LE -r 96000 -c 2 ./file.wav Recording WAVE './file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, Stereo arecord: pcm_read:2153: read error: Input/output error aplay -D hw:INTEGRA7 -f S32_LE -r 96000 -c 2 ./other-file.wav Playing WAVE './other-file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, Stereo Roland R-26 doesn't capture, but plays perfectly (only 96 kHz mode tested): arecord -D hw:R26AUDIO -f S32_LE -r 96000 -c 2 ./file.wav Recording WAVE './file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, Stereo arecord: pcm_read:2153: read error: Input/output error aplay -D hw:R26AUDIO -f S32_LE -r 96000 -c 2 ./other-file.wav Playing WAVE './other-file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, Stereo Roland Boutique D-05 doesn't capture, but plays perfectly: arecord -D hw:Boutique -f S32_LE -r 96000 -c 2 ./file.wav Recording WAVE './file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, Stereo arecord: pcm_read:2153: read error: Input/output error aplay -D hw:Boutique -f S32_LE -r 96000 -c 2 ./other-file.wav Playing WAVE './other-file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, Stereo EDIROL UA-4FX Perfect! (only tested 48 kHz mode): arecord -D hw:UA4FX -f S24_3LE -r 48000 -c 2 ./file.wav Recording WAVE './file.wav' : Signed 24 bit Little Endian in 3bytes, Rate 48000 Hz, Stereo ^CAborted by signal Interrupt... aplay -D hw:UA4FX -f S24_3LE -r 48000 -c 2 ./file.wav Playing WAVE './file.wav' : Signed 24 bit Little Endian in 3bytes, Rate 48000 Hz, Stereo EDIROL UA-25EX Perfect! (only tested 48 kHz mode): arecord -D hw:UA25EX -f S24_3LE -r 48000 -c 2 ./file.wav Recording WAVE './file.wav' : Signed 24 bit Little Endian in 3bytes, Rate 48000 Hz, Stereo ^CAborted by signal Interrupt... aplay -D hw:UA25EX -f S24_3LE -r 48000 -c 2 ./file.wav Playing WAVE './file.wav' : Signed 24 bit Little Endian in 3bytes, Rate 48000 Hz, Stereo EDIROL UA-101 full-speed (USB 1.1) and high-speed (USB 2.0) not detected for capture or playback (only 48 kHz mode tested): arecord -D hw:UA101 -f S32_LE -r 48000 -c 2 ./file.wav ALSA lib pcm_hw.c:1829:(_snd_pcm_hw_open) Invalid value for card arecord: main:830: audio open error: No such device aplay -D hw:UA101 -f S32_LE -r 48000 -c 2 ./other-file.wav ALSA lib pcm_hw.c:1829:(_snd_pcm_hw_open) Invalid value for card aplay: main:830: audio open error: No such device I'm guessing another look at "lsusb -v" would help, but don't know what to look for and have run out of time tonight. Thanks again Takashi, Lucas
On Wed, 21 Apr 2021 06:59:51 +0200, Lucas wrote: > > First, thanks very much for trying to cover all devices through detection! I > had hoped something like this could be done, but sadly, it has created a mixed > result: > > Roland VG-99 Perfect!: > 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 > ^CAborted by signal Interrupt... > > aplay -D hw:VG99 -f S24_3LE -r 44100 -c 2 ./file.wav > Playing WAVE './file.wav' : Signed 24 bit Little Endian in 3bytes, Rate 44100 > Hz, Stereo > > Roland INTEGRA-7 doesn't capture, but plays perfectly (only 96 kHz mode > tested): > arecord -D hw:INTEGRA7 -f S32_LE -r 96000 -c 2 ./file.wav > Recording WAVE './file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, > Stereo > arecord: pcm_read:2153: read error: Input/output error > > aplay -D hw:INTEGRA7 -f S32_LE -r 96000 -c 2 ./other-file.wav > Playing WAVE './other-file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, > Stereo Hm, that's bad. And INTEGRA7 did work before the patching? I saw that this has the right EP numbers (0x0d and 0x8e), so the static quirk entry was correct. > > Roland R-26 doesn't capture, but plays perfectly (only 96 kHz mode tested): R-26 seems also to be the one with both ASYNC and EP 0x0d/0x8e, so the same reason as INTEGRA7. > arecord -D hw:R26AUDIO -f S32_LE -r 96000 -c 2 ./file.wav > Recording WAVE './file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, > Stereo > arecord: pcm_read:2153: read error: Input/output error > > aplay -D hw:R26AUDIO -f S32_LE -r 96000 -c 2 ./other-file.wav > Playing WAVE './other-file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, > Stereo > > Roland Boutique D-05 doesn't capture, but plays perfectly: OK, this one looks slightly different. D-05 has two EPs in each interface, both are ASYNC and 0x0d/0x8e. So another adjustment might be needed here. > EDIROL UA-101 full-speed (USB 1.1) and high-speed (USB 2.0) not detected for > capture or playback (only 48 kHz mode tested): This is a different driver, so we can forget for now. > I'm guessing another look at "lsusb -v" would help, but don't know what to > look for and have run out of time tonight. Could you post the archive of your lsusb -v outputs? Put in a tarball and post with attachment. Below is a revised patch. Let me know if this works better. 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 */ }; @@ -204,30 +206,70 @@ static int add_generic_uac2_implicit_fb(struct snd_usb_audio *chip, ifnum, alts); } -/* Like the function above, but specific to Roland with vendor class and hack */ +static bool roland_sanity_check_iface(struct usb_host_interface *alts) +{ + if (alts->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC || + (alts->desc.bInterfaceSubClass != 2 && + alts->desc.bInterfaceProtocol != 2) || + alts->desc.bNumEndpoints < 1) + return false; + return true; +} + +/* Like the UAC2 case above, but specific to Roland with vendor class and hack */ static int add_roland_implicit_fb(struct snd_usb_audio *chip, struct audioformat *fmt, - unsigned int ifnum, - unsigned int altsetting) + struct usb_host_interface *alts) { - struct usb_host_interface *alts; struct usb_endpoint_descriptor *epd; - alts = snd_usb_get_host_interface(chip, ifnum, altsetting); - if (!alts) + if (!roland_sanity_check_iface(alts)) return 0; - if (alts->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC || - (alts->desc.bInterfaceSubClass != 2 && - alts->desc.bInterfaceProtocol != 2) || - alts->desc.bNumEndpoints < 1) + /* only when both streams are with ASYNC type */ + epd = get_endpoint(alts, 0); + if (!usb_endpoint_is_isoc_out(epd) || + (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) != USB_ENDPOINT_SYNC_ASYNC) + return 0; + + /* check capture EP */ + alts = snd_usb_get_host_interface(chip, + alts->desc.bInterfaceNumber + 1, + alts->desc.bAlternateSetting); + if (!alts || !roland_sanity_check_iface(alts)) 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); + alts->desc.bInterfaceNumber, alts); +} + +/* capture quirk for Roland device; always full-duplex */ +static int add_roland_capture_quirk(struct snd_usb_audio *chip, + struct audioformat *fmt, + struct usb_host_interface *alts) +{ + struct usb_endpoint_descriptor *epd; + + if (!roland_sanity_check_iface(alts)) + return 0; + epd = get_endpoint(alts, 0); + if (!usb_endpoint_is_isoc_in(epd) || + (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) != USB_ENDPOINT_SYNC_ASYNC) + return 0; + + alts = snd_usb_get_host_interface(chip, + alts->desc.bInterfaceNumber - 1, + alts->desc.bAlternateSetting); + if (!alts || !roland_sanity_check_iface(alts)) + 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, + alts->desc.bInterfaceNumber, alts); } /* Playback and capture EPs on Pioneer devices share the same iface/altset @@ -365,14 +407,8 @@ static int audioformat_implicit_fb_quirk(struct snd_usb_audio *chip, } /* Roland/BOSS implicit feedback with vendor spec class */ - if (attr == USB_ENDPOINT_SYNC_ASYNC && - 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_implicit_fb(chip, fmt, - alts->desc.bInterfaceNumber + 1, - alts->desc.bAlternateSetting)) + if (USB_ID_VENDOR(chip->usb_id) == 0x0582) { + if (add_roland_implicit_fb(chip, fmt, alts) > 0) return 1; } @@ -404,6 +440,13 @@ 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 (USB_ID_VENDOR(chip->usb_id) == 0x0582) { + if (add_roland_capture_quirk(chip, fmt, alts) > 0) + return 1; + } + if (is_pioneer_implicit_fb(chip, alts)) return 1; /* skip the quirk, also don't handle generic sync EP */ return 0;
I'll have to try the revised patch later tonight when I have time, but for now, I'm attaching the requested lsusb -v outputs. The only one I left out was the UA-101, due to its other driver. Yes, with the previous IMPLICIT_FB_BOTH_DEV capture table entries, the INTEGRA-7, R-26, and Boutique D-05 all worked on their highest sample rates. I love that this probably will be working automatically for everything Roland/BOSS/EDIROL in the end! Thanks for your continued efforts Takashi!, Lucas On Wed, Apr 21, 2021 at 3:59 AM Takashi Iwai <tiwai@suse.de> wrote: > On Wed, 21 Apr 2021 06:59:51 +0200, > Lucas wrote: > > > > First, thanks very much for trying to cover all devices through > detection! I > > had hoped something like this could be done, but sadly, it has created a > mixed > > result: > > > > Roland VG-99 Perfect!: > > 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 > > ^CAborted by signal Interrupt... > > > > aplay -D hw:VG99 -f S24_3LE -r 44100 -c 2 ./file.wav > > Playing WAVE './file.wav' : Signed 24 bit Little Endian in 3bytes, Rate > 44100 > > Hz, Stereo > > > > Roland INTEGRA-7 doesn't capture, but plays perfectly (only 96 kHz mode > > tested): > > arecord -D hw:INTEGRA7 -f S32_LE -r 96000 -c 2 ./file.wav > > Recording WAVE './file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, > > Stereo > > arecord: pcm_read:2153: read error: Input/output error > > > > aplay -D hw:INTEGRA7 -f S32_LE -r 96000 -c 2 ./other-file.wav > > Playing WAVE './other-file.wav' : Signed 32 bit Little Endian, Rate > 96000 Hz, > > Stereo > > Hm, that's bad. And INTEGRA7 did work before the patching? > I saw that this has the right EP numbers (0x0d and 0x8e), so the > static quirk entry was correct. > > > > > Roland R-26 doesn't capture, but plays perfectly (only 96 kHz mode > tested): > > R-26 seems also to be the one with both ASYNC and EP 0x0d/0x8e, so the > same reason as INTEGRA7. > > > arecord -D hw:R26AUDIO -f S32_LE -r 96000 -c 2 ./file.wav > > Recording WAVE './file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, > > Stereo > > arecord: pcm_read:2153: read error: Input/output error > > > > aplay -D hw:R26AUDIO -f S32_LE -r 96000 -c 2 ./other-file.wav > > Playing WAVE './other-file.wav' : Signed 32 bit Little Endian, Rate > 96000 Hz, > > Stereo > > > > Roland Boutique D-05 doesn't capture, but plays perfectly: > > OK, this one looks slightly different. > D-05 has two EPs in each interface, both are ASYNC and 0x0d/0x8e. > So another adjustment might be needed here. > > > EDIROL UA-101 full-speed (USB 1.1) and high-speed (USB 2.0) not detected > for > > capture or playback (only 48 kHz mode tested): > > This is a different driver, so we can forget for now. > > > I'm guessing another look at "lsusb -v" would help, but don't know what > to > > look for and have run out of time tonight. > > Could you post the archive of your lsusb -v outputs? Put in a tarball > and post with attachment. > > Below is a revised patch. Let me know if this works better. > > > 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 */ > }; > @@ -204,30 +206,70 @@ static int add_generic_uac2_implicit_fb(struct > snd_usb_audio *chip, > ifnum, alts); > } > > -/* Like the function above, but specific to Roland with vendor class and > hack */ > +static bool roland_sanity_check_iface(struct usb_host_interface *alts) > +{ > + if (alts->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC || > + (alts->desc.bInterfaceSubClass != 2 && > + alts->desc.bInterfaceProtocol != 2) || > + alts->desc.bNumEndpoints < 1) > + return false; > + return true; > +} > + > +/* Like the UAC2 case above, but specific to Roland with vendor class and > hack */ > static int add_roland_implicit_fb(struct snd_usb_audio *chip, > struct audioformat *fmt, > - unsigned int ifnum, > - unsigned int altsetting) > + struct usb_host_interface *alts) > { > - struct usb_host_interface *alts; > struct usb_endpoint_descriptor *epd; > > - alts = snd_usb_get_host_interface(chip, ifnum, altsetting); > - if (!alts) > + if (!roland_sanity_check_iface(alts)) > return 0; > - if (alts->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC || > - (alts->desc.bInterfaceSubClass != 2 && > - alts->desc.bInterfaceProtocol != 2) || > - alts->desc.bNumEndpoints < 1) > + /* only when both streams are with ASYNC type */ > + epd = get_endpoint(alts, 0); > + if (!usb_endpoint_is_isoc_out(epd) || > + (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) != > USB_ENDPOINT_SYNC_ASYNC) > + return 0; > + > + /* check capture EP */ > + alts = snd_usb_get_host_interface(chip, > + alts->desc.bInterfaceNumber + 1, > + alts->desc.bAlternateSetting); > + if (!alts || !roland_sanity_check_iface(alts)) > 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); > + alts->desc.bInterfaceNumber, alts); > +} > + > +/* capture quirk for Roland device; always full-duplex */ > +static int add_roland_capture_quirk(struct snd_usb_audio *chip, > + struct audioformat *fmt, > + struct usb_host_interface *alts) > +{ > + struct usb_endpoint_descriptor *epd; > + > + if (!roland_sanity_check_iface(alts)) > + return 0; > + epd = get_endpoint(alts, 0); > + if (!usb_endpoint_is_isoc_in(epd) || > + (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) != > USB_ENDPOINT_SYNC_ASYNC) > + return 0; > + > + alts = snd_usb_get_host_interface(chip, > + alts->desc.bInterfaceNumber - 1, > + alts->desc.bAlternateSetting); > + if (!alts || !roland_sanity_check_iface(alts)) > + 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, > + alts->desc.bInterfaceNumber, alts); > } > > /* Playback and capture EPs on Pioneer devices share the same iface/altset > @@ -365,14 +407,8 @@ static int audioformat_implicit_fb_quirk(struct > snd_usb_audio *chip, > } > > /* Roland/BOSS implicit feedback with vendor spec class */ > - if (attr == USB_ENDPOINT_SYNC_ASYNC && > - 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_implicit_fb(chip, fmt, > - alts->desc.bInterfaceNumber + 1, > - alts->desc.bAlternateSetting)) > + if (USB_ID_VENDOR(chip->usb_id) == 0x0582) { > + if (add_roland_implicit_fb(chip, fmt, alts) > 0) > return 1; > } > > @@ -404,6 +440,13 @@ 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 (USB_ID_VENDOR(chip->usb_id) == 0x0582) { > + if (add_roland_capture_quirk(chip, fmt, alts) > 0) > + return 1; > + } > + > if (is_pioneer_implicit_fb(chip, alts)) > return 1; /* skip the quirk, also don't handle generic > sync EP */ > return 0; >
It does work perfectly now, thanks! First, I just want to remind you that UA-1000/UA-101 seems enabled in snd-usb-audio still (or I need to mix that patch with your last), as it isn't detected for either capture or playback. Here are the specifics tested: Roland VG-99 Perfect!: 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 ^CAborted by signal Interrupt... aplay -D hw:VG99 -f S24_3LE -r 44100 -c 2 ./file.wav Playing WAVE './file.wav' : Signed 24 bit Little Endian in 3bytes, Rate 44100 Hz, Stereo Roland INTEGRA-7 Perfect! (only 96 kHz mode tested): arecord -D hw:INTEGRA7 -f S32_LE -r 96000 -c 2 ./file.wav Recording WAVE './file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, Stereo ^CAborted by signal Interrupt... aplay -D hw:INTEGRA7 -f S32_LE -r 96000 -c 2 ./file.wav Playing WAVE './file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, Stereo Roland R-26 Perfect! (only 96 kHz mode tested): arecord -D hw:R26AUDIO -f S32_LE -r 96000 -c 2 ./file.wav Recording WAVE './file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, Stereo ^CAborted by signal Interrupt... aplay -D hw:R26AUDIO -f S32_LE -r 96000 -c 2 ./file.wav Playing WAVE './file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, Stereo Roland Boutique D-05 Perfect!: arecord -D hw:Boutique -f S32_LE -r 96000 -c 2 ./file.wav Recording WAVE './file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, Stereo ^CAborted by signal Interrupt... aplay -D hw:Boutique -f S32_LE -r 96000 -c 2 ./file.wav Playing WAVE './file.wav' : Signed 32 bit Little Endian, Rate 96000 Hz, Stereo EDIROL UA-4FX Perfect! (only tested 48 kHz mode): arecord -D hw:UA4FX -f S24_3LE -r 48000 -c 2 ./file.wav Recording WAVE './file.wav' : Signed 24 bit Little Endian in 3bytes, Rate 48000 Hz, Stereo aplay -D hw:UA4FX -f S24_3LE -r 48000 -c 2 ./file.wav Playing WAVE './file.wav' : Signed 24 bit Little Endian in 3bytes, Rate 48000 Hz, Stereo EDIROL UA-25EX Perfect! (only tested 48 kHz mode): arecord -D hw:UA25EX -f S24_3LE -r 48000 -c 2 ./file.wav Recording WAVE './file.wav' : Signed 24 bit Little Endian in 3bytes, Rate 48000 Hz, Stereo aplay -D hw:UA25EX -f S24_3LE -r 48000 -c 2 ./file.wav Playing WAVE './file.wav' : Signed 24 bit Little Endian in 3bytes, Rate 48000 Hz, Stereo Unless you decide to simplify it, no improvements seem necessary. Thanks for your grand achievement, Takahashi! I really appreciate it!, Lucas On Wed, Apr 21, 2021 at 3:59 AM Takashi Iwai <tiwai@suse.de> wrote: > Below is a revised patch. Let me know if this works better. >> >> >> 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 */ >> }; >> @@ -204,30 +206,70 @@ static int add_generic_uac2_implicit_fb(struct >> snd_usb_audio *chip, >> ifnum, alts); >> } >> >> -/* Like the function above, but specific to Roland with vendor class and >> hack */ >> +static bool roland_sanity_check_iface(struct usb_host_interface *alts) >> +{ >> + if (alts->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC || >> + (alts->desc.bInterfaceSubClass != 2 && >> + alts->desc.bInterfaceProtocol != 2) || >> + alts->desc.bNumEndpoints < 1) >> + return false; >> + return true; >> +} >> + >> +/* Like the UAC2 case above, but specific to Roland with vendor class >> and hack */ >> static int add_roland_implicit_fb(struct snd_usb_audio *chip, >> struct audioformat *fmt, >> - unsigned int ifnum, >> - unsigned int altsetting) >> + struct usb_host_interface *alts) >> { >> - struct usb_host_interface *alts; >> struct usb_endpoint_descriptor *epd; >> >> - alts = snd_usb_get_host_interface(chip, ifnum, altsetting); >> - if (!alts) >> + if (!roland_sanity_check_iface(alts)) >> return 0; >> - if (alts->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC || >> - (alts->desc.bInterfaceSubClass != 2 && >> - alts->desc.bInterfaceProtocol != 2) || >> - alts->desc.bNumEndpoints < 1) >> + /* only when both streams are with ASYNC type */ >> + epd = get_endpoint(alts, 0); >> + if (!usb_endpoint_is_isoc_out(epd) || >> + (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) != >> USB_ENDPOINT_SYNC_ASYNC) >> + return 0; >> + >> + /* check capture EP */ >> + alts = snd_usb_get_host_interface(chip, >> + alts->desc.bInterfaceNumber + 1, >> + alts->desc.bAlternateSetting); >> + if (!alts || !roland_sanity_check_iface(alts)) >> 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); >> + alts->desc.bInterfaceNumber, alts); >> +} >> + >> +/* capture quirk for Roland device; always full-duplex */ >> +static int add_roland_capture_quirk(struct snd_usb_audio *chip, >> + struct audioformat *fmt, >> + struct usb_host_interface *alts) >> +{ >> + struct usb_endpoint_descriptor *epd; >> + >> + if (!roland_sanity_check_iface(alts)) >> + return 0; >> + epd = get_endpoint(alts, 0); >> + if (!usb_endpoint_is_isoc_in(epd) || >> + (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) != >> USB_ENDPOINT_SYNC_ASYNC) >> + return 0; >> + >> + alts = snd_usb_get_host_interface(chip, >> + alts->desc.bInterfaceNumber - 1, >> + alts->desc.bAlternateSetting); >> + if (!alts || !roland_sanity_check_iface(alts)) >> + 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, >> + alts->desc.bInterfaceNumber, alts); >> } >> >> /* Playback and capture EPs on Pioneer devices share the same >> iface/altset >> @@ -365,14 +407,8 @@ static int audioformat_implicit_fb_quirk(struct >> snd_usb_audio *chip, >> } >> >> /* Roland/BOSS implicit feedback with vendor spec class */ >> - if (attr == USB_ENDPOINT_SYNC_ASYNC && >> - 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_implicit_fb(chip, fmt, >> - alts->desc.bInterfaceNumber + >> 1, >> - alts->desc.bAlternateSetting)) >> + if (USB_ID_VENDOR(chip->usb_id) == 0x0582) { >> + if (add_roland_implicit_fb(chip, fmt, alts) > 0) >> return 1; >> } >> >> @@ -404,6 +440,13 @@ 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 (USB_ID_VENDOR(chip->usb_id) == 0x0582) { >> + if (add_roland_capture_quirk(chip, fmt, alts) > 0) >> + return 1; >> + } >> + >> if (is_pioneer_implicit_fb(chip, alts)) >> return 1; /* skip the quirk, also don't handle generic >> sync EP */ >> return 0; >> >
Sorry, for my typo. Here's the correction: "Thanks for your grand achievement, Takashi Iwai!" (It ran through my head just after sending it as these things often do for me. Too bad there's no after-sent edit option for email.)
On Thu, 22 Apr 2021 06:10:10 +0200, Lucas wrote: > > Sorry, for my typo. Here's the correction: "Thanks for your grand > achievement, Takashi Iwai!" (It ran through my head just after sending it as > these things often do for me. Too bad there's no after-sent edit option for > email.) Don't worry, it's a typical typo :) And it's good to hear that we finally nailed down. I'm going to submit the proper patches soon later. I decided to put to two patches, a simple revert of the previous quirk addition and the conversion to the generic implicit fb matching. Takashi
On Tuesday, 20 April 2021 05:20:57 BST Lucas wrote: > Keith, if it's anything like the Roland devices I've tested with this > long-awaited patch, could you show the JS-8's "lsusb -v"? Output pasted further down. > > 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. When testing Takashi-san's earlier patches, I had a mixture of experiences including (depending on the patch) some devices recording silence, some playing back silence, and some where the record would lock up until playback was started (or vice versa). > > Takashi Iwai has indicated a likely connection between needing the > IMPLICIT_FB_BOTH_DEVICE line if there's "Asynchronous" "Synch Type" for > both input and output endpoints. Otherwise, possibly, it should be > reverted to IMPLICIT_FB_FIXED_DEVICE for the JS-8. As you can see, this has "Asynchronous" "Synch Type" on EP4 IN and EP 5 OUT, so I don't think it's this. Bus 001 Device 007: ID 0582:0109 Roland Corp. eBand JS-8 Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass 255 Vendor Specific Class bDeviceSubClass 0 bDeviceProtocol 255 bMaxPacketSize0 64 idVendor 0x0582 Roland Corp. idProduct 0x0109 eBand JS-8 bcdDevice 0.01 iManufacturer 1 BOSS iProduct 2 JS-8 iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 0x00a7 bNumInterfaces 3 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xc0 Self Powered MaxPower 0mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 0 bNumEndpoints 0 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 2 bInterfaceProtocol 2 iInterface 0 ** UNRECOGNIZED: 06 24 f1 01 00 00 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 1 bNumEndpoints 1 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 2 bInterfaceProtocol 2 iInterface 0 ** UNRECOGNIZED: 07 24 01 01 00 01 00 ** UNRECOGNIZED: 0b 24 02 01 02 04 18 01 44 ac 00 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x05 EP 5 OUT bmAttributes 5 Transfer Type Isochronous Synch Type Asynchronous Usage Type Data wMaxPacketSize 0x0070 1x 112 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 1 bAlternateSetting 0 bNumEndpoints 0 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 2 bInterfaceProtocol 1 iInterface 0 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 1 bAlternateSetting 1 bNumEndpoints 1 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 2 bInterfaceProtocol 1 iInterface 0 ** UNRECOGNIZED: 07 24 01 07 00 01 00 ** UNRECOGNIZED: 0b 24 02 01 02 04 18 01 44 ac 00 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 37 Transfer Type Isochronous Synch Type Asynchronous Usage Type Implicit feedback Data wMaxPacketSize 0x0070 1x 112 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 2 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 3 bInterfaceProtocol 0 iInterface 0 ** UNRECOGNIZED: 06 24 f1 02 02 02 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x03 EP 3 OUT bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 2 bAlternateSetting 1 bNumEndpoints 2 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 3 bInterfaceProtocol 0 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x03 EP 3 OUT bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 1 Device Qualifier (for other device speed): bLength 10 bDescriptorType 6 bcdUSB 2.00 bDeviceClass 8 Mass Storage bDeviceSubClass 6 SCSI bDeviceProtocol 80 Bulk-Only bMaxPacketSize0 64 bNumConfigurations 0 can't get debug descriptor: Resource temporarily unavailable Device Status: 0x0001 Self Powered Regards, Keith
Does it help if you use this line in "static const struct
snd_usb_implicit_fb_match capture_implicit_fb_quirks[] = {":
IMPLICIT_FB_BOTH_DEV(0x0582, 0x0109, 0x05, 0x01), /* BOSS eBand
JS-8 */
For it, I attempted using your specific 0x05 ASYNC OUT endpoint address
this time.
I may be wrong, but I think Takashi's current patches are reading endpoint
addresses directly from the hardware, and if so your BOSS eBand JS-8 would
work if that line does.
Takashi would know best, though, so I'm bracing myself for any corrections.
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 102d53515a76..f4c3d2b38abb 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -1375,7 +1375,8 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) if (!ep_state_update(ep, EP_STATE_STOPPED, EP_STATE_RUNNING)) goto __error; - if (snd_usb_endpoint_implicit_feedback_sink(ep)) { + if (snd_usb_endpoint_implicit_feedback_sink(ep) && + !ep->chip->playback_first) { for (i = 0; i < ep->nurbs; i++) { struct snd_urb_ctx *ctx = ep->urb + i; list_add_tail(&ctx->ready_list, &ep->ready_playback_urbs); diff --git a/sound/usb/implicit.c b/sound/usb/implicit.c index 19622c58b72b..4bd9c105a529 100644 --- a/sound/usb/implicit.c +++ b/sound/usb/implicit.c @@ -21,6 +21,7 @@ enum { IMPLICIT_FB_NONE, IMPLICIT_FB_GENERIC, IMPLICIT_FB_FIXED, + IMPLICIT_FB_BOTH, /* generic playback + capture (for BOSS) */ }; struct snd_usb_implicit_fb_match { @@ -36,6 +37,9 @@ struct snd_usb_implicit_fb_match { #define IMPLICIT_FB_FIXED_DEV(vend, prod, ep, ifnum) \ { .id = USB_ID(vend, prod), .type = IMPLICIT_FB_FIXED, .ep_num = (ep),\ .iface = (ifnum) } +#define IMPLICIT_FB_BOTH_DEV(vend, prod, ep, ifnum) \ + { .id = USB_ID(vend, prod), .type = IMPLICIT_FB_BOTH, .ep_num = (ep),\ + .iface = (ifnum) } #define IMPLICIT_FB_SKIP_DEV(vend, prod) \ { .id = USB_ID(vend, prod), .type = IMPLICIT_FB_NONE } @@ -75,14 +79,14 @@ 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[] = { - IMPLICIT_FB_FIXED_DEV(0x0582, 0x0130, 0x0d, 0x01), /* BOSS BR-80 */ - IMPLICIT_FB_FIXED_DEV(0x0582, 0x0171, 0x0d, 0x01), /* BOSS RC-505 */ - IMPLICIT_FB_FIXED_DEV(0x0582, 0x0185, 0x0d, 0x01), /* BOSS GP-10 */ - IMPLICIT_FB_FIXED_DEV(0x0582, 0x0189, 0x0d, 0x01), /* BOSS GT-100v2 */ - IMPLICIT_FB_FIXED_DEV(0x0582, 0x01d6, 0x0d, 0x01), /* BOSS GT-1 */ - IMPLICIT_FB_FIXED_DEV(0x0582, 0x01d8, 0x0d, 0x01), /* BOSS Katana */ - IMPLICIT_FB_FIXED_DEV(0x0582, 0x01e5, 0x0d, 0x01), /* BOSS GT-001 */ - IMPLICIT_FB_FIXED_DEV(0x0582, 0x0203, 0x0d, 0x01), /* BOSS AD-10 */ + IMPLICIT_FB_BOTH_DEV(0x0582, 0x0130, 0x0d, 0x01), /* BOSS BR-80 */ + IMPLICIT_FB_BOTH_DEV(0x0582, 0x0171, 0x0d, 0x01), /* BOSS RC-505 */ + IMPLICIT_FB_BOTH_DEV(0x0582, 0x0185, 0x0d, 0x01), /* BOSS GP-10 */ + IMPLICIT_FB_BOTH_DEV(0x0582, 0x0189, 0x0d, 0x01), /* BOSS GT-100v2 */ + IMPLICIT_FB_BOTH_DEV(0x0582, 0x01d6, 0x0d, 0x01), /* BOSS GT-1 */ + IMPLICIT_FB_BOTH_DEV(0x0582, 0x01d8, 0x0d, 0x01), /* BOSS Katana */ + IMPLICIT_FB_BOTH_DEV(0x0582, 0x01e5, 0x0d, 0x01), /* BOSS GT-001 */ + IMPLICIT_FB_BOTH_DEV(0x0582, 0x0203, 0x0d, 0x01), /* BOSS AD-10 */ {} /* terminator */ }; @@ -268,10 +272,17 @@ static int audioformat_implicit_fb_quirk(struct snd_usb_audio *chip, } } - /* Don't apply playback quirks for the devices with capture quirk */ + /* Special handling for devices with capture quirks */ p = find_implicit_fb_entry(chip, capture_implicit_fb_quirks, alts); - if (p && p->type == IMPLICIT_FB_FIXED) - return 0; /* no quirk */ + if (p) { + switch (p->type) { + case IMPLICIT_FB_FIXED: + return 0; /* no quirk */ + case IMPLICIT_FB_BOTH: + chip->playback_first = 1; + return add_generic_implicit_fb(chip, fmt, alts); + } + } /* Generic UAC2 implicit feedback */ if (attr == USB_ENDPOINT_SYNC_ASYNC && @@ -321,7 +332,7 @@ static int audioformat_capture_quirk(struct snd_usb_audio *chip, const struct snd_usb_implicit_fb_match *p; p = find_implicit_fb_entry(chip, capture_implicit_fb_quirks, alts); - if (p && p->type == IMPLICIT_FB_FIXED) + 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); return 0; diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index a536ee33d36e..538831cbd925 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -37,6 +37,7 @@ struct snd_usb_audio { unsigned int txfr_quirk:1; /* Subframe boundaries on transfers */ unsigned int tx_length_quirk:1; /* Put length specifier in transfers */ unsigned int need_delayed_register:1; /* warn for delayed registration */ + unsigned int playback_first:1; /* for implicit fb: don't wait for the first capture URBs */ int num_interfaces; int num_suspended_intf; int sample_rate_read_error;
During the recent rewrite of the implicit feedback support, we've tested to apply the implicit fb on BOSS devices, but it failed, as the capture stream didn't start without the playback. As the end result, it got another type of quirk for tying both streams but starts playback always (commit 6234fdc1cede "ALSA: usb-audio: Quirk for BOSS GT-001"). Meanwhile, Mike Oliphant has tested the real implicit feedback mode for the playback again with the latest code, and found out that it actually works if the initial feedback sync is skipped; that is, on those BOSS devices, the playback stream has to be started at first without waiting for the capture URB completions. Otherwise it gets stuck. In the rest operations after the capture stream processed, we can take them as the implicit feedback source. This patch is an attempt to improve the support for BOSS devices with the implicit feedback mode in the way described above. It adds a new flag to snd_usb_audio, playback_first, indicating that the playback stream starts without sync with the initial capture completion. This flag is set in the quirk table with the new IMPLICIT_FB_BOTH type. Reported-and-tested-by: Mike Oliphant <oliphant@nostatic.org> Signed-off-by: Takashi Iwai <tiwai@suse.de> --- sound/usb/endpoint.c | 3 ++- sound/usb/implicit.c | 35 +++++++++++++++++++++++------------ sound/usb/usbaudio.h | 1 + 3 files changed, 26 insertions(+), 13 deletions(-)