| 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 |
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.
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
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 --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 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(-)