bcm2835-audio: Backport upstream commits #2948
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
commit fb4b9f02986fcb5ae751106ef9b027806b5dd750 upstream. This reverts commit fb8cc99f05687ca5565dc53a7ee0dd86aefad952.
commit 4eae66777a262ac9707980ea0cfe902afadfb577 upstream. This reverts commit 02d205a57c4c943fc2a5b1ac7c912ce01944f700.
commit ed4c2e5dc4216d5dded502bfcf594d3984e6bccd upstream. This reverts commit 786ced30fec053b27248ed5b24dcde61ed3f47f6.
commit ce4bb1aa271a97047b80ac917a5d91b54925913b upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit ab91e26229eaca2832df51e13c1285aea3be33ab upstream. The "IEC958 Playback Stream" control does basically the very same thing as "IEC958 Playback Default" redundantly. The former should have been stream-specific and restored after closing the stream, but we don't do in that way. Since it's nothing but confusion, remove this fake. Signed-off-by: Takashi Iwai <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 821950d3da4bf97bcfedcb812176a0f26b833db0 upstream. Only a few of them are really needed. Signed-off-by: Takashi Iwai <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 14b1f4cba853a11c7b381ad919622f38eb194bd7 upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 495e5a0d83d3902c741771f267a702ae19da8ab6 upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 124950ebe9fa8547c59e8d4acc8d6c59e6278ed6 upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit ad13924de6b07cb52714ea1809c57b2e72a24504 upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 02f2376321d75e78117f39ff81f215254ee6b4ef upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 7d2a91f5f1bcf08ca257bcf1ed9721fcd341f834 upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 5332f6f012c0bf3a45c77dbc0f79814443a884d4 upstream. The alsa_stream->lock is never used. Kill it. Signed-off-by: Takashi Iwai <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit b8f7fdd50890b848e085c0519469aed4ff4d9b54 upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 7318ec896f4856fae2bb013858e422fa078201e1 upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 8bcf9f252c29c2d5bcce3db605c0ebf1ef230f9c upstream. alsa_stream->chip can be never NULL. Signed-off-by: Takashi Iwai <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit fee5638fe552ff8222c3a5bdcc4a34255e248d8c upstream. When the parameter setup fails, the driver should propagate the error code instead of silently ignoring it. Signed-off-by: Takashi Iwai <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 055e1c330d04df87d4730a5db837161c11ddaafc upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit f0eb15d055380ff127e5f12c8fad2b36bdb3c006 upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 93c66acaf68b5247c3121a46a71ff6a70fc1d492 upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 326a6edcb2ada56375bd7d3fc24c83f58e8da7f3 upstream. The bcm2835_audio_instance object contains the array of VCHI_SERVICE_HANDLE_T, while the code assumes and uses only the first element explicitly. Let's reduce to a single vchi handle for simplifying the code. Signed-off-by: Takashi Iwai <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 769a8e9bf5cf39813f52962fdafdf7e4d52ad585 upstream. This is a cleanup and code refactoring in bcm2835-vchiq.c. The major code changes are to provide local helpers for easier use of lock / unlock, and message passing with/without response wait. This allows us to reduce lots of open codes. Also, the max packet is set at opening the stream, not at each time when the write gets called. Signed-off-by: Takashi Iwai <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 5c7883e5f27e829f3f3a2ba174d4a724bfd5f026 upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 898001a0c845cefe5d47d133485712412853f0a8 upstream. Instead of allocating a separate snd_device object, let snd_card_new() allocate the private resource. This simplifies the code. Signed-off-by: Takashi Iwai <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit b7584b64168208ebc14160770c0966b8b12fc16b upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 7e46fff5f19ce2b8a9891e4c08631c64d06e9e17 upstream. Yet a few header files are included unnecessarily. Drop them. Also remove trivial comments. Signed-off-by: Takashi Iwai <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit b876f2075808e95e244053caa53fa7e86e929a99 upstream. For more consistency, move the module parameter description right after its variable definition. Signed-off-by: Takashi Iwai <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit ad29c6e6cbf6f2af7362b043adad51a3be3d39c7 upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit b59d6a5f73501f74848d6700101e7736afe3d54a upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 74470ffeb9aed5548654cfca881bf1d7469fe9c4 upstream. All three functions to create PCM objects are fairly resemble, and can be unified to a single common helper. Signed-off-by: Takashi Iwai <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit dc5c0eb1e8601206dffbfc302cbd190f89dcd040 upstream. Just a minor code refactoring and adding some const prefix. Signed-off-by: Takashi Iwai <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 872ae2d63d516a2a3b9c833d8685afcfa7814542 upstream. 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 <[email protected]> Tested-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit a90d8f49cc7fd7220aa24b85fc74ef3cfd62b96f upstream. The device communicates with the audio core using FOURCC codes. The driver was generating them using different macros/expressions. We now use the same macro to create them and centralize all the definitions. Signed-off-by: Nicolas Saenz Julienne <[email protected]> Reviewed-by: Takashi Iwai <[email protected]> Acked-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 2e5f59fb77397cab3bc3d156e8be4164a67d32ef upstream. The memory is being allocated with devres_alloc(), wich ultimately uses __GFP_ZERO to call kmalloc. We don't need to zero the memory area again in bcm2835-audio. Signed-off-by: Nicolas Saenz Julienne <[email protected]> Reviewed-by: Takashi Iwai <[email protected]> Acked-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
…l comments commit d048385a070552ae819f99f05bd03ec41072783d upstream. 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 <[email protected]> Reviewed-by: Takashi Iwai <[email protected]> Acked-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 9c2eaf7da855d314a369d48b9cbf8ac80717a1d0 upstream. In this case explicitly naming the union doesn't help overall code comprehension and clutters it. Signed-off-by: Nicolas Saenz Julienne <[email protected]> Reviewed-by: Takashi Iwai <[email protected]> Acked-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 96f3bd8ae6516898c7b411ecb87064bb0dd25415 upstream. There will only be one probe function, there is no use for appendig "_dt" the end of the name. Signed-off-by: Nicolas Saenz Julienne <[email protected]> Reviewed-by: Takashi Iwai <[email protected]> Acked-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 82cdc0c6b6faf877e2aecb957cffa9cb578cc572 upstream. It was called bcm2835_alsa0_driver, that "0" didn't mean much. Suggested-by: Takashi Iwai <[email protected]> Signed-off-by: Nicolas Saenz Julienne <[email protected]> Acked-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 01ec7398c56e8f1b903ecb3c5c75400e263eef43 upstream. The following tasks were completed or not the right solution: 1/2- Not the proper solution, we should register a platform device in vchiq the same way it's done with bcm2835-camera as commented here: https://lkml.org/lkml/2018/10/16/1131 2/3- Fixed by Takashi Iwai here: https://lkml.org/lkml/2018/9/4/587 Also, adds a new task as per mailing list conversation. Signed-off-by: Nicolas Saenz Julienne <[email protected]> Acked-by: Stefan Wahren <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit a105a3a72824e0ac685a0711a67e4dbe29de62d0 upstream. 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 <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 458d4866a34d0c129ffc3bd56345b2166ba46d77 upstream. Enable the compilation test for bcm2835-audio to gain more build coverage. Signed-off-by: Stefan Wahren <[email protected]> Reviewed-by: Nicolas Saenz Julienne <[email protected]> Reviewed-by: Dan Carpenter <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 1e55d56344b0777d6cee9b9e4a813d53728ee798 upstream. There is not much value behind this boilerplate, so use module_platform_driver() instead. Signed-off-by: Stefan Wahren <[email protected]> Reviewed-by: Nicolas Saenz Julienne <[email protected]> Reviewed-by: Dan Carpenter <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 438fc48260a0afc4cee733e5bc20234ff2bbef56 upstream. Just like the bcm2835-video make this a platform driver which is probed by vchiq. In order to change the number of channels use a module parameter instead, but use the maximum as default. Signed-off-by: Stefan Wahren <[email protected]> Reviewed-by: Nicolas Saenz Julienne <[email protected]> Reviewed-by: Dan Carpenter <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 136ff5e49271c4c8fceeca5491c48e66b961564b upstream. We free instance here and in the caller. It should be only the caller which handles it. Fixes: d7ca3a7 ("staging: bcm2835-audio: Operate non-atomic PCM ops") Signed-off-by: Dan Carpenter <[email protected]> Reviewed-by: Takashi Iwai <[email protected]> Cc: stable <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
popcornmix
added a commit
to raspberrypi/firmware
that referenced
this pull request
Apr 30, 2019
kernel: Add PiGlow overlay See: raspberrypi/linux#2946 kernel: bcm2835-audio: Backport upstream commits See: raspberrypi/linux#2948
popcornmix
added a commit
to Hexxeh/rpi-firmware
that referenced
this pull request
Apr 30, 2019
kernel: Add PiGlow overlay See: raspberrypi/linux#2946 kernel: bcm2835-audio: Backport upstream commits See: raspberrypi/linux#2948
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR pulls in audio commits from upstream (5.1 kernel).
These commits have come from people with more knowledge of alsa than us, and they look sensible and don't obviously break anything.
I've reverted 3 commits we already had pulled in to allow the upstream commits to be cherry-picked untouched. The 3 commits gets added back in later.