diff mbox series

[1/2] Revert "ASoC: meson: axg-tdm-interface: manage formatters in trigger"

Message ID 20220421155725.2589089-1-narmstrong@baylibre.com (mailing list archive)
State Accepted
Commit c26830b6c5c534d273ce007eb33d5a2d2ad4e969
Headers show
Series [1/2] Revert "ASoC: meson: axg-tdm-interface: manage formatters in trigger" | expand

Commit Message

Neil Armstrong April 21, 2022, 3:57 p.m. UTC
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(-)

Comments

Jerome Brunet April 21, 2022, 4:17 p.m. UTC | #1
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.
Mark Brown April 21, 2022, 4:58 p.m. UTC | #2
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.
Mark Brown April 21, 2022, 5:20 p.m. UTC | #3
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.
Mark Brown April 21, 2022, 7:18 p.m. UTC | #4
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
Neil Armstrong April 22, 2022, 8:26 a.m. UTC | #5
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
diff mbox series

Patch

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 */