[PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Peter Ujfalusi]

Correct the hw_params callback to configure the codec correctly in case of
S24_3LE format.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/codecs/tlv320aic3x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
index 470fbfb4b386..31b6ae2334bc 100644
--- a/sound/soc/codecs/tlv320aic3x.c
+++ b/sound/soc/codecs/tlv320aic3x.c
@@ -879,7 +879,7 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_FORMAT_S20_3LE:
 		data |= (0x01 << 4);
 		break;
-	case SNDRV_PCM_FORMAT_S24_LE:
+	case SNDRV_PCM_FORMAT_S24_3LE:
 		data |= (0x02 << 4);
 		break;
 	case SNDRV_PCM_FORMAT_S32_LE:
-- 
1.8.5.1

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 420 bytes --]

On Fri, Dec 13, 2013 at 02:58:19PM +0200, Peter Ujfalusi wrote:

> -	case SNDRV_PCM_FORMAT_S24_LE:
> +	case SNDRV_PCM_FORMAT_S24_3LE:
>  		data |= (0x02 << 4);
>  		break;

This should be adding the case for the new format rather than replacing
the old one shouldn't it?  They ought to turn out the same on the AIF so
the CODECs shouldn't care about the difference, ideally the core would
hide the difference from them.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Peter Ujfalusi]

On 12/13/2013 03:04 PM, Mark Brown wrote:
> On Fri, Dec 13, 2013 at 02:58:19PM +0200, Peter Ujfalusi wrote:
> 
>> -	case SNDRV_PCM_FORMAT_S24_LE:
>> +	case SNDRV_PCM_FORMAT_S24_3LE:
>>  		data |= (0x02 << 4);
>>  		break;
> 
> This should be adding the case for the new format rather than replacing
> the old one shouldn't it?  They ought to turn out the same on the AIF so
> the CODECs shouldn't care about the difference, ideally the core would
> hide the difference from them.

Not really since the codec has only field to specify the data format. The
codec can not support S24_LE (S24_LE is basically S32_LE msbits==24) since we
can not say to the codec to ignore the 8bit over the 24 bits of real data.
In case of S24_3LE the I2S bus will have 24 clocks/per channel which can not
be used to stream S24_LE either.

-- 
Péter

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 980 bytes --]

On Fri, Dec 13, 2013 at 03:29:33PM +0200, Peter Ujfalusi wrote:
> On 12/13/2013 03:04 PM, Mark Brown wrote:

> > This should be adding the case for the new format rather than replacing
> > the old one shouldn't it?  They ought to turn out the same on the AIF so
> > the CODECs shouldn't care about the difference, ideally the core would
> > hide the difference from them.

> Not really since the codec has only field to specify the data format. The
> codec can not support S24_LE (S24_LE is basically S32_LE msbits==24) since we
> can not say to the codec to ignore the 8bit over the 24 bits of real data.
> In case of S24_3LE the I2S bus will have 24 clocks/per channel which can not
> be used to stream S24_LE either.

No, I'd expect the wire behaviour to be identical for any 24 bit samples
(that's certainly what most drivers are written for).  The memory layout
differences shouldn't be visible to CODEC drivers.

Like I say we ought to be handling this stuff in the core :/

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Takashi Iwai]

At Fri, 13 Dec 2013 13:34:44 +0000,
Mark Brown wrote:
> 
> On Fri, Dec 13, 2013 at 03:29:33PM +0200, Peter Ujfalusi wrote:
> > On 12/13/2013 03:04 PM, Mark Brown wrote:
> 
> > > This should be adding the case for the new format rather than replacing
> > > the old one shouldn't it?  They ought to turn out the same on the AIF so
> > > the CODECs shouldn't care about the difference, ideally the core would
> > > hide the difference from them.
> 
> > Not really since the codec has only field to specify the data format. The
> > codec can not support S24_LE (S24_LE is basically S32_LE msbits==24) since we
> > can not say to the codec to ignore the 8bit over the 24 bits of real data.
> > In case of S24_3LE the I2S bus will have 24 clocks/per channel which can not
> > be used to stream S24_LE either.
> 
> No, I'd expect the wire behaviour to be identical for any 24 bit samples
> (that's certainly what most drivers are written for).  The memory layout
> differences shouldn't be visible to CODEC drivers.
> 
> Like I say we ought to be handling this stuff in the core :/

But this codec driver declares the available formats as:

#define AIC3X_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE | \
			 SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S32_LE)

so the fix for inconsistency is needed anyway.


Takashi

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 704 bytes --]

On Fri, Dec 13, 2013 at 02:41:44PM +0100, Takashi Iwai wrote:
> Mark Brown wrote:

> > No, I'd expect the wire behaviour to be identical for any 24 bit samples
> > (that's certainly what most drivers are written for).  The memory layout
> > differences shouldn't be visible to CODEC drivers.

> > Like I say we ought to be handling this stuff in the core :/

> But this codec driver declares the available formats as:

> #define AIC3X_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE | \
> 			 SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S32_LE)

> so the fix for inconsistency is needed anyway.

Ah, indeed - and that one should go to stable.  The changelog ought to
be more accurate though.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Peter Ujfalusi]

