On Fri, 2021-07-09 at 19:02 +0100, Mark Brown wrote:
On Fri, Jul 09, 2021 at 05:05:47PM +0000, Robert Hancock wrote:
On Fri, 2021-07-09 at 13:38 +0100, Mark Brown wrote:
For something like this I think the driver should be able to figure out the ratio based on the configured MCLK and sample rate. For the most part set_clkdiv() should be a legacy thing, it's very manual and hard to see why a system would do something different to the obvious ratio usually.
Possibly the I2S transmitter should be implementing set_sysclk rather than set_clkdiv then? simple-audio-card seems like it would already propagate that through into the CPU DAI in asoc_simple_hw_params and then it could figure out the right divider value to use.
I think so.
The tricky part is that the Audio Formatter (used as the "plat" component here) also needs to know what the mclk-fs value is. (I really don't know why it cares, I would think it would just push data to the output stream as fast as it was accepted, but indeed it does have a register to set the sample rate to MCLK divider, and if it's not set properly the I2S transmitter downstream seems to constantly get underruns.) I'm not sure if there's any mechanism for it to determine the value right now, or if something new would need to be added?
Given that it knows the MCLK if set_sysclk() is used and it knows the sample rate it should just be able to calculate the ratio?
I see that snd_soc_component_driver has a set_sysclk callback as well, so that allows the formatter to handle setting the divider. However, right now with simple-audio-card that callback is not being invoked on the formatter, though it is on the I2S transmitter.
I'm thinking something needs to be added to asoc_simple_hw_params to call snd_soc_component_set_sysclk on the platform component(s) like it calls snd_soc_dai_set_sysclk for the codec DAI and CPU DAI.
Not sure exactly how that should be done though - we could use for_each_rtd_components to iterate through all of the components and call snd_soc_component_set_sysclk on all of them, though that would also potentially duplicate some settings already done by the snd_soc_dai_set_sysclk calls on the CPU and codec DAIs. I'm not sure if that really hurts anything though?