On Fri, Dec 21, 2018 at 03:05:51PM +0200, Dimitris Papavasiliou wrote: > 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/c53a137bd151130e29d7fc43947ac3e579a29ce4#diff-723fa079c49ec85d48e290fe84994b36 I didn't upstream the patch as I wasn't really satisfied with it and also think it's quite hackish. Back then when the Hifiberry folks submitted their machine driver to the RPi kernel they sneaked in a change to the pcm512x which forced S24 data to use 64fs clocking https://github.com/raspberrypi/linux/pull/1152 This change had the nasty side effect that using pcm512x eg in slave mode with the simple card driver broke as the setup on both sides no longer matched. bcm2835 by default uses 32/48/64fs for 16/24/32 bit data - like most drivers do (also upstream pcm512x driver). pcm512x with the change used 32/64/64fs - something that's not possible to express in simple-card's binding. So that change had to go and after dropping it we realized some control over fs was needed if the machine driver wanted to support S24_LE (just dropping S24_LE support and relying on alsalib to do automatic 24->32bit conversion would have been another option). Doing that via the set_bclk_ratio interface would probably have been the cleaner approach, OTOH set_tdm_slot had the nice feature that it already had simple card DT bindings - so I (ab)used that. I don't have a Hifiberry DAC+ card (or other cards with on-board oscillators plus pcm512x in master mode) so I haven't looked into better solutions. Main focus was to have some workaround that didn't cause collateral damage but allowed the card to keep working. I wouldn't mind at all if you do a proper implementation and submit it upstream so the downstream patch can be dropped. so long, Hias > > (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. > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@xxxxxxxxxxxxxxxx > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
