[alsa-devel] How to use implicit feedback with full duplex?
Hi,
I thought I'd try to use implicit feedback with my simple audio device:
Interface Descriptor: bInterfaceNumber 1 bAlternateSetting 0 Interface Descriptor: bInterfaceNumber 1 bAlternateSetting 1 AudioStreaming Interface Descriptor: ... Endpoint Descriptor: bEndpointAddress 0x01 EP 1 OUT Interface Descriptor: bInterfaceNumber 2 bAlternateSetting 0 Interface Descriptor: bInterfaceNumber 2 bAlternateSetting 1 AudioStreaming Interface Descriptor: ... Endpoint Descriptor: bEndpointAddress 0x82 EP 2 IN
So I wrote a patch to configure it like those M-Audio devices:
--- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -391,6 +391,17 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) attr = fmt->ep_attr & USB_ENDPOINT_SYNCTYPE;
switch (subs->stream->chip->usb_id) { + case USB_ID(0x0582, 0x0018): + if (is_playback) { + implicit_fb = 1; + ep = 0x82; + iface = usb_ifnum_to_if(dev, 2); + if (!iface || iface->num_altsetting < 2) + return -EINVAL; + alts = &iface->altsetting[1]; + goto add_sync_ep; + } + break; case USB_ID(0x0763, 0x2030): /* M-Audio Fast Track C400 */ if (is_playback) { implicit_fb = 1;
This works fine when playing something:
kernel: setting usb interface 1:1 kernel: Creating new playback data endpoint #1 kernel: Creating new capture data endpoint #82 kernel: Setting params for ep #1 (type 0, 8 urbs), ret=0 kernel: match_endpoint_audioformats: (fmt @ffff8801fefa8900) score 2 kernel: Setting params for ep #82 (type 0, 8 urbs), ret=0 kernel: Starting data EP @ffff8801fd4e8000 kernel: Starting sync EP @ffff8801fd2ac000
But when I then try to record at the same time, the driver refuses to configure the input endpoint (to the only format, which is already set):
kernel: setting usb interface 2:1 kernel: Re-using EP 82 in iface 2,1 @ffff8801fd2ac000 kernel: Unable to change format on ep #82: already in use
And despite that "alreay in use" check, the input endpoint is affected so much that playback breaks.
Is full duplex supposed to work? Does it work with other devices?
Regards, Clemens
Hi Clemens,
On Tue, 5 Feb 2013, Clemens Ladisch wrote:
Hi,
I thought I'd try to use implicit feedback with my simple audio device:
Interface Descriptor: bInterfaceNumber 1 bAlternateSetting 0 Interface Descriptor: bInterfaceNumber 1 bAlternateSetting 1 AudioStreaming Interface Descriptor: ... Endpoint Descriptor: bEndpointAddress 0x01 EP 1 OUT Interface Descriptor: bInterfaceNumber 2 bAlternateSetting 0 Interface Descriptor: bInterfaceNumber 2 bAlternateSetting 1 AudioStreaming Interface Descriptor: ... Endpoint Descriptor: bEndpointAddress 0x82 EP 2 IN
So I wrote a patch to configure it like those M-Audio devices:
--- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -391,6 +391,17 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) attr = fmt->ep_attr & USB_ENDPOINT_SYNCTYPE;
switch (subs->stream->chip->usb_id) {
- case USB_ID(0x0582, 0x0018):
if (is_playback) {
implicit_fb = 1;
ep = 0x82;
iface = usb_ifnum_to_if(dev, 2);
if (!iface || iface->num_altsetting < 2)
return -EINVAL;
alts = &iface->altsetting[1];
goto add_sync_ep;
}
break;
Looks good to me.
case USB_ID(0x0763, 0x2030): /* M-Audio Fast Track C400 */ if (is_playback) { implicit_fb = 1;
This works fine when playing something:
...
But when I then try to record at the same time, the driver refuses to configure the input endpoint (to the only format, which is already set):
kernel: setting usb interface 2:1 kernel: Re-using EP 82 in iface 2,1 @ffff8801fd2ac000 kernel: Unable to change format on ep #82: already in use
And despite that "alreay in use" check, the input endpoint is affected so much that playback breaks.
Is full duplex supposed to work? Does it work with other devices?
This is probably a "yes, but" :) I use my device mostly full duplex, but with jack opening both playback and capture at the same time.
I assume you are opening two different streams, one for playback and one for capture. In that case, the capture EP is of course already in use, as a source for timing. If I try opening 2 different streams I get the same here - but playback doesn't break for me. I wonder why that happens.
Can you try using jackd -d alsa -d hw:x with the device and see if that works for you?
Cheers, Eldad
Eldad Zack wrote:
On Tue, 5 Feb 2013, Clemens Ladisch wrote:
I thought I'd try to use implicit feedback with my simple audio device: [...] This works fine when playing something:
...
But when I then try to record at the same time, the driver refuses to configure the input endpoint (to the only format, which is already set): And despite that "alreay in use" check, the input endpoint is affected so much that playback breaks.
Is full duplex supposed to work? Does it work with other devices?
This is probably a "yes, but" :) I use my device mostly full duplex, but with jack opening both playback and capture at the same time.
I assume you are opening two different streams, one for playback and one for capture.
Jack *also* uses two different streams, but it opens them at the same time.
Can you try using jackd -d alsa -d hw:x with the device and see if that works for you?
That works. This means that there is a race condition in the driver, or that the different open/hw_params/prepare order trips it up.
Anyway, I feel safe now to enable implicit feedback for other devices.
Regards, Clemens
On 06.02.2013 21:13, Clemens Ladisch wrote:
Eldad Zack wrote:
On Tue, 5 Feb 2013, Clemens Ladisch wrote:
I thought I'd try to use implicit feedback with my simple audio device: [...] This works fine when playing something:
...
But when I then try to record at the same time, the driver refuses to configure the input endpoint (to the only format, which is already set): And despite that "alreay in use" check, the input endpoint is affected so much that playback breaks.
Is full duplex supposed to work? Does it work with other devices?
This is probably a "yes, but" :) I use my device mostly full duplex, but with jack opening both playback and capture at the same time.
I assume you are opening two different streams, one for playback and one for capture.
Jack *also* uses two different streams, but it opens them at the same time.
Can you try using jackd -d alsa -d hw:x with the device and see if that works for you?
That works. This means that there is a race condition in the driver, or that the different open/hw_params/prepare order trips it up.
I'll look for the FTU and re-try this with Eldad's latest additions once I find some time. If there are races, we should definitely fix them.
Anyway, I feel safe now to enable implicit feedback for other devices.
Nice.
On a general note, my idea from the beginning and part of the motivation to split the driver into the snd_usb_endpoint logic was to pull that class out to a separate library, so we can use it from other drivers as well. Much like what snd-usbmidi does.
There are few bits to clean up in order to remove dependencies between the endpoint code and the rest of the driver, but then your ua101 driver, snd-usb-caiaq and probably some more drivers in the future can share the generic streaming functionality.
Daniel
Hi Daniel, Hi Eldad.
2013/2/6 Daniel Mack zonque@gmail.com
I'll look for the FTU and re-try this with Eldad's latest additions once I find some time.
I'll probably find some time later next week. I haven't followed thoroughly what Eldad did for the C400/C600 but wanted to play around with the FTU anyway (if I remember correctly there was a patch dropping the first n implicit feedback packets on the C400 that might possibly be of benefit for the FTUs as well).
@Eldad Have your latest additions already made it into Takashi's tree or where can I find them?
At Thu, 7 Feb 2013 10:03:20 +0100, Felix Homann wrote:
Hi Daniel, Hi Eldad.
2013/2/6 Daniel Mack zonque@gmail.com
I'll look for the FTU and re-try this with Eldad's latest additions once I find some time.
I'll probably find some time later next week. I haven't followed thoroughly what Eldad did for the C400/C600 but wanted to play around with the FTU anyway (if I remember correctly there was a patch dropping the first n implicit feedback packets on the C400 that might possibly be of benefit for the FTUs as well).
@Eldad Have your latest additions already made it into Takashi's tree or where can I find them?
It should be all in my tree. If not, let me know. The merge window for 3.9 is pretty close.
thanks,
Takashi
On Thu, 7 Feb 2013, Takashi Iwai wrote:
At Thu, 7 Feb 2013 10:03:20 +0100, Felix Homann wrote:
Hi Daniel, Hi Eldad.
2013/2/6 Daniel Mack zonque@gmail.com
I'll look for the FTU and re-try this with Eldad's latest additions once I find some time.
I'll probably find some time later next week. I haven't followed thoroughly what Eldad did for the C400/C600 but wanted to play around with the FTU anyway (if I remember correctly there was a patch dropping the first n implicit feedback packets on the C400 that might possibly be of benefit for the FTUs as well).
@Eldad Have your latest additions already made it into Takashi's tree or where can I find them?
It should be all in my tree. If not, let me know. The merge window for 3.9 is pretty close.
Thanks for the heads up! I do have some general addition to do automatic clock switching in case the clock source currently selected is not valid, but it's not ready for submission. I just didn't have time to put it into good shape yet.
Cheers, Eldad
Hi Felix,
On Thu, 7 Feb 2013, Felix Homann wrote:
Hi Daniel, Hi Eldad.
2013/2/6 Daniel Mack zonque@gmail.com I'll look for the FTU and re-try this with Eldad's latest additions once I find some time.
I'll probably find some time later next week. I haven't followed thoroughly what Eldad did for the C400/C600 but wanted to play around with the FTU anyway (if I remember correctly there was a patch dropping the first n implicit feedback packets on the C400 that might possibly be of benefit for the FTUs as well).
I only added C400 - but I hope the asymmetry-related patches will save time with the C600.
Do you get different latency every time you start up a stream on the FTU? That's the reason for the skip_packet quirk for the C400.
@Eldad Have your latest additions already made it into Takashi's tree or where can I find them?
Yes and they are all merged upstream by now. But they are mostly small things and specific to the C400.
Cheers, Eldad
On Wed, 6 Feb 2013, Clemens Ladisch wrote:
Eldad Zack wrote:
On Tue, 5 Feb 2013, Clemens Ladisch wrote:
I thought I'd try to use implicit feedback with my simple audio device: [...] This works fine when playing something:
...
But when I then try to record at the same time, the driver refuses to configure the input endpoint (to the only format, which is already set): And despite that "alreay in use" check, the input endpoint is affected so much that playback breaks.
Is full duplex supposed to work? Does it work with other devices?
This is probably a "yes, but" :) I use my device mostly full duplex, but with jack opening both playback and capture at the same time.
I assume you are opening two different streams, one for playback and one for capture.
Jack *also* uses two different streams, but it opens them at the same time.
Oh, right.
Can you try using jackd -d alsa -d hw:x with the device and see if that works for you?
That works. This means that there is a race condition in the driver, or that the different open/hw_params/prepare order trips it up.
Hmm. I seem to have tested this wrong the other day or just had luck (I tried it just once). But today I can trigger breaking the playback when opening capture later every time I try, just like you described.
I get this too when the audio breaks: [ 766.987497] xhci_hcd 0000:00:14.0: shutdown urb ffff880244f89600 ep1in-iso [ 766.987510] xhci_hcd 0000:00:14.0: shutdown urb ffff880244f89400 ep1in-iso [ 766.987519] xhci_hcd 0000:00:14.0: shutdown urb ffff8802438e6000 ep1in-iso [ 766.987527] xhci_hcd 0000:00:14.0: shutdown urb ffff8802438e6200 ep1in-iso [ 766.987534] xhci_hcd 0000:00:14.0: WARN Event TRB for slot 1 ep 2 with no TDs queued? [ 766.987550] xhci_hcd 0000:00:14.0: shutdown urb ffff8802438e6400 ep1in-iso [ 766.987562] xhci_hcd 0000:00:14.0: shutdown urb ffff880244f89000 ep1in-iso [ 766.987576] xhci_hcd 0000:00:14.0: shutdown urb ffff880244f89200 ep1in-iso [ 766.987584] xhci_hcd 0000:00:14.0: shutdown urb ffff880244f88800 ep1in-iso
I will try to figure out what's happening on the weekend.
Cheers, Eldad
Hi Clemens,
On Wed, 6 Feb 2013, Clemens Ladisch wrote:
Eldad Zack wrote:
On Tue, 5 Feb 2013, Clemens Ladisch wrote:
I thought I'd try to use implicit feedback with my simple audio device: [...] This works fine when playing something:
...
But when I then try to record at the same time, the driver refuses to configure the input endpoint (to the only format, which is already set): And despite that "alreay in use" check, the input endpoint is affected so much that playback breaks.
Is full duplex supposed to work? Does it work with other devices?
This is probably a "yes, but" :) I use my device mostly full duplex, but with jack opening both playback and capture at the same time.
I assume you are opening two different streams, one for playback and one for capture.
Jack *also* uses two different streams, but it opens them at the same time.
Can you try using jackd -d alsa -d hw:x with the device and see if that works for you?
That works. This means that there is a race condition in the driver, or that the different open/hw_params/prepare order trips it up.
Yes, and also on closing (close/hw_free). I finally had some time to look into this - patch below adds some checks and with it:
* Starting playback, waiting, starting capture: capture doesn't start, playback continues without breaking.
* Starting capture, waiting, starting playback: playback doesn't start, capture breaks - but it is possible to restart the streams afterwards. [On my setup (with xhci), when the streams break I must restart my box to get them to work again with current code].
* Starting both - no change, works good.
This is the order the ops get called when both are start at once, if it helps anyone:
playback capture open open hw_params set_format prepare set_format hw_params set_format prepare set_format prepare set_format prepare set_format trigger trigger
I'll try to figure out why capture breaks next. I still quite slow around the code and don't understand some parts of it so this might take me a while.
@Daniel, do you have any hints for this case (capture breaking when starting playback)?
I can also submit this as a proper patch if it makes sense.
Cheers, Eldad
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 7e9c55a..7cdf9b3 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -934,12 +934,12 @@ int snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep) if (!ep) return -EINVAL;
- deactivate_urbs(ep, true); - wait_clear_urbs(ep); - if (ep->use_count != 0) return 0;
+ deactivate_urbs(ep, true); + wait_clear_urbs(ep); + clear_bit(EP_FLAG_ACTIVATED, &ep->flags);
return 0; diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 9531355..76d7e18 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -323,6 +323,19 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) if (fmt == subs->cur_audiofmt) return 0;
+ subs->data_endpoint = snd_usb_add_endpoint(subs->stream->chip, + alts, fmt->endpoint, subs->direction, + SND_USB_ENDPOINT_TYPE_DATA); + if (!subs->data_endpoint) + return -EINVAL; + + if (subs->data_endpoint->use_count != 0) { + snd_printk("set_format ep use count !0\n"); + subs->cur_audiofmt = fmt; + snd_usb_set_format_quirk(subs, fmt); + return 0; + } + /* close the old interface */ if (subs->interface >= 0 && subs->interface != fmt->iface) { err = usb_set_interface(subs->dev, subs->interface, 0); @@ -350,12 +363,6 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) subs->altset_idx = fmt->altset_idx; }
- subs->data_endpoint = snd_usb_add_endpoint(subs->stream->chip, - alts, fmt->endpoint, subs->direction, - SND_USB_ENDPOINT_TYPE_DATA); - if (!subs->data_endpoint) - return -EINVAL; - /* we need a sync pipe in async OUT or adaptive IN mode */ /* check the number of EP, since some devices have broken * descriptors which fool us. if it has only one EP, @@ -1128,7 +1135,8 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream, int direction)
stop_endpoints(subs, true);
- if (!as->chip->shutdown && subs->interface >= 0) { + if (!as->chip->shutdown && subs->interface >= 0 && + subs->data_endpoint->use_count == 0) { usb_set_interface(subs->dev, subs->interface, 0); subs->interface = -1; }
On 24.03.2013 23:49, Eldad Zack wrote:
Hi Clemens,
On Wed, 6 Feb 2013, Clemens Ladisch wrote:
Eldad Zack wrote:
On Tue, 5 Feb 2013, Clemens Ladisch wrote:
I thought I'd try to use implicit feedback with my simple audio device: [...] This works fine when playing something:
...
But when I then try to record at the same time, the driver refuses to configure the input endpoint (to the only format, which is already set): And despite that "alreay in use" check, the input endpoint is affected so much that playback breaks.
Is full duplex supposed to work? Does it work with other devices?
This is probably a "yes, but" :) I use my device mostly full duplex, but with jack opening both playback and capture at the same time.
I assume you are opening two different streams, one for playback and one for capture.
Jack *also* uses two different streams, but it opens them at the same time.
Can you try using jackd -d alsa -d hw:x with the device and see if that works for you?
That works. This means that there is a race condition in the driver, or that the different open/hw_params/prepare order trips it up.
Yes, and also on closing (close/hw_free). I finally had some time to look into this - patch below adds some checks and with it:
Starting playback, waiting, starting capture: capture doesn't start, playback continues without breaking.
Starting capture, waiting, starting playback: playback doesn't start, capture breaks - but it is possible to restart the streams afterwards. [On my setup (with xhci), when the streams break I must restart my box to get them to work again with current code].
Starting both - no change, works good.
This is the order the ops get called when both are start at once, if it helps anyone:
playback capture open open hw_params set_format prepare set_format hw_params set_format prepare set_format prepare set_format prepare set_format trigger trigger
I'll try to figure out why capture breaks next. I still quite slow around the code and don't understand some parts of it so this might take me a while.
Thanks for looking into this!
@Daniel, do you have any hints for this case (capture breaking when starting playback)?
Give me some time to catch up and rebuild my FTU setup please. I hope I'll be able to get back to you by the end of this week.
Daniel
On Tue, 26 Mar 2013, Daniel Mack wrote:
On 24.03.2013 23:49, Eldad Zack wrote:
On Wed, 6 Feb 2013, Clemens Ladisch wrote:
Can you try using jackd -d alsa -d hw:x with the device and see if that works for you?
That works. This means that there is a race condition in the driver, or that the different open/hw_params/prepare order trips it up.
Yes, and also on closing (close/hw_free). I finally had some time to look into this - patch below adds some checks and with it:
Starting playback, waiting, starting capture: capture doesn't start, playback continues without breaking.
Starting capture, waiting, starting playback: playback doesn't start, capture breaks - but it is possible to restart the streams afterwards. [On my setup (with xhci), when the streams break I must restart my box to get them to work again with current code].
Starting both - no change, works good.
This is the order the ops get called when both are start at once, if it helps anyone:
playback capture open open hw_params set_format prepare set_format hw_params set_format prepare set_format prepare set_format prepare set_format trigger trigger
I'll try to figure out why capture breaks next. I still quite slow around the code and don't understand some parts of it so this might take me a while.
Thanks for looking into this!
@Daniel, do you have any hints for this case (capture breaking when starting playback)?
Give me some time to catch up and rebuild my FTU setup please. I hope I'll be able to get back to you by the end of this week.
Cool, thank you!
And it just remembered something. I tried to make the "in use" check non-fatal (return 0) and with a couple of more changes (very hacky) I got it to start in both orders.
But the problem was that when I stopped the playback* stream my kernel "exploded" (hard lock up). But if I stopped the capture, it was ok. Playback didn't even stop. (It might have been the other way around, but I think it was playback)
I can find that branch when I find some time if you think it might help.
Cheers, Eldad
participants (5)
-
Clemens Ladisch
-
Daniel Mack
-
Eldad Zack
-
Felix Homann
-
Takashi Iwai