The recent support of low latency playback in USB-audio driver made
the snd_usb_queue_pending_output_urbs() function to be called via PCM
ack ops. In the new code path, the function is performed already in
the PCM stream lock. The problem is that, when an XRUN is detected,
the function calls snd_pcm_xrun() to notify, but snd_pcm_xrun() is
supposed to be called only outside the stream lock. As a result, it
leads to a deadlock of PCM stream locking.
For avoiding such a recursive locking, this patch adds an additional
check to the code paths in PCM core that call the ack callback; now it
checks the error code from the callback, and if it's -EPIPE, the XRUN
is handled in the PCM core side gracefully. Along with it, the
USB-audio driver code is changed to follow that, i.e. -EPIPE is
returned instead of the explicit snd_pcm_xrun() call when the function
is performed already in the stream lock.
Fixes: d5f871f89e ("ALSA: usb-audio: Improved lowlatency playback support")
Reported-and-tested-by: John Keeping <john@metanate.com>
Link: https://lore.kernel.org/r/20230317195128.3911155-1-john@metanate.com
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Reviewed-by; Takashi Sakamoto <o-takashi@sakamocchi.jp>
Link: https://lore.kernel.org/r/20230320142838.494-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
It seems that the firmware is broken and does not accept
the UAC_EP_CS_ATTR_SAMPLE_RATE URB. There is only one rate (48000Hz)
available in the descriptors for the output endpoint.
Create a new quirk QUIRK_FLAG_FIXED_RATE to skip the rate setup
when only one rate is available (fixed).
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=216798
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Link: https://lore.kernel.org/r/20221215153037.1163786-1-perex@perex.cz
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This is a second attempt to fix the bug appearing on Android with the
recent kernel; the first try was ff878b408a and reverted at commit
79764ec772.
The details taken from the v1 patch:
One of the former changes for the endpoint management was the more
consistent setup of endpoints at hw_params.
snd_usb_endpoint_configure() is a single function that does the full
setup, and it's called from both PCM hw_params and prepare callbacks.
Although the EP setup at the prepare phase is usually skipped (by
checking need_setup flag), it may be still effective in some cases
like suspend/resume that requires the interface setup again.
As it's a full and single setup, the invocation of
snd_usb_endpoint_configure() includes not only the USB interface setup
but also the buffer release and allocation. OTOH, doing the buffer
release and re-allocation at PCM prepare phase is rather superfluous,
and better to be done only in the hw_params phase.
For those optimizations, this patch splits the endpoint setup to two
phases: snd_usb_endpoint_set_params() and snd_usb_endpoint_prepare(),
to be called from hw_params and from prepare, respectively.
Note that this patch changes the driver operation slightly,
effectively moving the USB interface setup again to PCM prepare stage
instead of hw_params stage, while the buffer allocation and such
initializations are still done at hw_params stage.
And, the change of the USB interface setup timing (moving to prepare)
gave an interesting "fix", too: it was reported that the recent
kernels caused silent output at the beginning on playbacks on some
devices on Android, and this change casually fixed the regression.
It seems that those devices are picky about the sample rate change (or
the interface change?), and don't follow the too immediate rate
changes.
Meanwhile, Android operates the PCM in the following order:
- open, then hw_params with the possibly highest sample rate
- close without prepare
- re-open, hw_params with the normal sample rate
- prepare, and start streaming
This procedure ended up the hw_params twice with different rates, and
because the recent kernel did set up the sample rate twice one and
after, it screwed up the device. OTOH, the earlier kernels didn't set
up the USB interface at hw_params, hence this problem didn't appear.
Now, with this patch, the USB interface setup is again back to the
prepare phase, and it works around the problem automagically.
Although we should address the sample rate problem in a more solid
way in future, let's keep things working as before for now.
***
What's new in the take#2 patch:
- The regression caused by the v1 patch (bko#216500) was due to the
missing check of need_setup flag at hw_params. Now the check is
added, and the snd_usb_endpoint_set_params() call is skipped when
the running EP is re-opened.
- There was another bug in v1 where the clock reference rate wasn't
updated at hw_params phase, which may lead to a lack of the proper
hw constraints when an application doesn't issue the prepare but
only the hw_params call. This patch fixes it as well by tracking
the clock rate change in the prepare callback with a new flag
"need_update" for the clock reference object, just like others.
- The configure_endpoints() are simplified and folded back into
snd_usb_pcm_prepare().
Fixes: bf6313a0ff ("ALSA: usb-audio: Refactor endpoint management")
Fixes: ff878b408a ("ALSA: usb-audio: Split endpoint setups for hw_params and prepare")
Reported-by: chihhao chen <chihhao.chen@mediatek.com>
Link: https://lore.kernel.org/r/87e6d6ae69d68dc588ac9acc8c0f24d6188375c3.camel@mediatek.com
Link: https://lore.kernel.org/r/20220901124136.4984-1-tiwai@suse.de
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216500
Link: https://lore.kernel.org/r/20220920181106.4894-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This reverts commit ff878b408a.
Unfortunately the recent fix seems bringing another regressions with
PulseAudio / pipewire, at least for Steinberg and MOTU devices.
As a temporary solution, do a straight revert. The issue for Android
will be revisited again later by another different fix (if any).
Fixes: ff878b408a ("ALSA: usb-audio: Split endpoint setups for hw_params and prepare")
Cc: <stable@vger.kernel.org>
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=216500
Link: https://lore.kernel.org/r/20220920113929.25162-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
One of the former changes for the endpoint management was the more
consistent setup of endpoints at hw_params.
snd_usb_endpoint_configure() is a single function that does the full
setup, and it's called from both PCM hw_params and prepare callbacks.
Although the EP setup at the prepare phase is usually skipped (by
checking need_setup flag), it may be still effective in some cases
like suspend/resume that requires the interface setup again.
As it's a full and single setup, the invocation of
snd_usb_endpoint_configure() includes not only the USB interface setup
but also the buffer release and allocation. OTOH, doing the buffer
release and re-allocation at PCM prepare phase is rather superfluous,
and better to be done only in the hw_params phase.
For those optimizations, this patch splits the endpoint setup to two
phases: snd_usb_endpoint_set_params() and snd_usb_endpoint_prepare(),
to be called from hw_params and from prepare, respectively.
Note that this patch changes the driver operation slightly,
effectively moving the USB interface setup again to PCM prepare stage
instead of hw_params stage, while the buffer allocation and such
initializations are still done at hw_params stage.
And, the change of the USB interface setup timing (moving to prepare)
gave an interesting "fix", too: it was reported that the recent
kernels caused silent output at the beginning on playbacks on some
devices on Android, and this change casually fixed the regression.
It seems that those devices are picky about the sample rate change (or
the interface change?), and don't follow the too immediate rate
changes.
Meanwhile, Android operates the PCM in the following order:
- open, then hw_params with the possibly highest sample rate
- close without prepare
- re-open, hw_params with the normal sample rate
- prepare, and start streaming
This procedure ended up the hw_params twice with different rates, and
because the recent kernel did set up the sample rate twice one and
after, it screwed up the device. OTOH, the earlier kernels didn't set
up the USB interface at hw_params, hence this problem didn't appear.
Now, with this patch, the USB interface setup is again back to the
prepare phase, and it works around the problem automagically.
Although we should address the sample rate problem in a more solid
way in future, let's keep things working as before for now.
Fixes: bf6313a0ff ("ALSA: usb-audio: Refactor endpoint management")
Cc: <stable@vger.kernel.org>
Reported-by: chihhao chen <chihhao.chen@mediatek.com>
Link: https://lore.kernel.org/r/87e6d6ae69d68dc588ac9acc8c0f24d6188375c3.camel@mediatek.com
Link: https://lore.kernel.org/r/20220901124136.4984-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
While draining a stream, ALSA PCM core stops the stream by issuing
snd_pcm_stop() after all data has been sent out. And, at PCM trigger
stop, currently USB-audio driver kills the in-flight URBs explicitly,
then at sync-stop ops, sync with the finish of all remaining URBs.
This might result in a drop of the drained samples as most of
USB-audio devices / hosts allow relatively long in-flight samples (as
a sort of FIFO).
For avoiding the trimming, this patch changes the stream-stop behavior
during PCM draining state. Under that condition, the pending URBs
won't be killed. The leftover in-flight URBs are caught by the
sync-stop operation that shall be performed after the trigger-stop
operation.
Link: https://lore.kernel.org/r/20210929080844.11583-10-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This is another attempt to improve further the handling of playback
stream in the low latency mode. The latest workaround in commit
4267c5a8f3 ("ALSA: usb-audio: Work around for XRUN with low latency
playback") revealed that submitting URBs forcibly in advance may
trigger XRUN easily. In the classical mode, this problem was avoided
by practically delaying the submission of the actual data with the
pre-submissions of silent data before triggering the stream start.
But that is exactly what we want to avoid.
Now, in this patch, instead of the previous workaround, we take a
similar approach as used in the implicit feedback mode. The URBs are
queued at the PCM trigger start like before, but we check whether the
buffer has been already filled enough before each submission, and
stop queuing if the data overcomes the threshold. The remaining URBs
are kept in the ready list, and they will be retrieved in the URB
complete callback of other (already queued) URBs. In the complete
callback, we try to fill the data and submit as much as possible
again. When there is no more available in-flight URBs that may handle
the pending data, we'll check in PCM ack callback and submit and
process URBs there in addition. In this way, the amount of in-flight
URBs may vary dynamically and flexibly depending on the available data
without hitting XRUN.
The following things are changed to achieve the behavior above:
* The endpoint prepare callback is changed to return an error code;
when there is no enough data available, it may return -EAGAIN.
Currently only prepare_playback_urb() returns the error.
The evaluation of the available data is a bit messy here; we can't
check with snd_pcm_avail() at the point of prepare callback (as
runtime->status->hwptr hasn't been updated yet), hence we manually
estimate the appl_ptr and compare with the internal hwptr_done to
calculate the available frames.
* snd_usb_endpoint_start() doesn't submit full URBs if the prepare
callback returns -EAGAIN, and puts the remaining URBs to the ready
list for the later submission.
* snd_complete_urb() treats the URBs in the low-latency mode similarly
like the implicit feedback mode, and submissions are done in
(now exported) snd_usb_queue_pending_output_urbs().
* snd_usb_queue_pending_output_urbs() again checks the error value
from the prepare callback. If it's -EAGAIN for the normal stream
(i.e. not implicit feedback mode), we push it back to the ready list
again.
* PCM ack callback is introduced for the playback stream, and it calls
snd_usb_queue_pending_output_urbs() if there is no in-flight URB
while the stream is running. This corresponds to the case where the
system needs the appl_ptr update for re-submitting a new URB.
* snd_usb_queue_pending_output_urbs() and the prepare EP callback
receive in_stream_lock argument, which is a bool flag indicating the
call path from PCM ack. It's needed for avoiding the deadlock of
snd_pcm_period_elapsed() calls.
* Set the new SNDRV_PCM_INFO_EXPLICIT_SYNC flag when the new
low-latency mode is deployed. This assures catching each applptr
update even in the mmap mode.
Fixes: 4267c5a8f3 ("ALSA: usb-audio: Work around for XRUN with low latency playback")
Link: https://lore.kernel.org/r/20210929080844.11583-9-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This is yet more preparation for the upcoming changes.
Extend snd_usb_endpoint_next_packet_size() to check the available
frames and return -EAGAIN if the next packet size is equal or exceeds
the given size. This will be needed for avoiding XRUN during the low
latency operation.
As of this patch, avail=0 is passed, i.e. the check is skipped and no
behavior change.
Link: https://lore.kernel.org/r/20210929080844.11583-7-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When a single clock source is shared among several endpoints, we have
to keep the same rate on all active endpoints as long as the clock is
being used. For dealing with such a case, this patch adds one more
check in the hw params constraint for the rate to take the shared
clocks into account. The current rate is evaluated from the endpoint
list that applies the same clock source.
BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1190418
Link: https://lore.kernel.org/r/20210929080844.11583-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The current endpoint handling assumed (more or less) a unique 1:1
relation between the endpoint and the iface/altset. The exception was
the sync EP without the implicit feedback which has usually the
secondary EP of the same altset. This works fine for most devices,
but it turned out that some unusual devices like Pinoeer's ones have
both playback and capture endpoints in the same iface/altsetting and
use both for the implicit feedback mode. For handling such a case, we
need to extend the endpoint management to take the shared interface
into account.
This patch does that: it adds a new object snd_usb_iface_ref for
managing the reference counts of the each USB interface that is used
by each endpoint. The interface setup is performed only once for the
(sharing) endpoints, and the doubly initialization is avoided.
Along with this, the resource release of endpoints and interface
refcounts are put into a single function, snd_usb_endpoint_free_all()
instead of looping in the caller side.
Fixes: bf6313a0ff ("ALSA: usb-audio: Refactor endpoint management")
Link: https://lore.kernel.org/r/20210108075219.21463-4-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
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>
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>
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>
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>
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>
Factor out the code to obtain snd_usb_endpoint object matching with
the given endpoint. It'll be used in the later patch to add the
implicit feedback hw-constraint.
No functional change by this patch itself.
Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-6-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
For computation of the the next frame size current value of fs/fps and
accumulated fractional parts of fs/fps are used, where values are stored
in Q16.16 format. This is quite natural for computing frame size for
asynchronous endpoints driven by explicit feedback, since in this case
fs/fps is a value provided by the feedback endpoint and it's already in
the Q format. If an error is accumulated over time, the device can
adjust fs/fps value to prevent buffer overruns/underruns.
But for synchronous endpoints the accuracy provided by these computations
is not enough. Due to accumulated error the driver periodically produces
frames with incorrect size (+/- 1 audio sample).
This patch fixes this issue by implementing a different algorithm for
frame size computation. It is based on accumulating of the remainders
from division fs/fps and it doesn't accumulate errors over time. This
new method is enabled for synchronous and adaptive playback endpoints.
Signed-off-by: Alexander Tsoy <alexander@tsoy.me>
Link: https://lore.kernel.org/r/20200424022449.14972-1-alexander@tsoy.me
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Many source files in the tree are missing licensing information, which
makes it harder for compliance tools to determine the correct license.
By default all files without license information are under the default
license of the kernel, which is GPL version 2.
Update the files which contain no license information with the 'GPL-2.0'
SPDX license identifier. The SPDX identifier is a legally binding
shorthand, which can be used instead of the full boiler plate text.
This patch is based on work done by Thomas Gleixner and Kate Stewart and
Philippe Ombredanne.
How this work was done:
Patches were generated and checked against linux-4.14-rc6 for a subset of
the use cases:
- file had no licensing information it it.
- file was a */uapi/* one with no licensing information in it,
- file was a */uapi/* one with existing licensing information,
Further patches will be generated in subsequent months to fix up cases
where non-standard license headers were used, and references to license
had to be inferred by heuristics based on keywords.
The analysis to determine which SPDX License Identifier to be applied to
a file was done in a spreadsheet of side by side results from of the
output of two independent scanners (ScanCode & Windriver) producing SPDX
tag:value files created by Philippe Ombredanne. Philippe prepared the
base worksheet, and did an initial spot review of a few 1000 files.
The 4.13 kernel was the starting point of the analysis with 60,537 files
assessed. Kate Stewart did a file by file comparison of the scanner
results in the spreadsheet to determine which SPDX license identifier(s)
to be applied to the file. She confirmed any determination that was not
immediately clear with lawyers working with the Linux Foundation.
Criteria used to select files for SPDX license identifier tagging was:
- Files considered eligible had to be source code files.
- Make and config files were included as candidates if they contained >5
lines of source
- File already had some variant of a license header in it (even if <5
lines).
All documentation files were explicitly excluded.
The following heuristics were used to determine which SPDX license
identifiers to apply.
- when both scanners couldn't find any license traces, file was
considered to have no license information in it, and the top level
COPYING file license applied.
For non */uapi/* files that summary was:
SPDX license identifier # files
---------------------------------------------------|-------
GPL-2.0 11139
and resulted in the first patch in this series.
If that file was a */uapi/* path one, it was "GPL-2.0 WITH
Linux-syscall-note" otherwise it was "GPL-2.0". Results of that was:
SPDX license identifier # files
---------------------------------------------------|-------
GPL-2.0 WITH Linux-syscall-note 930
and resulted in the second patch in this series.
- if a file had some form of licensing information in it, and was one
of the */uapi/* ones, it was denoted with the Linux-syscall-note if
any GPL family license was found in the file or had no licensing in
it (per prior point). Results summary:
SPDX license identifier # files
---------------------------------------------------|------
GPL-2.0 WITH Linux-syscall-note 270
GPL-2.0+ WITH Linux-syscall-note 169
((GPL-2.0 WITH Linux-syscall-note) OR BSD-2-Clause) 21
((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) 17
LGPL-2.1+ WITH Linux-syscall-note 15
GPL-1.0+ WITH Linux-syscall-note 14
((GPL-2.0+ WITH Linux-syscall-note) OR BSD-3-Clause) 5
LGPL-2.0+ WITH Linux-syscall-note 4
LGPL-2.1 WITH Linux-syscall-note 3
((GPL-2.0 WITH Linux-syscall-note) OR MIT) 3
((GPL-2.0 WITH Linux-syscall-note) AND MIT) 1
and that resulted in the third patch in this series.
- when the two scanners agreed on the detected license(s), that became
the concluded license(s).
- when there was disagreement between the two scanners (one detected a
license but the other didn't, or they both detected different
licenses) a manual inspection of the file occurred.
- In most cases a manual inspection of the information in the file
resulted in a clear resolution of the license that should apply (and
which scanner probably needed to revisit its heuristics).
- When it was not immediately clear, the license identifier was
confirmed with lawyers working with the Linux Foundation.
- If there was any question as to the appropriate license identifier,
the file was flagged for further research and to be revisited later
in time.
In total, over 70 hours of logged manual review was done on the
spreadsheet to determine the SPDX license identifiers to apply to the
source files by Kate, Philippe, Thomas and, in some cases, confirmation
by lawyers working with the Linux Foundation.
Kate also obtained a third independent scan of the 4.13 code base from
FOSSology, and compared selected files where the other two scanners
disagreed against that SPDX file, to see if there was new insights. The
Windriver scanner is based on an older version of FOSSology in part, so
they are related.
Thomas did random spot checks in about 500 files from the spreadsheets
for the uapi headers and agreed with SPDX license identifier in the
files he inspected. For the non-uapi files Thomas did random spot checks
in about 15000 files.
In initial set of patches against 4.14-rc6, 3 files were found to have
copy/paste license identifier errors, and have been fixed to reflect the
correct identifier.
Additionally Philippe spent 10 hours this week doing a detailed manual
inspection and review of the 12,461 patched files from the initial patch
version early this week with:
- a full scancode scan run, collecting the matched texts, detected
license ids and scores
- reviewing anything where there was a license detected (about 500+
files) to ensure that the applied SPDX license was correct
- reviewing anything where there was no detection but the patch license
was not GPL-2.0 WITH Linux-syscall-note to ensure that the applied
SPDX license was correct
This produced a worksheet with 20 files needing minor correction. This
worksheet was then exported into 3 different .csv files for the
different types of files to be modified.
These .csv files were then reviewed by Greg. Thomas wrote a script to
parse the csv files and add the proper SPDX tag to the file, in the
format that the file expected. This script was further refined by Greg
based on the output to detect more types of files automatically and to
distinguish between header and source .c files (which need different
comment types.) Finally Greg ran the script using the .csv files to
generate the patches.
Reviewed-by: Kate Stewart <kstewart@linuxfoundation.org>
Reviewed-by: Philippe Ombredanne <pombredanne@nexb.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Commit 16200948d8 ("ALSA: usb-audio: Fix race at stopping the stream") was
incomplete causing another more severe kernel panic, so it got reverted.
This fixes both the original problem and its fallout kernel race/crash.
The original fix is to move the endpoint member NULL clearing logic inside
wait_clear_urbs() so the irq triggering the urb completion doesn't call
retire_capture/playback_urb() after the NULL clearing and generate a panic.
However this creates a new race between snd_usb_endpoint_start()'s call
to wait_clear_urbs() and the irq urb completion handler which again calls
retire_capture/playback_urb() leading to a new NULL dereference.
We keep the EP deactivation code in snd_usb_endpoint_start() because
removing it will break the EP reference counting (see [1] [2] for info),
however we don't need the "can_sleep" mechanism anymore because a new
function was introduced (snd_usb_endpoint_sync_pending_stop()) which
synchronizes pending stops and gets called inside the pcm prepare callback.
It also makes sense to remove can_sleep because it was also removed from
deactivate_urbs() signature in [3] so we benefit from more simplification.
[1] commit 015618b90 ("ALSA: snd-usb: Fix URB cancellation at stream start")
[2] commit e9ba389c5 ("ALSA: usb-audio: Fix scheduling-while-atomic bug in PCM capture stream")
[3] commit ccc1696d5 ("ALSA: usb-audio: simplify endpoint deactivation code")
Fixes: f8114f8583 ("Revert "ALSA: usb-audio: Fix race at stopping the stream"")
Signed-off-by: Ioan-Adrian Ratiu <adi@adirat.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Some functions in mixer.c and endpoint.c receive list_head instead of
the object itself. This is not obvious and rather error-prone. Let's
pass the proper object directly instead.
The functions in midi.c still receive list_head and this can't be
changed since the object definition isn't exposed to the outside of
midi.c, so left as is.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When a USB-audio device is disconnected while PCM is still running, we
still see some race: the disconnect callback calls
snd_usb_endpoint_free() that calls release_urbs() and then kfree()
while a PCM stream would be closed at the same time and calls
stop_endpoints() that leads to wait_clear_urbs(). That is, the EP
object might be deallocated while a PCM stream is syncing with
wait_clear_urbs() with the same EP.
Basically calling multiple wait_clear_urbs() would work fine, also
calling wait_clear_urbs() and release_urbs() would work, too, as
wait_clear_urbs() just reads some fields in ep. The problem is the
succeeding kfree() in snd_pcm_endpoint_free().
This patch moves out the EP deallocation into the later point, the
destructor callback. At this stage, all PCMs must have been already
closed, so it's safe to free the objects.
Reported-by: Alan Stern <stern@rowland.harvard.edu>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The return value of snd_usb_endpoint_deactivate() is not used,
make the function have no return value.
Update the documentation to reflect what the function is actually
doing.
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This patch changes the way URBs are allocated and their sizes are
determined for PCM playback in the snd-usb-audio driver. Currently
the driver allocates too few URBs for endpoints that don't use
implicit sync, making underruns more likely to occur. This may be a
holdover from before I/O delays could be measured accurately; in any
case, it is no longer necessary.
The patch allocates as many URBs as possible, subject to four
limitations:
The total number of URBs for the endpoint is not allowed to
exceed MAX_URBS (which the patch increases from 8 to 12).
The total number of packets per URB is not allowed to exceed
MAX_PACKS (or MAX_PACKS_HS for high-speed devices), which is
decreased from 20 to 6.
The total duration of queued data is not allowed to exceed
MAX_QUEUE, which is decreased from 24 ms to 18 ms.
The total number of ALSA frames in the output queue is not
allowed to exceed the ALSA buffer size.
The last requirement is the hardest to implement. Currently the
number of URBs needed to fill a buffer cannot be determined in
advance, because a buffer contains a fixed number of frames whereas
the number of frames in an URB varies to match shifts in the device's
clock rate. To solve this problem, the patch changes the logic for
deciding how many packets an URB should contain. Rather than using as
many as possible without exceeding an ALSA period boundary, now the
driver uses only as many packets as needed to transfer a predetermined
number of frames. As a result, unless the device's clock has an
exceedingly variable rate, the number of URBs making up each period
(and hence each buffer) will remain constant.
The overall effect of the patch is that playback works better in
low-latency settings. The user can still specify values for
frames/period and periods/buffer that exceed the capabilities of the
hardware, of course. But for values that are within those
capabilities, the performance will be improved. For example, testing
shows that a high-speed device can handle 32 frames/period and 3
periods/buffer at 48 KHz, whereas the current driver starts to get
glitchy at 64 frames/period and 2 periods/buffer.
A side effect of these changes is that the "nrpacks" module parameter
is no longer used. The patch removes it.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: Clemens Ladisch <clemens@ladisch.de>
Tested-by: Daniel Mack <zonque@gmail.com>
Tested-by: Eldad Zack <eldad@fogrefinery.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Correct spelling of snd_usb_endpoint_implict_feedback_sink in all
occurances.
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
As we are stopping the endpoints asynchronously now, it's better to
trigger the stop of both data and sync endpoints and wait for pending
stopping operations, instead of the sequential trigger-and-wait
procedure.
So the wait argument in snd_usb_endpoint_stop() is dropped, and it's
expected that the caller synchronizes explicitly by calling
snd_usb_endpoint_sync_pending_stop(). (Actually there is only one
place calling this, so it was safe to change.)
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Reduce the redundant arguments for snd_usb_endpoint_start() and
snd_usb_endpoint_stop(). Also replaced from int to bool.
No functional changes by this commit.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
There are bug reports of a crash with USB-audio devices when PCM
prepare is performed immediately after the stream is stopped via
trigger callback. It turned out that the problem is that we don't
wait until all URBs are killed.
This patch adds a new function to synchronize the pending stop
operation on an endpoint, and calls in the prepare callback for
avoiding the crash above.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=49181
Reported-and-tested-by: Artem S. Tashkinov <t.artem@lycos.com>
Cc: <stable@vger.kernel.org> [v3.6]
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Change the interface to configure an endpoint so that it doesn't require
a hw_params struct. This will allow it to be called from prepare
instead of hw_params, configuring it after system resume.
Signed-off-by: Dylan Reid <dgreid@chromium.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
In order to support devices with implicit feedback streaming models,
packet sizes are now stored with each individual urb, and the PCM
handling code which fills the buffers purely relies on the size fields
now.
However, calling snd_usb_audio_next_packet_size() for all possible
packets in an URB at once, prior to letting the PCM code do its job
does in fact not lead to the same behaviour than what the old code did:
The PCM code will break its loop once a period boundary is reached,
consequently using up less packets that it really could.
As snd_usb_audio_next_packet_size() implements a feedback mechanism to
the endpoints phase accumulator, the number of calls to that function
matters, and when called too often, the data rate runs out of bounds.
Fix this by making the next_packet function public, and call it from the
PCM code as before if the packet data sizes are not defined.
Signed-off-by: Daniel Mack <zonque@gmail.com>
Cc: stable@kernel.org [v3.5+]
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Commit e9ba389c5 ("ALSA: usb-audio: Fix scheduling-while-atomic bug in
PCM capture stream") fixed a scheduling-while-atomic bug that happened
when snd_usb_endpoint_start was called from the trigger callback, which
is an atmic context. However, the patch breaks the idea of the endpoints
reference counting, which is the reason why the driver has been
refactored lately.
Revert that commit and let snd_usb_endpoint_start() take care of the URB
cancellation again. As this function is called from both atomic and
non-atomic context, add a flag to denote whether the function may sleep.
Signed-off-by: Daniel Mack <zonque@gmail.com>
Cc: stable@kernel.org [3.5+]
Signed-off-by: Takashi Iwai <tiwai@suse.de>
With the previous commit that added the new streaming model, all
endpoint and streaming related code is now in endpoint.c, and pcm.c
only acts as a wrapper for handling the packet's payload.
Signed-off-by: Daniel Mack <zonque@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This patch adds a new generic streaming logic for audio over USB.
It defines a model (snd_usb_endpoint) that handles everything that
is related to an USB endpoint and its streaming. There are functions to
activate and deactivate an endpoint (which call usb_set_interface()),
and to start and stop its URBs. It also has function pointers to be
called when data was received or is about to be sent, and pointer to
a sync slave (another snd_usb_endpoint) that is informed when data has
been received.
A snd_usb_endpoint knows about its state and implements a refcounting,
so only the first user will actually start the URBs and only the last
one to stop it will tear them down again.
With this sort of abstraction, the actual streaming is decoupled from
the pcm handling, which makes the "implicit feedback" mechanisms easy to
implement.
In order to split changes properly, this patch only adds the new
implementation but leaves the old one around, so the the driver doesn't
change its behaviour. The switch to actually use the new code is
submitted separately.
Signed-off-by: Daniel Mack <zonque@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
No code altered at this point, simply preparing for upcoming
refactorizations.
Signed-off-by: Daniel Mack <zonque@gmail.com>
Acked-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Move code from endpoint.c into a new file called stream.c and rename
functions so that their names actually reflect what they're doing.
This way, endpoint.c will be available to functions that hold all the
endpoint logic.
Signed-off-by: Daniel Mack <zonque@gmail.com>
Acked-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Clean up the usb audio driver by factoring out a lot of functions to
separate files. Code for procfs, quirks, urbs, format parsers etc all
got a new home now.
Moved almost all special quirk handling to quirks.c and introduced new
generic functions to handle them, so the exceptions do not pollute the
whole driver.
Renamed usbaudio.c to card.c because this is what it actually does now.
Renamed usbmidi.c to midi.c for namespace clarity.
Removed more things from usbaudio.h.
The non-standard drivers were adopted accordingly.
Signed-off-by: Daniel Mack <daniel@caiaq.de>
Cc: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
