Re: [PATCH 1/2] ASoC: meson: gx-formatter: prepare on attach

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving formatter prepare logic to the DAPM PRE_PMU event breaks hardware reconfiguration for back-to-back playbacks or rapid restarts.
--

commit 0be8c647d2188e02c8519d4d117bc9cb1bdb746c
Author: Valerio Setti <vsetti@baylibre.com>

ASoC: meson: gx-formatter: prepare on attach

This commit splits the prepare and enable operations for the formatter
driver. It moves the prepare operation to happen when the widget is powered
up rather than at trigger time, aiming to resolve 24-bit I2S playback
corruption caused by pipeline reset ordering conflicts.

> diff --git a/sound/soc/meson/gx-formatter.c b/sound/soc/meson/gx-formatter.c
> index 311e63affb239..c7333f624b155 100644
> --- a/sound/soc/meson/gx-formatter.c
> +++ b/sound/soc/meson/gx-formatter.c

[ ... ]

> -static int gx_formatter_enable(struct gx_formatter *formatter)
> +static int gx_formatter_prepare(struct gx_formatter *formatter)
>  {
>  	int ret;
>  
> -	/* Do nothing if the formatter is already enabled */
> -	if (formatter->enabled)
> +	if (formatter->prepared)
>  		return 0;

[Severity: High]
Will this prevent hardware reconfiguration for back-to-back playbacks?

Because the ALSA core utilizes pmdown_time (delayed power down), the DAPM
widget remains in the powered-on state for a short time after playback
stops.

If a new stream with different parameters (e.g., changing from 16-bit to
24-bit) starts rapidly before the power-down occurs, dapm_power_widgets()
will not detect a power state change and will not emit a new
SND_SOC_DAPM_PRE_PMU event.

Since the widget wasn't powered down, gx_formatter_detach() hasn't cleared
formatter->prepared. This check will return early and skip updating the
hardware for the new sample format, causing the new stream to play through
incorrectly configured hardware.

>  
>  	/* Setup the stream parameter in the formatter */
>  	if (formatter->drv->ops->prepare) {
>  		ret = formatter->drv->ops->prepare(formatter->map,
> -					   formatter->drv->quirks,
> -					   formatter->stream);
> +						   formatter->drv->quirks,
> +						   formatter->stream);
>  		if (ret)
>  			return ret;
>  	}

[ ... ]

> @@ -63,6 +74,12 @@ static int gx_formatter_attach(struct gx_formatter *formatter)
>  
>  	mutex_lock(&ts->lock);
>  
> +	ret = gx_formatter_prepare(formatter);
> +	if (ret) {
> +		pr_err("failed to prepare the formatter\n");
> +		goto out;
> +	}
> +
>  	/* Catch up if the stream is already running when we attach */
>  	if (ts->ready) {
>  		ret = gx_formatter_enable(formatter);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-fix-24-bit-i2s-playback-v1-0-bcd3a43fe32a@baylibre.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic