[alsa-devel] Need help fixing pop/click artifacts in an ASOC driver

[Pierre-Louis Bossart]

On 12/18/18 5:32 AM, Dimitris Papavasiliou wrote:
> On 12/17/18 8:08 PM, Pierre-Louis Bossart wrote:
>
>> The machine driver should use clk_set_rate() and not directly handle 
>> regmap or codec stuff. If it does, or if the clock framework isn't 
>> relevant here then we can simplify all this as suggested in 
>> https://patchwork.kernel.org/patch/10444387/. What I was trying to do 
>> with the github update is to keep the clock framework, tie it closer 
>> with the codec parts with a state variable that prevents wild changes 
>> without going back to a 'safe' idle state (similar idea as PulseAudio 
>> clock changes, which can only happen when the PCM is not opened and 
>> used).
>
> I have documented, in a previous message, various approaches I tried
> to prevent the pop, which were mostly based on the assumption that the
> clock source was changed too "abruptly", or wasn't allowed to settle.
> In the end, the only state, where the clocks can be switched without a
> pop seemed to be when the chip is suspended.
>
> It would of course be great if this could be achieved in a natural
> manner, by waiting for the card to be suspended and only switching
> clocks in such a state, but I don't see how this can be achieved
> robustly.  The power management behavior is outside of the control of
> the machine driver, so it can only refuse to switch sample rate when
> it's not suspended.  Perhaps this will work in practice, but I fear it
> would break applications depending on less restrained control of the
> hardware.

My point was that the machine driver can track DAPM events and put the 
device in a 'safe' state on SND_SOC_DAPM_EVENT_OFF so that the rate can 
be changed when playback restarts. I don't see what prevents us from 
using the same config as during suspend? It's pretty common to play with 
clocks with these events, and calling clk_disable_unprepare() could lead 
to whatever configuration sequence is needed for pop-free restart on 
startup.