On 06/23/2014 06:18 PM, Mark Brown wrote:
On Mon, Jun 23, 2014 at 04:01:13PM +0300, Peter Ujfalusi wrote:
AIC3X_FORMATS is correct: #define AIC3X_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE | \ SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S32_LE)
While in hw_params the switch was not handling the S24_3LE but it was defined as S24_LE. So the code was not correct: It advertised that it supportsd S24_3LE and was not handling it, but it was handling S24_LE which would not happen with this codec driver since the format is not supported - as it was defined by the AIC3X_FORMATS.
This is a fix for the current driver.
Yes, but the fix should be to support both.
I would rather fix the S24_3LE with this patch and add the S24_LE with a separate one. But IMHO the S24_LE support is not obvious.
I really need to get round to doing the rest of the conversions to use params_width() which should ensure this.
Not sure how this would work at the end to be honest for all platforms and codecs. How should the bitclock need to be configured for S24_3LE, S24_LE and S32_LE msbits=24? They are at the end have 24bits audio data but in memory they are stored in different ways.
The number of bits on the wire should be the number specified in the format.
So you are saying that for S24_3LE, S24_LE and S32_LE (msbits=24) we should have 24 bits on the wire? At least twl4030/5030 expects the 24bit data on the wire with 32 clocks (24 bits data + 8bits padding). OMAP McBSP can only support 24bit audio with S32_LE (msbits=24) and it needs to have 32 clock cycle on the bus to do this due to system constraints.
So if we say that S32_LE (msbits=24) should have 24 bits on the wire neither OMAP McBSP nor twl4030/5030 could do that.
Not sure about other SoCs, but I would not be so surprised that they also want to shift out all the bits they have got form the main memory, ie 24 bits in case of S24_3LE, 32bits in case of S24_LE or S32_LE (msbits=24).
Aic3x for example need to be configured to 24bit mode (24 clocks/channel if it is master) so it is going to 'pull' that amount from the CPU side. This can not be used with OMAP's sDMA/McBSP but davinci's eDMA/McASP can support it. I can even get daVinci to 'extract' the valid 24 bits from the 32bit sample coming via eDMA (mask/rotate/reverse operation).
Sure, so it only supports exactly 24 bit sample formats in master mode. That's what's expected by default.
So I still think that this is a bit more complicated than it looks
I'm not seeing anything particularly hard here. The main thing is that we ought to do something for dealing with extra BCLKs (eg, a device that can tolerate extra BCLKs can have the other end generating more data quite happily).