diff mbox series

ASoC: mediatek: disable buffer pre-allocation

Message ID 20241219105303.548437-1-wenst@chromium.org (mailing list archive)
State Accepted
Commit 32c9c06adb5b157ef259233775a063a43746d699
Headers show
Series ASoC: mediatek: disable buffer pre-allocation | expand

Commit Message

Chen-Yu Tsai Dec. 19, 2024, 10:53 a.m. UTC
On Chromebooks based on Mediatek MT8195 or MT8188, the audio frontend
(AFE) is limited to accessing a very small window (1 MiB) of memory,
which is described as a reserved memory region in the device tree.

On these two platforms, the maximum buffer size is given as 512 KiB.
The MediaTek common code uses the same value for preallocations. This
means that only the first two PCM substreams get preallocations, and
then the whole space is exhausted, barring any other substreams from
working. Since the substreams used are not always the first two, this
means audio won't work correctly.

This is observed on the MT8188 Geralt Chromebooks, on which the
"mediatek,dai-link" property was dropped when it was upstreamed. That
property causes the driver to only register the PCM substreams listed
in the property, and in the order given.

Instead of trying to compute an optimal value and figuring out which
streams are used, simply disable preallocation. The PCM buffers are
managed by the core and are allocated and released on the fly. There
should be no impact to any of the other MediaTek platforms.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 sound/soc/mediatek/common/mtk-afe-platform-driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

AngeloGioacchino Del Regno Dec. 19, 2024, 11:06 a.m. UTC | #1
Il 19/12/24 11:53, Chen-Yu Tsai ha scritto:
> On Chromebooks based on Mediatek MT8195 or MT8188, the audio frontend
> (AFE) is limited to accessing a very small window (1 MiB) of memory,
> which is described as a reserved memory region in the device tree.
> 
> On these two platforms, the maximum buffer size is given as 512 KiB.
> The MediaTek common code uses the same value for preallocations. This
> means that only the first two PCM substreams get preallocations, and
> then the whole space is exhausted, barring any other substreams from
> working. Since the substreams used are not always the first two, this
> means audio won't work correctly.
> 
> This is observed on the MT8188 Geralt Chromebooks, on which the
> "mediatek,dai-link" property was dropped when it was upstreamed. That
> property causes the driver to only register the PCM substreams listed
> in the property, and in the order given.
> 
> Instead of trying to compute an optimal value and figuring out which
> streams are used, simply disable preallocation. The PCM buffers are
> managed by the core and are allocated and released on the fly. There
> should be no impact to any of the other MediaTek platforms.
> 
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

No there's no impact on the older ones, it's fine and I agree.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> ---
>   sound/soc/mediatek/common/mtk-afe-platform-driver.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/mediatek/common/mtk-afe-platform-driver.c b/sound/soc/mediatek/common/mtk-afe-platform-driver.c
> index 9b72b2a7ae91..6b6330583941 100644
> --- a/sound/soc/mediatek/common/mtk-afe-platform-driver.c
> +++ b/sound/soc/mediatek/common/mtk-afe-platform-driver.c
> @@ -120,8 +120,8 @@ int mtk_afe_pcm_new(struct snd_soc_component *component,
>   	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
>   
>   	size = afe->mtk_afe_hardware->buffer_bytes_max;
> -	snd_pcm_set_managed_buffer_all(pcm, SNDRV_DMA_TYPE_DEV,
> -				       afe->dev, size, size);
> +	snd_pcm_set_managed_buffer_all(pcm, SNDRV_DMA_TYPE_DEV, afe->dev, 0, size);
> +
>   	return 0;
>   }
>   EXPORT_SYMBOL_GPL(mtk_afe_pcm_new);
Mark Brown Dec. 19, 2024, 4:19 p.m. UTC | #2
On Thu, 19 Dec 2024 18:53:02 +0800, Chen-Yu Tsai wrote:
> On Chromebooks based on Mediatek MT8195 or MT8188, the audio frontend
> (AFE) is limited to accessing a very small window (1 MiB) of memory,
> which is described as a reserved memory region in the device tree.
> 
> On these two platforms, the maximum buffer size is given as 512 KiB.
> The MediaTek common code uses the same value for preallocations. This
> means that only the first two PCM substreams get preallocations, and
> then the whole space is exhausted, barring any other substreams from
> working. Since the substreams used are not always the first two, this
> means audio won't work correctly.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: mediatek: disable buffer pre-allocation
      commit: 32c9c06adb5b157ef259233775a063a43746d699

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
diff mbox series

Patch

diff --git a/sound/soc/mediatek/common/mtk-afe-platform-driver.c b/sound/soc/mediatek/common/mtk-afe-platform-driver.c
index 9b72b2a7ae91..6b6330583941 100644
--- a/sound/soc/mediatek/common/mtk-afe-platform-driver.c
+++ b/sound/soc/mediatek/common/mtk-afe-platform-driver.c
@@ -120,8 +120,8 @@  int mtk_afe_pcm_new(struct snd_soc_component *component,
 	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
 
 	size = afe->mtk_afe_hardware->buffer_bytes_max;
-	snd_pcm_set_managed_buffer_all(pcm, SNDRV_DMA_TYPE_DEV,
-				       afe->dev, size, size);
+	snd_pcm_set_managed_buffer_all(pcm, SNDRV_DMA_TYPE_DEV, afe->dev, 0, size);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(mtk_afe_pcm_new);