Works for me Dr. Iwai, thank you!
Much more better styled now, and less hacky.
-Geraldo
Em Ter, 6 de abr de 2021 08:48, Takashi Iwai <tiwai@xxxxxxx> escreveu:
> On Mon, 05 Apr 2021 15:49:20 +0200,
> Geraldo wrote:
> >
> > Dear Linux users of Pioneer gear, please disregard v1 of this patch and
> test
> > this instead if at all possible.
> >
> > My initial assessment that we needed to let the capture EP be opened
> twice was
> > clearly proven wrong by further testing. This is good because if we
> really
> > needed that it would require a lot of reengineering inside ALSA.
> >
> > One thing that still boggles my mind though is why we need a special
> Pioneer
> > case inside snd_usb_parse_implicit_fb_quirk that returns 1 like a capture
> > quirk was found.
> >
> > This is a highly experimental patch on top of 5.12-rc6 that's supposed to
> > engage implicit feedback sync on the playback for Pioneer devices.
> Without
> > implicit feedback sync the inputs started glitching due to clock drift in
> > about an hour in my Pioneer DJ DDJ-SR2.
> >
> > Once again I'd like to thank Takashi Iwai. He's the true author of the
> bulk of
> > this code, I just adapted it and coerced it into working.
> >
> > Signed-off-by: Geraldo Nascimento <geraldogabriel@xxxxxxxxx>
>
> Thanks for the patch!
>
> It's interesting that Pioneer devices would actually work with the
> implicit feedback mode. It seems that the key point is to skip the
> capture side; IIRC, we checked applying the quirk to the playback
> side, but this wasn't enough or not properly working on some devices.
>
> If that's the case, I believe a patch like below would be safer and
> more inconsistent: it checks the device from both playback and capture
> quirks with the same helper function.
>
> Could you check whether this one works? (It's totally untested.)
>
>
> thanks,
>
> Takashi
>
> ---
> --- a/sound/usb/implicit.c
> +++ b/sound/usb/implicit.c
> @@ -167,18 +167,27 @@ static int add_roland_implicit_fb(struct
> snd_usb_audio *chip,
> ifnum, alts);
> }
>
> -/* Playback and capture EPs on Pioneer devices share the same
> iface/altset,
> - * but they don't seem working with the implicit fb mode well, hence we
> - * just return as if the sync were already set up.
> +/* Playback and capture EPs on Pioneer devices share the same iface/altset
> + * for the implicit feedback operation
> */
> -static int skip_pioneer_sync_ep(struct snd_usb_audio *chip,
> - struct audioformat *fmt,
> - struct usb_host_interface *alts)
> +static bool is_pioneer_implicit_fb(struct snd_usb_audio *chip,
> + struct usb_host_interface *alts)
> +
> {
> struct usb_endpoint_descriptor *epd;
>
> + if (USB_ID_VENDOR(chip->usb_id) != 0x2b73 &&
> + USB_ID_VENDOR(chip->usb_id) != 0x08e4)
> + return false;
> + if (alts->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC)
> + return false;
> if (alts->desc.bNumEndpoints != 2)
> - return 0;
> + return false;
> +
> + epd = get_endpoint(alts, 0);
> + if (!usb_endpoint_is_isoc_out(epd) ||
> + (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) !=
> USB_ENDPOINT_SYNC_ASYNC)
> + return false;
>
> epd = get_endpoint(alts, 1);
> if (!usb_endpoint_is_isoc_in(epd) ||
> @@ -187,8 +196,9 @@ static int skip_pioneer_sync_ep(struct snd_usb_audio
> *chip,
> USB_ENDPOINT_USAGE_DATA &&
> (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) !=
> USB_ENDPOINT_USAGE_IMPLICIT_FB))
> - return 0;
> - return 1; /* don't handle with the implicit fb, just skip sync EP
> */
> + return false;
> +
> + return true;
> }
>
> static int __add_generic_implicit_fb(struct snd_usb_audio *chip,
> @@ -297,13 +307,11 @@ static int audioformat_implicit_fb_quirk(struct
> snd_usb_audio *chip,
> }
>
> /* Pioneer devices with vendor spec class */
> - if (attr == USB_ENDPOINT_SYNC_ASYNC &&
> - alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
> - (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */
> - USB_ID_VENDOR(chip->usb_id) == 0x08e4 /* Pioneer */)) {
> - if (skip_pioneer_sync_ep(chip, fmt, alts))
> - return 1;
> - }
> + if (is_pioneer_implicit_fb(chip, alts))
> + return add_implicit_fb_sync_ep(chip, fmt,
> + get_endpoint(alts,
> 1)->bEndpointAddress,
> + 1,
> alts->desc.bInterfaceNumber,
> + alts);
>
> /* Try the generic implicit fb if available */
> if (chip->generic_implicit_fb)
> @@ -324,6 +332,8 @@ static int audioformat_capture_quirk(struct
> snd_usb_audio *chip,
> if (p && p->type == IMPLICIT_FB_FIXED)
> return add_implicit_fb_sync_ep(chip, fmt, p->ep_num, 0,
> p->iface, NULL);
> + if (is_pioneer_implicit_fb(chip, alts))
> + return 1; /* skip */
> return 0;
> }
>
>
>
