[alsa-devel] [PATCH 1/2] ALSA: pcm: add SNDRV_PCM_FORMAT_{S, U}20_4
This format is similar to existing SNDRV_PCM_FORMAT_{S,U}20_3 that keep 20-bit PCM samples in 3 bytes, however i.MX6 platform SSI FIFO does not allow 3-byte accesses (including DMA) so a 4-byte format is needed for it.
Signed-off-by: Maciej S. Szmigiero mail@maciej.szmigiero.name --- include/sound/pcm.h | 8 ++++++++ include/sound/soc-dai.h | 2 ++ include/uapi/sound/asound.h | 10 +++++++++- sound/core/pcm_misc.c | 16 ++++++++++++++++ 4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 24febf9e177c..7ad2d3f0934f 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -191,6 +191,10 @@ struct snd_pcm_ops { #define SNDRV_PCM_FMTBIT_DSD_U32_LE _SNDRV_PCM_FMTBIT(DSD_U32_LE) #define SNDRV_PCM_FMTBIT_DSD_U16_BE _SNDRV_PCM_FMTBIT(DSD_U16_BE) #define SNDRV_PCM_FMTBIT_DSD_U32_BE _SNDRV_PCM_FMTBIT(DSD_U32_BE) +#define SNDRV_PCM_FMTBIT_S20_4LE _SNDRV_PCM_FMTBIT(S20_4LE) +#define SNDRV_PCM_FMTBIT_U20_4LE _SNDRV_PCM_FMTBIT(U20_4LE) +#define SNDRV_PCM_FMTBIT_S20_4BE _SNDRV_PCM_FMTBIT(S20_4BE) +#define SNDRV_PCM_FMTBIT_U20_4BE _SNDRV_PCM_FMTBIT(U20_4BE)
#ifdef SNDRV_LITTLE_ENDIAN #define SNDRV_PCM_FMTBIT_S16 SNDRV_PCM_FMTBIT_S16_LE @@ -202,6 +206,8 @@ struct snd_pcm_ops { #define SNDRV_PCM_FMTBIT_FLOAT SNDRV_PCM_FMTBIT_FLOAT_LE #define SNDRV_PCM_FMTBIT_FLOAT64 SNDRV_PCM_FMTBIT_FLOAT64_LE #define SNDRV_PCM_FMTBIT_IEC958_SUBFRAME SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE +#define SNDRV_PCM_FMTBIT_S20_4 SNDRV_PCM_FMTBIT_S20_4LE +#define SNDRV_PCM_FMTBIT_U20_4 SNDRV_PCM_FMTBIT_U20_4LE #endif #ifdef SNDRV_BIG_ENDIAN #define SNDRV_PCM_FMTBIT_S16 SNDRV_PCM_FMTBIT_S16_BE @@ -213,6 +219,8 @@ struct snd_pcm_ops { #define SNDRV_PCM_FMTBIT_FLOAT SNDRV_PCM_FMTBIT_FLOAT_BE #define SNDRV_PCM_FMTBIT_FLOAT64 SNDRV_PCM_FMTBIT_FLOAT64_BE #define SNDRV_PCM_FMTBIT_IEC958_SUBFRAME SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_BE +#define SNDRV_PCM_FMTBIT_S20_4 SNDRV_PCM_FMTBIT_S20_4BE +#define SNDRV_PCM_FMTBIT_U20_4 SNDRV_PCM_FMTBIT_U20_4BE #endif
struct snd_pcm_file { diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 58acd00cae19..16aec0cc96f5 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -102,6 +102,8 @@ struct snd_compr_stream; SNDRV_PCM_FMTBIT_S16_BE |\ SNDRV_PCM_FMTBIT_S20_3LE |\ SNDRV_PCM_FMTBIT_S20_3BE |\ + SNDRV_PCM_FMTBIT_S20_4LE |\ + SNDRV_PCM_FMTBIT_S20_4BE |\ SNDRV_PCM_FMTBIT_S24_3LE |\ SNDRV_PCM_FMTBIT_S24_3BE |\ SNDRV_PCM_FMTBIT_S32_LE |\ diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index c227ccba60ae..69b661816491 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -236,7 +236,11 @@ typedef int __bitwise snd_pcm_format_t; #define SNDRV_PCM_FORMAT_DSD_U32_LE ((__force snd_pcm_format_t) 50) /* DSD, 4-byte samples DSD (x32), little endian */ #define SNDRV_PCM_FORMAT_DSD_U16_BE ((__force snd_pcm_format_t) 51) /* DSD, 2-byte samples DSD (x16), big endian */ #define SNDRV_PCM_FORMAT_DSD_U32_BE ((__force snd_pcm_format_t) 52) /* DSD, 4-byte samples DSD (x32), big endian */ -#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_DSD_U32_BE +#define SNDRV_PCM_FORMAT_S20_4LE ((__force snd_pcm_format_t) 53) /* in four bytes */ +#define SNDRV_PCM_FORMAT_S20_4BE ((__force snd_pcm_format_t) 54) /* in four bytes */ +#define SNDRV_PCM_FORMAT_U20_4LE ((__force snd_pcm_format_t) 55) /* in four bytes */ +#define SNDRV_PCM_FORMAT_U20_4BE ((__force snd_pcm_format_t) 56) /* in four bytes */ +#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_U20_4BE
#ifdef SNDRV_LITTLE_ENDIAN #define SNDRV_PCM_FORMAT_S16 SNDRV_PCM_FORMAT_S16_LE @@ -248,6 +252,8 @@ typedef int __bitwise snd_pcm_format_t; #define SNDRV_PCM_FORMAT_FLOAT SNDRV_PCM_FORMAT_FLOAT_LE #define SNDRV_PCM_FORMAT_FLOAT64 SNDRV_PCM_FORMAT_FLOAT64_LE #define SNDRV_PCM_FORMAT_IEC958_SUBFRAME SNDRV_PCM_FORMAT_IEC958_SUBFRAME_LE +#define SNDRV_PCM_FORMAT_S20_4 SNDRV_PCM_FORMAT_S20_4LE +#define SNDRV_PCM_FORMAT_U20_4 SNDRV_PCM_FORMAT_U20_4LE #endif #ifdef SNDRV_BIG_ENDIAN #define SNDRV_PCM_FORMAT_S16 SNDRV_PCM_FORMAT_S16_BE @@ -259,6 +265,8 @@ typedef int __bitwise snd_pcm_format_t; #define SNDRV_PCM_FORMAT_FLOAT SNDRV_PCM_FORMAT_FLOAT_BE #define SNDRV_PCM_FORMAT_FLOAT64 SNDRV_PCM_FORMAT_FLOAT64_BE #define SNDRV_PCM_FORMAT_IEC958_SUBFRAME SNDRV_PCM_FORMAT_IEC958_SUBFRAME_BE +#define SNDRV_PCM_FORMAT_S20_4 SNDRV_PCM_FORMAT_S20_4BE +#define SNDRV_PCM_FORMAT_U20_4 SNDRV_PCM_FORMAT_U20_4BE #endif
typedef int __bitwise snd_pcm_subformat_t; diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c index 9be81025372f..132ee41edae1 100644 --- a/sound/core/pcm_misc.c +++ b/sound/core/pcm_misc.c @@ -229,6 +229,22 @@ static struct pcm_format_data pcm_formats[(INT)SNDRV_PCM_FORMAT_LAST+1] = { .width = 5, .phys = 8, .le = -1, .signd = -1, .silence = {}, }, + [SNDRV_PCM_FORMAT_S20_4LE] = { + .width = 20, .phys = 32, .le = 1, .signd = 1, + .silence = {}, + }, + [SNDRV_PCM_FORMAT_S20_4BE] = { + .width = 20, .phys = 32, .le = 0, .signd = 1, + .silence = {}, + }, + [SNDRV_PCM_FORMAT_U20_4LE] = { + .width = 20, .phys = 32, .le = 1, .signd = 0, + .silence = { 0x00, 0x00, 0x08, 0x00 }, + }, + [SNDRV_PCM_FORMAT_U20_4BE] = { + .width = 20, .phys = 32, .le = 0, .signd = 0, + .silence = { 0x00, 0x08, 0x00, 0x00 }, + }, };
On Nov 23 2017 04:17, Maciej S. Szmigiero wrote:
This format is similar to existing SNDRV_PCM_FORMAT_{S,U}20_3 that keep 20-bit PCM samples in 3 bytes, however i.MX6 platform SSI FIFO does not allow 3-byte accesses (including DMA) so a 4-byte format is needed for it.
Signed-off-by: Maciej S. Szmigiero mail@maciej.szmigiero.name
include/sound/pcm.h | 8 ++++++++ include/sound/soc-dai.h | 2 ++ include/uapi/sound/asound.h | 10 +++++++++- sound/core/pcm_misc.c | 16 ++++++++++++++++ 4 files changed, 35 insertions(+), 1 deletion(-)
... > diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index c227ccba60ae..69b661816491 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -236,7 +236,11 @@ typedef int __bitwise snd_pcm_format_t; #define SNDRV_PCM_FORMAT_DSD_U32_LE ((__force snd_pcm_format_t) 50) /* DSD, 4-byte samples DSD (x32), little endian */ #define SNDRV_PCM_FORMAT_DSD_U16_BE ((__force snd_pcm_format_t) 51) /* DSD, 2-byte samples DSD (x16), big endian */ #define SNDRV_PCM_FORMAT_DSD_U32_BE ((__force snd_pcm_format_t) 52) /* DSD, 4-byte samples DSD (x32), big endian */ -#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_DSD_U32_BE +#define SNDRV_PCM_FORMAT_S20_4LE ((__force snd_pcm_format_t) 53) /* in four bytes */ +#define SNDRV_PCM_FORMAT_S20_4BE ((__force snd_pcm_format_t) 54) /* in four bytes */ +#define SNDRV_PCM_FORMAT_U20_4LE ((__force snd_pcm_format_t) 55) /* in four bytes */ +#define SNDRV_PCM_FORMAT_U20_4BE ((__force snd_pcm_format_t) 56) /* in four bytes */ +#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_U20_4BE
In my opinion, for this type of definition, it's better to declare left/right-adjusted or padding side. (Of course, silence definition is already a hint, however the lack of information forces developers to have a careful behaviour to handle entries on the list.)
(I note that in current ALSA PCM interface there's no way to deliver MSB/LSB-first information about sample format.)
Additionally, alsa-lib includes some codes related to the definition[1]. If you'd like to thing goes well out of ALSA SoC part, it's better to submit changes to the library as well.
[1] http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/pcm/pcm_misc.c;h=54...
Regards
Takashi Sakamoto
On 23.11.2017 00:27, Takashi Sakamoto wrote:
On Nov 23 2017 04:17, Maciej S. Szmigiero wrote:
(..)
--- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -236,7 +236,11 @@ typedef int __bitwise snd_pcm_format_t; #define SNDRV_PCM_FORMAT_DSD_U32_LE ((__force snd_pcm_format_t) 50) /* DSD, 4-byte samples DSD (x32), little endian */ #define SNDRV_PCM_FORMAT_DSD_U16_BE ((__force snd_pcm_format_t) 51) /* DSD, 2-byte samples DSD (x16), big endian */ #define SNDRV_PCM_FORMAT_DSD_U32_BE ((__force snd_pcm_format_t) 52) /* DSD, 4-byte samples DSD (x32), big endian */ -#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_DSD_U32_BE +#define SNDRV_PCM_FORMAT_S20_4LE ((__force snd_pcm_format_t) 53) /* in four bytes */ +#define SNDRV_PCM_FORMAT_S20_4BE ((__force snd_pcm_format_t) 54) /* in four bytes */ +#define SNDRV_PCM_FORMAT_U20_4LE ((__force snd_pcm_format_t) 55) /* in four bytes */ +#define SNDRV_PCM_FORMAT_U20_4BE ((__force snd_pcm_format_t) 56) /* in four bytes */ +#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_U20_4BE
In my opinion, for this type of definition, it's better to declare left/right-adjusted or padding side. (Of course, silence definition is already a hint, however the lack of information forces developers to have a careful behaviour to handle entries on the list.> (I note that in current ALSA PCM interface there's no way to deliver MSB/LSB-first information about sample format.)
No other sound format includes this information in its name so if we name these formats SNDRV_PCM_FORMAT_{S, U}20LSB_4 they are going to have it inconsistent with every other one (I assume you meant to include such information in a format name?).
But information about whether this format is MSB or LSB justified can be added in a comment so the situation is clear for other developers from the definition without needing to read the actual processing code.
Additionally, alsa-lib includes some codes related to the definition[1]. If you'd like to thing goes well out of ALSA SoC part, it's better to submit changes to the library as well.
[1] http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/pcm/pcm_misc.c;h=54...
I have alsa-lib changes ready for these formats - they were needed to test these patches, will post them when this is merged on the kernel side (in case some changes are needed which affect both).
Regards
Takashi Sakamoto
Best regards, Maciej Szmigiero
On Nov 23 2017 08:44, Maciej S. Szmigiero wrote:
On 23.11.2017 00:27, Takashi Sakamoto wrote:
On Nov 23 2017 04:17, Maciej S. Szmigiero wrote:
(..)
--- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -236,7 +236,11 @@ typedef int __bitwise snd_pcm_format_t; #define SNDRV_PCM_FORMAT_DSD_U32_LE ((__force snd_pcm_format_t) 50) /* DSD, 4-byte samples DSD (x32), little endian */ #define SNDRV_PCM_FORMAT_DSD_U16_BE ((__force snd_pcm_format_t) 51) /* DSD, 2-byte samples DSD (x16), big endian */ #define SNDRV_PCM_FORMAT_DSD_U32_BE ((__force snd_pcm_format_t) 52) /* DSD, 4-byte samples DSD (x32), big endian */ -#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_DSD_U32_BE +#define SNDRV_PCM_FORMAT_S20_4LE ((__force snd_pcm_format_t) 53) /* in four bytes */ +#define SNDRV_PCM_FORMAT_S20_4BE ((__force snd_pcm_format_t) 54) /* in four bytes */ +#define SNDRV_PCM_FORMAT_U20_4LE ((__force snd_pcm_format_t) 55) /* in four bytes */ +#define SNDRV_PCM_FORMAT_U20_4BE ((__force snd_pcm_format_t) 56) /* in four bytes */ +#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_U20_4BE
In my opinion, for this type of definition, it's better to declare left/right-adjusted or padding side. (Of course, silence definition is already a hint, however the lack of information forces developers to have a careful behaviour to handle entries on the list. (I note that in current ALSA PCM interface there's no way to deliver MSB/LSB-first information about sample format.)
No other sound format includes this information in its name
You overlook comments in 'SNDRV_PCM_FORMAT_[U|S]24_[LE|BE]'. Let me refer to them [1]:
198 #define SNDRV_PCM_FORMAT_S24_LE ((__force snd_pcm_format_t) 6) /* low three bytes */ 199 #define SNDRV_PCM_FORMAT_S24_BE ((__force snd_pcm_format_t) 7) /* low three bytes */ 200 #define SNDRV_PCM_FORMAT_U24_LE ((__force snd_pcm_format_t) 8) /* low three bytes */ 201 #define SNDRV_PCM_FORMAT_U24_BE ((__force snd_pcm_format_t) 9) /* low three bytes */
In your way, these types of format can be represented by 'SNDRV_PCM_FORMAT_[U|S]24_4[LE|BE]', thus for playback direction they mean:
``` #include <sound/asound.h> #include <endian.h>
uint32_t *buf; uint32_t sample; snd_pcm_format_t format;
sample = generate_a_sample(); (sample & ~0x00ffffff) /* invalid bits as sample */
if (format == SNDRV_PCM_FORMAT_[U|S]24_LE) { buf[0] = htole32(sample); else buf[0] = htobe32(sample);
/* transfer content of the buf via ALSA kernel stuffs. */ ```
The comments are good enough for application developers in an aspect of a position for padding.
In general, studying from the past is preferable behaviour to be genius, however accumulated history includes mistakes and defects. Just pretending the past is not so genius, without further consideration.
Actually additions of the rest of entries for PCM format were done without enough cares of what information they give to application developers. Adding new entries is easier than fixing and improving them once exposed. It's a reason that they're left what they're.
I wish you had enough care to assist applications developers. Without applications, drivers are worthless and just waste of code base.
so if we name these formats SNDRV_PCM_FORMAT_{S, U}20LSB_4 they are going to have it inconsistent with every other one
(I assume you meant to include such information in a format name?).
But information about whether this format is MSB or LSB justified can be added in a comment so the situation is clear for other developers from the definition without needing to read the actual processing code.
For consistency of the other entries, this is not so preferable, in my opinion. So I didn't suggest it and just noted.
Additionally, alsa-lib includes some codes related to the definition[1]. If you'd like to thing goes well out of ALSA SoC part, it's better to submit changes to the library as well.
[1] http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/pcm/pcm_misc.c;h=54...
I have alsa-lib changes ready for these formats - they were needed to test these patches, will post them when this is merged on the kernel side (in case some changes are needed which affect both).
Please pay enough care when writing patch comment. Silence means nothing, at least for reviewers, even if you have good preparations.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/tree/include...
Regards
Takashi Sakamoto
On 23.11.2017 08:40, Takashi Sakamoto wrote:
On Nov 23 2017 08:44, Maciej S. Szmigiero wrote:
On 23.11.2017 00:27, Takashi Sakamoto wrote:
On Nov 23 2017 04:17, Maciej S. Szmigiero wrote:
(..)
--- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -236,7 +236,11 @@ typedef int __bitwise snd_pcm_format_t; #define SNDRV_PCM_FORMAT_DSD_U32_LE ((__force snd_pcm_format_t) 50) /* DSD, 4-byte samples DSD (x32), little endian */ #define SNDRV_PCM_FORMAT_DSD_U16_BE ((__force snd_pcm_format_t) 51) /* DSD, 2-byte samples DSD (x16), big endian */ #define SNDRV_PCM_FORMAT_DSD_U32_BE ((__force snd_pcm_format_t) 52) /* DSD, 4-byte samples DSD (x32), big endian */ -#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_DSD_U32_BE +#define SNDRV_PCM_FORMAT_S20_4LE ((__force snd_pcm_format_t) 53) /* in four bytes */ +#define SNDRV_PCM_FORMAT_S20_4BE ((__force snd_pcm_format_t) 54) /* in four bytes */ +#define SNDRV_PCM_FORMAT_U20_4LE ((__force snd_pcm_format_t) 55) /* in four bytes */ +#define SNDRV_PCM_FORMAT_U20_4BE ((__force snd_pcm_format_t) 56) /* in four bytes */ +#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_U20_4BE
In my opinion, for this type of definition, it's better to declare left/right-adjusted or padding side. (Of course, silence definition is already a hint, however the lack of information forces developers to have a careful behaviour to handle entries on the list. (I note that in current ALSA PCM interface there's no way to deliver MSB/LSB-first information about sample format.)
No other sound format includes this information in its name
You overlook comments in 'SNDRV_PCM_FORMAT_[U|S]24_[LE|BE]'. Let me refer to them [1]:
198 #define SNDRV_PCM_FORMAT_S24_LE ((__force snd_pcm_format_t) 6) /* low three bytes */ 199 #define SNDRV_PCM_FORMAT_S24_BE ((__force snd_pcm_format_t) 7) /* low three bytes */ 200 #define SNDRV_PCM_FORMAT_U24_LE ((__force snd_pcm_format_t) 8) /* low three bytes */ 201 #define SNDRV_PCM_FORMAT_U24_BE ((__force snd_pcm_format_t) 9) /* low three bytes */
Yes, I can add this information in a comment, just like these formats are described as "low three bytes" (in other words, LSB justified formats)
In your way, these types of format can be represented by 'SNDRV_PCM_FORMAT_[U|S]24_4[LE|BE]', thus for playback direction they mean:
#include <sound/asound.h> #include <endian.h> uint32_t *buf; uint32_t sample; snd_pcm_format_t format; sample = generate_a_sample(); (sample & ~0x00ffffff) /* invalid bits as sample */ if (format == SNDRV_PCM_FORMAT_[U|S]24_LE) { buf[0] = htole32(sample); else buf[0] = htobe32(sample); /* transfer content of the buf via ALSA kernel stuffs. */The comments are good enough for application developers in an aspect of a position for padding.
In general, studying from the past is preferable behaviour to be genius, however accumulated history includes mistakes and defects. Just pretending the past is not so genius, without further consideration.
Actually additions of the rest of entries for PCM format were done without enough cares of what information they give to application developers. Adding new entries is easier than fixing and improving them once exposed. It's a reason that they're left what they're.
I wish you had enough care to assist applications developers. Without applications, drivers are worthless and just waste of code base.
Right, I will add this information to a comment.
(..)
Additionally, alsa-lib includes some codes related to the definition[1]. If you'd like to thing goes well out of ALSA SoC part, it's better to submit changes to the library as well.
[1] http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/pcm/pcm_misc.c;h=54...
I have alsa-lib changes ready for these formats - they were needed to test these patches, will post them when this is merged on the kernel side (in case some changes are needed which affect both).
Please pay enough care when writing patch comment. Silence means nothing, at least for reviewers, even if you have good preparations.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/tree/include...
I will add this information (about alsa-lib changes) to commit notes in this patch description.
Regards
Takashi Sakamoto
Best regards, Maciej Szmigiero
On Wed, 22 Nov 2017 20:17:34 +0100, Maciej S. Szmigiero wrote:
This format is similar to existing SNDRV_PCM_FORMAT_{S,U}20_3 that keep 20-bit PCM samples in 3 bytes, however i.MX6 platform SSI FIFO does not allow 3-byte accesses (including DMA) so a 4-byte format is needed for it.
Signed-off-by: Maciej S. Szmigiero mail@maciej.szmigiero.name
include/sound/pcm.h | 8 ++++++++ include/sound/soc-dai.h | 2 ++ include/uapi/sound/asound.h | 10 +++++++++- sound/core/pcm_misc.c | 16 ++++++++++++++++ 4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 24febf9e177c..7ad2d3f0934f 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -191,6 +191,10 @@ struct snd_pcm_ops { #define SNDRV_PCM_FMTBIT_DSD_U32_LE _SNDRV_PCM_FMTBIT(DSD_U32_LE) #define SNDRV_PCM_FMTBIT_DSD_U16_BE _SNDRV_PCM_FMTBIT(DSD_U16_BE) #define SNDRV_PCM_FMTBIT_DSD_U32_BE _SNDRV_PCM_FMTBIT(DSD_U32_BE) +#define SNDRV_PCM_FMTBIT_S20_4LE _SNDRV_PCM_FMTBIT(S20_4LE) +#define SNDRV_PCM_FMTBIT_U20_4LE _SNDRV_PCM_FMTBIT(U20_4LE) +#define SNDRV_PCM_FMTBIT_S20_4BE _SNDRV_PCM_FMTBIT(S20_4BE) +#define SNDRV_PCM_FMTBIT_U20_4BE _SNDRV_PCM_FMTBIT(U20_4BE)
The conventional names aren't with "4" suffix, e.g. SNDRV_PCM_FMTBIT_S20_LE.
Also, there are still empty slots under 32, e.g. start from 25. The formats over 31 were used for 3 bytes or other unusual formats (although nowadays it makes little sense), and the slots < 32 would fit for 4 bytes linear format.
It's still an open question whether we should increase the protocol number when we add a new PCM format definition. I guess it's not, as the ABI behavior itself doesn't change, but I might have overlooked some possible breakage.
thanks,
Takashi
On 23.11.2017 09:08, Takashi Iwai wrote:
On Wed, 22 Nov 2017 20:17:34 +0100, Maciej S. Szmigiero wrote:
This format is similar to existing SNDRV_PCM_FORMAT_{S,U}20_3 that keep 20-bit PCM samples in 3 bytes, however i.MX6 platform SSI FIFO does not allow 3-byte accesses (including DMA) so a 4-byte format is needed for it.
Signed-off-by: Maciej S. Szmigiero mail@maciej.szmigiero.name
include/sound/pcm.h | 8 ++++++++ include/sound/soc-dai.h | 2 ++ include/uapi/sound/asound.h | 10 +++++++++- sound/core/pcm_misc.c | 16 ++++++++++++++++ 4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 24febf9e177c..7ad2d3f0934f 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -191,6 +191,10 @@ struct snd_pcm_ops { #define SNDRV_PCM_FMTBIT_DSD_U32_LE _SNDRV_PCM_FMTBIT(DSD_U32_LE) #define SNDRV_PCM_FMTBIT_DSD_U16_BE _SNDRV_PCM_FMTBIT(DSD_U16_BE) #define SNDRV_PCM_FMTBIT_DSD_U32_BE _SNDRV_PCM_FMTBIT(DSD_U32_BE) +#define SNDRV_PCM_FMTBIT_S20_4LE _SNDRV_PCM_FMTBIT(S20_4LE) +#define SNDRV_PCM_FMTBIT_U20_4LE _SNDRV_PCM_FMTBIT(U20_4LE) +#define SNDRV_PCM_FMTBIT_S20_4BE _SNDRV_PCM_FMTBIT(S20_4BE) +#define SNDRV_PCM_FMTBIT_U20_4BE _SNDRV_PCM_FMTBIT(U20_4BE)
The conventional names aren't with "4" suffix, e.g. SNDRV_PCM_FMTBIT_S20_LE.
Will rename it in a respin.
Also, there are still empty slots under 32, e.g. start from 25. The formats over 31 were used for 3 bytes or other unusual formats (although nowadays it makes little sense), and the slots < 32 would fit for 4 bytes linear format.
Will renumber these formats to start from 25 in a respin.
It's still an open question whether we should increase the protocol number when we add a new PCM format definition. I guess it's not, as the ABI behavior itself doesn't change, but I might have overlooked some possible breakage.
This isn't an incompatible, application-breaking, ABI change (it is rather kind of an additional "ABI"), so I think an ALSA protocol number should not be increased for it.
thanks,
Takashi
Thanks, Maciej
participants (3)
-
Maciej S. Szmigiero -
Takashi Iwai -
Takashi Sakamoto