On 12/21/18 12:57 PM, Mark Brown wrote:
Should we then accept, that some pops will be generated and hope that they'll be suppressed by the digital_mute callback?
It's probably easiest.
All right then; I'll just add the one line, setting .ignore_pmdown_time = 1 to the machine driver and leave it at that. I've been using the card with just the digital_mute patch for several weeks and no pop got through, so it seems to be good enough, even with just that.
First, I'd like to thank you for your support both in troubleshooting this issue and in reviewing the patch I submitted. I fully appreciate I wasn't the easiest man to work with, being new to all of this and mostly clueless, so your help is very much appreciated.
Now, I apologize in advance, but I'll ask you if you can comment on the following final issue I have with this driver, which I was determined to ignore, but can't: I've already mentioned that I'm using a card made for the Raspberry Pi, and I can't really apply my patch there as-is, because they seem to have changed the pcm512x CODEC driver, without pushing the changes upstream. You can see the patch in question here:
https://github.com/raspberrypi/linux/commit/c53a137bd151130e29d7fc43947ac3e5...
(My patch, by the way, does run without the above change if you use the card without the external crystals, so I was able to test it in the form I submitted it in.)
I can apply the patch via merge, so bothering with this is not at all necessary, but if this change is useful, perhaps it's worth submitting another patch for it. I doubt this though, as it looks a bit hackish to me.
To provide you with some context: The card uses a 24.576MHz crystal for 48kHz-derived sampling rates, and it supports the S24_LE format, which leads to non-integer LRCK dividers, and somewhat faster-than-normal playback. A solution to this, would be to play 24bit samples as 32-bit with the MSB zeroed out, as the chip supports this. As far as I understand it, the patch implements this, by sending stereo streams as TDM data with two slots, one for each channel, wide enough for the physical size of the sample, which "happens" to be 32bits. On the machine driver size the following is done:
width = snd_pcm_format_physical_width(params_format(params)); ret = snd_soc_dai_set_tdm_slot(rtd->cpu_dai, 0x03, 0x03, channels, width); ret = snd_soc_dai_set_tdm_slot(rtd->codec_dai, 0x03, 0x03, channels, width);
This works, but I'm not sure TDM is meant for things like that. Anyway, if you think it's worth disucssing this, I can start a new thread if you prefer. If there's a straightforward way to fix this at the machine driver level, that'd be even better, as it would allow me to get rid of the divergence between the Pi and mainline kernel, without bothering the latter. If none of the above is the case, I can happily just merge the patch as-is.