ASoC: tegra: Fix Master Volume Control
Subsystems (4)
Jon Hunter
maintainerPATCH 1/1 ASoC: tegra: Fix Master Volume Control
Commit 3ed2b549b39f ("ALSA: pcm: fix wait_time calculations") corrected the PCM wait_time calculations and in doing so reduced the calculated wait_time. This exposed an issue with the Tegra Master Volume Control (MVC) device where the reduced wait_time caused the MVC to fail. For now fix this by setting the default wait_time for Tegra to be 500ms.
--- sound/soc/tegra/tegra_pcm.c | 3 +++ 1 file changed, 3 insertions(+)
sound/soc/tegra/tegra_pcm.c +3 -0
@@ -117,6 +117,9 @@ int tegra_pcm_open(struct snd_soc_component *component,return ret; } +/* Set wait time to 500ms by default */ + substream->wait_time =500 ; +return 0 ; }EXPORT_SYMBOL_GPL (tegra_pcm_open);
Signature · 1 line
2.34.1
Quoted 9 lines · Jon Hunter
Commit
3ed2b549b39f("ALSA: pcm: fixwait_timecalculations") corrected the PCMwait_timecalculations and in doing so reduced the calculatedwait_time. This exposed an issue with the Tegra Master Volume Control (MVC) device where the reducedwait_timecaused the MVC to fail. For now fix this by setting the defaultwait_timefor Tegra to be 500ms.Fixes 3ed2b549b39f ("ALSA: pcm: fix wait_time calculations")Signed-off-by Jon Hunter <[email protected]>
Hm, it's still not clear why it fails. The commit above changes the timeout of wait_for_avail() to the full-buffer + 10% margin. In thoery, the loop should abort after the full buffer read, and that must be enough. If there were a large FIFO behind, it might be overflow, but the fifo_size of Tegra driver seems 4, so it's negligible.
If extending the timeout "fixes" the problem, it might indicate that the period update IRQ is triggered too late. Could you measure the timing of each snd_pcm_period_elapsed() and wait_for_avail() call?
thanks,
Takashi
Quoted 21 lines
--- sound/soc/tegra/tegra_pcm.c | 3 +++ 1 file changed, 3 insertions(+)sound/soc/tegra/tegra_pcm.c +3
-0@@ -117,6 +117,9 @@ int tegra_pcm_open(struct snd_soc_component *component,return ret; } +/* Set wait time to 500ms by default */ + substream->wait_time =500 ; +return 0 ; }EXPORT_SYMBOL_GPL (tegra_pcm_open);Signature · 1 line
2.34.1
Quoted 20 lines · Takashi Iwai
Quoted 9 lines · Jon Hunter
Commit
3ed2b549b39f("ALSA: pcm: fixwait_timecalculations") corrected the PCMwait_timecalculations and in doing so reduced the calculatedwait_time. This exposed an issue with the Tegra Master Volume Control (MVC) device where the reducedwait_timecaused the MVC to fail. For now fix this by setting the defaultwait_timefor Tegra to be 500ms.Fixes 3ed2b549b39f ("ALSA: pcm: fix wait_time calculations")Signed-off-by Jon Hunter <[email protected]>Hm, it's still not clear why it fails. The commit above changes the timeout of
wait_for_avail()to the full-buffer + 10% margin. In thoery, the loop should abort after the full buffer read, and that must be enough. If there were a large FIFO behind, it might be overflow, but thefifo_sizeof Tegra driver seems 4, so it's negligible.
Quoted 3 lines
If extending the timeout "fixes" the problem, it might indicate that the period update IRQ is triggered too late. Could you measure the timing of each
snd_pcm_period_elapsed()andwait_for_avail()call?
OTOH, it's already at a pretty late stage for 6.4, and we need an urgent regression fix. So it's better to paper over it now, while hunting further for the real culprit.
Takashi
Jon Hunter
maintainerQuoted 22 lines · Takashi Iwai
Quoted 20 lines · Takashi Iwai
Quoted 9 lines · Jon Hunter
Commit
3ed2b549b39f("ALSA: pcm: fixwait_timecalculations") corrected the PCMwait_timecalculations and in doing so reduced the calculatedwait_time. This exposed an issue with the Tegra Master Volume Control (MVC) device where the reducedwait_timecaused the MVC to fail. For now fix this by setting the defaultwait_timefor Tegra to be 500ms.Fixes 3ed2b549b39f ("ALSA: pcm: fix wait_time calculations")Signed-off-by Jon Hunter <[email protected]>Hm, it's still not clear why it fails. The commit above changes the timeout of
wait_for_avail()to the full-buffer + 10% margin. In thoery, the loop should abort after the full buffer read, and that must be enough. If there were a large FIFO behind, it might be overflow, but thefifo_sizeof Tegra driver seems 4, so it's negligible.
Quoted 7 lines
Quoted 3 lines
If extending the timeout "fixes" the problem, it might indicate that the period update IRQ is triggered too late. Could you measure the timing of each
snd_pcm_period_elapsed()andwait_for_avail()call?OTOH, it's already at a pretty late stage for 6.4, and we need an urgent regression fix. So it's better to paper over it now, while hunting further for the real culprit.
I have filed a bug report internally to investigate this, but yes for now was hoping to get something in place for v6.4 to avoid any regressions.
Thanks
Jon
Signature · 1 line
nvpublic
[CCing the regression list, as it should be in the loop for regressions:
https://docs.kernel.org/admin-guide/reporting-regressions.html]
[TLDR: I'm adding this report to the list of tracked Linux kernel regressions; the text you find below is based on a few templates paragraphs you might have encountered already in similar form. See link in footer if these mails annoy you.]
Quoted 10 lines · Jon Hunter
Commit
3ed2b549b39f("ALSA: pcm: fixwait_timecalculations") corrected the PCMwait_timecalculations and in doing so reduced the calculatedwait_time. This exposed an issue with the Tegra Master Volume Control (MVC) device where the reducedwait_timecaused the MVC to fail. For now fix this by setting the defaultwait_timefor Tegra to be 500ms.Fixes 3ed2b549b39f ("ALSA: pcm: fix wait_time calculations")Signed-off-by Jon Hunter <[email protected]>---
[...]
Thanks for the report. To be sure the issue doesn't fall through the cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression tracking bot:
#regzbot ^introduced 3ed2b549b39f
#regzbot title ASoC: tegra: Master Volume Control broken #regzbot ignore-activity
This isn't a regression? This issue or a fix for it are already discussed somewhere else? It was fixed already? You want to clarify when the regression started to happen? Or point out I got the title or something else totally wrong? Then just reply and tell me -- ideally while also telling regzbot about it, as explained by the page listed in the footer of this mail.
Developers: When fixing the issue, remember to add 'Link:' tags pointing to the report (the parent of this mail). See page linked in footer for details.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) --
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.
Mark Brown
maintainerQuoted 8 lines · Jon Hunter
Commit
3ed2b549b39f("ALSA: pcm: fixwait_timecalculations") corrected the PCMwait_timecalculations and in doing so reduced the calculatedwait_time. This exposed an issue with the Tegra Master Volume Control (MVC) device where the reducedwait_timecaused the MVC to fail. For now fix this by setting the defaultwait_timefor Tegra to be 500ms.[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: tegra: Fix Master Volume Control
commit: f9fd804aa0a36f15a35ca070ec4c52650876cc29
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