diff mbox

[v2,1/2] ALSA: pcm: add SNDRV_PCM_FORMAT_{S, U}20

Message ID a54e6905-6124-9a59-b7ca-97cc650a8f99@maciej.szmigiero.name (mailing list archive)
State New, archived
Headers show

Commit Message

Maciej S. Szmigiero Nov. 23, 2017, 11:31 p.m. UTC
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 (more conventional)
format is needed for it.

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
Changes from v1: Drop "_4" suffix from these formats since they aren't
non-standard ones, use empty format slots starting from format number 25
for them, add information that they are LSB justified formats.

Corresponding alsa-lib changes will be posted as soon as this patch is
merged on the kernel side, to keep alsa-lib and kernel synchronized.

 include/sound/pcm.h         |  8 ++++++++
 include/sound/soc-dai.h     |  2 ++
 include/uapi/sound/asound.h |  9 +++++++++
 sound/core/pcm_misc.c       | 16 ++++++++++++++++
 4 files changed, 35 insertions(+)

Comments

Takashi Sakamoto Nov. 26, 2017, 9:27 a.m. UTC | #1
Hi,

On Nov 24 2017 08:31, 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 (more conventional)
> format is needed for it.
> 
> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> ---
> Changes from v1: Drop "_4" suffix from these formats since they aren't
> non-standard ones, use empty format slots starting from format number 25
> for them, add information that they are LSB justified formats.
> 
> Corresponding alsa-lib changes will be posted as soon as this patch is
> merged on the kernel side, to keep alsa-lib and kernel synchronized.
> 
>   include/sound/pcm.h         |  8 ++++++++
>   include/sound/soc-dai.h     |  2 ++
>   include/uapi/sound/asound.h |  9 +++++++++
>   sound/core/pcm_misc.c       | 16 ++++++++++++++++
>   4 files changed, 35 insertions(+)
> ...
> diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c
> index 9be81025372f..c62bfe27106f 100644
> --- a/sound/core/pcm_misc.c
> +++ b/sound/core/pcm_misc.c
> @@ -170,6 +170,22 @@ static struct pcm_format_data pcm_formats[(INT)SNDRV_PCM_FORMAT_LAST+1] = {
>   	[SNDRV_PCM_FORMAT_GSM] = {
>   		.le = -1, .signd = -1,
>   	},
> +	[SNDRV_PCM_FORMAT_S20_LE] = {
> +		.width = 20, .phys = 32, .le = 1, .signd = 1,
> +		.silence = {},
> +	},
> +	[SNDRV_PCM_FORMAT_S20_BE] = {
> +		.width = 20, .phys = 32, .le = 0, .signd = 1,
> +		.silence = {},
> +	},
> +	[SNDRV_PCM_FORMAT_U20_LE] = {
> +		.width = 20, .phys = 32, .le = 1, .signd = 0,
> +		.silence = { 0x00, 0x00, 0x08, 0x00 },
> +	},
> +	[SNDRV_PCM_FORMAT_U20_BE] = {
> +		.width = 20, .phys = 32, .le = 0, .signd = 0,
> +		.silence = { 0x00, 0x08, 0x00, 0x00 },
> +	},
>   	[SNDRV_PCM_FORMAT_SPECIAL] = {
>   		.le = -1, .signd = -1,
>   	},

Before applying this patch:
166         /* FIXME: the following three formats are not defined 
properly yet */
167         [SNDRV_PCM_FORMAT_MPEG] = {
168                 .le = -1, .signd = -1,
169         },
170         [SNDRV_PCM_FORMAT_GSM] = {
171                 .le = -1, .signd = -1,
172         },
173         [SNDRV_PCM_FORMAT_SPECIAL] = {
174                 .le = -1, .signd = -1,
175         },

After applying this patch:

166         /* FIXME: the following three formats are not defined 
properly yet */
167         [SNDRV_PCM_FORMAT_MPEG] = {
168                 .le = -1, .signd = -1,
169         },
170         [SNDRV_PCM_FORMAT_GSM] = {
171                 .le = -1, .signd = -1,
172         },
173         [SNDRV_PCM_FORMAT_S20_LE] = {
174                 .width = 20, .phys = 32, .le = 1, .signd = 1,
175                 .silence = {},
176         },
177         [SNDRV_PCM_FORMAT_S20_BE] = {
178                 .width = 20, .phys = 32, .le = 0, .signd = 1,
179                 .silence = {},
180         },
181         [SNDRV_PCM_FORMAT_U20_LE] = {
182                 .width = 20, .phys = 32, .le = 1, .signd = 0,
183                 .silence = { 0x00, 0x00, 0x08, 0x00 },
184         },
185         [SNDRV_PCM_FORMAT_U20_BE] = {
186                 .width = 20, .phys = 32, .le = 0, .signd = 0,
187                 .silence = { 0x00, 0x08, 0x00, 0x00 },
188         },
189         [SNDRV_PCM_FORMAT_SPECIAL] = {
190                 .le = -1, .signd = -1,
191         },

I think it good to add an alternative comment for each of entry which is 
not defined yet, like:

-> 166         /* FIXME: this format is not defined properly yet */
167         [SNDRV_PCM_FORMAT_MPEG] = {
168                 .le = -1, .signd = -1,
169         },
-> 170         /* FIXME: this format is not defined properly yet */
171         [SNDRV_PCM_FORMAT_GSM] = {
172                 .le = -1, .signd = -1,
173         },
174         [SNDRV_PCM_FORMAT_S20_LE] = {
175                 .width = 20, .phys = 32, .le = 1, .signd = 1,
176                 .silence = {},
177         },
178         [SNDRV_PCM_FORMAT_S20_BE] = {
179                 .width = 20, .phys = 32, .le = 0, .signd = 1,
180                 .silence = {},
181         },
182         [SNDRV_PCM_FORMAT_U20_LE] = {
183                 .width = 20, .phys = 32, .le = 1, .signd = 0,
184                 .silence = { 0x00, 0x00, 0x08, 0x00 },
185         },
186         [SNDRV_PCM_FORMAT_U20_BE] = {
187                 .width = 20, .phys = 32, .le = 0, .signd = 0,
188                 .silence = { 0x00, 0x08, 0x00, 0x00 },
189         },
-> 190         /* FIXME: this format is not defined properly yet */
191         [SNDRV_PCM_FORMAT_SPECIAL] = {
192                 .le = -1, .signd = -1,
193         },


Regards

Takashi Sakamoto
Maciej S. Szmigiero Nov. 26, 2017, 11:13 p.m. UTC | #2
Hi,

On 26.11.2017 10:27, Takashi Sakamoto wrote:
> Hi,
> 
(..)
> Before applying this patch:
> 166         /* FIXME: the following three formats are not defined properly yet */
> 167         [SNDRV_PCM_FORMAT_MPEG] = {
> 168                 .le = -1, .signd = -1,
> 169         },
> 170         [SNDRV_PCM_FORMAT_GSM] = {
> 171                 .le = -1, .signd = -1,
> 172         },
> 173         [SNDRV_PCM_FORMAT_SPECIAL] = {
> 174                 .le = -1, .signd = -1,
> 175         },
> 
> After applying this patch:
> 
> 166         /* FIXME: the following three formats are not defined properly yet */
> 167         [SNDRV_PCM_FORMAT_MPEG] = {
> 168                 .le = -1, .signd = -1,
> 169         },
> 170         [SNDRV_PCM_FORMAT_GSM] = {
> 171                 .le = -1, .signd = -1,
> 172         },
> 173         [SNDRV_PCM_FORMAT_S20_LE] = {
(..)> 
> I think it good to add an alternative comment for each of entry which is not defined yet, like:
> 
> -> 166         /* FIXME: this format is not defined properly yet */
> 167         [SNDRV_PCM_FORMAT_MPEG] = {
> 168                 .le = -1, .signd = -1,
> 169         },
> -> 170         /* FIXME: this format is not defined properly yet */
> 171         [SNDRV_PCM_FORMAT_GSM] = {
> 172                 .le = -1, .signd = -1,
> 173         },
> 174         [SNDRV_PCM_FORMAT_S20_LE] = {
> 175                 .width = 20, .phys = 32, .le = 1, .signd = 1,
> 176                 .silence = {},
> 177         },
> 178         [SNDRV_PCM_FORMAT_S20_BE] = {
> 179                 .width = 20, .phys = 32, .le = 0, .signd = 1,
> 180                 .silence = {},
> 181         },
> 182         [SNDRV_PCM_FORMAT_U20_LE] = {
> 183                 .width = 20, .phys = 32, .le = 1, .signd = 0,
> 184                 .silence = { 0x00, 0x00, 0x08, 0x00 },
> 185         },
> 186         [SNDRV_PCM_FORMAT_U20_BE] = {
> 187                 .width = 20, .phys = 32, .le = 0, .signd = 0,
> 188                 .silence = { 0x00, 0x08, 0x00, 0x00 },
> 189         },
> -> 190         /* FIXME: this format is not defined properly yet */
> 191         [SNDRV_PCM_FORMAT_SPECIAL] = {
> 192                 .le = -1, .signd = -1,
> 193         },
> 

Thanks, fixed now in v3.

> Regards
> 
> Takashi Sakamoto

Best regards,
Maciej Szmigiero
diff mbox

Patch

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 24febf9e177c..e054c583d3b3 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -169,6 +169,10 @@  struct snd_pcm_ops {
 #define SNDRV_PCM_FMTBIT_IMA_ADPCM	_SNDRV_PCM_FMTBIT(IMA_ADPCM)
 #define SNDRV_PCM_FMTBIT_MPEG		_SNDRV_PCM_FMTBIT(MPEG)
 #define SNDRV_PCM_FMTBIT_GSM		_SNDRV_PCM_FMTBIT(GSM)
+#define SNDRV_PCM_FMTBIT_S20_LE	_SNDRV_PCM_FMTBIT(S20_LE)
+#define SNDRV_PCM_FMTBIT_U20_LE	_SNDRV_PCM_FMTBIT(U20_LE)
+#define SNDRV_PCM_FMTBIT_S20_BE	_SNDRV_PCM_FMTBIT(S20_BE)
+#define SNDRV_PCM_FMTBIT_U20_BE	_SNDRV_PCM_FMTBIT(U20_BE)
 #define SNDRV_PCM_FMTBIT_SPECIAL	_SNDRV_PCM_FMTBIT(SPECIAL)
 #define SNDRV_PCM_FMTBIT_S24_3LE	_SNDRV_PCM_FMTBIT(S24_3LE)
 #define SNDRV_PCM_FMTBIT_U24_3LE	_SNDRV_PCM_FMTBIT(U24_3LE)
@@ -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		SNDRV_PCM_FMTBIT_S20_LE
+#define SNDRV_PCM_FMTBIT_U20		SNDRV_PCM_FMTBIT_U20_LE
 #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		SNDRV_PCM_FMTBIT_S20_BE
+#define SNDRV_PCM_FMTBIT_U20		SNDRV_PCM_FMTBIT_U20_BE
 #endif
 
 struct snd_pcm_file {
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index 58acd00cae19..d970879944fc 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_LE |\
+			       SNDRV_PCM_FMTBIT_S20_BE |\
 			       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..7385024041d2 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -214,6 +214,11 @@  typedef int __bitwise snd_pcm_format_t;
 #define	SNDRV_PCM_FORMAT_IMA_ADPCM	((__force snd_pcm_format_t) 22)
 #define	SNDRV_PCM_FORMAT_MPEG		((__force snd_pcm_format_t) 23)
 #define	SNDRV_PCM_FORMAT_GSM		((__force snd_pcm_format_t) 24)
+#define	SNDRV_PCM_FORMAT_S20_LE	((__force snd_pcm_format_t) 25) /* \ 			*/
+#define	SNDRV_PCM_FORMAT_S20_BE	((__force snd_pcm_format_t) 26) /* | 			*/
+#define	SNDRV_PCM_FORMAT_U20_LE	((__force snd_pcm_format_t) 27) /* | in four bytes,	*/
+#define	SNDRV_PCM_FORMAT_U20_BE	((__force snd_pcm_format_t) 28) /* / LSB justified	*/
+/* gap in the numbering for a future standard linear format */
 #define	SNDRV_PCM_FORMAT_SPECIAL	((__force snd_pcm_format_t) 31)
 #define	SNDRV_PCM_FORMAT_S24_3LE	((__force snd_pcm_format_t) 32)	/* in three bytes */
 #define	SNDRV_PCM_FORMAT_S24_3BE	((__force snd_pcm_format_t) 33)	/* in three bytes */
@@ -248,6 +253,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		SNDRV_PCM_FORMAT_S20_LE
+#define	SNDRV_PCM_FORMAT_U20		SNDRV_PCM_FORMAT_U20_LE
 #endif
 #ifdef SNDRV_BIG_ENDIAN
 #define	SNDRV_PCM_FORMAT_S16		SNDRV_PCM_FORMAT_S16_BE
@@ -259,6 +266,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		SNDRV_PCM_FORMAT_S20_BE
+#define	SNDRV_PCM_FORMAT_U20		SNDRV_PCM_FORMAT_U20_BE
 #endif
 
 typedef int __bitwise snd_pcm_subformat_t;
diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c
index 9be81025372f..c62bfe27106f 100644
--- a/sound/core/pcm_misc.c
+++ b/sound/core/pcm_misc.c
@@ -170,6 +170,22 @@  static struct pcm_format_data pcm_formats[(INT)SNDRV_PCM_FORMAT_LAST+1] = {
 	[SNDRV_PCM_FORMAT_GSM] = {
 		.le = -1, .signd = -1,
 	},
+	[SNDRV_PCM_FORMAT_S20_LE] = {
+		.width = 20, .phys = 32, .le = 1, .signd = 1,
+		.silence = {},
+	},
+	[SNDRV_PCM_FORMAT_S20_BE] = {
+		.width = 20, .phys = 32, .le = 0, .signd = 1,
+		.silence = {},
+	},
+	[SNDRV_PCM_FORMAT_U20_LE] = {
+		.width = 20, .phys = 32, .le = 1, .signd = 0,
+		.silence = { 0x00, 0x00, 0x08, 0x00 },
+	},
+	[SNDRV_PCM_FORMAT_U20_BE] = {
+		.width = 20, .phys = 32, .le = 0, .signd = 0,
+		.silence = { 0x00, 0x08, 0x00, 0x00 },
+	},
 	[SNDRV_PCM_FORMAT_SPECIAL] = {
 		.le = -1, .signd = -1,
 	},