[PATCH v2] Pioneer devices: engage implicit feedback sync for playback
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@gmail.com
--- implicit.c.git 2021-04-04 20:51:57.226754632 -0300 +++ implicit.c 2021-04-05 10:02:41.059816581 -0300 @@ -177,30 +177,31 @@ static int add_roland_implicit_fb(struct 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. +/* Pioneer devices: playback and capture streams sharing the same iface/altset */ -static int skip_pioneer_sync_ep(struct snd_usb_audio *chip, - struct audioformat *fmt, - struct usb_host_interface *alts) -{ - struct usb_endpoint_descriptor *epd; - - if (alts->desc.bNumEndpoints != 2) - return 0; - - epd = get_endpoint(alts, 1); - if (!usb_endpoint_is_isoc_in(epd) || - (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) != USB_ENDPOINT_SYNC_ASYNC || - ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) != - 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 */ +static int add_pioneer_implicit_fb(struct snd_usb_audio *chip, + struct audioformat *fmt, + struct usb_host_interface *alts) +{ + struct usb_endpoint_descriptor *epd; + + if (alts->desc.bNumEndpoints != 2) + return 0; + + epd = get_endpoint(alts, 1); + + if (!usb_endpoint_is_isoc_in(epd) || + (epd->bmAttributes & USB_ENDPOINT_SYNCTYPE) != USB_ENDPOINT_SYNC_ASYNC || + ((epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) != + USB_ENDPOINT_USAGE_DATA && + (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) != + USB_ENDPOINT_USAGE_IMPLICIT_FB)) + return 0; + return add_implicit_fb_sync_ep(chip, fmt, epd->bEndpointAddress, 1, + alts->desc.bInterfaceNumber, alts); }
+ static int __add_generic_implicit_fb(struct snd_usb_audio *chip, struct audioformat *fmt, int iface, int altset) @@ -306,7 +307,7 @@ static int audioformat_implicit_fb_quirk 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)) + if (add_pioneer_implicit_fb(chip, fmt, alts)) return 1; }
@@ -339,8 +340,20 @@ int snd_usb_parse_implicit_fb_quirk(stru struct audioformat *fmt, struct usb_host_interface *alts) { - if (fmt->endpoint & USB_DIR_IN) - return audioformat_capture_quirk(chip, fmt, alts); + bool isPioneer; + + if (alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC && + (USB_ID_VENDOR(chip->usb_id) == 0x2b73 || /* Pioneer */ + USB_ID_VENDOR(chip->usb_id) == 0x08e4 /* Pioneer */)) + isPioneer = true; + + if (fmt->endpoint & USB_DIR_IN) { + if (isPioneer == true) + return 1; + else + return audioformat_capture_quirk(chip, fmt, alts); + } + else return audioformat_implicit_fb_quirk(chip, fmt, alts); }
Dr. Iwai, here's the additional debugging information you asked for.
First dyndbg log is for successful engagement of implicit feedback sync on the playback. Second case I modified snd_usb_parse_implicit_fb_quirk to return 0 as it would normally, unsuccessful. Third is lsusb -v of Pioneer DJ DDJ-SR2.
I think I know now why we need to return 1 like a capture quirk was found inside snd_usb_parse_implicit_fb_quirk: if we don't it triggers sync ep code inside pcm.c
As always, thank you for your time.
---
[ 6397.520597] xhci_hcd 0000:00:14.0: Port change event, 3-1, id 1, portsc: 0xa0206e1 [ 6397.520619] xhci_hcd 0000:00:14.0: resume root hub [ 6397.520647] xhci_hcd 0000:00:14.0: handle_port_status: starting port polling. [ 6397.520747] xhci_hcd 0000:00:14.0: Get port status 3-1 read: 0x206e1, return 0x10101 [ 6397.520950] xhci_hcd 0000:00:14.0: clear port1 connect change, portsc: 0x6e1 [ 6397.521111] xhci_hcd 0000:00:14.0: Get port status 3-2 read: 0x2a0, return 0x100 [ 6397.521147] xhci_hcd 0000:00:14.0: Get port status 3-3 read: 0x2a0, return 0x100 [ 6397.521179] xhci_hcd 0000:00:14.0: Get port status 3-4 read: 0x2a0, return 0x100 [ 6397.626337] xhci_hcd 0000:00:14.0: Get port status 3-1 read: 0x6e1, return 0x101 [ 6397.626387] xhci_hcd 0000:00:14.0: // Ding dong! [ 6397.626432] xhci_hcd 0000:00:14.0: Adding 1 ep ctx, 1 now active. [ 6397.626441] xhci_hcd 0000:00:14.0: Slot 5 output ctx = 0x13efdc000 (dma) [ 6397.626447] xhci_hcd 0000:00:14.0: Slot 5 input ctx = 0x13ec75000 (dma) [ 6397.626456] xhci_hcd 0000:00:14.0: Set slot id 5 dcbaa entry 00000000225b73b8 to 0x13efdc000 [ 6397.626515] xhci_hcd 0000:00:14.0: set port reset, actual port 3-1 status = 0x791 [ 6397.629635] xhci_hcd 0000:00:14.0: xhci_hub_status_data: stopping port polling. [ 6397.681751] xhci_hcd 0000:00:14.0: Port change event, 3-1, id 1, portsc: 0x200e03 [ 6397.681768] xhci_hcd 0000:00:14.0: handle_port_status: starting port polling. [ 6397.693020] xhci_hcd 0000:00:14.0: Get port status 3-1 read: 0x200e03, return 0x100503 [ 6397.693084] xhci_hcd 0000:00:14.0: clear port1 reset change, portsc: 0xe03 [ 6397.719619] xhci_hcd 0000:00:14.0: xhci_hub_status_data: stopping port polling. [ 6397.749662] usb 3-1: new high-speed USB device number 6 using xhci_hcd [ 6397.749689] xhci_hcd 0000:00:14.0: Set root hub portnum to 1 [ 6397.749697] xhci_hcd 0000:00:14.0: Set fake root hub portnum to 1 [ 6397.749702] xhci_hcd 0000:00:14.0: udev->tt = 0000000000000000 [ 6397.749709] xhci_hcd 0000:00:14.0: udev->ttport = 0x0 [ 6397.749720] xhci_hcd 0000:00:14.0: // Ding dong! [ 6397.749762] xhci_hcd 0000:00:14.0: Successful setup context command [ 6397.749771] xhci_hcd 0000:00:14.0: Op regs DCBAA ptr = 0x00000236cef000 [ 6397.749778] xhci_hcd 0000:00:14.0: Slot ID 5 dcbaa entry @00000000225b73b8 = 0x0000013efdc000 [ 6397.749787] xhci_hcd 0000:00:14.0: Output Context DMA address = 0x13efdc000 [ 6397.749792] xhci_hcd 0000:00:14.0: Internal device address = 0 [ 6397.749878] xhci_hcd 0000:00:14.0: Waiting for status stage event [ 6397.749925] xhci_hcd 0000:00:14.0: set port reset, actual port 3-1 status = 0xf91 [ 6397.805146] xhci_hcd 0000:00:14.0: Port change event, 3-1, id 1, portsc: 0x200e03 [ 6397.805161] xhci_hcd 0000:00:14.0: handle_port_status: starting port polling. [ 6397.816340] xhci_hcd 0000:00:14.0: Get port status 3-1 read: 0x200e03, return 0x100503 [ 6397.816403] xhci_hcd 0000:00:14.0: clear port1 reset change, portsc: 0xe03 [ 6397.872940] xhci_hcd 0000:00:14.0: Resetting device with slot ID 5 [ 6397.872964] xhci_hcd 0000:00:14.0: // Ding dong! [ 6397.872979] xhci_hcd 0000:00:14.0: Completed reset device command. [ 6397.872995] xhci_hcd 0000:00:14.0: Can't reset device (slot ID 5) in default state [ 6397.873001] xhci_hcd 0000:00:14.0: Not freeing device rings. [ 6397.873013] xhci_hcd 0000:00:14.0: // Ding dong! [ 6397.873057] xhci_hcd 0000:00:14.0: Successful setup address command [ 6397.873066] xhci_hcd 0000:00:14.0: Op regs DCBAA ptr = 0x00000236cef000 [ 6397.873073] xhci_hcd 0000:00:14.0: Slot ID 5 dcbaa entry @00000000225b73b8 = 0x0000013efdc000 [ 6397.873083] xhci_hcd 0000:00:14.0: Output Context DMA address = 0x13efdc000 [ 6397.873088] xhci_hcd 0000:00:14.0: Internal device address = 5 [ 6397.890400] xhci_hcd 0000:00:14.0: Waiting for status stage event [ 6397.890503] xhci_hcd 0000:00:14.0: Waiting for status stage event [ 6397.890743] xhci_hcd 0000:00:14.0: Waiting for status stage event [ 6397.890817] usb 3-1: New USB device found, idVendor=2b73, idProduct=001e, bcdDevice= 1.01 [ 6397.890828] usb 3-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0 [ 6397.890835] usb 3-1: Product: DDJ-SR2 [ 6397.890840] usb 3-1: Manufacturer: Pioneer DJ Corporation [ 6397.891132] xhci_hcd 0000:00:14.0: add ep 0x85, slot id 5, new drop flags = 0x0, new add flags = 0x800 [ 6397.891165] xhci_hcd 0000:00:14.0: add ep 0x4, slot id 5, new drop flags = 0x0, new add flags = 0x900 [ 6397.891183] xhci_hcd 0000:00:14.0: add ep 0x87, slot id 5, new drop flags = 0x0, new add flags = 0x8900 [ 6397.891196] xhci_hcd 0000:00:14.0: xhci_check_bandwidth called for udev 00000000298860cb [ 6397.891210] xhci_hcd 0000:00:14.0: Adding 3 ep ctxs, 4 now active. [ 6397.891225] xhci_hcd 0000:00:14.0: Recalculating BW for rootport 1 [ 6397.891236] xhci_hcd 0000:00:14.0: Final bandwidth: 42, Limit: 1607, Reserved: 322, Available: 77 percent [ 6397.891253] xhci_hcd 0000:00:14.0: // Ding dong! [ 6397.891348] xhci_hcd 0000:00:14.0: Successful Endpoint Configure command [ 6397.891468] xhci_hcd 0000:00:14.0: // Ding dong! [ 6397.891493] xhci_hcd 0000:00:14.0: Stopped on No-op or Link TRB for slot 5 ep 10 [ 6397.891522] xhci_hcd 0000:00:14.0: // Ding dong! [ 6397.891609] xhci_hcd 0000:00:14.0: // Ding dong! [ 6397.891841] xhci_hcd 0000:00:14.0: Stopped on No-op or Link TRB for slot 5 ep 7 [ 6397.891856] xhci_hcd 0000:00:14.0: // Ding dong! [ 6397.891923] xhci_hcd 0000:00:14.0: // Ding dong! [ 6397.891949] xhci_hcd 0000:00:14.0: Stopped on No-op or Link TRB for slot 5 ep 14 [ 6397.891962] xhci_hcd 0000:00:14.0: // Ding dong! [ 6397.892364] xhci_hcd 0000:00:14.0: Waiting for status stage event [ 6397.892476] xhci_hcd 0000:00:14.0: Waiting for status stage event [ 6397.892543] usb 3-1: 0:1: added playback implicit_fb sync_ep 82, iface 0:1 [ 6397.892562] usb 3-1: Creating new data endpoint #1 [ 6397.892568] usb 3-1: Creating new data endpoint #82 [ 6397.892574] xhci_hcd 0000:00:14.0: xhci_check_bandwidth called for udev 00000000298860cb [ 6397.892683] usb 3-1: 0:1 Set sample rate 44100, clock 0 [ 6397.892706] xhci_hcd 0000:00:14.0: xhci_check_bandwidth called for udev 00000000298860cb [ 6397.892771] usb 3-1: 0:1 Set sample rate 44100, clock 0 [ 6397.969579] xhci_hcd 0000:00:14.0: xhci_hub_status_data: stopping port polling. [ 6402.946313] xhci_hcd 0000:00:14.0: Cancel URB 00000000c89bac54, dev 1, ep 0x0, starting at offset 0x13ef91210 [ 6402.946353] xhci_hcd 0000:00:14.0: // Ding dong! [ 6402.946371] xhci_hcd 0000:00:14.0: Stopped on Transfer TRB for slot 5 ep 0 [ 6402.946383] xhci_hcd 0000:00:14.0: Removing canceled TD starting at 0x13ef91210 (dma). [ 6402.946392] xhci_hcd 0000:00:14.0: Set TR Deq ptr 0x13ef91230, cycle 1 [ 6402.946398] xhci_hcd 0000:00:14.0: // Ding dong! [ 6402.946409] xhci_hcd 0000:00:14.0: Successful Set TR Deq Ptr cmd, deq = @13ef91230 [ 6402.946417] xhci_hcd 0000:00:14.0: Giveback URB 00000000c89bac54, len = 0, expected = 0, status = -115 [ 6402.947692] hid-generic 0003:2B73:001E.0005: hiddev96,hidraw0: USB HID v1.11 Device [Pioneer DJ Corporation DDJ-SR2] on usb-0000:00:14.0-1/input3 [ 6424.552616] usb 3-1: Open EP 0x82, iface=0:1, idx=1 [ 6424.552626] usb 3-1: channels=6, rate=44100, format=S24_3LE, period_bytes=9216, periods=2, implicit_fb=0 [ 6424.552632] usb 3-1: Setting usb interface 0:0 for EP 0x82 [ 6424.552638] xhci_hcd 0000:00:14.0: xhci_check_bandwidth called for udev 00000000298860cb [ 6424.552715] usb 3-1: Setting usb interface 0:1 for EP 0x82 [ 6424.552725] xhci_hcd 0000:00:14.0: add ep 0x1, slot id 5, new drop flags = 0x0, new add flags = 0x5 [ 6424.552732] xhci_hcd 0000:00:14.0: add ep 0x82, slot id 5, new drop flags = 0x0, new add flags = 0x25 [ 6424.552736] xhci_hcd 0000:00:14.0: xhci_check_bandwidth called for udev 00000000298860cb [ 6424.552741] xhci_hcd 0000:00:14.0: Adding 2 ep ctxs, 6 now active. [ 6424.552746] xhci_hcd 0000:00:14.0: Recalculating BW for rootport 1 [ 6424.552750] xhci_hcd 0000:00:14.0: Final bandwidth: 282, Limit: 1607, Reserved: 322, Available: 62 percent [ 6424.552757] xhci_hcd 0000:00:14.0: // Ding dong! [ 6424.552811] xhci_hcd 0000:00:14.0: Successful Endpoint Configure command [ 6424.552983] usb 3-1: 0:1 Set sample rate 44100, clock 0 [ 6424.553011] usb 3-1: Setting params for data EP 0x82, pipe 0x10680 [ 6424.553021] usb 3-1: Set up 12 URBS, ret=0 [ 6424.553113] usb 3-1: Open EP 0x1, iface=0:1, idx=0 [ 6424.553117] usb 3-1: channels=4, rate=44100, format=S24_3LE, period_bytes=6144, periods=2, implicit_fb=1 [ 6424.553122] usb 3-1: Reopened EP 0x82 (count 1) [ 6424.553126] usb 3-1: 0:1 Set sample rate 44100, clock 0 [ 6424.553129] usb 3-1: Setting params for data EP 0x1, pipe 0x8600 [ 6424.553138] usb 3-1: Set up 12 URBS, ret=0 [ 6424.553163] usb 3-1: Starting data EP 0x1 (running 0) [ 6424.553168] usb 3-1: No URB submission due to implicit fb sync [ 6424.553170] usb 3-1: Starting data EP 0x82 (running 0) [ 6424.553219] usb 3-1: 12 URBs submitted for EP 0x82 [ 6424.555173] usb 3-1: Starting data EP 0x82 (running 1) [ 6424.555180] usb 3-1: 0:1 Start Capture PCM [ 6424.555184] usb 3-1: 0:1 Start Playback PCM
---
[ 6518.662829] xhci_hcd 0000:00:14.0: resume root hub [ 6518.662849] xhci_hcd 0000:00:14.0: handle_port_status: starting port polling. [ 6518.663623] xhci_hcd 0000:00:14.0: Get port status 3-1 read: 0x206e1, return 0x10101 [ 6518.663703] xhci_hcd 0000:00:14.0: clear port1 connect change, portsc: 0x6e1 [ 6518.663728] xhci_hcd 0000:00:14.0: Get port status 3-2 read: 0x2a0, return 0x100 [ 6518.663750] xhci_hcd 0000:00:14.0: Get port status 3-3 read: 0x2a0, return 0x100 [ 6518.663769] xhci_hcd 0000:00:14.0: Get port status 3-4 read: 0x2a0, return 0x100 [ 6518.719588] xhci_hcd 0000:00:14.0: xhci_hub_status_data: stopping port polling. [ 6518.769612] xhci_hcd 0000:00:14.0: Get port status 3-1 read: 0x6e1, return 0x101 [ 6518.769668] xhci_hcd 0000:00:14.0: // Ding dong! [ 6518.769712] xhci_hcd 0000:00:14.0: Adding 1 ep ctx, 1 now active. [ 6518.769725] xhci_hcd 0000:00:14.0: Slot 6 output ctx = 0x13efdc000 (dma) [ 6518.769734] xhci_hcd 0000:00:14.0: Slot 6 input ctx = 0x13ec75000 (dma) [ 6518.769744] xhci_hcd 0000:00:14.0: Set slot id 6 dcbaa entry 00000000d24104f6 to 0x13efdc000 [ 6518.769816] xhci_hcd 0000:00:14.0: set port reset, actual port 3-1 status = 0x791 [ 6518.772897] xhci_hcd 0000:00:14.0: xhci_hub_status_data: stopping port polling. [ 6518.824991] xhci_hcd 0000:00:14.0: Port change event, 3-1, id 1, portsc: 0x200e03 [ 6518.825012] xhci_hcd 0000:00:14.0: handle_port_status: starting port polling. [ 6518.836254] xhci_hcd 0000:00:14.0: Get port status 3-1 read: 0x200e03, return 0x100503 [ 6518.836306] xhci_hcd 0000:00:14.0: clear port1 reset change, portsc: 0xe03 [ 6518.892917] usb 3-1: new high-speed USB device number 7 using xhci_hcd [ 6518.892938] xhci_hcd 0000:00:14.0: Set root hub portnum to 1 [ 6518.892945] xhci_hcd 0000:00:14.0: Set fake root hub portnum to 1 [ 6518.892950] xhci_hcd 0000:00:14.0: udev->tt = 0000000000000000 [ 6518.892956] xhci_hcd 0000:00:14.0: udev->ttport = 0x0 [ 6518.892966] xhci_hcd 0000:00:14.0: // Ding dong! [ 6518.892999] xhci_hcd 0000:00:14.0: Successful setup context command [ 6518.893008] xhci_hcd 0000:00:14.0: Op regs DCBAA ptr = 0x00000236cef000 [ 6518.893015] xhci_hcd 0000:00:14.0: Slot ID 6 dcbaa entry @00000000d24104f6 = 0x0000013efdc000 [ 6518.893023] xhci_hcd 0000:00:14.0: Output Context DMA address = 0x13efdc000 [ 6518.893029] xhci_hcd 0000:00:14.0: Internal device address = 0 [ 6518.893208] xhci_hcd 0000:00:14.0: Waiting for status stage event [ 6518.893287] xhci_hcd 0000:00:14.0: set port reset, actual port 3-1 status = 0xf91 [ 6518.948510] xhci_hcd 0000:00:14.0: Port change event, 3-1, id 1, portsc: 0x200e03 [ 6518.948533] xhci_hcd 0000:00:14.0: handle_port_status: starting port polling. [ 6518.959642] xhci_hcd 0000:00:14.0: Get port status 3-1 read: 0x200e03, return 0x100503 [ 6518.959704] xhci_hcd 0000:00:14.0: clear port1 reset change, portsc: 0xe03 [ 6518.969594] xhci_hcd 0000:00:14.0: xhci_hub_status_data: stopping port polling. [ 6519.016304] xhci_hcd 0000:00:14.0: Resetting device with slot ID 6 [ 6519.016330] xhci_hcd 0000:00:14.0: // Ding dong! [ 6519.016351] xhci_hcd 0000:00:14.0: Completed reset device command. [ 6519.016370] xhci_hcd 0000:00:14.0: Can't reset device (slot ID 6) in default state [ 6519.016377] xhci_hcd 0000:00:14.0: Not freeing device rings. [ 6519.016390] xhci_hcd 0000:00:14.0: // Ding dong! [ 6519.016437] xhci_hcd 0000:00:14.0: Successful setup address command [ 6519.016446] xhci_hcd 0000:00:14.0: Op regs DCBAA ptr = 0x00000236cef000 [ 6519.016454] xhci_hcd 0000:00:14.0: Slot ID 6 dcbaa entry @00000000d24104f6 = 0x0000013efdc000 [ 6519.016463] xhci_hcd 0000:00:14.0: Output Context DMA address = 0x13efdc000 [ 6519.016469] xhci_hcd 0000:00:14.0: Internal device address = 6 [ 6519.033886] xhci_hcd 0000:00:14.0: Waiting for status stage event [ 6519.034170] xhci_hcd 0000:00:14.0: Waiting for status stage event [ 6519.034352] xhci_hcd 0000:00:14.0: Waiting for status stage event [ 6519.034416] usb 3-1: New USB device found, idVendor=2b73, idProduct=001e, bcdDevice= 1.01 [ 6519.034427] usb 3-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0 [ 6519.034435] usb 3-1: Product: DDJ-SR2 [ 6519.034440] usb 3-1: Manufacturer: Pioneer DJ Corporation [ 6519.034757] xhci_hcd 0000:00:14.0: add ep 0x85, slot id 6, new drop flags = 0x0, new add flags = 0x800 [ 6519.034776] xhci_hcd 0000:00:14.0: add ep 0x4, slot id 6, new drop flags = 0x0, new add flags = 0x900 [ 6519.034790] xhci_hcd 0000:00:14.0: add ep 0x87, slot id 6, new drop flags = 0x0, new add flags = 0x8900 [ 6519.034799] xhci_hcd 0000:00:14.0: xhci_check_bandwidth called for udev 0000000041e6ad6d [ 6519.034811] xhci_hcd 0000:00:14.0: Adding 3 ep ctxs, 4 now active. [ 6519.034821] xhci_hcd 0000:00:14.0: Recalculating BW for rootport 1 [ 6519.034828] xhci_hcd 0000:00:14.0: Final bandwidth: 42, Limit: 1607, Reserved: 322, Available: 77 percent [ 6519.034842] xhci_hcd 0000:00:14.0: // Ding dong! [ 6519.045845] xhci_hcd 0000:00:14.0: Successful Endpoint Configure command [ 6519.045948] xhci_hcd 0000:00:14.0: // Ding dong! [ 6519.045982] xhci_hcd 0000:00:14.0: Stopped on No-op or Link TRB for slot 6 ep 10 [ 6519.046008] xhci_hcd 0000:00:14.0: // Ding dong! [ 6519.046083] xhci_hcd 0000:00:14.0: // Ding dong! [ 6519.046107] xhci_hcd 0000:00:14.0: Stopped on No-op or Link TRB for slot 6 ep 7 [ 6519.046123] xhci_hcd 0000:00:14.0: // Ding dong! [ 6519.046239] xhci_hcd 0000:00:14.0: // Ding dong! [ 6519.046528] xhci_hcd 0000:00:14.0: Stopped on No-op or Link TRB for slot 6 ep 14 [ 6519.046813] xhci_hcd 0000:00:14.0: // Ding dong! [ 6524.122938] xhci_hcd 0000:00:14.0: Cancel URB 000000005dc311b3, dev 1, ep 0x0, starting at offset 0x13ef91170 [ 6524.122971] xhci_hcd 0000:00:14.0: // Ding dong! [ 6524.122989] xhci_hcd 0000:00:14.0: Stopped on Transfer TRB for slot 6 ep 0 [ 6524.123001] xhci_hcd 0000:00:14.0: Removing canceled TD starting at 0x13ef91170 (dma). [ 6524.123010] xhci_hcd 0000:00:14.0: Set TR Deq ptr 0x13ef91190, cycle 1 [ 6524.123016] xhci_hcd 0000:00:14.0: // Ding dong! [ 6524.123028] xhci_hcd 0000:00:14.0: Successful Set TR Deq Ptr cmd, deq = @13ef91190 [ 6524.123036] xhci_hcd 0000:00:14.0: Giveback URB 000000005dc311b3, len = 0, expected = 0, status = -115 [ 6524.124748] hid-generic 0003:2B73:001E.0006: hiddev96,hidraw0: USB HID v1.11 Device [Pioneer DJ Corporation DDJ-SR2] on usb-0000:00:14.0-1/input3 [ 6524.140705] xhci_hcd 0000:00:14.0: Waiting for status stage event [ 6524.140856] xhci_hcd 0000:00:14.0: Waiting for status stage event [ 6524.140936] usb 3-1: 0:1: added playback implicit_fb sync_ep 82, iface 0:1 [ 6524.140951] usb 3-1: Creating new data endpoint #1 [ 6524.140954] usb 3-1: Creating new data endpoint #82 [ 6524.140958] xhci_hcd 0000:00:14.0: xhci_check_bandwidth called for udev 0000000041e6ad6d [ 6524.141095] usb 3-1: 0:1 Set sample rate 44100, clock 0 [ 6524.141110] usb 3-1: 0:1: found sync_ep=0x82, iface=0, alt=1, implicit_fb=0 [ 6524.141118] xhci_hcd 0000:00:14.0: xhci_check_bandwidth called for udev 0000000041e6ad6d [ 6524.141274] usb 3-1: 0:1 Set sample rate 44100, clock 0 [ 6524.141683] usbcore: registered new interface driver snd-usb-audio [ 6530.912038] usb 3-1: Open EP 0x82, iface=0:1, idx=1 [ 6530.912047] usb 3-1: channels=6, rate=44100, format=S24_3LE, period_bytes=9216, periods=2, implicit_fb=0 [ 6530.912052] usb 3-1: Reopened EP 0x82 (count 1) [ 6530.912056] usb 3-1: Setting usb interface 0:0 for EP 0x82 [ 6530.912062] xhci_hcd 0000:00:14.0: xhci_check_bandwidth called for udev 0000000041e6ad6d [ 6530.912125] usb 3-1: Setting usb interface 0:1 for EP 0x82 [ 6530.912134] xhci_hcd 0000:00:14.0: add ep 0x1, slot id 6, new drop flags = 0x0, new add flags = 0x5 [ 6530.912141] xhci_hcd 0000:00:14.0: add ep 0x82, slot id 6, new drop flags = 0x0, new add flags = 0x25 [ 6530.912146] xhci_hcd 0000:00:14.0: xhci_check_bandwidth called for udev 0000000041e6ad6d [ 6530.912151] xhci_hcd 0000:00:14.0: Adding 2 ep ctxs, 6 now active. [ 6530.912156] xhci_hcd 0000:00:14.0: Recalculating BW for rootport 1 [ 6530.912160] xhci_hcd 0000:00:14.0: Final bandwidth: 282, Limit: 1607, Reserved: 322, Available: 62 percent [ 6530.912165] xhci_hcd 0000:00:14.0: // Ding dong! [ 6530.912221] xhci_hcd 0000:00:14.0: Successful Endpoint Configure command [ 6530.912412] usb 3-1: 0:1 Set sample rate 44100, clock 0 [ 6530.912419] usb 3-1: Setting params for data EP 0x82, pipe 0x10780 [ 6530.912428] usb 3-1: Set up 12 URBS, ret=0 [ 6530.912525] usb 3-1: Open EP 0x1, iface=0:1, idx=0 [ 6530.912530] usb 3-1: channels=4, rate=44100, format=S24_3LE, period_bytes=6144, periods=2, implicit_fb=1 [ 6530.912535] usb 3-1: Reopened EP 0x82 (count 2) [ 6530.912538] usb 3-1: 0:1 Set sample rate 44100, clock 0 [ 6530.912542] usb 3-1: Setting params for data EP 0x1, pipe 0x8700 [ 6530.912549] usb 3-1: Set up 12 URBS, ret=0 [ 6530.912575] usb 3-1: Starting data EP 0x1 (running 0) [ 6530.912579] usb 3-1: No URB submission due to implicit fb sync [ 6530.912582] usb 3-1: Starting data EP 0x82 (running 0) [ 6530.912623] usb 3-1: 12 URBs submitted for EP 0x82 [ 6530.915746] usb 3-1: Starting data EP 0x82 (running 1) [ 6530.915760] usb 3-1: Starting data EP 0x82 (running 2) [ 6530.915766] usb 3-1: 0:1 Start Capture PCM [ 6530.915772] usb 3-1: 0:1 Start Playback PCM [ 6548.340701] usb 3-1: Stopping data EP 0x82 (running 3) [ 6548.340709] usb 3-1: Stopping data EP 0x82 (running 2) [ 6548.340713] usb 3-1: 0:1 Stop Capture PCM [ 6548.340717] usb 3-1: Stopping data EP 0x82 (running 1) [ 6548.340724] xhci_hcd 0000:00:14.0: Cancel URB 0000000017eaed8f, dev 1, ep 0x82, starting at offset 0x13eef0b40 [ 6548.340731] xhci_hcd 0000:00:14.0: // Ding dong! [ 6548.340735] xhci_hcd 0000:00:14.0: Cancel URB 00000000d1c36d1d, dev 1, ep 0x82, starting at offset 0x13eef0b50 [ 6548.340741] xhci_hcd 0000:00:14.0: Stopped on Transfer TRB for slot 6 ep 4 [ 6548.340746] xhci_hcd 0000:00:14.0: Removing canceled TD starting at 0x13eef0b40 (dma). [ 6548.340750] xhci_hcd 0000:00:14.0: Removing canceled TD starting at 0x13eef0b50 (dma). [ 6548.340758] xhci_hcd 0000:00:14.0: Cancel URB 000000007dcb7021, dev 1, ep 0x82, starting at offset 0x13eef0b60 [ 6548.340762] xhci_hcd 0000:00:14.0: // Ding dong! [ 6548.340766] xhci_hcd 0000:00:14.0: Cancel URB 00000000b381540e, dev 1, ep 0x82, starting at offset 0x13eef0b70 [ 6548.340769] xhci_hcd 0000:00:14.0: Cancel URB 00000000059964b3, dev 1, ep 0x82, starting at offset 0x13eef0b80 [ 6548.340773] xhci_hcd 0000:00:14.0: Cancel URB 0000000009d6cac7, dev 1, ep 0x82, starting at offset 0x13eef0b90 [ 6548.340777] xhci_hcd 0000:00:14.0: Cancel URB 000000008fc1dbd6, dev 1, ep 0x82, starting at offset 0x13eef0ba0 [ 6548.340783] xhci_hcd 0000:00:14.0: Stopped on Transfer TRB for slot 6 ep 4 [ 6548.340786] xhci_hcd 0000:00:14.0: Removing canceled TD starting at 0x13eef0b60 (dma). [ 6548.340789] xhci_hcd 0000:00:14.0: Removing canceled TD starting at 0x13eef0b70 (dma). [ 6548.340792] xhci_hcd 0000:00:14.0: Removing canceled TD starting at 0x13eef0b80 (dma). [ 6548.340795] xhci_hcd 0000:00:14.0: Removing canceled TD starting at 0x13eef0b90 (dma). [ 6548.340798] xhci_hcd 0000:00:14.0: Removing canceled TD starting at 0x13eef0ba0 (dma). [ 6548.340806] xhci_hcd 0000:00:14.0: Cancel URB 00000000dd5ca99e, dev 1, ep 0x82, starting at offset 0x13eef0bb0 [ 6548.340810] xhci_hcd 0000:00:14.0: // Ding dong! [ 6548.340814] xhci_hcd 0000:00:14.0: Cancel URB 000000003c889fa7, dev 1, ep 0x82, starting at offset 0x13eef0bc0 [ 6548.340818] xhci_hcd 0000:00:14.0: Cancel URB 000000005c402de4, dev 1, ep 0x82, starting at offset 0x13eef0b10 [ 6548.340823] xhci_hcd 0000:00:14.0: Stopped on Transfer TRB for slot 6 ep 4 [ 6548.340826] xhci_hcd 0000:00:14.0: Removing canceled TD starting at 0x13eef0bb0 (dma). [ 6548.340829] xhci_hcd 0000:00:14.0: Removing canceled TD starting at 0x13eef0bc0 (dma). [ 6548.340832] xhci_hcd 0000:00:14.0: Removing canceled TD starting at 0x13eef0b10 (dma). [ 6548.340836] xhci_hcd 0000:00:14.0: Set TR Deq ptr 0x13eef0b20, cycle 1 [ 6548.340839] xhci_hcd 0000:00:14.0: // Ding dong! [ 6548.340848] xhci_hcd 0000:00:14.0: Cancel URB 0000000012679da4, dev 1, ep 0x82, starting at offset 0x13eef0b20 [ 6548.340851] xhci_hcd 0000:00:14.0: // Ding dong! [ 6548.340855] xhci_hcd 0000:00:14.0: Cancel URB 00000000353acb81, dev 1, ep 0x82, starting at offset 0x13eef0b30 [ 6548.340860] usb 3-1: Stopping data EP 0x1 (running 1) [ 6548.340861] xhci_hcd 0000:00:14.0: Successful Set TR Deq Ptr cmd, deq = @13eef0b20 [ 6548.340864] usb 3-1: 0:1 Stop Playback PCM [ 6548.340866] xhci_hcd 0000:00:14.0: Removing canceled TD starting at 0x13eef0b20 (dma). [ 6548.340869] xhci_hcd 0000:00:14.0: Removing canceled TD starting at 0x13eef0b30 (dma). [ 6548.340872] xhci_hcd 0000:00:14.0: Set TR Deq ptr 0x13eef0b30, cycle 1 [ 6548.340875] xhci_hcd 0000:00:14.0: // Ding dong! [ 6548.340901] xhci_hcd 0000:00:14.0: Successful Set TR Deq Ptr cmd, deq = @13eef0b30 [ 6548.350377] usb 3-1: Closing EP 0x82 (count 3) [ 6548.350384] usb 3-1: Closing EP 0x82 (count 2) [ 6548.350396] usb 3-1: Closing EP 0x1 (count 1) [ 6548.350398] usb 3-1: EP 0x1 closed [ 6548.350400] usb 3-1: Closing EP 0x82 (count 1) [ 6548.350401] usb 3-1: Setting usb interface 0:0 for EP 0x82 [ 6548.350406] xhci_hcd 0000:00:14.0: xhci_drop_endpoint called for udev 0000000041e6ad6d [ 6548.350423] xhci_hcd 0000:00:14.0: drop ep 0x1, slot id 6, new drop flags = 0x4, new add flags = 0x0 [ 6548.350426] xhci_hcd 0000:00:14.0: xhci_drop_endpoint called for udev 0000000041e6ad6d [ 6548.350435] xhci_hcd 0000:00:14.0: drop ep 0x82, slot id 6, new drop flags = 0x24, new add flags = 0x0 [ 6548.350438] xhci_hcd 0000:00:14.0: xhci_check_bandwidth called for udev 0000000041e6ad6d [ 6548.350442] xhci_hcd 0000:00:14.0: Adding 0 ep ctxs, 6 now active. [ 6548.350445] xhci_hcd 0000:00:14.0: Recalculating BW for rootport 1 [ 6548.350447] xhci_hcd 0000:00:14.0: Final bandwidth: 42, Limit: 1607, Reserved: 322, Available: 77 percent [ 6548.350451] xhci_hcd 0000:00:14.0: // Ding dong! [ 6548.350509] xhci_hcd 0000:00:14.0: Successful Endpoint Configure command [ 6548.350512] xhci_hcd 0000:00:14.0: Removing 2 dropped ep ctxs, 4 now active. [ 6548.350644] usb 3-1: EP 0x82 closed
---
Bus 003 Device 007: ID 2b73:001e Pioneer DJ Corporation DDJ-SR2 Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass 0 bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 64 idVendor 0x2b73 idProduct 0x001e bcdDevice 1.01 iManufacturer 1 Pioneer DJ Corporation iProduct 2 DDJ-SR2 iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 0x00a2 bNumInterfaces 4 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 0 bInterfaceProtocol 0 iInterface 0 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 1 bNumEndpoints 2 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 0 iInterface 0 Endpoint Descriptor: bLength 9 bDescriptorType 5 bEndpointAddress 0x01 EP 1 OUT bmAttributes 5 Transfer Type Isochronous Synch Type Asynchronous Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 3 bRefresh 0 bSynchAddress 0 Endpoint Descriptor: bLength 9 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 5 Transfer Type Isochronous Synch Type Asynchronous Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 3 bRefresh 0 bSynchAddress 0 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 1 bAlternateSetting 0 bNumEndpoints 0 bInterfaceClass 1 Audio bInterfaceSubClass 1 Control Device bInterfaceProtocol 0 iInterface 0 AudioControl Interface Descriptor: bLength 9 bDescriptorType 36 bDescriptorSubtype 1 (HEADER) bcdADC 1.00 wTotalLength 0x0009 bInCollection 1 baInterfaceNr(0) 2 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 2 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 1 Audio bInterfaceSubClass 3 MIDI Streaming bInterfaceProtocol 0 iInterface 0 MIDIStreaming Interface Descriptor: bLength 7 bDescriptorType 36 bDescriptorSubtype 1 (HEADER) bcdADC 1.00 wTotalLength 0x0041 MIDIStreaming Interface Descriptor: bLength 6 bDescriptorType 36 bDescriptorSubtype 2 (MIDI_IN_JACK) bJackType 1 Embedded bJackID 3 iJack 0 MIDIStreaming Interface Descriptor: bLength 6 bDescriptorType 36 bDescriptorSubtype 2 (MIDI_IN_JACK) bJackType 2 External bJackID 1 iJack 0 MIDIStreaming Interface Descriptor: bLength 9 bDescriptorType 36 bDescriptorSubtype 3 (MIDI_OUT_JACK) bJackType 2 External bJackID 4 bNrInputPins 1 baSourceID( 0) 3 BaSourcePin( 0) 1 iJack 0 MIDIStreaming Interface Descriptor: bLength 9 bDescriptorType 36 bDescriptorSubtype 3 (MIDI_OUT_JACK) bJackType 1 Embedded bJackID 2 bNrInputPins 1 baSourceID( 0) 1 BaSourcePin( 0) 1 iJack 0 Endpoint Descriptor: bLength 9 bDescriptorType 5 bEndpointAddress 0x85 EP 5 IN bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 bRefresh 0 bSynchAddress 0 MIDIStreaming Endpoint Descriptor: bLength 5 bDescriptorType 37 bDescriptorSubtype 1 (GENERAL) bNumEmbMIDIJack 1 baAssocJackID( 0) 2 Endpoint Descriptor: bLength 9 bDescriptorType 5 bEndpointAddress 0x04 EP 4 OUT bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 bRefresh 0 bSynchAddress 0 MIDIStreaming Endpoint Descriptor: bLength 5 bDescriptorType 37 bDescriptorSubtype 1 (GENERAL) bNumEmbMIDIJack 1 baAssocJackID( 0) 3 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 3 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 3 Human Interface Device bInterfaceSubClass 0 bInterfaceProtocol 0 iInterface 0 HID Device Descriptor: bLength 9 bDescriptorType 33 bcdHID 1.11 bCountryCode 0 Not supported bNumDescriptors 1 bDescriptorType 34 Report wDescriptorLength 52 Report Descriptors: ** UNAVAILABLE ** Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x87 EP 7 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 4 can't get debug descriptor: Resource temporarily unavailable Device Status: 0x0001 Self Powered
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@gmail.com
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; }
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@suse.de 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@gmail.com
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;}
On Mon, Apr 05, 2021 at 10:49:20AM -0300, 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@gmail.com
Thank you Geraldo and Takashi for working on this patch. I'm currently in the process of testing with the DJM-750 but it currently does not work as expected. I'll debug futher and report back but would like to make you aware of this in the meantime. It may not be a a fundamental issue with the patch, but rather something incidental.
Kind regards, Olivia
Hi, Olivia, thank you, glad you catched this and are able to test and debug it. Let me know if I can be of any assistance.
Em Sex, 9 de abr de 2021 08:09, Olivia Mackintosh livvy@base.nu escreveu:
On Mon, Apr 05, 2021 at 10:49:20AM -0300, 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@gmail.com
Thank you Geraldo and Takashi for working on this patch. I'm currently in the process of testing with the DJM-750 but it currently does not work as expected. I'll debug futher and report back but would like to make you aware of this in the meantime. It may not be a a fundamental issue with the patch, but rather something incidental.
Kind regards, Olivia
On Fri, 09 Apr 2021 13:07:45 +0200, Olivia Mackintosh wrote:
On Mon, Apr 05, 2021 at 10:49:20AM -0300, 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@gmail.com
Thank you Geraldo and Takashi for working on this patch. I'm currently in the process of testing with the DJM-750 but it currently does not work as expected. I'll debug futher and report back but would like to make you aware of this in the meantime. It may not be a a fundamental issue with the patch, but rather something incidental.
Which kernel version have you tested? There have been quite a few development about USB-audio recently, so something might be missing or conflicting? Let's see.
FWIW, below is a proper patch that is applied on top of for-next branch of sound.git tree (destined for 5.13 kernel merge).
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: usb-audio: Re-apply implicit feedback mode to Pioneer devices
Pioneer devices are supposed to be working with the implicit feedback mode, but so far the attempt to applying this caused issues, hence we explicitly skipped the implicit feedback mode for them. Recently, Geraldo discovered that the device actually works if you skip the generic matching of the sync EPs for the capture stream. That is, we should apply the implicit feedback setup for the playback like other similar devices, while we need to return 1 from audioformat_capture_quirk() so that no further matching will be done.
This patch implement the application of the implicit feedback to Pioneer devices. The former skip_pioneer_sync_ep() was dropped, and instead we provide is_pioneer_implicit_fb() to check the Pioneer devices that need the implicit feedback. In the audioformat_implicit_fb_quirk(), simply apply the implicit fb for playback if matching, and in audioformat_capture_quirk()(), it returns 1 if matching, as mentioned in the above.
Reported-by: Geraldo geraldogabriel@gmail.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/implicit.c | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-)
diff --git a/sound/usb/implicit.c b/sound/usb/implicit.c index 4bd9c105a529..427ed0ff3b7b 100644 --- a/sound/usb/implicit.c +++ b/sound/usb/implicit.c @@ -171,18 +171,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) || @@ -191,8 +200,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, @@ -308,13 +318,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) @@ -335,6 +343,8 @@ 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); + if (is_pioneer_implicit_fb(chip, alts)) + return 1; /* skip */ return 0; }
Which kernel version have you tested? There have been quite a few development about USB-audio recently, so something might be missing or conflicting? Let's see.
I have just tested d86f43b17ed4cd751f73d890ea63f818ffa5ef3d with and without the patch:
- Without the patch, everything works fine. - With the patch, speaker-test times out. I'll try to collect some more infomation from dyndb and try to have a look to see what the problem is.
There's no obvious mistakes in the patch as far as I can tell.
Olivia
Sorry to hear that, Olivia.
I have to ask, though, is capture working for your device? If not, then it might be related.
Em Sáb, 17 de abr de 2021 19:28, Olivia Mackintosh livvy@base.nu escreveu:
Which kernel version have you tested? There have been quite a few development about USB-audio recently, so something might be missing or conflicting? Let's see.
I have just tested d86f43b17ed4cd751f73d890ea63f818ffa5ef3d with and without the patch:
- Without the patch, everything works fine.
- With the patch, speaker-test times out. I'll try to collect some more infomation from dyndb and try to have a look to see what the problem is.
There's no obvious mistakes in the patch as far as I can tell.
Olivia
On Sun, 18 Apr 2021 00:24:21 +0200, Olivia Mackintosh wrote:
Which kernel version have you tested? There have been quite a few development about USB-audio recently, so something might be missing or conflicting? Let's see.
I have just tested d86f43b17ed4cd751f73d890ea63f818ffa5ef3d with and without the patch:
- Without the patch, everything works fine.
- With the patch, speaker-test times out. I'll try to collect some more infomation from dyndb and try to have a look to see what the problem is.
There's no obvious mistakes in the patch as far as I can tell.
Thanks for testing. A possible difference of speaker-test from others is that speaker-test tends to set up with the larger period and buffer sizes. I guess the same problem could be seen when you run aplay with the same parameters shown in /proc/asound/card*/pcm0p/sub0/hw_params, too.
As a blind shot: might the stall be avoided by passing the recently introduced chip->playback_first flag? The revised patch is below.
Takashi
--- --- a/sound/usb/implicit.c +++ b/sound/usb/implicit.c @@ -230,18 +230,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) || @@ -250,8 +259,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, @@ -367,12 +377,12 @@ 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)) { + chip->playback_first = 1; + 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 */ @@ -394,6 +404,8 @@ 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); + if (is_pioneer_implicit_fb(chip, alts)) + return 1; /* skip */ return 0; }
As a blind shot: might the stall be avoided by passing the recently introduced chip->playback_first flag? The revised patch is below.
This appears to fix the issue, thank you! I am curious as to why this is not required for Geraldo's DDJ-SR2.
My initial thought was that certain devices that restrict capture unless regular SysEx/Control URBs are sent may mean that Playback first support is required (e.g. 750, 850, 450). Please correct me if this intuition is incorrect.
Olivia
Em Dom, 18 de abr de 2021 09:45, Olivia Mackintosh livvy@base.nu escreveu:
As a blind shot: might the stall be avoided by passing the recently introduced chip->playback_first flag? The revised patch is below.
This appears to fix the issue, thank you! I am curious as to why this is not required for Geraldo's DDJ-SR2.
Heh, sometimes I think Takashi Iwai is psychic ;-)
Just kidding, he's "just" very talented and very experienced. Anyway, the DDJ-SR2 is originally a Serato controller. It still requires two MIDI SysEx messages to unmute the RCA inputs - engaging "Serato Mode" in marketing-speak.
My point is that Pioneer may be a little more compliant / a little less deviant from the UAC2 standard when designing gear for use with third-party software.
My initial thought was that certain devices that restrict capture unless regular SysEx/Control URBs are sent may mean that Playback first support is required (e.g. 750, 850, 450). Please correct me if this intuition is incorrect.
Olivia
On Sun, 18 Apr 2021 16:16:56 +0200, Geraldo Nascimento wrote:
Em Dom, 18 de abr de 2021 09:45, Olivia Mackintosh livvy@base.nu escreveu:
> As a blind shot: might the stall be avoided by passing the recently > introduced chip->playback_first flag? The revised patch is below. This appears to fix the issue, thank you! I am curious as to why this is not required for Geraldo's DDJ-SR2.Heh, sometimes I think Takashi Iwai is psychic ;-)
Heh, it was our luck that we've worked on the similar problem very recently.
Just kidding, he's "just" very talented and very experienced. Anyway, the DDJ-SR2 is originally a Serato controller. It still requires two MIDI SysEx messages to unmute the RCA inputs - engaging "Serato Mode" in marketing-speak.
My point is that Pioneer may be a little more compliant / a little less deviant from the UAC2 standard when designing gear for use with third-party software.
I think many devices require the implicit feedback seem following this pattern: for such devices, the full-duplex streams are mandatory, i.e. both streams have to be started always. And the implicit feedback is triggered at the later point for a proper sync.
My initial thought was that certain devices that restrict capture unless regular SysEx/Control URBs are sent may mean that Playback first support is required (e.g. 750, 850, 450). Please correct me if this intuition is incorrect.
As mentioned previously, it might be the difference of the PCM parameters to be deployed. I guess Geraldo didn't test speaker-test program but maybe only the standard sound backends like JACK, etc.
In anyway, I'm going to format and submit the proper patch for merging.
thanks,
Takashi
participants (4)
-
Geraldo -
Geraldo Nascimento
-
Olivia Mackintosh -
Takashi Iwai