[PATCH 1/2] Revert "ASoC: meson: axg-tdm-interface: manage formatters in trigger"
This reverts commit bf5e4887eeddb48480568466536aa08ec7f179a5 because the following and required commit e138233e56e9829e65b6293887063a1a3ccb2d68 causes the following system crash when using audio: BUG: sleeping function called from invalid context at kernel/locking/mutex.c:282
Reported-by: Dmitry Shmidt dimitrysh@google.com Signed-off-by: Neil Armstrong narmstrong@baylibre.com --- sound/soc/meson/axg-tdm-interface.c | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/sound/soc/meson/axg-tdm-interface.c b/sound/soc/meson/axg-tdm-interface.c index 0c31934a9630..e076ced30025 100644 --- a/sound/soc/meson/axg-tdm-interface.c +++ b/sound/soc/meson/axg-tdm-interface.c @@ -351,29 +351,13 @@ static int axg_tdm_iface_hw_free(struct snd_pcm_substream *substream, return 0; }
-static int axg_tdm_iface_trigger(struct snd_pcm_substream *substream, - int cmd, +static int axg_tdm_iface_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { - struct axg_tdm_stream *ts = - snd_soc_dai_get_dma_data(dai, substream); - - switch (cmd) { - case SNDRV_PCM_TRIGGER_START: - case SNDRV_PCM_TRIGGER_RESUME: - case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - axg_tdm_stream_start(ts); - break; - case SNDRV_PCM_TRIGGER_SUSPEND: - case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - case SNDRV_PCM_TRIGGER_STOP: - axg_tdm_stream_stop(ts); - break; - default: - return -EINVAL; - } + struct axg_tdm_stream *ts = snd_soc_dai_get_dma_data(dai, substream);
- return 0; + /* Force all attached formatters to update */ + return axg_tdm_stream_reset(ts); }
static int axg_tdm_iface_remove_dai(struct snd_soc_dai *dai) @@ -413,8 +397,8 @@ static const struct snd_soc_dai_ops axg_tdm_iface_ops = { .set_fmt = axg_tdm_iface_set_fmt, .startup = axg_tdm_iface_startup, .hw_params = axg_tdm_iface_hw_params, + .prepare = axg_tdm_iface_prepare, .hw_free = axg_tdm_iface_hw_free, - .trigger = axg_tdm_iface_trigger, };
/* TDM Backend DAIs */
This commit e138233e56e9829e65b6293887063a1a3ccb2d68 causes the following system crash when using audio on G12A/G12B & SM1 systems: BUG: sleeping function called from invalid context at kernel/locking/mutex.c:282 in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 0, name: swapper/0 preempt_count: 10001, expected: 0 RCU nest depth: 0, expected: 0 Preemption disabled at: schedule_preempt_disabled+0x20/0x2c CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 5.17.0-rc6-03747-gd403c3588f77-dirty #957 Hardware name: SEI Robotics SEI610 (DT) Call trace: dump_backtrace+0xd8/0xf4 show_stack+0x18/0x30 dump_stack_lvl+0x70/0x8c dump_stack+0x18/0x38 __might_resched+0x154/0x164 __might_sleep+0x48/0x78 mutex_lock+0x24/0x60 _snd_pcm_stream_lock_irqsave+0x20/0x3c snd_pcm_period_elapsed+0x24/0xa4 axg_fifo_pcm_irq_block+0x64/0xdc __handle_irq_event_percpu+0x104/0x264 handle_irq_event+0x48/0xb4 ... start_kernel+0x3f0/0x484 __primary_switched+0xc0/0xc8
Revert this commit until the crash is fixed.
Reported-by: Dmitry Shmidt dimitrysh@google.com Signed-off-by: Neil Armstrong narmstrong@baylibre.com --- sound/soc/meson/axg-card.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/meson/axg-card.c b/sound/soc/meson/axg-card.c index cbbaa55d92a6..2b77010c2c5c 100644 --- a/sound/soc/meson/axg-card.c +++ b/sound/soc/meson/axg-card.c @@ -320,7 +320,6 @@ static int axg_card_add_link(struct snd_soc_card *card, struct device_node *np,
dai_link->cpus = cpu; dai_link->num_cpus = 1; - dai_link->nonatomic = true;
ret = meson_card_parse_dai(card, np, &dai_link->cpus->of_node, &dai_link->cpus->dai_name);
On Thu 21 Apr 2022 at 17:57, Neil Armstrong narmstrong@baylibre.com wrote:
This reverts commit bf5e4887eeddb48480568466536aa08ec7f179a5 because the following and required commit e138233e56e9829e65b6293887063a1a3ccb2d68 causes the following system crash when using audio: BUG: sleeping function called from invalid context at kernel/locking/mutex.c:282
Reported-by: Dmitry Shmidt dimitrysh@google.com Signed-off-by: Neil Armstrong narmstrong@baylibre.com
For both: Acked-by: Jerome Brunet jbrunet@baylibre.com
The main reason for the this was to be able to configure the start order between the DPCM Backend and Frontend. Only the trigger() callback has that capability for now.
This HW require the BE to start before FE, otherwise channels get randomly shifted in the output stream if there is more than 2 slots on the link, mainly on the capture path.
This HW require mutexes to handle the TDM formatters (because it uses the CCF API). This why I moved to non-atomic to use trigger(), forgetting that doing so would make period_elapsed() take a mutex from the IRQ ... :/
To properly fix this, I'll need to extend ASoC so the prepare() callback BE/FE call order can also be configured.
On Thu, Apr 21, 2022 at 05:57:24PM +0200, Neil Armstrong wrote:
This reverts commit bf5e4887eeddb48480568466536aa08ec7f179a5 because the following and required commit e138233e56e9829e65b6293887063a1a3ccb2d68 causes the following system crash when using audio:
Please submit patches using subject lines reflecting the style for the subsystem, this makes it easier for people to identify relevant patches. Look at what existing commits in the area you're changing are doing and make sure your subject lines visually resemble what they're doing. There's no need to resubmit to fix this alone.
On Thu, Apr 21, 2022 at 05:57:24PM +0200, Neil Armstrong wrote:
This reverts commit bf5e4887eeddb48480568466536aa08ec7f179a5 because the following and required commit e138233e56e9829e65b6293887063a1a3ccb2d68
One other thing - these should be Fixes: tags, that helps tooling figure out things like backports.
Also:
Please include human readable descriptions of things like commits and issues being discussed in e-mail in your mails, this makes them much easier for humans to read especially when they have no internet access. I do frequently catch up on my mail on flights or while otherwise travelling so this is even more pressing for me than just being about making things a bit easier to read.
Hi Mark,
On 21/04/2022 19:20, Mark Brown wrote:
On Thu, Apr 21, 2022 at 05:57:24PM +0200, Neil Armstrong wrote:
This reverts commit bf5e4887eeddb48480568466536aa08ec7f179a5 because the following and required commit e138233e56e9829e65b6293887063a1a3ccb2d68
One other thing - these should be Fixes: tags, that helps tooling figure out things like backports.
Also:
Please include human readable descriptions of things like commits and issues being discussed in e-mail in your mails, this makes them much easier for humans to read especially when they have no internet access. I do frequently catch up on my mail on flights or while otherwise travelling so this is even more pressing for me than just being about making things a bit easier to read.
Thanks, I'll think of this for the next time.
Neil
On Thu, 21 Apr 2022 17:57:24 +0200, Neil Armstrong wrote:
This reverts commit bf5e4887eeddb48480568466536aa08ec7f179a5 because the following and required commit e138233e56e9829e65b6293887063a1a3ccb2d68 causes the following system crash when using audio: BUG: sleeping function called from invalid context at kernel/locking/mutex.c:282
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] Revert "ASoC: meson: axg-tdm-interface: manage formatters in trigger" commit: c26830b6c5c534d273ce007eb33d5a2d2ad4e969 [2/2] Revert "ASoC: meson: axg-card: make links nonatomic" commit: 0c9b152c72e53016e96593bdbb8cffe2176694b9
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (3)
-
Jerome Brunet -
Mark Brown -
Neil Armstrong