On 09/21/15 12:31, Russell King - ARM Linux wrote:
On Sat, Sep 19, 2015 at 10:54:51AM -0700, Mark Brown wrote:
On Fri, Sep 18, 2015 at 02:06:40PM +0300, Jyri Sarha wrote:
+#define SPDIF_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\
SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_3BE |\SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE)+#define I2S_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\
SNDRV_PCM_FMTBIT_S18_3LE | SNDRV_PCM_FMTBIT_S18_3BE |\SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_3BE |\SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE |\SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S32_BE)I'm not sure how abstracted this I2S and S/PDIF DAI business is - it doesn't feel like something that's going to be a property of all HDMI devices, and the specific sets of formats above cause me to raise a bit of an eyebrow. Should this be an interface where the HDMI chip registers multiple interfaces if it has them instead of automatically getting this split as is?
The inclusion of the 32-bit formats does raise an eyebrow here too. Audio transmission across the HDMI link is S/PDIF, supporting up to 24-bit uncompressed audio (aka L-PCM).
The device may accept 32 bit I2S, but it would have to be truncated to 24 bit before transmitting it to the sink. This should be mentioned somewhere.
There is ".sig_bits = 24" in hdmi_i2s_dai, but I can add an explicit comment about it.
We needed 32bit format in practice until Peter got 24_LE properly working with McASP. I just thought the may still be some platforms out there that can not produce 24bit i2s samples for some reason, but work just fine with 32bit samples. Those platforms would be limited to less than 24bit precision without 32bit formats. But as said, it is not critical to us any more and I can drop the 32bit formats as well.
BR, Jyri