On Mon, Feb 25, 2019 at 10:58:34PM +0200, Jyri Sarha wrote:
On 25/02/2019 16:03, Russell King - ARM Linux admin wrote:
As mentioned in the commit message, this is what TDA998x does today, and I have to assume that the contributor tested this, who seems to be one Jyri Sarha in commit 95db3b255fde ("drm/i2c: tda998x: Improve tda998x_configure_audio() audio related pdata").
Yes, my original implementation was a bit optimistic. I only had limited information about the chip and a somewhat working undocumented hackish driver implementation. By now it's clear that rejecting anything but 16-, 24-, or 32-bit sample widths (32, 48, or 64 bclk ratios) would have been right way to go.
If we don't do it this way, then converting TDA998x to use this defaulting would change the already established behaviour. As there are no users of this by hdmi-codec implementations, and as there are no callers of snd_soc_dai_set_bclk_ratio(), the only way to maintain the current behaviour in TDA998x is to either place a default in hdmi-codec.c, or to have hdmi-codec.c pass zero into TDA998x and have that encode the above.
I think there are other options too. The obvious one would be passing the blck_ratio callback trough HDMI-codec to HDMI-bridge-driver, but yes, I do not like that either.
The other would be changing the implicit bclk_ratio to 2 x sample-width, and accepting blck_ratios of 36 and 40 in tda998x, and set CTS_N_M = 3 and CTS_N_K = 2 for them too.
This way my bad choices would not spread to all hdmi-codec users.
Then again, I don't think anyone is using 18- or 20-bit samples with tda998x. They most likely do not even work. So simply refusing the 36 and 40 blck_ratios would probably be just fine too.
Another would be keepign the existing code with an additional WARN_ON_ONCE(1) if invoked, which would have the effect of encouraging drivers to be fixed up to call snd_soc_dai_set_bclk_ratio() - which has surely to be a good thing? However, the risk is that no one reports the problem cases...