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? -- Robert Hancock Senior Hardware Designer, Calian Advanced Technologies www.calian.com
