After the recent fix of runtime PM for USB-audio driver, we got a
lockdep warning like:
=============================================
[ INFO: possible recursive locking detected ]
4.2.0-rc8+ #61 Not tainted
---------------------------------------------
pulseaudio/980 is trying to acquire lock:
(&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
but task is already holding lock:
(&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
This comes from snd_usb_autoresume() invoking down_read() and it's
used in a nested way. Although it's basically safe, per se (as these
are read locks), it's better to reduce such spurious warnings.
The read lock is needed to guarantee the execution of "shutdown"
(cleanup at disconnection) task after all concurrent tasks are
finished. This can be implemented in another better way.
Also, the current check of chip->in_pm isn't good enough for
protecting the racy execution of multiple auto-resumes.
This patch rewrites the logic of snd_usb_autoresume() & co; namely,
- The recursive call of autopm is avoided by the new refcount,
chip->active. The chip->in_pm flag is removed accordingly.
- Instead of rwsem, another refcount, chip->usage_count, is introduced
for tracking the period to delay the shutdown procedure. At
the last clear of this refcount, wake_up() to the shutdown waiter is
called.
- The shutdown flag is replaced with shutdown atomic count; this is
for reducing the lock.
- Two new helpers are introduced to simplify the management of these
refcounts; snd_usb_lock_shutdown() increases the usage_count, checks
the shutdown state, and does autoresume. snd_usb_unlock_shutdown()
does the opposite. Most of mixer and other codes just need this,
and simply returns an error if it receives an error from lock.
Fixes: 9003ebb13f ('ALSA: usb-audio: Fix runtime PM unbalance')
Reported-and-tested-by: Alexnader Kuleshov <kuleshovmail@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The M-Audio Transit exposes an interface with a SYNC_NONE attribute.
This is not a valid value according to the USB audio classspec. However
there is a sync endpoint associated to this record. Changing the logic to
try to use this sync endpoint allows for seamless transitions between
altset 2 and altset 3. If any errors happen, the behavior remains the same.
$ more /proc/asound/card1/stream0
M-Audio Transit USB at usb-0000:00:14.0-2, full speed : USB Audio
Playback:
Status: Stop
Interface 1
Altset 1
Format: S24_3LE
Channels: 2
Endpoint: 3 OUT (ADAPTIVE)
Rates: 48001 - 96000 (continuous)
Interface 1
Altset 2
Format: S24_3LE
Channels: 2
Endpoint: 3 OUT (NONE)
Rates: 8000 - 48000 (continuous)
Interface 1
Altset 3
Format: S16_LE
Channels: 2
Endpoint: 3 OUT (ASYNC)
Rates: 8000 - 48000 (continuous)
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When a transition occurs between alternate settings that do not use the
same synchronization method, the substream pointers were not reset.
This prevents audio from being played during the second transition.
Identified and tested with M-Audio Transit device
(0763:2006 Midiman M-Audio Transit)
Details of the issue:
First playback to adaptive endpoint:
$ aplay -Dhw:1,0 ~/24_96.wav
Playing WAVE '/home/plb/24_96.wav' : Signed 24 bit Little Endian in 3bytes,
Rate 96000 Hz, Stereo
[ 3169.297556] usb 1-2: setting usb interface 1:1
[ 3169.297568] usb 1-2: Creating new playback data endpoint #3
[ 3169.298563] usb 1-2: Setting params for ep #3 (type 0, 3 urbs), ret=0
[ 3169.298574] usb 1-2: Starting data EP @ffff880035fc8000
first playback to asynchronous endpoint:
$ aplay -Dhw:1,0 ~/16_48.wav
Playing WAVE '/home/plb/16_48.wav' : Signed 16 bit Little Endian,
Rate 48000 Hz, Stereo
[ 3204.520251] usb 1-2: setting usb interface 1:3
[ 3204.520264] usb 1-2: Creating new playback data endpoint #3
[ 3204.520272] usb 1-2: Creating new capture sync endpoint #83
[ 3204.521162] usb 1-2: Setting params for ep #3 (type 0, 4 urbs), ret=0
[ 3204.521177] usb 1-2: Setting params for ep #83 (type 1, 4 urbs), ret=0
[ 3204.521182] usb 1-2: Starting data EP @ffff880035fce000
[ 3204.521204] usb 1-2: Starting sync EP @ffff8800bd616000
second playback to adaptive endpoint: no audio and error on terminal:
$ aplay -Dhw:1,0 ~/24_96.wav
Playing WAVE '/home/plb/24_96.wav' : Signed 24 bit Little Endian in 3bytes,
Rate 96000 Hz, Stereo
aplay: pcm_write:1939: write error: Input/output error
[ 3239.483589] usb 1-2: setting usb interface 1:1
[ 3239.483601] usb 1-2: Re-using EP 3 in iface 1,1 @ffff880035fc8000
[ 3239.484590] usb 1-2: Setting params for ep #3 (type 0, 4 urbs), ret=0
[ 3239.484606] usb 1-2: Setting params for ep #83 (type 1, 4 urbs), ret=0
This last line shows that a sync endpoint is used when it shouldn't.
The sync endpoint is no longer valid and the pointers are corrupted
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The first URBs are submitted during the prepare stage. When .trigger is
called, the ALSA core saves a trigger tstamp that doesn't correspond to
the actual time when the samples are submitted. The trigger_tstamp is
now updated when the first data are submitted to avoid any time offsets.
A usb-specific trigger_tstamp_pending_update flag is used for now,
at some point the flag would need to move to the ALSA core, USB
is not the only interface where silent block transfers are programmed
as part of the prepare stage, with actual data enabled when .trigger
is called.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Denon/Marantz USB DACs need a specific vendor command to switch between PCM and
DSD mode. This patch adds a new quirk function to switch between the two modes
using the specific USB vendor command.
This patch applies to the following devices:
- Marantz SA-14S1
- Marantz HD-DAC1
Signed-off-by: Jurgen Kramer <gtmkramer@xs4all.nl>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This (widely used) construction:
if(printk_ratelimit())
dev_dbg()
Causes the ratelimiting to spam the kernel log with the "callbacks suppressed"
message below, even while the dev_dbg it is supposed to rate limit wouldn't
print anything because DEBUG is not defined for this device.
[ 533.803964] retire_playback_urb: 852 callbacks suppressed
[ 538.807930] retire_playback_urb: 852 callbacks suppressed
[ 543.811897] retire_playback_urb: 852 callbacks suppressed
[ 548.815745] retire_playback_urb: 852 callbacks suppressed
[ 553.819826] retire_playback_urb: 852 callbacks suppressed
So use dev_dbg_ratelimited() instead of this construction.
Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
BugLink: http://bugs.launchpad.net/bugs/1305133
Malfunctioning or slow devices can cause a flood of dmesg SPAM.
I've ignored checkpatch.pl complaints about the use of printk_ratelimit() in favour
of prior art in sound/usb/pcm.c.
WARNING: Prefer printk_ratelimited or pr_<level>_ratelimited to printk_ratelimit
+ if (printk_ratelimit() &&
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Eldad Zack <eldad@fogrefinery.com>
Cc: Daniel Mack <zonque@gmail.com>
Cc: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Convert with dev_err() and co from snd_printk(), etc.
As there are too deep indirections (e.g. ep->chip->dev->dev),
a few new local macros, usb_audio_err() & co, are introduced.
Also, the device numbers in some messages are dropped, as they are
shown in the prefix automatically.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
As Clemens Ladisch kindly explained:
"Please note that there are two methods to identify alternate settings:
the number, which is the value in bAlternateSetting, and the index,
which is the index in the descriptor array. There might be some wording
in the USB spec that these two values must be the same, but in reality,
[insert standard rant about firmware writers], bAlternateSetting
must be treated as a random ID value."
This patch changes the name to express the correct usage semantics.
No functional change.
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
If setting the interface fails, the SUBSTREAM_FLAG_SYNC_EP_STARTED
should be cleared.
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The only call site for deactivate_endpoints() at snd_usb_hw_free().
The return value is not checked there, as it is irrelevant if it
fails on hw_free.
This patch moves the deactivation of the endpoints directly into
snd_usb_hw_free().
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>
Since the quirks all apply to implicit feedback (the source endpoint
is always a data endpoint), there's no need to set and check
a flag for it.
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
An implicit feedback endpoint can only be a capture source. The
consumer (sink) of the implicit feedback endpoint is therefore limited
to playback EPs.
Check if the target endpoint is a playback first and remove redundant
checks.
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Since implicit_fb is not changed, !implicit_fb will always
be true - it is set only after these checks.
Similarly, there's also no need to set it at the top of the function.
Change the type of implicit_fb to bool (more appropriate).
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Reverse logic on the conditions required to qualify for a sync endpoint
and remove one level of indendation.
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Separate setting implicit feedback quirks from setting
a sync endpoint (which may also be explicit feedback or async).
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Setting the sync endpoint currently takes up about half of set_format().
Move it to a dedicated function.
No functional change.
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
All the Roland/Edirol/BOSS USB audio devices that need implicit feedback
show this unambiguously in their descriptors, so it might be a good idea
to let the driver detect this.
This should make playback work correctly (at least with Jack) with the
following devices:
- BOSS GT-100
- BOSS JS-8 Jam Station
- Edirol M-16DX
- Roland GAIA SH-01
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Instead of reading bInterfaceProtocol from the descriptor whenever it's
needed, store this value in the audioformat structure. Besides
simplifying some code, this will allow us to correctly handle vendor-
specific devices where the descriptors are marked with other values.
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Add a function to handle conversion from snd_pcm_format_t
to bitwise with proper typing.
Change such conversions to use this function and silence sparse
warnings.
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
There is quite some confusion around the bit-ordering in DSD samples,
and no general agreement that defines whether hardware is supposed to
expect the oldest sample in the MSB or the LSB of a byte.
ALSA will hence set the rule that on the software API layer, bytes
always carry the oldest bit in the most significant bit of a byte, and
the driver has to translate that at runtime in order to match the
hardware layout.
This patch adds support for this by adding a boolean flag to the
audio format struct.
Signed-off-by: Daniel Mack <zonque@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
In order to provide a compatibility way for pushing DSD
samples through ordinary PCM channels, the "DoP open Standard" was
invented. See http://www.dsd-guide.com for the official document.
The host is required to stuff DSD marker bytes (0x05, 0xfa,
alternating) in the MSB of 24 bit wide samples on the bus, in addition
to the 16 bits of actual DSD sample payload.
To support this, the hardware and software stride logic in the driver
has to be tweaked a bit, as we make the userspace believe we're
operating on 16 bit samples, while we in fact push one more byte per
channel down to the hardware.
The DOP runtime information is stored in struct snd_usb_substream, so
we can keep track of our state across multiple calls to
prepare_playback_urb_dsd_dop().
Signed-off-by: Daniel Mack <zonque@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
For normal PCM transfer, this change has no effect, as the endpoint's
stride is always frame_bits/8. For DSD DOP streams, however, which is
added later, the hardware stride differs from the software stride, and
the endpoint has the correct information in these cases.
Signed-off-by: Daniel Mack <zonque@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When recording at 176.2KHz or 192Khz, the device adds a 32-bit length
header to the capture packets, which obviously needs to be ignored for
recording to work properly.
Userspace expected: L0 L1 L2 R0 R1 R2
...but actually got: R2 L0 L1 L2 R0 R1
Also, the last byte of the length header being interpreted as L0 of
the first sample caused spikes every 0.5ms, resulting in a loud 16KHz
tone (about the highest 'B' on a piano) being present throughout
captures.
Tested at all sample rates on an E-Mu 0404USB, and tested for
regressions on a generic USB headset.
Signed-off-by: Calvin Owens <jcalvinowens@gmail.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
It turns out the devices from Playback Design need the delay quirk
after usb_set_interface from clocks.c as well. Make it a proper
quirks function and factor out the code to quirks.c.
Signed-off-by: Daniel Mack <zonque@gmail.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>
"Playback Design" products need a 50ms delay after setting the USB
interface.
Signed-off-by: Daniel Mack <zonque@gmail.com>
Reported-by: Andreas Koch <andreas@akdesigninc.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Adds quirks and mixer support for the M-Audio Fast Track C600 USB
audio interface. This device is very similar to the C400 - the C600
simply has some more inputs and outputs, so the existing C400 support
is extended to support this device as well.
Signed-off-by: Matt Gruskin <matthew.gruskin@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Because currently snd_printd() and snd_printdd() macros are expanded
to empty when CONFIG_SND_DEBUG=n, a compile warning like below
appears sometimes, and we had to covert it by ugly ifdefs:
sound/pci/hda/patch_sigmatel.c: In function ‘stac92hd71bxx_fixup_hp’:
sound/pci/hda/patch_sigmatel.c:2434:24: warning: unused variable ‘spec’ [-Wunused-variable]
For "fixing" these issues better, this patch replaces snd_printd() and
snd_printdd() definitions with empty inline functions instead of
macros. This should have the same effect but shut up warnings like
above.
But since we had already put ifdefs, changing to inline functions
would trigger compile errors. So, such ifdefs is removed in this
patch.
In addition, snd_pci_quirk name field is defined only when
CONFIG_SND_DEBUG_VERBOSE is set, and the reference to it in
snd_printdd() argument triggers the build errors, too. For avoiding
these errors, introduce a new macro snd_pci_quirk_name() that is
defined no matter how the debug option is set.
Reported-by: Stratos Karafotis <stratosk@semaphore.gr>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The commit [0d9741c0: ALSA: usb-audio: sync ep init fix for
audioformat mismatch] introduced the correction of parameters to be
set for sync EP. But since the new code assumes that the sync EP is
always paired with the data EP of another direction, it triggers Oops
when a device only with a single direction is used.
This patch adds a proper check of sync EP type and the presence of the
paired substream for avoiding the crash.
Reported-and-tested-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Enable delay report on capture path. The delay is reset when an
URB is retired and increment at each call to .pointer based
on frame counter changes. The precision of the delay
information is limited to 1ms as in the playback case.
This reverts commit 3f94fad095.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Commit 947d299686 , "ALSA: snd-usb:
properly initialize the sync endpoint", while correcting the
initialization of the sync endpoint when opening just the data
endpoint, prevents devices that has a sync endpoint, with a channel
number different than that of the data endpoint, from functioning.
Due to a different channel and period bytes count, attempting to
initialize the sync endpoint will fail at the usb host driver.
For example, when using xhci:
cannot submit urb 0, error -90: internal error
With this patch, if a sync endpoint has multiple audioformats, a
matching audioformat is preferred. An audioformat must be found
with at least one channel and support the requested sample rate
and PCM format, otherwise the stream will not be opened.
If the number of channels differ between the selected audioformat
and the requested format, adjust the period bytes count accordingly.
It is safe to perform the calculation on the basis of the channel
count, since the requested PCM audio format and the rate must be
supported by the selected audioformat.
Cc: Jeffrey Barish <jeff_barish@earthlink.net>
Cc: Daniel Mack <zonque@gmail.com>
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The playback endpoint uses implicit feedback mode, similar
to the M-Audio FTU. Like with the FTU, we need to associate
the sync pipe ourselves.
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
When a playback stream is paused, the stream isn't actually stopped,
thus we still need to take care of the in-flight data amount for the
delay calculation. Otherwise the value of subs->last_delay is no
longer reliable and can give a bogus value after resuming from pause.
This will result in "delay: estimated XX, actual YY" error messages.
Also, during pause after all in flight data are processed
(i.e. last_delay = 0), we don't have to calculate the actual delay
from the current frame. Give a short path in such a case.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
It doesn't make sense to calculate the delay for capture streams in
the current implementation. It's always zero, so we should skip the
computation in snd_usb_pcm_pointer() in the case of capture.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Jeffrey Barish reported an obvious bug in the pcm part of the usb-audio
driver which causes the code to not initialize the sync endpoint from
configure_endpoint().
Reported-by: Jeffrey Barish <jeff_barish@earthlink.net>
Signed-off-by: Daniel Mack <zonque@gmail.com>
Cc: stable@kernel.org [3.5+]
Signed-off-by: Takashi Iwai <tiwai@suse.de>
PCM hw_free and close should wait until all the pending stop
operations have been finished. Basically only PCM trigger callback
should use non-wait calls.
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>
Replace mutex with rwsem for codec->shutdown protection so that
concurrent accesses are allowed.
Also add the protection to snd_usb_autosuspend() and
snd_usb_autoresume(), too.
Reported-by: Matthieu CASTET <matthieu.castet@parrot.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Takashi Iwai