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

[Valerio Setti <vsetti@baylibre.com>]

On 7/2/26 1:10 PM, sashiko-bot@kernel.org wrote:
>> 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.
> 

Uh, this seems right, but with a caveat. When I tested this series I 
launched 'speaker-test' with 16-bits and 24-bits in sequence and it 
worked perfectly.
To generate the problem described here I had to create a custom binary 
that played the two sine tones, one at 16-bits and the one at 24-bits, 
without closing the PCM in the middle (as the 'speaker-test' case was 
doing instead). This way I was able to recreate the problem. I don't 
know how common it is for an application to switch sample width "on the 
fly", but it's worth fixing it.

For the records, I just tried removing this commit and both test 
scenarios work just fine so I'm inclined in sending a v2 with this 
commit dropped.

-- 
Valerio


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