[alsa-devel] [PATCH v2] ALSA: ASoC: McASP: add support for 24 bit samples
Sorry for the lack of quoting, but I onle get the digest.
These are wrong:
+ case SNDRV_PCM_FORMAT_U24_LE: + case SNDRV_PCM_FORMAT_S24_LE:
These pack a 24-bit sample value in a 32-bit word. The codec will send 32 bits to the McASP, and you should transfer 32 bits to the user, not just 24. Hence, SNDRV_PCM_FORMAT_S24_LE must be treated just like SNDRV_PCM_FORMAT_S32_LE.
I've tested that on a DA850-alike board with several TLV320AIC3256 codecs, treating them as 3-byte samples will reasult in invalid data.
Met vriendelijke groet / kind regards,
Mike Looijmans
TOPIC Embedded Systems Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: (+31) - (0)499 - 33.69.79 Telefax: (+31) - (0)499 - 33.69.70 E-mail: mike.looijmans@topic.nl Website: www.topic.nl
Dit e-mail bericht en de eventueel daarbij behorende bijlagen zijn uitsluitend bestemd voor de geadresseerde, zoals die blijkt uit het e-mail bericht en/of de bijlagen. Er kunnen gegevens met betrekking tot een derde instaan. Indien u als niet-geadresseerde dit bericht en de bijlagen ontvangt, terwijl u niet bevoegd of gemachtigd bent om dit bericht namens de geadresseerde te ontvangen, wordt u verzocht de afzender hierover direct te informeren en het e-mail bericht met de bijlagen te vernietigen. Ieder gebruik van de inhoud van het e-mail bericht, waaronder de daarbij behorende bijlagen, door een ander dan de geadresseerde is onrechtmatig jegens ons dan wel de eventueel in het e-mail bericht of de bijlagen voorkomende andere personen. TOPIC Embedded Systems is niet aansprakelijk voor enigerlei schade voortvloeiend uit het gebruik en/of acceptatie van dit e-mail bericht of de daarbij behorende bijlagen.
The contents of this message, as well as any enclosures, are addressed personally to, and thus solely intended for the addressee. They may contain information regarding a third party. A recipient who is neither the addressee, nor empowered to receive this message on behalf of the addressee, is kindly requested to immediately inform the sender of receipt, and to destroy the message and the enclosures. Any use of the contents of this message and/or the enclosures by any other person than the addressee or person who is empowered to receive this message, is illegal towards the sender and/or the aforementioned third party. TOPIC Embedded Systems is not liable for any damage as a result of the use and/or acceptance of this message and as well as any enclosures.
On 09.10.2012 11:41, Mike Looijmans wrote:
Sorry for the lack of quoting, but I onle get the digest.
These are wrong:
case SNDRV_PCM_FORMAT_U24_LE: + caseSNDRV_PCM_FORMAT_S24_LE:
These pack a 24-bit sample value in a 32-bit word. The codec will send 32 bits to the McASP, and you should transfer 32 bits to the user, not just 24. Hence, SNDRV_PCM_FORMAT_S24_LE must be treated just like SNDRV_PCM_FORMAT_S32_LE.
I've tested that on a DA850-alike board with several TLV320AIC3256 codecs, treating them as 3-byte samples will reasult in invalid data.
Ok, thanks for reporting this. Would like to send a patch or want me to fix it?
Daniel
Please just fix it, the fix is simply to move those "cases" a few lines down. The mcasp code in my tree is severely bashed because i'm using it to record from up to 8 codecs simultaneously...
Met vriendelijke groet / kind regards,
Mike Looijmans
TOPIC Embedded Systems Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: (+31) - (0)499 - 33.69.79 Telefax: (+31) - (0)499 - 33.69.70 E-mail: mike.looijmans@topic.nl Website: www.topic.nl
Dit e-mail bericht en de eventueel daarbij behorende bijlagen zijn uitsluitend bestemd voor de geadresseerde, zoals die blijkt uit het e-mail bericht en/of de bijlagen. Er kunnen gegevens met betrekking tot een derde instaan. Indien u als niet-geadresseerde dit bericht en de bijlagen ontvangt, terwijl u niet bevoegd of gemachtigd bent om dit bericht namens de geadresseerde te ontvangen, wordt u verzocht de afzender hierover direct te informeren en het e-mail bericht met de bijlagen te vernietigen. Ieder gebruik van de inhoud van het e-mail bericht, waaronder de daarbij behorende bijlagen, door een ander dan de geadresseerde is onrechtmatig jegens ons dan wel de eventueel in het e-mail bericht of de bijlagen voorkomende andere personen. TOPIC Embedded Systems is niet aansprakelijk voor enigerlei schade voortvloeiend uit het gebruik en/of acceptatie van dit e-mail bericht of de daarbij behorende bijlagen.
The contents of this message, as well as any enclosures, are addressed personally to, and thus solely intended for the addressee. They may contain information regarding a third party. A recipient who is neither the addressee, nor empowered to receive this message on behalf of the addressee, is kindly requested to immediately inform the sender of receipt, and to destroy the message and the enclosures. Any use of the contents of this message and/or the enclosures by any other person than the addressee or person who is empowered to receive this message, is illegal towards the sender and/or the aforementioned third party. TOPIC Embedded Systems is not liable for any damage as a result of the use and/or acceptance of this message and as well as any enclosures. ________________________________________ Van: Daniel Mack [zonque@gmail.com] Verzonden: dinsdag 9 oktober 2012 11:47 Aan: Mike Looijmans CC: alsa-devel@alsa-project.org; Mark Brown Onderwerp: Re: [alsa-devel] [PATCH v2] ALSA: ASoC: McASP: add support for 24 bit samples
On 09.10.2012 11:41, Mike Looijmans wrote:
Sorry for the lack of quoting, but I onle get the digest.
These are wrong:
case SNDRV_PCM_FORMAT_U24_LE: + caseSNDRV_PCM_FORMAT_S24_LE:
These pack a 24-bit sample value in a 32-bit word. The codec will send 32 bits to the McASP, and you should transfer 32 bits to the user, not just 24. Hence, SNDRV_PCM_FORMAT_S24_LE must be treated just like SNDRV_PCM_FORMAT_S32_LE.
I've tested that on a DA850-alike board with several TLV320AIC3256 codecs, treating them as 3-byte samples will reasult in invalid data.
Ok, thanks for reporting this. Would like to send a patch or want me to fix it?
Daniel
On Tue, Oct 09, 2012 at 11:47:57AM +0200, Daniel Mack wrote:
On 09.10.2012 11:41, Mike Looijmans wrote:
Sorry for the lack of quoting, but I onle get the digest.
These are wrong:
case SNDRV_PCM_FORMAT_U24_LE: + caseSNDRV_PCM_FORMAT_S24_LE:
These pack a 24-bit sample value in a 32-bit word. The codec will send 32 bits to the McASP, and you should transfer 32 bits to the user, not just 24. Hence, SNDRV_PCM_FORMAT_S24_LE must be treated just like SNDRV_PCM_FORMAT_S32_LE.
I've tested that on a DA850-alike board with several TLV320AIC3256 codecs, treating them as 3-byte samples will reasult in invalid data.
Ok, thanks for reporting this. Would like to send a patch or want me to fix it?
The above explanation isn't quite right. For 24 bit the CODEC should be working with 24 bits on the wire (though obviously extra BCLKs are allowed) and the AP should be working with 32 bit words in memory. The issue here is probably that you're working with real 24 bit data in RAM too.
On 09.10.2012 12:24, Mark Brown wrote:
On Tue, Oct 09, 2012 at 11:47:57AM +0200, Daniel Mack wrote:
On 09.10.2012 11:41, Mike Looijmans wrote:
Sorry for the lack of quoting, but I onle get the digest.
These are wrong:
case SNDRV_PCM_FORMAT_U24_LE: + caseSNDRV_PCM_FORMAT_S24_LE:
These pack a 24-bit sample value in a 32-bit word. The codec will send 32 bits to the McASP, and you should transfer 32 bits to the user, not just 24. Hence, SNDRV_PCM_FORMAT_S24_LE must be treated just like SNDRV_PCM_FORMAT_S32_LE.
I've tested that on a DA850-alike board with several TLV320AIC3256 codecs, treating them as 3-byte samples will reasult in invalid data.
Ok, thanks for reporting this. Would like to send a patch or want me to fix it?
The above explanation isn't quite right. For 24 bit the CODEC should be working with 24 bits on the wire (though obviously extra BCLKs are allowed) and the AP should be working with 32 bit words in memory. The issue here is probably that you're working with real 24 bit data in RAM too.
Hmm, I don't understand. I thought S24_3LE exists to denote a 3-byte representation, because S24_LE uses 32 bits? Hence Mike would be right that S24_LE has to use a 4-byte dma transfer size, no?
I'm using S24_3LE here, hence that didn't hit me.
Where's the confusion? :)
Daniel
Daniel Mack wrote:
On 09.10.2012 12:24, Mark Brown wrote:
On 09.10.2012 11:41, Mike Looijmans wrote:
Sorry for the lack of quoting, but I onle get the digest.
These are wrong:
case SNDRV_PCM_FORMAT_U24_LE:case SNDRV_PCM_FORMAT_S24_LE:These pack a 24-bit sample value in a 32-bit word. The codec will send 32 bits to the McASP, and you should transfer 32 bits to the user, not just 24. Hence, SNDRV_PCM_FORMAT_S24_LE must be treated just like SNDRV_PCM_FORMAT_S32_LE.
The above explanation isn't quite right. For 24 bit the CODEC should be working with 24 bits on the wire (though obviously extra BCLKs are allowed) and the AP should be working with 32 bit words in memory. The issue here is probably that you're working with real 24 bit data in RAM too.
Hmm, I don't understand. I thought S24_3LE exists to denote a 3-byte representation
Yes.
The three possibilities to store little-endian signed 24-bit samples in memory are (where L=LSB, M=MSB):
0padding LLLLLLLL MMMMMMMM HHHHHHHH S32_LE LLLLLLLL MMMMMMMM HHHHHHHH 0padding S24_LE LLLLLLLL MMMMMMMM HHHHHHHH S24_3LE
The first two require the DMA controller to transfer four bytes, the last one, three bytes.
All three use (at least) 24 bits on the wire.
Mark is right, unless your DMA controller actually happens to support three-byte samples, with all the alignment problems that entails. S24_3LE was introduced for USB controllers that are too dumb to differentiate between memory and on-the-wire formats; nobody else should want to use it. (But if the hardware supports it, you might as well allow it, if you implement it correctly.)
Hence Mike would be right that S24_LE has to use a 4-byte dma transfer size, no?
Yes.
Regards, Clemens
On Tue, Oct 09, 2012 at 02:21:48PM +0200, Clemens Ladisch wrote:
Hence Mike would be right that S24_LE has to use a 4-byte dma transfer size, no?
Yes.
That's what Mike meant but it wasn't quite what he said - he said the CODEC would send 32 bits which isn't the case, the 32 bits thing only applies inside the CPU.
On 10/09/2012 02:21 PM, Clemens Ladisch wrote:
The three possibilities to store little-endian signed 24-bit samples in memory are (where L=LSB, M=MSB):
0padding LLLLLLLL MMMMMMMM HHHHHHHH S32_LE LLLLLLLL MMMMMMMM HHHHHHHH 0padding S24_LE LLLLLLLL MMMMMMMM HHHHHHHH S24_3LE
The first two require the DMA controller to transfer four bytes, the last one, three bytes.
All three use (at least) 24 bits on the wire.
Oh, I always assumed that S32_LE was just a 32-bit format. That's what I've been assuming so far, and the resulting data files from "arecord -f {S32_LE|S24_LE|S24_3LE}" were perfectly understood by audio programs. Basically, what you're claiming here is that S24_LE must store the audio data in the lower 3 bytes? Because in my experiments so far I concluded I had to fill the high order bits for both S24_LE and S32_LE formats.
Hence Mike would be right that S24_LE has to use a 4-byte dma transfer size, no?
Yes.
It's not strictly what I wrote, but it's what I meant, yes.
Now we're on the topic of DMA controllers: When reading multiple channels (e.g. 2 or more codecs on the serializers either using TDM or multiple AXR pins), the DMA controller must always read 32 bits per sample from the serializer, regardless of the sample format used. Currently I implement this by enforcing S32_LE or S24_LE formats, and denying all others when multiple serializers are involved. Would it be possible to instruct the davinci's DMA controller to read 32 bits but write only 8, 16 or 24 bits to memory for each sample?
Met vriendelijke groet / kind regards,
Mike Looijmans
TOPIC Embedded Systems Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: (+31) – (0)499 - 33.69.79 Telefax: (+31) - (0)499 - 33.69.70 E-mail: mike.looijmans@topic.nl Website: www.topic.nl
Dit e-mail bericht en de eventueel daarbij behorende bijlagen zijn uitsluitend bestemd voor de geadresseerde, zoals die blijkt uit het e-mail bericht en/of de bijlagen. Er kunnen gegevens met betrekking tot een derde instaan. Indien u als niet-geadresseerde dit bericht en de bijlagen ontvangt, terwijl u niet bevoegd of gemachtigd bent om dit bericht namens de geadresseerde te ontvangen, wordt u verzocht de afzender hierover direct te informeren en het e-mail bericht met de bijlagen te vernietigen. Ieder gebruik van de inhoud van het e-mail bericht, waaronder de daarbij behorende bijlagen, door een ander dan de geadresseerde is onrechtmatig jegens ons dan wel de eventueel in het e-mail bericht of de bijlagen voorkomende andere personen. TOPIC Embedded Systems is niet aansprakelijk voor enigerlei schade voortvloeiend uit het gebruik en/of acceptatie van dit e-mail bericht of de daarbij behorende bijlagen.
The contents of this message, as well as any enclosures, are addressed personally to, and thus solely intended for the addressee. They may contain information regarding a third party. A recipient who is neither the addressee, nor empowered to receive this message on behalf of the addressee, is kindly requested to immediately inform the sender of receipt, and to destroy the message and the enclosures. Any use of the contents of this message and/or the enclosures by any other person than the addressee or person who is empowered to receive this message, is illegal towards the sender and/or the aforementioned third party. TOPIC Embedded Systems is not liable for any damage as a result of the use and/or acceptance of this message and as well as any enclosures.
participants (4)
-
Clemens Ladisch -
Daniel Mack -
Mark Brown -
Mike Looijmans