This project is mirrored from https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git.
Pull mirroring updated .
- 27 Aug, 2021 1 commit
-
-
Takashi Iwai authored
The recent change for low latency playback works in most of test cases but it turned out still to hit errors on some use cases, most notably with JACK with small buffer sizes. This is because USB-audio driver fills up and submits full URBs at the beginning, while the URBs would return immediately and try to fill more -- that can easily trigger XRUN. It was more or less expected, but in the small buffer size, the problem became pretty obvious. Fixing this behavior properly would require the change of the fundamental driver design, so it's no trivial task, unfortunately. Instead, here we work around the problem just by switching back to the old method when the given configuration is too fragile with the low latency stream handling. As a threshold, we calculate the total buffer bytes in all plus one URBs, and check whether it's beyond the PCM buffer bytes. The one extra URB is needed because XRUN happens at the next submission after the first round. Fixes: 307cc9ba ("ALSA: usb-audio: Reduce latency at playback start, take#2") Cc: <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20210827203311.5987-1-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
- 07 Jul, 2021 1 commit
-
-
Takashi Iwai authored
This is another attempt for the reduction of the latency at the start of a USB audio playback stream. The first attempt in the commit 9ce650a7 caused an unexpected regression (a deadlock with pipewire usage) and was later reverted by the commit 4b820e16. The devils are always living in details, of course; the cause of the deadlock was the call of snd_pcm_period_elapsed() inside prepare_playback_urb() callback. In the original code, this callback is never called from the stream lock context as it's driven solely from the URB complete callback. Along with the movement of the URB submission into the trigger START, this prepare call may be also executed in the stream lock context, hence it deadlocked with the another lock in snd_pcm_period_elapsed(). (Note that this happens only conditionally with a small period size that matches with the URB buffer length, which was a reason I overlooked during my tests. Also, the problem wasn't seen in the capture stream because the capture stream handles the period-elapsed only at retire callback that isn't executed at the trigger.) If it were only about avoiding the deadlock, it'd be possible to use snd_pcm_period_elapsed_under_stream_lock() as a solution. However, in general, the period elapsed notification must be sent after the actual stream start, and replacing the call wouldn't satisfy the pattern. A better option is to delay the notification after the stream start procedure finished, instead. In the case of USB framework, one of the fitting place would be the complete callback of the first URB. So, as a workaround of the deadlock and the order fixes above, in addition to the re-applying the changes in the commit 9ce650a7, this patch introduces a new flag indicating the delayed period-elapsed handling and sets it under the possible deadlock condition (i.e. prepare callback being called before subs->running is set). Once when the flag is set, the period-elapsed call is handled at a later URB complete call instead. As a reference for the original motivation for the low-latency change, I cite here again: | USB-audio driver behaves a bit strangely for the playback stream -- | namely, it starts sending silent packets at PCM prepare state while | the actual data is submitted at first when the trigger START is | kicked off. This is a workaround for the behavior where URBs are | processed too quickly at the beginning. That is, if we start | submitting URBs at trigger START, the first few URBs will be | immediately completed, and this would result in the immediate | period-elapsed calls right after the start, which may confuse | applications. | | OTOH, submitting the data after silent URBs would, of course, result | in a certain delay of the actual data processing, and this is rather | more serious problem on modern systems, in practice. | | This patch tries to revert the workaround and lets the URB | submission starting at PCM trigger for the playback again. As far | as I've tested with various backends (native ALSA, PA, JACK, PW), I | haven't seen any problems (famous last words :) | | Note that the capture stream handling needs no such workaround, | since the capture is driven per received URB. Link: https://lore.kernel.org/r/4e71531f-4535-fd46-040e-506a3c256bbd@marcan.st Link: https://lore.kernel.org/r/s5hbl7li0fe.wl-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de> Link: https://lore.kernel.org/r/20210707112447.27485-1-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
- 03 Jul, 2021 1 commit
-
-
Linus Torvalds authored
This reverts commit 9ce650a7 . This commit causes watchdog lockups on my machine, and while I have no idea what the cause is, it bisected right to this commit, and reverting the change promptly fixes it. At least occasionally one of the watchdog call traces was Call Trace: _raw_spin_lock_irqsave+0x35/0x40 snd_pcm_period_elapsed+0x1b/0xa0 [snd_pcm] snd_usb_endpoint_start+0x1a0/0x3c0 [snd_usb_audio] start_endpoints+0x23/0x90 [snd_usb_audio] snd_usb_substream_playback_trigger+0x7b/0x1a0 [snd_usb_audio] snd_pcm_common_ioctl+0x1c44/0x2360 [snd_pcm] snd_pcm_ioctl+0x2e/0x40 [snd_pcm] __se_sys_ioctl+0x72/0xc0 do_syscall_64+0x4c/0xa0 entry_SYSCALL_64_after_hwframe+0x44/0xae so presumably it's a locking error on that substream spinlock that snd_pcm_period_elapsed() takes. But at this point I just want to have a working system so that I can continue the merge window work tomorrow. Cc: Takashi Iwai <tiwai@suse.de> Signed-off-by:
Linus Torvalds <torvalds@linux-foundation.org>
-
- 02 Jun, 2021 5 commits
-
-
Takashi Iwai authored
USB-audio driver behaves a bit strangely for the playback stream -- namely, it starts sending silent packets at PCM prepare state while the actual data is submitted at first when the trigger START is kicked off. This is a workaround for the behavior where URBs are processed too quickly at the beginning. That is, if we start submitting URBs at trigger START, the first few URBs will be immediately completed, and this would result in the immediate period-elapsed calls right after the start, which may confuse applications. OTOH, submitting the data after silent URBs would, of course, result in a certain delay of the actual data processing, and this is rather more serious problem on modern systems, in practice. This patch tries to revert the workaround and lets the URB submission starting at PCM trigger for the playback again. As far as I've tested with various backends (native ALSA, PA, JACK, PW), I haven't seen any problems (famous last words :) Note that the capture stream handling needs no such workaround, since the capture is driven per received URB. Link: https://lore.kernel.org/r/20210601162457.4877-6-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
Just minor code refactoring. Like DOP DSD code, it can be better in a separate function for code readability. Link: https://lore.kernel.org/r/20210601162457.4877-5-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
The PCM delay accounting in USB-audio driver is a bit complex to follow, and this is an attempt to improve the readability and provide some potential fix. Basically, the PCM position delay is calculated from two factors: the in-flight data on URBs and the USB frame counter. For the playback stream, we advance the hwptr already at submitting URBs. Those "in-flight" data amount is now tracked, and this is used as the base value for the PCM delay correction. The in-flight data is decreased again at URB completion in return. For the capture stream, OTOH, there is no in-flight data, hence the delay base is zero. The USB frame counter is used in addition for correcting the current position. The reference frame counter is updated at each submission and receiving time, and the difference from the current counter value is taken into account. In this patch, each in-flight data bytes is recorded in the new snd_usb_ctx.queued field, and the total in-flight amount is tracked in snd_usb_substream.inflight_bytes field, as the replacement of last_delay field. Note that updating the hwptr after URB completion doesn't work for PulseAudio who tries to scratch the buffer on the fly; USB-audio is basically a double-buffer implementation, hence the scratching the buffer can't work for the already submitted data. So we always update hwptr beforehand. It's not ideal, but the delay account should give enough correctness. Link: https://lore.kernel.org/r/20210601162457.4877-4-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
There are a bunch of lines calculating the buffer size in bytes at each time. Keep the value in subs->buffer_bytes and use it consistently for the code simplicity. Link: https://lore.kernel.org/r/20210601162457.4877-3-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
It's a local function, let's make it static. Link: https://lore.kernel.org/r/20210601162457.4877-2-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
- 28 Feb, 2021 1 commit
-
-
Takashi Iwai authored
The recent fix for the hw constraints for implicit feedback streams via commit e4ea77f8 ("ALSA: usb-audio: Always apply the hw constraints for implicit fb sync") added the check of the matching endpoints and whether those EPs are already opened. This is needed and correct, per se, even for the normal streams without the implicit feedback, as the endpoint setup is exclusive. However, it's reported that there seem applications that behave in unexpected ways to update the hw_params without clearing the previous setup via hw_free, and those hit a problem now: then hw_params is called with still the previous EP setup kept, hence it's restricted with the previous own setup. Although the obvious fix is to call snd_pcm_hw_free() API in the application side, it's a kind of unwelcome change. This patch tries to ease the situation: in the endpoint check, we add a couple of more conditions and now skip the endpoint that is being used only by the stream in question itself. That is, in addition to the presence check of ep (ep->cur_audiofmt is non-NULL), when the following conditions are met, we skip such an ep: - ep->opened == 1, and - ep->cur_audiofmt == subs->cur_audiofmt. subs->cur_audiofmt is non-NULL only if it's a re-setup of hw_params, and ep->cur_audiofmt points to the currently set up parameters. So if those match, it must be this stream itself. Fixes: e4ea77f8 ("ALSA: usb-audio: Always apply the hw constraints for implicit fb sync") BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=211941 Cc: <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20210228080138.9936-1-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
- 08 Feb, 2021 1 commit
-
-
Takashi Iwai authored
In the later patch, we're going to issue the PCM sync_stop calls at disconnection. But currently the USB-audio driver can't handle it because it has a check of shutdown flag for stopping the URBs. This is basically superfluous (the stopping URBs are safe at disconnection state), so let's drop the check. Fixes: dc5eafe7 ("ALSA: usb-audio: Support PCM sync_stop") Cc: <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20210206203052.15606-4-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
- 05 Feb, 2021 1 commit
-
-
Takashi Iwai authored
The commit f274baa4 ("ALSA: usb-audio: Allow non-vmalloc buffer for PCM buffers") introduced the mode to allocate coherent pages for PCM buffers, and it used bus->controller device as its DMA device. It turned out, however, that bus->sysdev is a more appropriate device to be used for DMA mapping in HCD code. This patch corrects the device reference accordingly. Note that, on most platforms, both point to the very same device, hence this patch doesn't change anything practically. But on platforms like xhcd-plat hcd, the change becomes effective. Fixes: f274baa4 ("ALSA: usb-audio: Allow non-vmalloc buffer for PCM buffers") Cc: <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20210205144559.29555-1-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
- 20 Jan, 2021 1 commit
-
-
Takashi Iwai authored
Since the recent refactoring, it's been reported that some USB-audio devices (typically webcams) are no longer detected properly by PulseAudio. The debug session revealed that it's failing at probing by PA to try the sample rate 44.1kHz while the device has discrete sample rates other than 44.1kHz. But the puzzle was that arecord works as is, and some other devices with the discrete rates work, either. After all, this turned out to be the lack of the dependencies in a few hw constraint rules: snd_pcm_hw_rule_add() has the (variable) arguments specifying the dependent parameters, and some functions didn't set the target parameter itself as the dependencies. This resulted in an invalid parameter that could be generated only in a certain call pattern. This bug itself has been present in the code, but it didn't trigger errors just because the rules were casually avoiding such a corner case. After the recent refactoring and cleanup, however, the hw constraints work "as expected", and the problem surfaced now. For fixing the problem above, this patch adds the missing dependent parameters to each snd_pcm_hw_rule() call. Fixes: bc4e94aa ("ALSA: usb-audio: Handle discrete rates properly in hw constraints") BugLink: http://bugzilla.opensuse.org/show_bug.cgi?id=1181014 Link: https://lore.kernel.org/r/20210120204554.30177-1-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
- 13 Jan, 2021 1 commit
-
-
Takashi Iwai authored
Since the commit 5a6c3e11 ("ALSA: usb-audio: Add hw constraint for implicit fb sync"), we apply the hw constraints for the implicit feedback sync to make the secondary open aligned with the already opened stream setup. This change assumed that the secondary open is performed after the first stream has been already set up, and adds the hw constraints to sync with the first stream's parameters only when the EP setup for the first stream was confirmed at the open time. However, most of applications handling the full-duplex operations do open both playback and capture streams at first, then set up both streams. This results in skipping the additional hw constraints since the counter-part stream hasn't been set up yet at the open of the second stream, and it eventually leads to "incompatible EP" error in the end. This patch corrects the behavior by always applying the hw constraints for the implicit fb sync. The hw constraint rules are defined so that they check the sync EP dynamically at each invocation, instead. This covers the concurrent stream setups better and lets the hw refine calls resolving to the right configuration. Also this patch corrects a minor error that has existed in the debug print that isn't built as default. Fixes: 5a6c3e11 ("ALSA: usb-audio: Add hw constraint for implicit fb sync") Link: https://lore.kernel.org/r/20210111081611.12790-1-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
- 23 Nov, 2020 27 commits
-
-
Takashi Iwai authored
The code dealing with the implicit feedback mode grew recently, and it's becoming messy. As we receive more and more devices that need the similar handling, it's better to be processed through a table instead of the open code. This patch moves the code that is relevant with parsing the implicit feedback mode and some helpers into another file, implicit.c. The detection and the setup of the implicit feedback sync EPs are rewritten to use the ID/class matching table instead. There should be no functional changes. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-38-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
The capture stream of BOSS GT-001 seems always requiring to be tied with the playback stream. OTOH, the playback stream of this device doesn't seem working in the implicit fb mode, per se, since the playback must be running before the capture stream. This patch tries to address the points above: - Avoid the implicit fb mode for the playback - Set up a fake sync EP for the capture stream with the hard-coded playback stream using the implicit fb mode Reported-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-37-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
There are two places calculating the next packet size for the playback stream in the exactly same way. Provide the single helper for this purpose and use it from both places gracefully. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-32-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
Some fields like interface and alt_idx in snd_usb_substream are mostly useless now as they can be referred via either cur_audiofmt or data_endpoint assigned to the substream. Drop those, and also assure the concurrency about the access of cur_audiofmt field. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-31-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
The audioformat is referred in many places but most of usages are read-only. Let's add const prefix in the possible places. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-28-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
This is an intensive surgery for the endpoint and stream management for achieving more robust and clean code. The goals of this patch are: - More clear endpoint resource changes - The interface altsetting control in a single place Below are brief description of the whole changes. First off, most of the endpoint operations are moved into endpoint.c, so that the snd_usb_endpoint object is only referred in other places. The endpoint object is acquired and released via the new functions snd_usb_endpoint_open() and snd_usb_endpoint_close() that are called at PCM hw_params and hw_free callbacks, respectively. Those are ref-counted and EPs can manage the multiple opens. The open callback receives the audioformat and hw_params arguments, and those are used for initializing the EP parameters; especially the endpoint, interface and altset numbers are read from there, as well as the PCM parameters like the format, rate and channels. Those are stored in snd_usb_endpoint object. If it's the secondary open, the function checks whether the given parameters are compatible with the already opened EP setup, too. The coupling with a sync EP (including an implicit feedback sync) is done by the sole snd_usb_endpoint_set_sync() call. The configuration of each endpoint is done in a single shot via snd_usb_endpoint_configure() call. This is the place where most of PCM configurations are done. A few flags and special handling in the snd_usb_substream are dropped along with this change. A significant difference wrt the configuration from the previous code is the order of USB host interface setups. Now the interface is always disabled at beginning and (re-)enabled at the last step of snd_usb_endpoint_configure(), in order to be compliant with the standard UAC2/3. For UAC1, the interface is set before the parameter setups since there seem devices that require it (e.g. Yamaha THR10), just like how it was done in the previous driver code. The start/stop are almost same as before, also single-shots. The URB callbacks need to be set via snd_usb_endpoint_set_callback() like the previous code at the trigger phase, too. Finally, the flag for the re-setup is set at the device suspend through the full EP list, instead of PCM trigger. This catches the overlooked cases where the PCM hasn't been running yet but the device needs the full setup after resume. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-26-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
The function to evaluate the match of the parameters with an EP assumes only the discrete rate tables and doesn't handle the continuous rates properly. This patch fixes match_endpoint_audioformats() to handle the continuous rates. Also the almost useless debug prints there are dropped. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-25-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
The commit 92adc96f ("ALSA: usb-audio: set the interface format after resume on Dell WD19") introduced the workaround for the broken setup after the resume specifically on a Dell dock model. However, the full setup should have been performed after the resume on all devices, as we can't guarantee the same state. So this patch removes the conditional check and applies the workaround always. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-24-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
The prepare_data_urb and retire_data_urb fields of the endpoint object are set dynamically at PCM trigger start/stop. Those are evaluated in the endpoint handler, but there can be a race, especially if two different PCM substreams are handling the same endpoint for the implicit feedback case. Also, the data_subs field of the endpoint is set and accessed dynamically, too, which has the same risk. As a slight improvement for the concurrency, this patch introduces the function to set the callbacks and the data in a shot with the memory barrier. In the reader side, it's also fetched with the memory barrier. There is still a room of race if prepare and retire callbacks are set during executing the URB completion. But such an inconsistency may happen only for the implicit fb source, i.e. it's only about the capture stream. And luckily, the capture stream never sets the prepare callback, hence the problem doesn't happen practically. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-23-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
start_endpoints() may leave the data endpoint running if an error happens at starting the sync endpoint. We should stop both streams properly, instead. While we're at it, move the debug prints into the endpoint.c that is a more suitable place. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-22-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
A preliminary change for the later big changes. This is a minor code refactoring to drop the unnecessary arguments that can be retrieved in a different way. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-21-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
A preliminary change for the later big changes. This is a minor code refactoring to drop the unnecessary arguments that can be retrieved in a different way. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-20-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
A preliminary patch for the later big change. Just a minor code refactoring. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-19-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
Add a helper function to retrieve the usb_host_interface object from the given interface and altsetting number pair, which is a commonly used procedure in the driver code. No functional changes, just minor code refactoring. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-17-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
This behavior turned out to be invalid from the USB spec POV and shouldn't be applied. As it's an optional flag that is set only via an card control element that must be hardly used, let's drop it again. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-16-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
Currently snd_usb_endpoint objects are created at first when the substream is opened and tries to assign the endpoints corresponding to the matching audioformat. But since basically the all endpoints have been already parsed and the information have been obtained, we may create the endpoint objects statically at the init phase. It's easier to manage for the implicit fb case, for example. This patch changes the endpoint object management and lets the parser to create the all endpoint objects. This change shouldn't bring any functional changes. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-15-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
The implicit feedback mode initializes both the main data stream and the sync data stream. When a sync stream was already opened, this would result in the doubly initialization and might screw up things. Add the check of already opened sync streams and skip the unnecessary initialization. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-14-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
The file debug.h contains a simple macro for debug prints, and it's used only in two places, the format parser and the hw_params rules. The former actually should print a more informative message instead, so the only users are the hw_parmas rules. This patch moves the contents of debug.h into the hw_params rules local code and remove the unneeded includes. Also, the debug print in the format parser is replaced with the information print with more useful information, and the raw printk() call is replaced with pr_debug(). Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-13-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
Several hw_params functions narrows the interval via min/max rule in the very similar way, so factor out those into a helper function and use commonly. No functional changes, just minor code refactoring. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-12-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
In the current code, there is no check at the stream open time whether the endpoint is being already used by others. In the normal operations, this shouldn't happen, but in the case of the implicit feedback mode, it's a common problem with the full duplex operation, because the capture stream is always opened by the playback stream as an implicit sync source. Although we recently introduced the check of such a conflict of parameters at the PCM hw_params time, it doesn't give any hint at the hw_params itself and just gives the error. This isn't quite comfortable, and it caused problems on many applications. This patch attempts to make the parameter handling easier by introducing the strict hw constraint matching with the counterpart stream that is being used. That said, when an implicit feedback playback stream is running before a capture stream is opened, the capture stream carries the PCM hw-constraint to allow only the same sample rate, format, periods and period frames as the running playback stream. If not opened or there is no conflict of endpoints, the behavior remains as same as before. Note that this kind of "weak link" should work for most cases, but this is no concrete solution; e.g. if an application changes the hw params multiple times while another stream is opened, this would lead to inconsistencies. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-11-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
This is a preliminary work for the upcoming hw-constraint change for the implicit feedback mode. Currently snd_usb_autoresume() is called at the end of setup_hwinfo(). It's a bit confusing; because of this implicit refcount usage, the caller side needs to call snd_usb_autosuspend() later in the error path although it's not seen inside the function. Instead, it's clearer to call both snd_usb_autoresume() and suspend() in the very same function. It's only refactoring and no functional changes. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-10-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
Instead of parsing and evaluating the sync endpoint and the implicit feedback mode at each time the audio stream is opened, let's parse it once at the probe time, as the all needed information can be obtained statically from the descriptor or from the quirk. This patch extends audioformat struct to record the sync endpoint, interface and altsetting as well as the implicit feedback flag, which are filled at parsing the streams. Then, set_sync_endpoint() is much simplified just to follow the already parsed data. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-9-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
There are a few rooms for improvements wrt the debug prints: - The EP debug print is shown only at starting, not at stopping - The EP debug print contains useless object addresses - Some helpers show the urb and the EP object addresses, too This patch addresses those shortcomings. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-8-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
The sync EP setup isn't cleared at stopping the stream but expected to be cleared at the next stream start. This may leave the sync link setup stale and can spoof wrongly when full duplex streams were running in the implicit fb sync. Let's initialize them properly at start and end of the stream. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-7-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
It seems that many UAC2 devices are with the implicit feedback, but they couldn't be probed properly because the assumption the driver takes currently isn't applied: they have the single endpoint for both data and implicit-fb streams, while we checked only the classical sync endpoints assigned to the next altsetting in the same interface. This patch extends the search to match with those typical cases where the implicit fb stream is found in the next interface number. While we're at it, slightly refactor the code, not returning 0/-ERROR but use the standard bool to success/failur, which is more intuitive in this particular case. Reported-by:
Dylan Robinson <dylan_robinson@motu.com> Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-5-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
The PCM trigger callback is atomic, hence we must not call a function like usb_set_interface() there. Calling it from there would lead to a kernel Oops. Fix it by moving the usb_set_interface() call to set_sync_endpoint(). Also, apply the snd_usb_set_interface_quirk() for consistency, too. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-3-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-
Takashi Iwai authored
In the current code, when the device provides the discrete sample rate tables with unusual sample rates, the driver tries to gather the whole values from the audioformat entries and create a hw-constraint rule to restrict with this single rate list. This is rather inefficient and may overlook the rates that are associated only with the certain audioformat entries. This patch improves the hw constraint setup by rewriting the existing hw_rule_rate(). The discrete sample rates (identified by rate_table and nr_rates of format entry) are checked in the existing hw_rule_rate() instead of extra rules; in the case of discrete rates, the function compares with each rate table entry and calculates the min/max values from there. For the contiguous rates, the behavior doesn't change. Along with it, snd_usb_pcm_check_knot() and snb_usb_substream rate_list field become superfluous, thus those are dropped. Tested-by:
Keith Milner <kamilner@superlative.org> Tested-by:
Dylan Robinson <dylan_robinson@motu.com> Link: https://lore.kernel.org/r/20201123085347.19667-2-tiwai@suse.de Signed-off-by:
Takashi Iwai <tiwai@suse.de>
-