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

[Lucas <jaffa225man@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 9768 bytes --]

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


-- 
Protect your digital freedom and privacy, eliminate DRM, learn more at
http://www.defectivebydesign.org/what_is_drm
On a related note, also see https://www.fsf.org/campaigns/surveillance

[-- Attachment #2: lsusb_-v.tar --]
[-- Type: application/x-tar, Size: 40960 bytes --]