On 12/13/2013 03:34 PM, Mark Brown wrote:
> On Fri, Dec 13, 2013 at 03:29:33PM +0200, Peter Ujfalusi wrote:
>> On 12/13/2013 03:04 PM, Mark Brown wrote:
> 
>>> This should be adding the case for the new format rather than replacing
>>> the old one shouldn't it?  They ought to turn out the same on the AIF so
>>> the CODECs shouldn't care about the difference, ideally the core would
>>> hide the difference from them.
> 
>> Not really since the codec has only field to specify the data format. The
>> codec can not support S24_LE (S24_LE is basically S32_LE msbits==24) since we
>> can not say to the codec to ignore the 8bit over the 24 bits of real data.
>> In case of S24_3LE the I2S bus will have 24 clocks/per channel which can not
>> be used to stream S24_LE either.
> 
> No, I'd expect the wire behaviour to be identical for any 24 bit samples
> (that's certainly what most drivers are written for).  The memory layout
> differences shouldn't be visible to CODEC drivers.

We can not change the HW... for example:
twl4030/twl5030: 32 clock cycle/channel and 24 bits used out of that.
tlv320aic3106: 24 clock cycle/channel for 24 bit audio.

The wire behavior is different and this need to be known by the CPU side as well.

-- 
Péter

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1492 bytes --]

On Fri, Dec 13, 2013 at 03:57:24PM +0200, Peter Ujfalusi wrote:
> On 12/13/2013 03:34 PM, Mark Brown wrote:

> > No, I'd expect the wire behaviour to be identical for any 24 bit samples
> > (that's certainly what most drivers are written for).  The memory layout
> > differences shouldn't be visible to CODEC drivers.

> We can not change the HW... for example:

This isn't a hardware issue, this is a software issue.

> twl4030/twl5030: 32 clock cycle/channel and 24 bits used out of that.

These shouldn't be advertising themselves as supporting 24 bit format
for ASoC then since they need 32 bit samples on the bus - this is
actually pretty standard for things that say they support 32 bits, they
usually just discard some of the lower bits but can cope with 32 bit on
the interface.

The reason everything says 24_LE right now is that we're being dumb
about how we do the constraints and CPUs do care about the memory
layout, not because the CODECs require the blank cycles.  Of course this
means that almost anything that does I2S should be able to advertise
arbatrary sample size support which is one of the resons we ought to be
doing stuff in the core to make this nicer.

> tlv320aic3106: 24 clock cycle/channel for 24 bit audio.

This is normal ASoC behaviour.

> The wire behavior is different and this need to be known by the CPU side as well.

This isn't the way to do it, though - if the TWL drivers were ever used
with other CPUs (admittedly unlikely) they'd run into trouble.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Peter Ujfalusi]

On 12/13/2013 04:18 PM, Mark Brown wrote:
> On Fri, Dec 13, 2013 at 03:57:24PM +0200, Peter Ujfalusi wrote:
>> On 12/13/2013 03:34 PM, Mark Brown wrote:
> 
>>> No, I'd expect the wire behaviour to be identical for any 24 bit samples
>>> (that's certainly what most drivers are written for).  The memory layout
>>> differences shouldn't be visible to CODEC drivers.
> 
>> We can not change the HW... for example:
> 
> This isn't a hardware issue, this is a software issue.
> 
>> twl4030/twl5030: 32 clock cycle/channel and 24 bits used out of that.
> 
> These shouldn't be advertising themselves as supporting 24 bit format
> for ASoC then since they need 32 bit samples on the bus - this is
> actually pretty standard for things that say they support 32 bits, they
> usually just discard some of the lower bits but can cope with 32 bit on
> the interface.
> 
> The reason everything says 24_LE right now is that we're being dumb
> about how we do the constraints and CPUs do care about the memory
> layout, not because the CODECs require the blank cycles.  Of course this
> means that almost anything that does I2S should be able to advertise
> arbatrary sample size support which is one of the resons we ought to be
> doing stuff in the core to make this nicer.

Hrm, not sure about other SoCs but on OMAP most of the formats from memory
ends up on the I2S bus without any bits added/removed so the format actually
applies to the codec side as well since the data arriving to the codec will
have the same layout as it had in the memory.
But, yes some level of logic in the core might help.

>> tlv320aic3106: 24 clock cycle/channel for 24 bit audio.
> 
> This is normal ASoC behaviour.
> 
>> The wire behavior is different and this need to be known by the CPU side as well.
> 
> This isn't the way to do it, though - if the TWL drivers were ever used
> with other CPUs (admittedly unlikely) they'd run into trouble.

The twl drivers (and the dac33) is doing the correct thing IMHO: reporting
S32_LE with set .sig_bits = 24. Just to let the user space know that thay
should not use the whole 32bit range.

-- 
Péter

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 955 bytes --]

On Fri, Dec 13, 2013 at 04:31:40PM +0200, Peter Ujfalusi wrote:

> Hrm, not sure about other SoCs but on OMAP most of the formats from memory
> ends up on the I2S bus without any bits added/removed so the format actually
> applies to the codec side as well since the data arriving to the codec will
> have the same layout as it had in the memory.
> But, yes some level of logic in the core might help.

For every other CPU or CODEC where I know what it does the number of
bits expected on the bus is the sample size.

> > This isn't the way to do it, though - if the TWL drivers were ever used
> > with other CPUs (admittedly unlikely) they'd run into trouble.

> The twl drivers (and the dac33) is doing the correct thing IMHO: reporting
> S32_LE with set .sig_bits = 24. Just to let the user space know that thay
> should not use the whole 32bit range.

Ah, OK - that's good and correct.  Your mail made me think they were
using one of the S24 formats.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Lars-Peter Clausen]

