At Mon, 7 Oct 2013 21:26:57 +0200 (CEST), Eldad Zack wrote:
On Mon, 7 Oct 2013, Takashi Iwai wrote:
At Sun, 6 Oct 2013 22:31:19 +0200, Eldad Zack wrote:
Start the endpoints at prepare also for capture endpoints, since it might be needed to wait for the URBs to be unlinked.
If an implicit feedback source endpoint stops being used by its sink endpoint, but immediately used as a data endpoint, usb_submit_urb will return -EBUSY.
Merge two trigger cases since they are now the same.
This change worries me about the timing. This change means that the capture stream isn't started at the moment the trigger callback is called but at the next urb handling. It means a possible regression in the case of realtime usage.
I'm not sure I understand. Do you mean it might cause the delay between capture and playback to vary at each startup?
No, I mean that the actual begin of the recording will be delayed in comparison with the driver without your patch. The delay can't be avoided in this style.
Is there any reason to do this except for clean up? IOW, does this fix any problem by itself?
Yes, I only became aware of it since I bumped into it with my test tool. Attached here - I hope the mailing list accepts attachments.
I understand that this will be required for the implicit feedback case. But why applying this to *all* other cases, too, although we know that this may regress slightly with respect to the latency? This is my biggest concern.
thanks,
Takashi
I reverted the patch right now (mainline rc4 + this series) and this is the exact sequence: $ ./atest --device hw:2 stream --first 0x0038f0 atest v1 Testing device hw:2, rate 96000, range 38f0:0
- Test stream, Steps: [pcm_open] [pcm_hw_params] [pcm_prepare] [pcm_start] [waitsleep] [pcm_stop] [pcm_close]
++ Test: stream, permutation: 0x0038f0
- [1/14] playback => pcm_open
- [2/14] playback => pcm_hw_params
- [3/14] playback => pcm_prepare
- [4/14] playback => pcm_start
- [5/14] capture => pcm_open
- [6/14] capture => pcm_hw_params
- [7/14] capture => pcm_prepare
- [8/14] capture => pcm_start
- [9/14] playback => waitsleep
- [10/14] playback => pcm_stop
++ Frames [playback]: 58560
- [11/14] playback => pcm_close
- [12/14] capture => waitsleep
- [13/14] capture => pcm_stop
++ Frames [capture]: 5760
- [14/14] capture => pcm_close
++ permutation: 0x0038f0, runtime: 0.228940 sec ++ Test: stream, permutation: 0x003907
- [1/14] capture => pcm_open
- [2/14] capture => pcm_hw_params
- [3/14] capture => pcm_prepare
- [4/14] playback => pcm_open
- [5/14] playback => pcm_hw_params
- [6/14] playback => pcm_prepare
- [7/14] playback => pcm_start
- [8/14] playback => waitsleep
- [9/14] capture => pcm_start
- [10/14] playback => pcm_stop
++ Frames [playback]: 58560
- [11/14] playback => pcm_close
capture_thread:209: error -32 on readi 0 ** IO Error (capture) ++ permutation: 0x003907, runtime: 0.142909 sec !! Test stream permutation 0x003907 failed !!
...and dmesg shows:
snd_usb_endpoint_start: cannot submit urb 0, error -16: device busy
After I removed the revert, no test fail.
Cheers, Eldad