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

[Dimitris Papavasiliou]

Hi,

A (commercial) DAC board, I use with a Raspberry Pi is plagued by clicks 
and pops and I'm trying to fix it at the driver level.  I have mostly 
fixed it, but I could use some guidance on the problem described below. 
I apologize in advance for the long read, but the situation is a bit 
convoluted (or my understanding of it is).

The board (a HifiBerry DAC+ Pro) uses a PCM5122 chip which is driven by 
the pcm512x codec driver, but the Pro version also includes a set of 
external clocks on the board (one each for 48Khz and 44.1Khz sample 
rates and multiples thereof).

There are two kinds of clicks and pops: on the one hand the board clicks 
when the device is opened/closed, mostly because the pcm512x codec 
driver doesn't support the digital_mute operation.  These are relatively 
mild clicks, which are an annoyance at most and I have a separate patch 
for the pcm512x, that addresses them, which I'm currently testing and 
will (hopefully) submit soon.

A separate case is a pop that's generated when the clock source is 
changed.  This pop is independent of the digital volume setting and very 
loud.  As far as I could determine, when the hw_params callback 
specified in the snd_soc_dai_link struct is called, it figures out what 
clock is needed for the giver sample rate and powers the appropriate 
clock through a GPIO on the PCM5122.  This essentially means that the 
external clock source fed into the 5122 is stopped and restarted at a 
different rate, potentially while the DAC chip is operating.  Later the 
hw_params callback defined in the pcm512x driver is called and it 
reconfigure the clock dividers for the new rate.  It is at this point 
that the pop seems to be generated, provided that the card is not 
powered down or muted.  If I comment out either the clock power-up/down 
in the dai_link hw_params, or the clock divider reconfiguration in the 
codec's hw_params, no pop is generated.

Now, the digital_mute patch described before, also happens to fix this 
loud pop.  I'm not sure whether that's a happy coincidence that might 
cease to be the case in the future though and was considering fixing the 
pop problem separately, at its source: the clock change.  One way to fix 
the problem is to switch the 5122 into standby before changing the clock 
source in the dai_link hw_params callback and switch it back into normal 
operation afterwards.  This seems to work regardless of whether 
digital_mute is implemented or not, but has the following problem: The 
register that's used to put the chip into/out of standby, is also used 
by the pcm512x driver, in the set_bias_level operation.  Since the 
hw_params operation is, as far as I can tell, not atomic, I'm concerned 
that race conditions may arise, causing the card to be left powered up, 
when it should be off and vice-versa.

Is there some way to synchronize the use of common chip registers in the 
DAI and codec drivers?  Is the digital_mute somehow guaranteed to fix 
the pop originating at the DAI/machine driver level (whatever the 
hw_params operation in the dai_link struct is supposed to be)? Am I 
perhaps missing something entirely?

Any help or guidance is welcome.

Thanks,
Dimitris