On 12/13/2013 02:29 PM, Peter Ujfalusi wrote:
> On 12/13/2013 03:04 PM, Mark Brown wrote:
>> On Fri, Dec 13, 2013 at 02:58:19PM +0200, Peter Ujfalusi wrote:
>>
>>> -	case SNDRV_PCM_FORMAT_S24_LE:
>>> +	case SNDRV_PCM_FORMAT_S24_3LE:
>>>  		data |= (0x02 << 4);
>>>  		break;
>>
>> This should be adding the case for the new format rather than replacing
>> the old one shouldn't it?  They ought to turn out the same on the AIF so
>> the CODECs shouldn't care about the difference, ideally the core would
>> hide the difference from them.
> 
> Not really since the codec has only field to specify the data format. The
> codec can not support S24_LE (S24_LE is basically S32_LE msbits==24) since we

S24_LE is a 24 bit sample in a 32 bit word, but it is right aligned, so
there are 8 bits of leading padding, while with S32_LE msbits=24 you'd have
8bits of trailing padding.

> can not say to the codec to ignore the 8bit over the 24 bits of real data.
> In case of S24_3LE the I2S bus will have 24 clocks/per channel which can not
> be used to stream S24_LE either.
> 

Normally you'd expect the I2S core to only put the 24 bits of data onto the
bus for both S24_3LE and S24_LE (it might add necessary trailing padding if
the bit clocks per frame is > 24). A CODEC driver should really not have to
care about the in memory layout of the data since all it will see is a
serialized bit stream. I think ideally we wouldn't check for params_format()
but rather for snd_pcm_format_width(params_format()).

- Lars

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 590 bytes --]

On Fri, Dec 13, 2013 at 02:42:32PM +0100, Lars-Peter Clausen wrote:

> Normally you'd expect the I2S core to only put the 24 bits of data onto the
> bus for both S24_3LE and S24_LE (it might add necessary trailing padding if
> the bit clocks per frame is > 24). A CODEC driver should really not have to
> care about the in memory layout of the data since all it will see is a
> serialized bit stream. I think ideally we wouldn't check for params_format()
> but rather for snd_pcm_format_width(params_format()).

Yes, that would be a much better approach.  In fact, let me go attack
that...

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Peter Ujfalusi]

Correct the hw_params callback to configure the codec correctly in case of
S24_3LE format.
S24_LE is not defined as supported format for the codec.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/codecs/tlv320aic3x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
index e12fafbb1e09..5360772bc1ad 100644
--- a/sound/soc/codecs/tlv320aic3x.c
+++ b/sound/soc/codecs/tlv320aic3x.c
@@ -879,7 +879,7 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_FORMAT_S20_3LE:
 		data |= (0x01 << 4);
 		break;
-	case SNDRV_PCM_FORMAT_S24_LE:
+	case SNDRV_PCM_FORMAT_S24_3LE:
 		data |= (0x02 << 4);
 		break;
 	case SNDRV_PCM_FORMAT_S32_LE:
-- 
2.0.0

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 442 bytes --]

On Fri, Jun 06, 2014 at 04:05:31PM +0300, Peter Ujfalusi wrote:

> Correct the hw_params callback to configure the codec correctly in case of
> S24_3LE format.
> S24_LE is not defined as supported format for the codec.

These should both be supported by the time things get down to the wire -
the memory layout shouldn't matter.  I really need to get round to doing
the rest of the conversions to use params_width() which should ensure
this.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Peter Ujfalusi]

On 06/09/2014 11:24 PM, Mark Brown wrote:
> On Fri, Jun 06, 2014 at 04:05:31PM +0300, Peter Ujfalusi wrote:
> 
>> Correct the hw_params callback to configure the codec correctly in case of
>> S24_3LE format.
>> S24_LE is not defined as supported format for the codec.
> 
> These should both be supported by the time things get down to the wire -
> the memory layout shouldn't matter.

The codec itself has support for 16, 20, 24 and 32 bits data. The
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.

> 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.
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).

So I still think that this is a bit more complicated than it looks

-- 
Péter

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1910 bytes --]

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 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.

> 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).

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Peter Ujfalusi]

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).
> 


-- 
Péter

Re: [PATCH] ASoC: tlv320aic3x: Correct S24_3LE support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 354 bytes --]

On Tue, Jun 24, 2014 at 09:23:28AM +0300, Peter Ujfalusi wrote:
> On 06/23/2014 06:18 PM, Mark Brown wrote:

> > 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?

No - when the format is S32_LE then that's 32 bits.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]