The PCM draining behavior got broken since the recent refactoring, and
this turned out to be the incorrect expectation of the firmware
behavior regarding "draining". While I expected the "drain" flag at
the stop operation would do processing the queued samples, it seems
rather dropping the samples.
As a quick fix, just drop the SNDRV_PCM_INFO_DRAIN_TRIGGER flag, so
that the driver uses the normal PCM draining procedure. Also, put
some caution comment to the function for future readers not to fall
into the same pitfall.
Fixes: d7ca3a7154 ("staging: bcm2835-audio: Operate non-atomic PCM ops")
BugLink: https://github.com/raspberrypi/linux/issues/2983
Cc: stable@vger.kernel.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Acked-by: Stefan Wahren <wahrenst@gmx.net>
Link: https://lore.kernel.org/r/20190914152405.7416-1-tiwai@suse.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When the BCM2835 audio output is used, userspace sees a jitter up to 10ms
in the audio position, aka "delay" -- the number of frames that must
be output before a new frame would be played.
Make this a bit nicer for userspace by interpolating the position
using the CPU clock.
The overhead is small -- an extra ktime_get() every time a GPU message
is sent -- and another call and a few calculations whenever the delay
is sought from userland.
At 48,000 frames per second, i.e. approximately 20 microseconds per
frame, it would take a clock inaccuracy of
20 microseconds in 10 milliseconds -- 2,000 parts per million --
to result in an inaccurate estimate, whereas
crystal- or resonator-based clocks typically have an
inaccuracy of 10s to 100s of parts per million.
Signed-off-by: Mike Brady <mikebrady@eircom.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When it comes to declaring variables it's preferred, when possible, to
use an inverted tree organization scheme.
Also, removes some comments that were useless.
Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Instead of creating a dummy child device to manage the card object,
just use devm stuff directly for releasing with snd_card_free().
This results in a lot of code reduction.
Since the dummy child devices are gone, the device object to be passed
to the memory allocator needs to be adjusted as well.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
All three functions to create PCM objects are fairly resemble, and can
be unified to a single common helper.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The recent ALSA PCM core supports the SNDRV_PCM_INFO_SYNC_APPLPTR flag
indicating that the driver needs the ack call at each appl_ptr
update. This is requirement for the indirect PCM implementations like
bcm2835-audio driver, too.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The memory access to the pages allocated with
SNDRV_DMA_TYPE_CONTINUOUS are basically non-coherent, and it becomes a
problem when a process accesses via mmap.
For the more consistent access, use the device coherent memory, just
by replacing the call pattern in the allocator helpers.
The only point we need to be careful for is the device object passed
there; since bcm2835-audio driver creates fake devices and each card
is created on top of that, we need to pass its parent device as the
real device object.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
For making the whole code more consistent, replace the home-made debug
print macros with the standard dev_err() & co.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This is the most significant part in the patch series.
The bcm2835-audio driver used to queue the commands to vc04 core via
workqueue, but basically the whole accesses to vc04 core are done in
the sleepable context, including the callback calls. In such a case,
rewriting the code using non-atomic PCM ops will simplify the logic a
lot.
This patch does it: all workqueue are gone and each former-work
implementation is now directly called from PCM ops like trigger and
write transfer.
Along with it, the DMA position updater, bcm2835_playback_fifo(), was
also rewritten to use a simpler logic. Now it handles the XRUN and
draining properly by calling snd_pcm_stop() conditionally.
The current position is kept in atomic_t value so that it can be read
concurrently from the pointer callback.
Also, the bcm2835_audio_instance object is allocated at the beginning
of bcm2835_audio_open(). This makes the resource management clearer.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
It seems that the resolution of vc04 callback is in 10 msec; i.e. the
minimal period size is also 10 msec.
This patch adds the corresponding hw constraint.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The chip->audio_mutex is used basically for protecting the opened
stream assignment, and the prepare callback is irrelevant with it.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
These debug messages worsen the code readability a lot while they give
little debuggability (which we already have via tracing, in anyway).
Let's clean them up. This allows us to reduce the
snd_bcm2835_pcm_lib_ioctl() function to be a direct call of the
snd_pcm_lib_ioctl callback (like most other drivers do), too.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When the parameter setup fails, the driver should propagate the error
code instead of silently ignoring it.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
alsa_stream->chip can be never NULL.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The hw_queue_size of PCM indirect helper doesn't need to be set up if
you use the whole given buffer size. Drop the useless
initialization, which just confuses readers.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Some fields in alsa_stream are the values we keep already in PCM
runtime object, hence they are redundant. Use the standard PCM
runtime values instead of the private copies.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The alsa_stream->lock is never used. Kill it.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The handling of SNDRV_PCM_TRIGGER_STOP at the trigger callback is
incorrect: when the STOP is issued, the driver is supposed to drop the
stream immediately. Meanwhile bcm2835 driver checks the DRAINING
state and tries to issue some different command.
This patch straightens things a bit, dropping the incorrect state
checks. The draining behavior would be still not perfect at this
point, but will be improved in a later patch.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The running flag of alsa_stream is basically useless. The running
state is strictly controlled in ALSA PCM core side, hence the check in
PCM trigger and close callbacks are superfluous.
Also, the prefill ack at trigger start became superfluous nowadays
with the ALSA PCM core update.
Let's rip them off.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
All the alsa_stream->open flag checks in the current code are
redundant, and they cannot be racy. For the code simplification,
let's remove the flag and its check.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
bcm2835_audio_setup(), bcm2835_audio_flush_buffers() and
bcm2835_audio_flush_playback_buffers() functions do implement
nothing.
Also, bcm2835_audio_set_ctls() is already called inside
bcm2835_audio_set_params(), so the later call is superfluous.
This patch removes these superfluous implementations.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
In the current code, the mute control is dealt in a special manner,
modifying the current volume and saving the old volume, etc. This is
inconsistent (e.g. change the volume while muted, then unmute), and
way too complex.
Also, the whole volume handling code has conversion between ALSA
volume and raw volume values, which can lead to another
inconsistency and complexity.
This patch simplifies these points:
- The ALSA volume value is saved in chip->volume
- volume->mute saves the mute state
- The mute state is evaluated only when the actual volume is passed to
the hardware, bcm2835_audio_set_ctls()
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The avail_substreams bit mask is checked for the possible racy
accesses, but this cannot happen in practice; i.e. the assignment and
the check are superfluous.
Let's rip them off.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
snd-bcm2835 driver takes the lock with mutex_lock_interruptible() in
all places, which don't make sense. Replace them with the simple
mutex_lock().
Also taking a mutex lock right after creating it for each PCM object
is nonsense, too. It cannot be racy at that point. We can get rid of
it.
Last but not least, initializing chip->audio_mutex at each place is
error-prone. Initialize properly at creating the chip object in
snd_bcm2835_create() instead.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Change 'unsigned *' to 'unsigned int *'. Issue found with checkpatch.
Signed-off-by: Nishka Dasgupta <nishka.dasgupta_ug18@ashoka.edu.in>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Format multiline comment by moving '*/' to a new line. Issue found with
checkpatch.
Signed-off-by: Nishka Dasgupta <nishka.dasgupta_ug18@ashoka.edu.in>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Add blank line after declaration. Issue found with checkpatch.
Signed-off-by: Nishka Dasgupta <nishka.dasgupta_ug18@ashoka.edu.in>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Now that the SPDX tag is in all
drivers/staging/vc04_services/bcm2835-audio/ files, that identifies the
license in a specific and legally-defined manner. So the extra GPL text
wording can be removed as it is no longer needed at all.
This is done on a quest to remove the 700+ different ways that files in
the kernel describe the GPL license text. And there's unneeded stuff
like the address (sometimes incorrect) for the FSF which is never
needed.
No copyright headers or other non-license-description text was removed.
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: <bcm-kernel-feedback-list@broadcom.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
It's good to have SPDX identifiers in all files to make it easier to
audit the kernel tree for correct licenses.
Fix up the bcm2835-audio driver to have a proper SPDX identifier, based
on the license text in the file itself. The SPDX identifier is a
legally binding shorthand, which can be used instead of the full boiler
plate text.
This work is based on a script and data from Thomas Gleixner, Philippe
Ombredanne, and Kate Stewart.
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: <bcm-kernel-feedback-list@broadcom.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Blank lines use up extra space in file and makes the file
larger. So do not use multiple blanklines
Signed-off-by: Keerthi Reddy <keerthigd4990@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Make these const as they are only used during a copy operation.
Done using Coccinelle.
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Check for snd_pcm_ops structures that are only stored in the ops field of
a snd_soc_platform_driver structure or passed as the third argument to
snd_pcm_set_ops. The corresponding field or parameter is declared const,
so snd_pcm_ops structures that have this property can be declared as
const also.
This issue was detected using Coccinelle and the following semantic patch:
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct snd_pcm_ops i@p = { ... };
@ok1@
identifier r.i;
struct snd_soc_platform_driver e;
position p;
@@
e.ops = &i@p;
@ok2@
identifier r.i;
expression e1, e2; position p;
@@
snd_pcm_set_ops(e1, e2, &i@p)
@bad@
position p != {r.p,ok1.p,ok2.p};
identifier r.i;
struct snd_pcm_ops e;
@@
e@i@p
@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
struct snd_pcm_ops i = { ... };
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Now that the indirect-PCM transfer helper gives back an error, we
should return the error from ack callbacks.
Acked-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Remove unnecessary log messages in the driver which are just tracking
function entry and exits.
Signed-off-by: Aishwarya Pant <aishpant@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The firmware for the Raspberry PI already supports simultanous output
of audio through both the HDMI and the Headphone jack. The current
implementation of ALSA doesn't expose this well to user mode since
the firmware audio is represented as a single card.
A newer approach is taken here and a virtual card is created for each
output(HDMI, Headphones, and Traditional ALSA). The firmware has
the concept of channels or streams for which the number to use is
passed in the device tree. These streams are allocated to each of the
virtual cards.
As a side effect of this change, since each output is represented
independenly it's now very easy to use PulseAudio to control the
priorities of the outputs.
Testing:
Audacity and VLC were both loaded at the same time. Each application
was assigned to a different card. With this change I was able to play
different music files at the same time through the HDMI and Headphones
jacks and control the audio independently.
Signed-off-by: Michael Zoran <mzoran@crowfest.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This was reported by checkpatch.pl
Signed-off-by: Narcisa Ana Maria Vasile <narcisaanamaria12@gmail.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This was reported by checkpatch.pl
Signed-off-by: Alexandru Jercaianu <alex.jercaianu@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The bcm2835-audio driver is part of v04_services, so it makes
sense for it to be located under vc04_services to make
configuration clearer.
Signed-off-by: Michael Zoran <mzoran@crowfest.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
