| Message ID | 20230613093453.13927-1-jonathanh@nvidia.com |
|---|---|
| State | Accepted |
| Commit | f9fd804aa0a36f15a35ca070ec4c52650876cc29 |
| Headers | show |
| Series | ASoC: tegra: Fix Master Volume Control | expand |
On Tue, 13 Jun 2023 11:55:16 +0200, Takashi Iwai wrote: > > On Tue, 13 Jun 2023 11:34:53 +0200, > Jon Hunter wrote: > > > > 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. > > > > Fixes: 3ed2b549b39f ("ALSA: pcm: fix wait_time calculations") > > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > > 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? 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
On 13/06/2023 10:57, Takashi Iwai wrote: > On Tue, 13 Jun 2023 11:55:16 +0200, > Takashi Iwai wrote: >> >> On Tue, 13 Jun 2023 11:34:53 +0200, >> Jon Hunter wrote: >>> >>> 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. >>> >>> Fixes: 3ed2b549b39f ("ALSA: pcm: fix wait_time calculations") >>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >> >> 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? > > 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
[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.] On 13.06.23 11:34, Jon Hunter wrote: > 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. > > Fixes: 3ed2b549b39f ("ALSA: pcm: fix wait_time calculations") > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- > [...] 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.
diff --git a/sound/soc/tegra/tegra_pcm.c b/sound/soc/tegra/tegra_pcm.c index 468c8e77de21..0b69cebc9a33 100644 --- a/sound/soc/tegra/tegra_pcm.c +++ b/sound/soc/tegra/tegra_pcm.c @@ -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);
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. Fixes: 3ed2b549b39f ("ALSA: pcm: fix wait_time calculations") Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- sound/soc/tegra/tegra_pcm.c | 3 +++ 1 file changed, 3 insertions(+)