[PATCH 01/19] ASoC: atmel: Don't set unused struct snd_pcm_hardware fields

[PATCH 01/19] ASoC: atmel: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Bo Shen <voice.shen@atmel.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 sound/soc/atmel/atmel-pcm-dma.c | 1 -
 sound/soc/atmel/atmel-pcm-pdc.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/sound/soc/atmel/atmel-pcm-dma.c b/sound/soc/atmel/atmel-pcm-dma.c
index 06082e5..b79a2a8 100644
--- a/sound/soc/atmel/atmel-pcm-dma.c
+++ b/sound/soc/atmel/atmel-pcm-dma.c
@@ -50,7 +50,6 @@ static const struct snd_pcm_hardware atmel_pcm_dma_hardware = {
 				  SNDRV_PCM_INFO_INTERLEAVED |
 				  SNDRV_PCM_INFO_RESUME |
 				  SNDRV_PCM_INFO_PAUSE,
-	.formats		= SNDRV_PCM_FMTBIT_S16_LE,
 	.period_bytes_min	= 256,		/* lighting DMA overhead */
 	.period_bytes_max	= 2 * 0xffff,	/* if 2 bytes format */
 	.periods_min		= 8,
diff --git a/sound/soc/atmel/atmel-pcm-pdc.c b/sound/soc/atmel/atmel-pcm-pdc.c
index 054ea4d..33ec592 100644
--- a/sound/soc/atmel/atmel-pcm-pdc.c
+++ b/sound/soc/atmel/atmel-pcm-pdc.c
@@ -58,7 +58,6 @@ static const struct snd_pcm_hardware atmel_pcm_hardware = {
 				  SNDRV_PCM_INFO_MMAP_VALID |
 				  SNDRV_PCM_INFO_INTERLEAVED |
 				  SNDRV_PCM_INFO_PAUSE,
-	.formats		= SNDRV_PCM_FMTBIT_S16_LE,
 	.period_bytes_min	= 32,
 	.period_bytes_max	= 8192,
 	.periods_min		= 2,
-- 
1.8.0

[PATCH 02/19] ASoC: au1x: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Manuel Lauss <manuel.lauss@googlemail.com>
---
 sound/soc/au1x/dbdma2.c |  9 ---------
 sound/soc/au1x/dma.c    | 14 --------------
 2 files changed, 23 deletions(-)

diff --git a/sound/soc/au1x/dbdma2.c b/sound/soc/au1x/dbdma2.c
index 3b4eafa..17a24d8 100644
--- a/sound/soc/au1x/dbdma2.c
+++ b/sound/soc/au1x/dbdma2.c
@@ -65,19 +65,10 @@ struct au1xpsc_audio_dmadata {
 #define AU1XPSC_PERIOD_MIN_BYTES	1024
 #define AU1XPSC_BUFFER_MIN_BYTES	65536
 
-#define AU1XPSC_PCM_FMTS					\
-	(SNDRV_PCM_FMTBIT_S8     | SNDRV_PCM_FMTBIT_U8 |	\
-	 SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |	\
-	 SNDRV_PCM_FMTBIT_U16_LE | SNDRV_PCM_FMTBIT_U16_BE |	\
-	 SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S32_BE |	\
-	 SNDRV_PCM_FMTBIT_U32_LE | SNDRV_PCM_FMTBIT_U32_BE |	\
-	 0)
-
 /* PCM hardware DMA capabilities - platform specific */
 static const struct snd_pcm_hardware au1xpsc_pcm_hardware = {
 	.info		  = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
 			    SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BATCH,
-	.formats	  = AU1XPSC_PCM_FMTS,
 	.period_bytes_min = AU1XPSC_PERIOD_MIN_BYTES,
 	.period_bytes_max = 4096 * 1024 - 1,
 	.periods_min	  = 2,
diff --git a/sound/soc/au1x/dma.c b/sound/soc/au1x/dma.c
index befd107..e920b60 100644
--- a/sound/soc/au1x/dma.c
+++ b/sound/soc/au1x/dma.c
@@ -21,14 +21,6 @@
 
 #include "psc.h"
 
-#define ALCHEMY_PCM_FMTS					\
-	(SNDRV_PCM_FMTBIT_S8     | SNDRV_PCM_FMTBIT_U8 |	\
-	 SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |	\
-	 SNDRV_PCM_FMTBIT_U16_LE | SNDRV_PCM_FMTBIT_U16_BE |	\
-	 SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S32_BE |	\
-	 SNDRV_PCM_FMTBIT_U32_LE | SNDRV_PCM_FMTBIT_U32_BE |	\
-	 0)
-
 struct pcm_period {
 	u32 start;
 	u32 relative_end;	/* relative to start of buffer */
@@ -171,12 +163,6 @@ static irqreturn_t au1000_dma_interrupt(int irq, void *ptr)
 static const struct snd_pcm_hardware alchemy_pcm_hardware = {
 	.info		  = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
 			    SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BATCH,
-	.formats	  = ALCHEMY_PCM_FMTS,
-	.rates		  = SNDRV_PCM_RATE_8000_192000,
-	.rate_min	  = SNDRV_PCM_RATE_8000,
-	.rate_max	  = SNDRV_PCM_RATE_192000,
-	.channels_min	  = 2,
-	.channels_max	  = 2,
 	.period_bytes_min = 1024,
 	.period_bytes_max = 16 * 1024 - 1,
 	.periods_min	  = 4,
-- 
1.8.0

Re: [PATCH 02/19] ASoC: au1x: Don't set unused struct snd_pcm_hardware fields

[Manuel Lauss]

My Alchemy hardware likes it.

Tested-by: Manuel Lauss <manuel.lauss@gmail.com>

On Fri, Dec 20, 2013 at 2:20 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Manuel Lauss <manuel.lauss@googlemail.com>
> ---
>  sound/soc/au1x/dbdma2.c |  9 ---------
>  sound/soc/au1x/dma.c    | 14 --------------
>  2 files changed, 23 deletions(-)
>
> diff --git a/sound/soc/au1x/dbdma2.c b/sound/soc/au1x/dbdma2.c
> index 3b4eafa..17a24d8 100644
> --- a/sound/soc/au1x/dbdma2.c
> +++ b/sound/soc/au1x/dbdma2.c
> @@ -65,19 +65,10 @@ struct au1xpsc_audio_dmadata {
>  #define AU1XPSC_PERIOD_MIN_BYTES       1024
>  #define AU1XPSC_BUFFER_MIN_BYTES       65536
>
> -#define AU1XPSC_PCM_FMTS                                       \
> -       (SNDRV_PCM_FMTBIT_S8     | SNDRV_PCM_FMTBIT_U8 |        \
> -        SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |    \
> -        SNDRV_PCM_FMTBIT_U16_LE | SNDRV_PCM_FMTBIT_U16_BE |    \
> -        SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S32_BE |    \
> -        SNDRV_PCM_FMTBIT_U32_LE | SNDRV_PCM_FMTBIT_U32_BE |    \
> -        0)
> -
>  /* PCM hardware DMA capabilities - platform specific */
>  static const struct snd_pcm_hardware au1xpsc_pcm_hardware = {
>         .info             = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
>                             SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BATCH,
> -       .formats          = AU1XPSC_PCM_FMTS,
>         .period_bytes_min = AU1XPSC_PERIOD_MIN_BYTES,
>         .period_bytes_max = 4096 * 1024 - 1,
>         .periods_min      = 2,
> diff --git a/sound/soc/au1x/dma.c b/sound/soc/au1x/dma.c
> index befd107..e920b60 100644
> --- a/sound/soc/au1x/dma.c
> +++ b/sound/soc/au1x/dma.c
> @@ -21,14 +21,6 @@
>
>  #include "psc.h"
>
> -#define ALCHEMY_PCM_FMTS                                       \
> -       (SNDRV_PCM_FMTBIT_S8     | SNDRV_PCM_FMTBIT_U8 |        \
> -        SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |    \
> -        SNDRV_PCM_FMTBIT_U16_LE | SNDRV_PCM_FMTBIT_U16_BE |    \
> -        SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S32_BE |    \
> -        SNDRV_PCM_FMTBIT_U32_LE | SNDRV_PCM_FMTBIT_U32_BE |    \
> -        0)
> -
>  struct pcm_period {
>         u32 start;
>         u32 relative_end;       /* relative to start of buffer */
> @@ -171,12 +163,6 @@ static irqreturn_t au1000_dma_interrupt(int irq, void *ptr)
>  static const struct snd_pcm_hardware alchemy_pcm_hardware = {
>         .info             = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
>                             SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BATCH,
> -       .formats          = ALCHEMY_PCM_FMTS,
> -       .rates            = SNDRV_PCM_RATE_8000_192000,
> -       .rate_min         = SNDRV_PCM_RATE_8000,
> -       .rate_max         = SNDRV_PCM_RATE_192000,
> -       .channels_min     = 2,
> -       .channels_max     = 2,
>         .period_bytes_min = 1024,
>         .period_bytes_max = 16 * 1024 - 1,
>         .periods_min      = 4,
> --
> 1.8.0
>

Re: [PATCH 02/19] ASoC: au1x: Don't set unused struct snd_pcm_hardware fields

[Mark Brown]

On Fri, Dec 20, 2013 at 02:20:08PM +0100, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

Applied, thanks.

[PATCH 03/19] ASoC: blackfin: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/blackfin/bf5xx-ac97-pcm.c | 1 -
 sound/soc/blackfin/bf5xx-i2s-pcm.c  | 3 ---
 2 files changed, 4 deletions(-)

diff --git a/sound/soc/blackfin/bf5xx-ac97-pcm.c b/sound/soc/blackfin/bf5xx-ac97-pcm.c
index 1d4c676..cdb8ee7 100644
--- a/sound/soc/blackfin/bf5xx-ac97-pcm.c
+++ b/sound/soc/blackfin/bf5xx-ac97-pcm.c
@@ -107,7 +107,6 @@ static const struct snd_pcm_hardware bf5xx_pcm_hardware = {
 #endif
 				   SNDRV_PCM_INFO_BLOCK_TRANSFER,
 
-	.formats		= SNDRV_PCM_FMTBIT_S16_LE,
 	.period_bytes_min	= 32,
 	.period_bytes_max	= 0x10000,
 	.periods_min		= 1,
diff --git a/sound/soc/blackfin/bf5xx-i2s-pcm.c b/sound/soc/blackfin/bf5xx-i2s-pcm.c
index 2a5b434..a3881c4 100644
--- a/sound/soc/blackfin/bf5xx-i2s-pcm.c
+++ b/sound/soc/blackfin/bf5xx-i2s-pcm.c
@@ -52,9 +52,6 @@ static const struct snd_pcm_hardware bf5xx_pcm_hardware = {
 	.info			= SNDRV_PCM_INFO_INTERLEAVED |
 				   SNDRV_PCM_INFO_MMAP_VALID |
 				   SNDRV_PCM_INFO_BLOCK_TRANSFER,
-	.formats		= SNDRV_PCM_FMTBIT_S16_LE |
-				   SNDRV_PCM_FMTBIT_S24_LE |
-				   SNDRV_PCM_FMTBIT_S32_LE,
 	.period_bytes_min	= 32,
 	.period_bytes_max	= 0x10000,
 	.periods_min		= 1,
-- 
1.8.0

Re: [PATCH 03/19] ASoC: blackfin: Don't set unused struct snd_pcm_hardware fields

[Mark Brown]

On Fri, Dec 20, 2013 at 02:20:09PM +0100, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

Applied, thanks.

[PATCH 04/19] ASoC: davinci: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
Cc: Olof Johansson <olof@lixom.net>
---
 sound/soc/davinci/davinci-pcm.c | 28 ----------------------------
 1 file changed, 28 deletions(-)

diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
index fb5d107..14145cd 100644
--- a/sound/soc/davinci/davinci-pcm.c
+++ b/sound/soc/davinci/davinci-pcm.c
@@ -46,33 +46,11 @@ static void print_buf_info(int slot, char *name)
 }
 #endif
 
-#define DAVINCI_PCM_FMTBITS	(\
-				SNDRV_PCM_FMTBIT_S8	|\
-				SNDRV_PCM_FMTBIT_U8	|\
-				SNDRV_PCM_FMTBIT_S16_LE	|\
-				SNDRV_PCM_FMTBIT_S16_BE	|\
-				SNDRV_PCM_FMTBIT_U16_LE	|\
-				SNDRV_PCM_FMTBIT_U16_BE	|\
-				SNDRV_PCM_FMTBIT_S24_LE	|\
-				SNDRV_PCM_FMTBIT_S24_BE	|\
-				SNDRV_PCM_FMTBIT_U24_LE	|\
-				SNDRV_PCM_FMTBIT_U24_BE	|\
-				SNDRV_PCM_FMTBIT_S32_LE	|\
-				SNDRV_PCM_FMTBIT_S32_BE	|\
-				SNDRV_PCM_FMTBIT_U32_LE	|\
-				SNDRV_PCM_FMTBIT_U32_BE)
-
 static struct snd_pcm_hardware pcm_hardware_playback = {
 	.info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
 		 SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
 		 SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME|
 		 SNDRV_PCM_INFO_BATCH),
-	.formats = DAVINCI_PCM_FMTBITS,
-	.rates = SNDRV_PCM_RATE_8000_192000 | SNDRV_PCM_RATE_KNOT,
-	.rate_min = 8000,
-	.rate_max = 192000,
-	.channels_min = 2,
-	.channels_max = 384,
 	.buffer_bytes_max = 128 * 1024,
 	.period_bytes_min = 32,
 	.period_bytes_max = 8 * 1024,
@@ -86,12 +64,6 @@ static struct snd_pcm_hardware pcm_hardware_capture = {
 		 SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
 		 SNDRV_PCM_INFO_PAUSE |
 		 SNDRV_PCM_INFO_BATCH),
-	.formats = DAVINCI_PCM_FMTBITS,
-	.rates = SNDRV_PCM_RATE_8000_192000 | SNDRV_PCM_RATE_KNOT,
-	.rate_min = 8000,
-	.rate_max = 192000,
-	.channels_min = 2,
-	.channels_max = 384,
 	.buffer_bytes_max = 128 * 1024,
 	.period_bytes_min = 32,
 	.period_bytes_max = 8 * 1024,
-- 
1.8.0

Re: [PATCH 04/19] ASoC: davinci: Don't set unused struct snd_pcm_hardware fields

[Peter Ujfalusi]

On 12/20/2013 03:20 PM, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Cc: Olof Johansson <olof@lixom.net>
> ---
>  sound/soc/davinci/davinci-pcm.c | 28 ----------------------------
>  1 file changed, 28 deletions(-)
> 
> diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
> index fb5d107..14145cd 100644
> --- a/sound/soc/davinci/davinci-pcm.c
> +++ b/sound/soc/davinci/davinci-pcm.c
> @@ -46,33 +46,11 @@ static void print_buf_info(int slot, char *name)
>  }
>  #endif
>  
> -#define DAVINCI_PCM_FMTBITS	(\
> -				SNDRV_PCM_FMTBIT_S8	|\
> -				SNDRV_PCM_FMTBIT_U8	|\
> -				SNDRV_PCM_FMTBIT_S16_LE	|\
> -				SNDRV_PCM_FMTBIT_S16_BE	|\
> -				SNDRV_PCM_FMTBIT_U16_LE	|\
> -				SNDRV_PCM_FMTBIT_U16_BE	|\
> -				SNDRV_PCM_FMTBIT_S24_LE	|\
> -				SNDRV_PCM_FMTBIT_S24_BE	|\
> -				SNDRV_PCM_FMTBIT_U24_LE	|\
> -				SNDRV_PCM_FMTBIT_U24_BE	|\
> -				SNDRV_PCM_FMTBIT_S32_LE	|\
> -				SNDRV_PCM_FMTBIT_S32_BE	|\
> -				SNDRV_PCM_FMTBIT_U32_LE	|\
> -				SNDRV_PCM_FMTBIT_U32_BE)
> -
>  static struct snd_pcm_hardware pcm_hardware_playback = {
>  	.info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
>  		 SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
>  		 SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME|
>  		 SNDRV_PCM_INFO_BATCH),
> -	.formats = DAVINCI_PCM_FMTBITS,
> -	.rates = SNDRV_PCM_RATE_8000_192000 | SNDRV_PCM_RATE_KNOT,
> -	.rate_min = 8000,
> -	.rate_max = 192000,
> -	.channels_min = 2,
> -	.channels_max = 384,
>  	.buffer_bytes_max = 128 * 1024,
>  	.period_bytes_min = 32,
>  	.period_bytes_max = 8 * 1024,
> @@ -86,12 +64,6 @@ static struct snd_pcm_hardware pcm_hardware_capture = {
>  		 SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
>  		 SNDRV_PCM_INFO_PAUSE |
>  		 SNDRV_PCM_INFO_BATCH),
> -	.formats = DAVINCI_PCM_FMTBITS,
> -	.rates = SNDRV_PCM_RATE_8000_192000 | SNDRV_PCM_RATE_KNOT,
> -	.rate_min = 8000,
> -	.rate_max = 192000,
> -	.channels_min = 2,
> -	.channels_max = 384,
>  	.buffer_bytes_max = 128 * 1024,
>  	.period_bytes_min = 32,
>  	.period_bytes_max = 8 * 1024,
> 

Re: [PATCH 04/19] ASoC: davinci: Don't set unused struct snd_pcm_hardware fields

[Mark Brown]

On Fri, Dec 20, 2013 at 02:20:10PM +0100, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

Applied, thanks.

[PATCH 05/19] ASoC: ep93xx: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Ryan Mallon <rmallon@gmail.com>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
---
 sound/soc/cirrus/ep93xx-pcm.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/sound/soc/cirrus/ep93xx-pcm.c b/sound/soc/cirrus/ep93xx-pcm.c
index ca6698d..5f66447 100644
--- a/sound/soc/cirrus/ep93xx-pcm.c
+++ b/sound/soc/cirrus/ep93xx-pcm.c
@@ -30,15 +30,6 @@ static const struct snd_pcm_hardware ep93xx_pcm_hardware = {
 				   SNDRV_PCM_INFO_MMAP_VALID	|
 				   SNDRV_PCM_INFO_INTERLEAVED	|
 				   SNDRV_PCM_INFO_BLOCK_TRANSFER),
-				   
-	.rates			= SNDRV_PCM_RATE_8000_192000,
-	.rate_min		= SNDRV_PCM_RATE_8000,
-	.rate_max		= SNDRV_PCM_RATE_192000,
-	
-	.formats		= (SNDRV_PCM_FMTBIT_S16_LE |
-				   SNDRV_PCM_FMTBIT_S24_LE |
-				   SNDRV_PCM_FMTBIT_S32_LE),
-	
 	.buffer_bytes_max	= 131072,
 	.period_bytes_min	= 32,
 	.period_bytes_max	= 32768,
-- 
1.8.0

Re: [PATCH 05/19] ASoC: ep93xx: Don't set unused struct snd_pcm_hardware fields

[Mark Brown]

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

On Fri, Dec 20, 2013 at 02:20:11PM +0100, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

Applied, thanks.

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

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

[PATCH 06/19] ASoC: fsl: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Timur Tabi <timur@tabi.org>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Nicolin Chen <b42378@freescale.com>
Cc: Markus Pargmann <mpa@pengutronix.de>
---
 sound/soc/fsl/fsl_dma.c     | 26 --------------------------
 sound/soc/fsl/imx-pcm-dma.c |  4 ----
 sound/soc/fsl/imx-pcm-fiq.c |  4 ----
 sound/soc/fsl/mpc5200_dma.c |  6 ------
 4 files changed, 40 deletions(-)

diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c
index d570f8c..2326831 100644
--- a/sound/soc/fsl/fsl_dma.c
+++ b/sound/soc/fsl/fsl_dma.c
@@ -37,28 +37,6 @@
 #include "fsl_dma.h"
 #include "fsl_ssi.h"	/* For the offset of stx0 and srx0 */
 
-/*
- * The formats that the DMA controller supports, which is anything
- * that is 8, 16, or 32 bits.
- */
-#define FSLDMA_PCM_FORMATS (SNDRV_PCM_FMTBIT_S8 	| \
-			    SNDRV_PCM_FMTBIT_U8 	| \
-			    SNDRV_PCM_FMTBIT_S16_LE     | \
-			    SNDRV_PCM_FMTBIT_S16_BE     | \
-			    SNDRV_PCM_FMTBIT_U16_LE     | \
-			    SNDRV_PCM_FMTBIT_U16_BE     | \
-			    SNDRV_PCM_FMTBIT_S24_LE     | \
-			    SNDRV_PCM_FMTBIT_S24_BE     | \
-			    SNDRV_PCM_FMTBIT_U24_LE     | \
-			    SNDRV_PCM_FMTBIT_U24_BE     | \
-			    SNDRV_PCM_FMTBIT_S32_LE     | \
-			    SNDRV_PCM_FMTBIT_S32_BE     | \
-			    SNDRV_PCM_FMTBIT_U32_LE     | \
-			    SNDRV_PCM_FMTBIT_U32_BE)
-
-#define FSLDMA_PCM_RATES (SNDRV_PCM_RATE_5512 | SNDRV_PCM_RATE_8000_192000 | \
-			  SNDRV_PCM_RATE_CONTINUOUS)
-
 struct dma_object {
 	struct snd_soc_platform_driver dai;
 	dma_addr_t ssi_stx_phys;
@@ -139,10 +117,6 @@ static const struct snd_pcm_hardware fsl_dma_hardware = {
 				  SNDRV_PCM_INFO_MMAP_VALID |
 				  SNDRV_PCM_INFO_JOINT_DUPLEX |
 				  SNDRV_PCM_INFO_PAUSE,
-	.formats		= FSLDMA_PCM_FORMATS,
-	.rates  		= FSLDMA_PCM_RATES,
-	.rate_min       	= 5512,
-	.rate_max       	= 192000,
 	.period_bytes_min       = 512,  	/* A reasonable limit */
 	.period_bytes_max       = (u32) -1,
 	.periods_min    	= NUM_DMA_LINKS,
diff --git a/sound/soc/fsl/imx-pcm-dma.c b/sound/soc/fsl/imx-pcm-dma.c
index c5e47f8..0849b7b 100644
--- a/sound/soc/fsl/imx-pcm-dma.c
+++ b/sound/soc/fsl/imx-pcm-dma.c
@@ -40,10 +40,6 @@ static const struct snd_pcm_hardware imx_pcm_hardware = {
 		SNDRV_PCM_INFO_MMAP_VALID |
 		SNDRV_PCM_INFO_PAUSE |
 		SNDRV_PCM_INFO_RESUME,
-	.formats = SNDRV_PCM_FMTBIT_S16_LE,
-	.rate_min = 8000,
-	.channels_min = 2,
-	.channels_max = 2,
 	.buffer_bytes_max = IMX_SSI_DMABUF_SIZE,
 	.period_bytes_min = 128,
 	.period_bytes_max = 65535, /* Limited by SDMA engine */
diff --git a/sound/soc/fsl/imx-pcm-fiq.c b/sound/soc/fsl/imx-pcm-fiq.c
index c75d43b..0d668bd 100644
--- a/sound/soc/fsl/imx-pcm-fiq.c
+++ b/sound/soc/fsl/imx-pcm-fiq.c
@@ -161,10 +161,6 @@ static struct snd_pcm_hardware snd_imx_hardware = {
 		SNDRV_PCM_INFO_MMAP_VALID |
 		SNDRV_PCM_INFO_PAUSE |
 		SNDRV_PCM_INFO_RESUME,
-	.formats = SNDRV_PCM_FMTBIT_S16_LE,
-	.rate_min = 8000,
-	.channels_min = 2,
-	.channels_max = 2,
 	.buffer_bytes_max = IMX_SSI_DMABUF_SIZE,
 	.period_bytes_min = 128,
 	.period_bytes_max = 16 * 1024,
diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c
index 71bf2f2..30d52e7 100644
--- a/sound/soc/fsl/mpc5200_dma.c
+++ b/sound/soc/fsl/mpc5200_dma.c
@@ -198,12 +198,6 @@ static const struct snd_pcm_hardware psc_dma_hardware = {
 	.info = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
 		SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
 		SNDRV_PCM_INFO_BATCH,
-	.formats = SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_BE |
-		SNDRV_PCM_FMTBIT_S24_BE | SNDRV_PCM_FMTBIT_S32_BE,
-	.rate_min = 8000,
-	.rate_max = 48000,
-	.channels_min = 1,
-	.channels_max = 2,
 	.period_bytes_max	= 1024 * 1024,
 	.period_bytes_min	= 32,
 	.periods_min		= 2,
-- 
1.8.0

Re: [PATCH 06/19] ASoC: fsl: Don't set unused struct snd_pcm_hardware fields

[Timur Tabi]

Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

Is this new?  There are formats that the codec and the SSI support that 
the DMA controller does NOT support, like packed 24-bit samples.  How do 
we ensure that we never get those?

Re: [PATCH 06/19] ASoC: fsl: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

On 12/20/2013 03:25 PM, Timur Tabi wrote:
> Lars-Peter Clausen wrote:
>> The ASoC core assumes that the PCM component of the ASoC card transparently
>> moves data around and does not impose any restrictions on the memory
>> layout or
>> the transfer speed. It ignores all fields from the snd_pcm_hardware struct
>> for
>> the PCM driver that are related to this. Setting these fields in the PCM
>> driver
>> might suggest otherwise though, so rather not set them.
> 
> Is this new?  There are formats that the codec and the SSI support that the
> DMA controller does NOT support, like packed 24-bit samples.  How do we
> ensure that we never get those?

No, this is how it has always been. If there are restrictions imposed by the
DMA controller we need to add support for expressing this inside the ASoC
framework. But I think it will probably be more complex than just saying the
DMA controller supports format A, B, C and the DAI controller supports
format B, C, D and then just do the intersection of both. E.g. the DAI
controller probably does not care whether the samples are packed or not if
it only sees one sample at a time, while

- Lars

Re: [PATCH 06/19] ASoC: fsl: Don't set unused struct snd_pcm_hardware fields

[Mark Brown]

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

On Fri, Dec 20, 2013 at 03:04:08PM +0100, Lars-Peter Clausen wrote:
> On 12/20/2013 03:25 PM, Timur Tabi wrote:

> > Is this new?  There are formats that the codec and the SSI support that the
> > DMA controller does NOT support, like packed 24-bit samples.  How do we
> > ensure that we never get those?

> No, this is how it has always been. If there are restrictions imposed by the
> DMA controller we need to add support for expressing this inside the ASoC
> framework. But I think it will probably be more complex than just saying the
> DMA controller supports format A, B, C and the DAI controller supports
> format B, C, D and then just do the intersection of both. E.g. the DAI
> controller probably does not care whether the samples are packed or not if
> it only sees one sample at a time, while

The most common pattern I've seen is that the DAIs expect to see whole
samples at a time get written into their FIFOs since the FIFOs tend to
be stored in samples rather than bytes.  With that pattern it'd be a bit
cleaner to have them advertise sample sizes and transfer sizes and then
have the core work out that if you can for example do 24 bit samples
with four byte transfers and have a DMA controller that needs a 1:1
mapping between data read and written then we can't do packed format.

I think in general we want to be moving the DAIs to a sample size based
interface and then mapping that onto the DMA controller when we connect
the DMA and CPU DAIs.  This would help with clarity on the CODEC side as
well.

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

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

Re: [PATCH 06/19] ASoC: fsl: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

On 12/20/2013 07:27 PM, Mark Brown wrote:
> On Fri, Dec 20, 2013 at 03:04:08PM +0100, Lars-Peter Clausen wrote:
>> On 12/20/2013 03:25 PM, Timur Tabi wrote:
> 
>>> Is this new?  There are formats that the codec and the SSI support that the
>>> DMA controller does NOT support, like packed 24-bit samples.  How do we
>>> ensure that we never get those?
> 
>> No, this is how it has always been. If there are restrictions imposed by the
>> DMA controller we need to add support for expressing this inside the ASoC
>> framework. But I think it will probably be more complex than just saying the
>> DMA controller supports format A, B, C and the DAI controller supports
>> format B, C, D and then just do the intersection of both. E.g. the DAI
>> controller probably does not care whether the samples are packed or not if
>> it only sees one sample at a time, while
> 
> The most common pattern I've seen is that the DAIs expect to see whole
> samples at a time get written into their FIFOs since the FIFOs tend to
> be stored in samples rather than bytes. With that pattern it'd be a bit
> cleaner to have them advertise sample sizes and transfer sizes and then
> have the core work out that if you can for example do 24 bit samples
> with four byte transfers and have a DMA controller that needs a 1:1
> mapping between data read and written then we can't do packed format.
> 

Yep. The other one I've seen is where the audio controller expects the DMA
to pack samples, that are smaller than the bus width, to one bus width word.
E.g. if the bus width is 32 and the sample width is 16 the DMA controller is
supposed to write 2 samples at once.

And then there might be different variations of the first one. Some
controllers might expect narrow writes/reads while others might expect full
bus width access with the upper bytes padded/discarded. But I think most
controllers are fine with both.

The audio controller should probably advertise what sample sizes it supports
and how it expects them to be written/read. And the DMA controller then
based on that list needs to figure out what kind of in memory
representations of the audio data it can support.

> I think in general we want to be moving the DAIs to a sample size based
> interface and then mapping that onto the DMA controller when we connect
> the DMA and CPU DAIs.  This would help with clarity on the CODEC side as
> well.
> 

Yep, something similar should probably be done for format negotiation
between DAI and CODEC.

[PATCH 07/19] ASoC: intel: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Vinod Koul <vinod.koul@linux.intel.com>
---
 sound/soc/intel/sst_platform.c | 10 ----------
 sound/soc/intel/sst_platform.h |  4 ----
 2 files changed, 14 deletions(-)

diff --git a/sound/soc/intel/sst_platform.c b/sound/soc/intel/sst_platform.c
index b6b5eb6..f465a81 100644
--- a/sound/soc/intel/sst_platform.c
+++ b/sound/soc/intel/sst_platform.c
@@ -89,16 +89,6 @@ static struct snd_pcm_hardware sst_platform_pcm_hw = {
 			SNDRV_PCM_INFO_MMAP_VALID |
 			SNDRV_PCM_INFO_BLOCK_TRANSFER |
 			SNDRV_PCM_INFO_SYNC_START),
-	.formats = (SNDRV_PCM_FMTBIT_S16 | SNDRV_PCM_FMTBIT_U16 |
-			SNDRV_PCM_FMTBIT_S24 | SNDRV_PCM_FMTBIT_U24 |
-			SNDRV_PCM_FMTBIT_S32 | SNDRV_PCM_FMTBIT_U32),
-	.rates = (SNDRV_PCM_RATE_8000|
-			SNDRV_PCM_RATE_44100 |
-			SNDRV_PCM_RATE_48000),
-	.rate_min = SST_MIN_RATE,
-	.rate_max = SST_MAX_RATE,
-	.channels_min =	SST_MIN_CHANNEL,
-	.channels_max =	SST_MAX_CHANNEL,
 	.buffer_bytes_max = SST_MAX_BUFFER,
 	.period_bytes_min = SST_MIN_PERIOD_BYTES,
 	.period_bytes_max = SST_MAX_PERIOD_BYTES,
diff --git a/sound/soc/intel/sst_platform.h b/sound/soc/intel/sst_platform.h
index cacc906..bee64fb 100644
--- a/sound/soc/intel/sst_platform.h
+++ b/sound/soc/intel/sst_platform.h
@@ -33,10 +33,6 @@
 #define SST_STEREO		2
 #define SST_MAX_CAP		5
 
-#define SST_MIN_RATE		8000
-#define SST_MAX_RATE		48000
-#define SST_MIN_CHANNEL		1
-#define SST_MAX_CHANNEL		5
 #define SST_MAX_BUFFER		(800*1024)
 #define SST_MIN_BUFFER		(800*1024)
 #define SST_MIN_PERIOD_BYTES	32
-- 
1.8.0

[PATCH 08/19] ASoC: kirkwood: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Jean-Francois Moine <moinejf@free.fr>
---
 sound/soc/kirkwood/kirkwood-dma.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/sound/soc/kirkwood/kirkwood-dma.c b/sound/soc/kirkwood/kirkwood-dma.c
index 4af1936..aac22fc 100644
--- a/sound/soc/kirkwood/kirkwood-dma.c
+++ b/sound/soc/kirkwood/kirkwood-dma.c
@@ -21,16 +21,6 @@
 #include <sound/soc.h>
 #include "kirkwood.h"
 
-#define KIRKWOOD_RATES \
-	(SNDRV_PCM_RATE_8000_192000 |		\
-	 SNDRV_PCM_RATE_CONTINUOUS |		\
-	 SNDRV_PCM_RATE_KNOT)
-
-#define KIRKWOOD_FORMATS \
-	(SNDRV_PCM_FMTBIT_S16_LE | \
-	 SNDRV_PCM_FMTBIT_S24_LE | \
-	 SNDRV_PCM_FMTBIT_S32_LE)
-
 static struct kirkwood_dma_data *kirkwood_priv(struct snd_pcm_substream *subs)
 {
 	struct snd_soc_pcm_runtime *soc_runtime = subs->private_data;
@@ -43,12 +33,6 @@ static struct snd_pcm_hardware kirkwood_dma_snd_hw = {
 		 SNDRV_PCM_INFO_MMAP_VALID |
 		 SNDRV_PCM_INFO_BLOCK_TRANSFER |
 		 SNDRV_PCM_INFO_PAUSE),
-	.formats		= KIRKWOOD_FORMATS,
-	.rates			= KIRKWOOD_RATES,
-	.rate_min		= 8000,
-	.rate_max		= 384000,
-	.channels_min		= 1,
-	.channels_max		= 8,
 	.buffer_bytes_max	= KIRKWOOD_SND_MAX_BUFFER_BYTES,
 	.period_bytes_min	= KIRKWOOD_SND_MIN_PERIOD_BYTES,
 	.period_bytes_max	= KIRKWOOD_SND_MAX_PERIOD_BYTES,
-- 
1.8.0

Re: [PATCH 08/19] ASoC: kirkwood: Don't set unused struct snd_pcm_hardware fields

[Jean-Francois Moine]

On Fri, 20 Dec 2013 14:20:14 +0100
Lars-Peter Clausen <lars@metafoo.de> wrote:

> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Jean-Francois Moine <moinejf@free.fr>
> ---
>  sound/soc/kirkwood/kirkwood-dma.c | 16 ----------------
>  1 file changed, 16 deletions(-)
> 
> diff --git a/sound/soc/kirkwood/kirkwood-dma.c b/sound/soc/kirkwood/kirkwood-dma.c
> index 4af1936..aac22fc 100644
> --- a/sound/soc/kirkwood/kirkwood-dma.c
> +++ b/sound/soc/kirkwood/kirkwood-dma.c
	[snip]
> @@ -43,12 +33,6 @@ static struct snd_pcm_hardware kirkwood_dma_snd_hw = {
>  		 SNDRV_PCM_INFO_MMAP_VALID |
>  		 SNDRV_PCM_INFO_BLOCK_TRANSFER |
>  		 SNDRV_PCM_INFO_PAUSE),
> -	.formats		= KIRKWOOD_FORMATS,
> -	.rates			= KIRKWOOD_RATES,
> -	.rate_min		= 8000,
> -	.rate_max		= 384000,
> -	.channels_min		= 1,
> -	.channels_max		= 8,
>  	.buffer_bytes_max	= KIRKWOOD_SND_MAX_BUFFER_BYTES,
>  	.period_bytes_min	= KIRKWOOD_SND_MIN_PERIOD_BYTES,
>  	.period_bytes_max	= KIRKWOOD_SND_MAX_PERIOD_BYTES,

Lars,

You removed too many things. The 'formats' field is mandatory.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 08/19] ASoC: kirkwood: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

On 12/20/2013 07:05 PM, Jean-Francois Moine wrote:
> On Fri, 20 Dec 2013 14:20:14 +0100
> Lars-Peter Clausen <lars@metafoo.de> wrote:
> 
>> The ASoC core assumes that the PCM component of the ASoC card transparently
>> moves data around and does not impose any restrictions on the memory layout or
>> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
>> the PCM driver that are related to this. Setting these fields in the PCM driver
>> might suggest otherwise though, so rather not set them.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> Cc: Jean-Francois Moine <moinejf@free.fr>
>> ---
>>  sound/soc/kirkwood/kirkwood-dma.c | 16 ----------------
>>  1 file changed, 16 deletions(-)
>>
>> diff --git a/sound/soc/kirkwood/kirkwood-dma.c b/sound/soc/kirkwood/kirkwood-dma.c
>> index 4af1936..aac22fc 100644
>> --- a/sound/soc/kirkwood/kirkwood-dma.c
>> +++ b/sound/soc/kirkwood/kirkwood-dma.c
> 	[snip]
>> @@ -43,12 +33,6 @@ static struct snd_pcm_hardware kirkwood_dma_snd_hw = {
>>  		 SNDRV_PCM_INFO_MMAP_VALID |
>>  		 SNDRV_PCM_INFO_BLOCK_TRANSFER |
>>  		 SNDRV_PCM_INFO_PAUSE),
>> -	.formats		= KIRKWOOD_FORMATS,
>> -	.rates			= KIRKWOOD_RATES,
>> -	.rate_min		= 8000,
>> -	.rate_max		= 384000,
>> -	.channels_min		= 1,
>> -	.channels_max		= 8,
>>  	.buffer_bytes_max	= KIRKWOOD_SND_MAX_BUFFER_BYTES,
>>  	.period_bytes_min	= KIRKWOOD_SND_MIN_PERIOD_BYTES,
>>  	.period_bytes_max	= KIRKWOOD_SND_MAX_PERIOD_BYTES,
> 
> Lars,
> 
> You removed too many things. The 'formats' field is mandatory.

No it is not. While snd_soc_set_runtime_hwparams() uses it it is later
overwritten again in soc_pcm_init_runtime_hw().

- Lars

Re: [PATCH 08/19] ASoC: kirkwood: Don't set unused struct snd_pcm_hardware fields

[Jean-Francois Moine]

On Fri, 20 Dec 2013 18:18:49 +0100
Lars-Peter Clausen <lars@metafoo.de> wrote:

> On 12/20/2013 07:05 PM, Jean-Francois Moine wrote:
> > On Fri, 20 Dec 2013 14:20:14 +0100
> > Lars-Peter Clausen <lars@metafoo.de> wrote:
> > 
> >> The ASoC core assumes that the PCM component of the ASoC card transparently
> >> moves data around and does not impose any restrictions on the memory layout or
> >> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> >> the PCM driver that are related to this. Setting these fields in the PCM driver
> >> might suggest otherwise though, so rather not set them.
> >>
> >> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> >> Cc: Jean-Francois Moine <moinejf@free.fr>
> >> ---
> >>  sound/soc/kirkwood/kirkwood-dma.c | 16 ----------------
> >>  1 file changed, 16 deletions(-)
> >>
> >> diff --git a/sound/soc/kirkwood/kirkwood-dma.c b/sound/soc/kirkwood/kirkwood-dma.c
> >> index 4af1936..aac22fc 100644
> >> --- a/sound/soc/kirkwood/kirkwood-dma.c
> >> +++ b/sound/soc/kirkwood/kirkwood-dma.c
> > 	[snip]
> >> @@ -43,12 +33,6 @@ static struct snd_pcm_hardware kirkwood_dma_snd_hw = {
> >>  		 SNDRV_PCM_INFO_MMAP_VALID |
> >>  		 SNDRV_PCM_INFO_BLOCK_TRANSFER |
> >>  		 SNDRV_PCM_INFO_PAUSE),
> >> -	.formats		= KIRKWOOD_FORMATS,
> >> -	.rates			= KIRKWOOD_RATES,
> >> -	.rate_min		= 8000,
> >> -	.rate_max		= 384000,
> >> -	.channels_min		= 1,
> >> -	.channels_max		= 8,
> >>  	.buffer_bytes_max	= KIRKWOOD_SND_MAX_BUFFER_BYTES,
> >>  	.period_bytes_min	= KIRKWOOD_SND_MIN_PERIOD_BYTES,
> >>  	.period_bytes_max	= KIRKWOOD_SND_MAX_PERIOD_BYTES,
> > 
> > Lars,
> > 
> > You removed too many things. The 'formats' field is mandatory.
> 
> No it is not. While snd_soc_set_runtime_hwparams() uses it it is later
> overwritten again in soc_pcm_init_runtime_hw().

I have a DPCM system and soc_pcm_init_runtime_hw() is not called
(either 'dynamic' or 'no_pcm' is set in the DAI links).

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 08/19] ASoC: kirkwood: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

On 12/20/2013 08:13 PM, Jean-Francois Moine wrote:
> On Fri, 20 Dec 2013 18:18:49 +0100
> Lars-Peter Clausen <lars@metafoo.de> wrote:
> 
>> On 12/20/2013 07:05 PM, Jean-Francois Moine wrote:
>>> On Fri, 20 Dec 2013 14:20:14 +0100
>>> Lars-Peter Clausen <lars@metafoo.de> wrote:
>>>
>>>> The ASoC core assumes that the PCM component of the ASoC card transparently
>>>> moves data around and does not impose any restrictions on the memory layout or
>>>> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
>>>> the PCM driver that are related to this. Setting these fields in the PCM driver
>>>> might suggest otherwise though, so rather not set them.
>>>>
>>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>>>> Cc: Jean-Francois Moine <moinejf@free.fr>
>>>> ---
>>>>  sound/soc/kirkwood/kirkwood-dma.c | 16 ----------------
>>>>  1 file changed, 16 deletions(-)
>>>>
>>>> diff --git a/sound/soc/kirkwood/kirkwood-dma.c b/sound/soc/kirkwood/kirkwood-dma.c
>>>> index 4af1936..aac22fc 100644
>>>> --- a/sound/soc/kirkwood/kirkwood-dma.c
>>>> +++ b/sound/soc/kirkwood/kirkwood-dma.c
>>> 	[snip]
>>>> @@ -43,12 +33,6 @@ static struct snd_pcm_hardware kirkwood_dma_snd_hw = {
>>>>  		 SNDRV_PCM_INFO_MMAP_VALID |
>>>>  		 SNDRV_PCM_INFO_BLOCK_TRANSFER |
>>>>  		 SNDRV_PCM_INFO_PAUSE),
>>>> -	.formats		= KIRKWOOD_FORMATS,
>>>> -	.rates			= KIRKWOOD_RATES,
>>>> -	.rate_min		= 8000,
>>>> -	.rate_max		= 384000,
>>>> -	.channels_min		= 1,
>>>> -	.channels_max		= 8,
>>>>  	.buffer_bytes_max	= KIRKWOOD_SND_MAX_BUFFER_BYTES,
>>>>  	.period_bytes_min	= KIRKWOOD_SND_MIN_PERIOD_BYTES,
>>>>  	.period_bytes_max	= KIRKWOOD_SND_MAX_PERIOD_BYTES,
>>>
>>> Lars,
>>>
>>> You removed too many things. The 'formats' field is mandatory.
>>
>> No it is not. While snd_soc_set_runtime_hwparams() uses it it is later
>> overwritten again in soc_pcm_init_runtime_hw().
> 
> I have a DPCM system and soc_pcm_init_runtime_hw() is not called
> (either 'dynamic' or 'no_pcm' is set in the DAI links).
> 

Ok, I see dpcm_set_fe_runtime() does things slightly different.

[PATCH 09/19] ASoC: mxs: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
---
 sound/soc/mxs/mxs-pcm.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/sound/soc/mxs/mxs-pcm.c b/sound/soc/mxs/mxs-pcm.c
index 04a6b0d..2e0863a 100644
--- a/sound/soc/mxs/mxs-pcm.c
+++ b/sound/soc/mxs/mxs-pcm.c
@@ -36,11 +36,6 @@ static const struct snd_pcm_hardware snd_mxs_hardware = {
 				  SNDRV_PCM_INFO_RESUME |
 				  SNDRV_PCM_INFO_INTERLEAVED |
 				  SNDRV_PCM_INFO_HALF_DUPLEX,
-	.formats		= SNDRV_PCM_FMTBIT_S16_LE |
-				  SNDRV_PCM_FMTBIT_S20_3LE |
-				  SNDRV_PCM_FMTBIT_S24_LE,
-	.channels_min		= 2,
-	.channels_max		= 2,
 	.period_bytes_min	= 32,
 	.period_bytes_max	= 8192,
 	.periods_min		= 1,
-- 
1.8.0

[PATCH 10/19] ASoC: nuc900: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Wan ZongShun <mcuos.com@gmail.com>
---
 sound/soc/nuc900/nuc900-pcm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/nuc900/nuc900-pcm.c b/sound/soc/nuc900/nuc900-pcm.c
index f588ee4..f434ed7 100644
--- a/sound/soc/nuc900/nuc900-pcm.c
+++ b/sound/soc/nuc900/nuc900-pcm.c
@@ -32,9 +32,6 @@ static const struct snd_pcm_hardware nuc900_pcm_hardware = {
 					SNDRV_PCM_INFO_MMAP_VALID |
 					SNDRV_PCM_INFO_PAUSE |
 					SNDRV_PCM_INFO_RESUME,
-	.formats		= SNDRV_PCM_FMTBIT_S16_LE,
-	.channels_min		= 1,
-	.channels_max		= 2,
 	.buffer_bytes_max	= 4*1024,
 	.period_bytes_min	= 1*1024,
 	.period_bytes_max	= 4*1024,
-- 
1.8.0

[PATCH 11/19] ASoC: omap: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
Cc: Jarkko Nikula <jarkko.nikula@bitmer.com>
---
 sound/soc/omap/omap-pcm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c
index b8fa986..07b8b7b 100644
--- a/sound/soc/omap/omap-pcm.c
+++ b/sound/soc/omap/omap-pcm.c
@@ -45,8 +45,6 @@ static const struct snd_pcm_hardware omap_pcm_hardware = {
 				  SNDRV_PCM_INFO_PAUSE |
 				  SNDRV_PCM_INFO_RESUME |
 				  SNDRV_PCM_INFO_NO_PERIOD_WAKEUP,
-	.formats		= SNDRV_PCM_FMTBIT_S16_LE |
-				  SNDRV_PCM_FMTBIT_S32_LE,
 	.period_bytes_min	= 32,
 	.period_bytes_max	= 64 * 1024,
 	.periods_min		= 2,
-- 
1.8.0

Re: [PATCH 11/19] ASoC: omap: Don't set unused struct snd_pcm_hardware fields

[Jarkko Nikula]

On Fri, 20 Dec 2013 14:20:17 +0100
Lars-Peter Clausen <lars@metafoo.de> wrote:

> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Cc: Jarkko Nikula <jarkko.nikula@bitmer.com>
> ---
>  sound/soc/omap/omap-pcm.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c
> index b8fa986..07b8b7b 100644
> --- a/sound/soc/omap/omap-pcm.c
> +++ b/sound/soc/omap/omap-pcm.c
> @@ -45,8 +45,6 @@ static const struct snd_pcm_hardware omap_pcm_hardware = {
>  				  SNDRV_PCM_INFO_PAUSE |
>  				  SNDRV_PCM_INFO_RESUME |
>  				  SNDRV_PCM_INFO_NO_PERIOD_WAKEUP,
> -	.formats		= SNDRV_PCM_FMTBIT_S16_LE |
> -				  SNDRV_PCM_FMTBIT_S32_LE,
Ok, I see it's overwritten here:

soc_pcm_init_runtime_hw()
hw->formats = codec_stream->formats & cpu_stream->formats;

Acked-by: Jarkko Nikula <jarkko.nikula@bitmer.com>

Re: [PATCH 11/19] ASoC: omap: Don't set unused struct snd_pcm_hardware fields

[Peter Ujfalusi]

On 12/20/2013 03:20 PM, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Cc: Jarkko Nikula <jarkko.nikula@bitmer.com>
> ---
>  sound/soc/omap/omap-pcm.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c
> index b8fa986..07b8b7b 100644
> --- a/sound/soc/omap/omap-pcm.c
> +++ b/sound/soc/omap/omap-pcm.c
> @@ -45,8 +45,6 @@ static const struct snd_pcm_hardware omap_pcm_hardware = {
>  				  SNDRV_PCM_INFO_PAUSE |
>  				  SNDRV_PCM_INFO_RESUME |
>  				  SNDRV_PCM_INFO_NO_PERIOD_WAKEUP,
> -	.formats		= SNDRV_PCM_FMTBIT_S16_LE |
> -				  SNDRV_PCM_FMTBIT_S32_LE,
>  	.period_bytes_min	= 32,
>  	.period_bytes_max	= 64 * 1024,
>  	.periods_min		= 2,

Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

Re: [PATCH 11/19] ASoC: omap: Don't set unused struct snd_pcm_hardware fields

[Mark Brown]

On Fri, Dec 20, 2013 at 02:20:17PM +0100, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

Applied, thanks.

[PATCH 12/19] ASoC: mmp: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Qiao Zhou <zhouqiao@marvell.com>
---
 sound/soc/pxa/mmp-pcm.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/sound/soc/pxa/mmp-pcm.c b/sound/soc/pxa/mmp-pcm.c
index 682ee52..5e8d813 100644
--- a/sound/soc/pxa/mmp-pcm.c
+++ b/sound/soc/pxa/mmp-pcm.c
@@ -36,14 +36,9 @@ struct mmp_dma_data {
 		SNDRV_PCM_INFO_PAUSE |		\
 		SNDRV_PCM_INFO_RESUME)
 
-#define MMP_PCM_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
-			 SNDRV_PCM_FMTBIT_S24_LE | \
-			 SNDRV_PCM_FMTBIT_S32_LE)
-
 static struct snd_pcm_hardware mmp_pcm_hardware[] = {
 	{
 		.info			= MMP_PCM_INFO,
-		.formats		= MMP_PCM_FORMATS,
 		.period_bytes_min	= 1024,
 		.period_bytes_max	= 2048,
 		.periods_min		= 2,
@@ -53,7 +48,6 @@ static struct snd_pcm_hardware mmp_pcm_hardware[] = {
 	},
 	{
 		.info			= MMP_PCM_INFO,
-		.formats		= MMP_PCM_FORMATS,
 		.period_bytes_min	= 1024,
 		.period_bytes_max	= 2048,
 		.periods_min		= 2,
-- 
1.8.0

Re: [PATCH 12/19] ASoC: mmp: Don't set unused struct snd_pcm_hardware fields

[Mark Brown]

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

On Fri, Dec 20, 2013 at 02:20:18PM +0100, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

Applied, thanks.

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

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

[PATCH 13/19] ASoC: s6000: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Daniel Glöckner <daniel-gl@gmx.net>
---
 sound/soc/s6000/s6000-pcm.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/sound/soc/s6000/s6000-pcm.c b/sound/soc/s6000/s6000-pcm.c
index d219880..fb8461e 100644
--- a/sound/soc/s6000/s6000-pcm.c
+++ b/sound/soc/s6000/s6000-pcm.c
@@ -33,13 +33,6 @@ static struct snd_pcm_hardware s6000_pcm_hardware = {
 	.info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
 		 SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
 		 SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_JOINT_DUPLEX),
-	.formats = (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S32_LE),
-	.rates = (SNDRV_PCM_RATE_CONTINUOUS | SNDRV_PCM_RATE_5512 | \
-		  SNDRV_PCM_RATE_8000_192000),
-	.rate_min = 0,
-	.rate_max = 1562500,
-	.channels_min = 2,
-	.channels_max = 8,
 	.buffer_bytes_max = 0x7ffffff0,
 	.period_bytes_min = 16,
 	.period_bytes_max = 0xfffff0,
-- 
1.8.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 13/19] ASoC: s6000: Don't set unused struct snd_pcm_hardware fields

[Daniel Glöckner]

On Fri, Dec 20, 2013 at 02:20:19PM +0100, Lars-Peter Clausen wrote:
> -	.formats = (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S32_LE),

I'm still interested in the outcome of the DPCM issue mentioned in the
Kirkwood thread, but as the only known board using this driver is not
affected:

Acked-by: Daniel Glöckner <daniel-gl@gmx.net>

Re: [PATCH 13/19] ASoC: s6000: Don't set unused struct snd_pcm_hardware fields

[Mark Brown]

On Fri, Dec 20, 2013 at 02:20:19PM +0100, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

Applied, thanks.

[PATCH 14/19] ASoC: samsung: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Sangbeom Kim <sbkim73@samsung.com>
---
 sound/soc/samsung/dma.c  | 6 ------
 sound/soc/samsung/idma.c | 8 --------
 2 files changed, 14 deletions(-)

diff --git a/sound/soc/samsung/dma.c b/sound/soc/samsung/dma.c
index ee23194..dc09b71 100644
--- a/sound/soc/samsung/dma.c
+++ b/sound/soc/samsung/dma.c
@@ -35,12 +35,6 @@ static const struct snd_pcm_hardware dma_hardware = {
 				    SNDRV_PCM_INFO_BLOCK_TRANSFER |
 				    SNDRV_PCM_INFO_MMAP |
 				    SNDRV_PCM_INFO_MMAP_VALID,
-	.formats		= SNDRV_PCM_FMTBIT_S16_LE |
-				    SNDRV_PCM_FMTBIT_U16_LE |
-				    SNDRV_PCM_FMTBIT_U8 |
-				    SNDRV_PCM_FMTBIT_S8,
-	.channels_min		= 2,
-	.channels_max		= 2,
 	.buffer_bytes_max	= 128*1024,
 	.period_bytes_min	= PAGE_SIZE,
 	.period_bytes_max	= PAGE_SIZE*2,
diff --git a/sound/soc/samsung/idma.c b/sound/soc/samsung/idma.c
index e4f318f..3d5cf15 100644
--- a/sound/soc/samsung/idma.c
+++ b/sound/soc/samsung/idma.c
@@ -35,14 +35,6 @@ static const struct snd_pcm_hardware idma_hardware = {
 		    SNDRV_PCM_INFO_MMAP_VALID |
 		    SNDRV_PCM_INFO_PAUSE |
 		    SNDRV_PCM_INFO_RESUME,
-	.formats = SNDRV_PCM_FMTBIT_S16_LE |
-		    SNDRV_PCM_FMTBIT_U16_LE |
-		    SNDRV_PCM_FMTBIT_S24_LE |
-		    SNDRV_PCM_FMTBIT_U24_LE |
-		    SNDRV_PCM_FMTBIT_U8 |
-		    SNDRV_PCM_FMTBIT_S8,
-	.channels_min = 2,
-	.channels_max = 2,
 	.buffer_bytes_max = MAX_IDMA_BUFFER,
 	.period_bytes_min = 128,
 	.period_bytes_max = MAX_IDMA_PERIOD,
-- 
1.8.0

Re: [PATCH 14/19] ASoC: samsung: Don't set unused struct snd_pcm_hardware fields

[Mark Brown]

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

On Fri, Dec 20, 2013 at 02:20:20PM +0100, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

Applied, thanks.

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

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

[PATCH 15/19] ASoC: sh: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/dma-sh7760.c | 17 -----------------
 sound/soc/sh/fsi.c        |  6 ------
 sound/soc/sh/rcar/core.c  |  6 ------
 3 files changed, 29 deletions(-)

diff --git a/sound/soc/sh/dma-sh7760.c b/sound/soc/sh/dma-sh7760.c
index 1a8b03e..c85f8eb 100644
--- a/sound/soc/sh/dma-sh7760.c
+++ b/sound/soc/sh/dma-sh7760.c
@@ -89,29 +89,12 @@ struct camelot_pcm {
 #define DMABRG_PREALLOC_BUFFER		32 * 1024
 #define DMABRG_PREALLOC_BUFFER_MAX	32 * 1024
 
-/* support everything the SSI supports */
-#define DMABRG_RATES	\
-	SNDRV_PCM_RATE_8000_192000
-
-#define DMABRG_FMTS	\
-	(SNDRV_PCM_FMTBIT_S8      | SNDRV_PCM_FMTBIT_U8      |	\
-	 SNDRV_PCM_FMTBIT_S16_LE  | SNDRV_PCM_FMTBIT_U16_LE  |	\
-	 SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_U20_3LE |	\
-	 SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_U24_3LE |	\
-	 SNDRV_PCM_FMTBIT_S32_LE  | SNDRV_PCM_FMTBIT_U32_LE)
-
 static struct snd_pcm_hardware camelot_pcm_hardware = {
 	.info = (SNDRV_PCM_INFO_MMAP |
 		SNDRV_PCM_INFO_INTERLEAVED |
 		SNDRV_PCM_INFO_BLOCK_TRANSFER |
 		SNDRV_PCM_INFO_MMAP_VALID |
 		SNDRV_PCM_INFO_BATCH),
-	.formats =	DMABRG_FMTS,
-	.rates =	DMABRG_RATES,
-	.rate_min =		8000,
-	.rate_max =		192000,
-	.channels_min =		2,
-	.channels_max =		8,		/* max of the SSI */
 	.buffer_bytes_max =	DMABRG_PERIOD_MAX,
 	.period_bytes_min =	DMABRG_PERIOD_MIN,
 	.period_bytes_max =	DMABRG_PERIOD_MAX / 2,
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 6101055..1967f44 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -1787,12 +1787,6 @@ static struct snd_pcm_hardware fsi_pcm_hardware = {
 			SNDRV_PCM_INFO_MMAP		|
 			SNDRV_PCM_INFO_MMAP_VALID	|
 			SNDRV_PCM_INFO_PAUSE,
-	.formats		= FSI_FMTS,
-	.rates			= FSI_RATES,
-	.rate_min		= 8000,
-	.rate_max		= 192000,
-	.channels_min		= 2,
-	.channels_max		= 2,
 	.buffer_bytes_max	= 64 * 1024,
 	.period_bytes_min	= 32,
 	.period_bytes_max	= 8192,
diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index b3653d3..743de5e 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -628,12 +628,6 @@ static struct snd_pcm_hardware rsnd_pcm_hardware = {
 			SNDRV_PCM_INFO_MMAP		|
 			SNDRV_PCM_INFO_MMAP_VALID	|
 			SNDRV_PCM_INFO_PAUSE,
-	.formats		= RSND_FMTS,
-	.rates			= RSND_RATES,
-	.rate_min		= 8000,
-	.rate_max		= 192000,
-	.channels_min		= 2,
-	.channels_max		= 2,
 	.buffer_bytes_max	= 64 * 1024,
 	.period_bytes_min	= 32,
 	.period_bytes_max	= 8192,
-- 
1.8.0

[PATCH 16/19] ASoC: tegra: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Stephen Warren <swarren@wwwdotorg.org>
---
 sound/soc/tegra/tegra_pcm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/tegra/tegra_pcm.c b/sound/soc/tegra/tegra_pcm.c
index 7b2d23b..c09ffd1 100644
--- a/sound/soc/tegra/tegra_pcm.c
+++ b/sound/soc/tegra/tegra_pcm.c
@@ -42,9 +42,6 @@ static const struct snd_pcm_hardware tegra_pcm_hardware = {
 	.info			= SNDRV_PCM_INFO_MMAP |
 				  SNDRV_PCM_INFO_MMAP_VALID |
 				  SNDRV_PCM_INFO_INTERLEAVED,
-	.formats		= SNDRV_PCM_FMTBIT_S16_LE,
-	.channels_min		= 2,
-	.channels_max		= 2,
 	.period_bytes_min	= 1024,
 	.period_bytes_max	= PAGE_SIZE,
 	.periods_min		= 2,
-- 
1.8.0

Re: [PATCH 16/19] ASoC: tegra: Don't set unused struct snd_pcm_hardware fields

[Stephen Warren]

On 12/20/2013 06:20 AM, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

I suppose that makes sense; the data format is determined by the FIFO in
the DAI/controller, not the DMA engine that's pushing the data there
(unless DMA can do data conversions, though I think our HW can't?).

Tested-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Stephen Warren <swarren@nvidia.com>

Re: [PATCH 16/19] ASoC: tegra: Don't set unused struct snd_pcm_hardware fields

[Mark Brown]

On Fri, Dec 20, 2013 at 02:20:22PM +0100, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

Applied, thanks.

[PATCH 17/19] ASoC: txx9: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/txx9/txx9aclc.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/sound/soc/txx9/txx9aclc.c b/sound/soc/txx9/txx9aclc.c
index fbd077f..f0829de 100644
--- a/sound/soc/txx9/txx9aclc.c
+++ b/sound/soc/txx9/txx9aclc.c
@@ -40,11 +40,6 @@ static const struct snd_pcm_hardware txx9aclc_pcm_hardware = {
 	.info		  = SNDRV_PCM_INFO_INTERLEAVED |
 			    SNDRV_PCM_INFO_BATCH |
 			    SNDRV_PCM_INFO_PAUSE,
-#ifdef __BIG_ENDIAN
-	.formats	  = SNDRV_PCM_FMTBIT_S16_BE,
-#else
-	.formats	  = SNDRV_PCM_FMTBIT_S16_LE,
-#endif
 	.period_bytes_min = 1024,
 	.period_bytes_max = 8 * 1024,
 	.periods_min	  = 2,
-- 
1.8.0

Re: [PATCH 17/19] ASoC: txx9: Don't set unused struct snd_pcm_hardware fields

[Mark Brown]

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

On Fri, Dec 20, 2013 at 02:20:23PM +0100, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

Applied, thanks.

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

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

[PATCH 18/19] ASoC: ux500: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Cc: Lee Jones <lee.jones@linaro.org>
---
 sound/soc/ux500/ux500_pcm.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c
index ce554de..fa71640 100644
--- a/sound/soc/ux500/ux500_pcm.c
+++ b/sound/soc/ux500/ux500_pcm.c
@@ -28,12 +28,6 @@
 #include "ux500_msp_i2s.h"
 #include "ux500_pcm.h"
 
-#define UX500_PLATFORM_MIN_RATE 8000
-#define UX500_PLATFORM_MAX_RATE 48000
-
-#define UX500_PLATFORM_MIN_CHANNELS 1
-#define UX500_PLATFORM_MAX_CHANNELS 8
-
 #define UX500_PLATFORM_PERIODS_BYTES_MIN	128
 #define UX500_PLATFORM_PERIODS_BYTES_MAX	(64 * PAGE_SIZE)
 #define UX500_PLATFORM_PERIODS_MIN		2
@@ -45,15 +39,6 @@ static const struct snd_pcm_hardware ux500_pcm_hw = {
 		SNDRV_PCM_INFO_MMAP |
 		SNDRV_PCM_INFO_RESUME |
 		SNDRV_PCM_INFO_PAUSE,
-	.formats = SNDRV_PCM_FMTBIT_S16_LE |
-		SNDRV_PCM_FMTBIT_U16_LE |
-		SNDRV_PCM_FMTBIT_S16_BE |
-		SNDRV_PCM_FMTBIT_U16_BE,
-	.rates = SNDRV_PCM_RATE_KNOT,
-	.rate_min = UX500_PLATFORM_MIN_RATE,
-	.rate_max = UX500_PLATFORM_MAX_RATE,
-	.channels_min = UX500_PLATFORM_MIN_CHANNELS,
-	.channels_max = UX500_PLATFORM_MAX_CHANNELS,
 	.buffer_bytes_max = UX500_PLATFORM_BUFFER_BYTES_MAX,
 	.period_bytes_min = UX500_PLATFORM_PERIODS_BYTES_MIN,
 	.period_bytes_max = UX500_PLATFORM_PERIODS_BYTES_MAX,
-- 
1.8.0

[PATCH 19/19] ASoC: soc-utils: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

The ASoC core assumes that the PCM component of the ASoC card transparently
moves data around and does not impose any restrictions on the memory layout or
the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
the PCM driver that are related to this. Setting these fields in the PCM driver
might suggest otherwise though, so rather not set them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/soc-utils.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/sound/soc/soc-utils.c b/sound/soc/soc-utils.c
index 5e63365..6ebdfd9 100644
--- a/sound/soc/soc-utils.c
+++ b/sound/soc/soc-utils.c
@@ -59,10 +59,6 @@ int snd_soc_params_to_bclk(struct snd_pcm_hw_params *params)
 EXPORT_SYMBOL_GPL(snd_soc_params_to_bclk);
 
 static const struct snd_pcm_hardware dummy_dma_hardware = {
-	.formats		= 0xffffffff,
-	.channels_min		= 1,
-	.channels_max		= UINT_MAX,
-
 	/* Random values to keep userspace happy when checking constraints */
 	.info			= SNDRV_PCM_INFO_INTERLEAVED |
 				  SNDRV_PCM_INFO_BLOCK_TRANSFER,
-- 
1.8.0

Re: [PATCH 19/19] ASoC: soc-utils: Don't set unused struct snd_pcm_hardware fields

[Mark Brown]

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

On Fri, Dec 20, 2013 at 02:20:25PM +0100, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

Applied, thanks.

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

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

Re: [PATCH 01/19] ASoC: atmel: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

On 12/20/2013 02:20 PM, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Bo Shen <voice.shen@atmel.com>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
>  sound/soc/atmel/atmel-pcm-dma.c | 1 -
>  sound/soc/atmel/atmel-pcm-pdc.c | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/sound/soc/atmel/atmel-pcm-dma.c b/sound/soc/atmel/atmel-pcm-dma.c
> index 06082e5..b79a2a8 100644
> --- a/sound/soc/atmel/atmel-pcm-dma.c
> +++ b/sound/soc/atmel/atmel-pcm-dma.c
> @@ -50,7 +50,6 @@ static const struct snd_pcm_hardware atmel_pcm_dma_hardware = {
>  				  SNDRV_PCM_INFO_INTERLEAVED |
>  				  SNDRV_PCM_INFO_RESUME |
>  				  SNDRV_PCM_INFO_PAUSE,
> -	.formats		= SNDRV_PCM_FMTBIT_S16_LE,

Atmel is one of the few platforms where the formats specified for the DMA
controller are more restrictive than the formats specified for the DAI. As
the restrictions specified by the PCM part have been ignored so far I'm
wondering if this is a real restriction and things have only worked by
chance or whether the restriction should be removed?

- Lars

Re: [PATCH 01/19] ASoC: atmel: Don't set unused struct snd_pcm_hardware fields

[Bo Shen]

Hi Lars,

On 01/05/2014 02:15 AM, Lars-Peter Clausen wrote:
> On 12/20/2013 02:20 PM, Lars-Peter Clausen wrote:
>> The ASoC core assumes that the PCM component of the ASoC card transparently
>> moves data around and does not impose any restrictions on the memory layout or
>> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
>> the PCM driver that are related to this. Setting these fields in the PCM driver
>> might suggest otherwise though, so rather not set them.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> Cc: Bo Shen <voice.shen@atmel.com>
>> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
>> ---
>>   sound/soc/atmel/atmel-pcm-dma.c | 1 -
>>   sound/soc/atmel/atmel-pcm-pdc.c | 1 -
>>   2 files changed, 2 deletions(-)
>>
>> diff --git a/sound/soc/atmel/atmel-pcm-dma.c b/sound/soc/atmel/atmel-pcm-dma.c
>> index 06082e5..b79a2a8 100644
>> --- a/sound/soc/atmel/atmel-pcm-dma.c
>> +++ b/sound/soc/atmel/atmel-pcm-dma.c
>> @@ -50,7 +50,6 @@ static const struct snd_pcm_hardware atmel_pcm_dma_hardware = {
>>   				  SNDRV_PCM_INFO_INTERLEAVED |
>>   				  SNDRV_PCM_INFO_RESUME |
>>   				  SNDRV_PCM_INFO_PAUSE,
>> -	.formats		= SNDRV_PCM_FMTBIT_S16_LE,
>
> Atmel is one of the few platforms where the formats specified for the DMA
> controller are more restrictive than the formats specified for the DAI. As
> the restrictions specified by the PCM part have been ignored so far I'm
> wondering if this is a real restriction and things have only worked by
> chance or whether the restriction should be removed?

This is not a real restriction for DMA, this can be removed. Only the 
SSC have the restriction, and we check it in DAI driver.

For this patch,
Acked-by: Bo Shen <voice.shen@atmel.com>

> - Lars
>

Best Regards,
Bo Shen

Re: [PATCH 01/19] ASoC: atmel: Don't set unused struct snd_pcm_hardware fields

[Mark Brown]

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

On Fri, Dec 20, 2013 at 02:20:07PM +0100, Lars-Peter Clausen wrote:
> The ASoC core assumes that the PCM component of the ASoC card transparently
> moves data around and does not impose any restrictions on the memory layout or
> the transfer speed. It ignores all fields from the snd_pcm_hardware struct for
> the PCM driver that are related to this. Setting these fields in the PCM driver
> might suggest otherwise though, so rather not set them.

Applied, thanks.

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

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

Re: [PATCH 08/19] ASoC: kirkwood: Don't set unused struct snd_pcm_hardware fields

[Jean-Francois Moine]

> On 12/20/2013 08:13 PM, Jean-Francois Moine wrote:
> > On Fri, 20 Dec 2013 18:18:49 +0100
> > Lars-Peter Clausen <lars@metafoo.de> wrote:
> >   
> >> On 12/20/2013 07:05 PM, Jean-Francois Moine wrote:  
> >>>> diff --git a/sound/soc/kirkwood/kirkwood-dma.c b/sound/soc/kirkwood/kirkwood-dma.c
> >>>> index 4af1936..aac22fc 100644
> >>>> --- a/sound/soc/kirkwood/kirkwood-dma.c
> >>>> +++ b/sound/soc/kirkwood/kirkwood-dma.c  
> >>> 	[snip]  
> >>>> @@ -43,12 +33,6 @@ static struct snd_pcm_hardware kirkwood_dma_snd_hw = {
> >>>>  		 SNDRV_PCM_INFO_MMAP_VALID |
> >>>>  		 SNDRV_PCM_INFO_BLOCK_TRANSFER |
> >>>>  		 SNDRV_PCM_INFO_PAUSE),
> >>>> -	.formats		= KIRKWOOD_FORMATS,
> >>>> -	.rates			= KIRKWOOD_RATES,
> >>>> -	.rate_min		= 8000,
> >>>> -	.rate_max		= 384000,
> >>>> -	.channels_min		= 1,
> >>>> -	.channels_max		= 8,
> >>>>  	.buffer_bytes_max	= KIRKWOOD_SND_MAX_BUFFER_BYTES,
> >>>>  	.period_bytes_min	= KIRKWOOD_SND_MIN_PERIOD_BYTES,
> >>>>  	.period_bytes_max	= KIRKWOOD_SND_MAX_PERIOD_BYTES,  
> >>>
> >>> Lars,
> >>>
> >>> You removed too many things. The 'formats' field is mandatory.  
> >>
> >> No it is not. While snd_soc_set_runtime_hwparams() uses it it is later
> >> overwritten again in soc_pcm_init_runtime_hw().  
> > 
> > I have a DPCM system and soc_pcm_init_runtime_hw() is not called
> > (either 'dynamic' or 'no_pcm' is set in the DAI links).
> 
> Ok, I see dpcm_set_fe_runtime() does things slightly different.

Well, I advanced a bit with DPCM, and I have a problem with this field.

In the driver, the front-end DAI is the audio controller and the
back-ends DAIs are the HDMI and SPDIF outputs. These back-end DAIs
have different rates and formats, as have the audio controller outputs
(I2S and SPDIF). So, I used intermediate DAIs which represent the audio
controller outputs, and they are described in the DAI links:

link 0 (FE): audio controller <-> dummy DAI
link 1 (BE): i2s audio controller output <-> HDMI output
link 2 (BE): spdif audio controller output <-> HDMI output
link 3 (BE): spdif audio controller output <-> SPDIF output

Without any patch in the core, the rates and formats are always the
rates and formats of the audio controller (FE). This is due to the
'goto dynamic' in soc_pcm_open(): as the back-ends are linked to real
DAIs, the rate and format constraints must be checked. So, as a
temporary patch, I replaced:

	if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm)
		goto dynamic;
by:
	if (rtd->dai_link->dynamic)
		goto dynamic;

(indeed, this will not work if the back-end is linked to the dummy DAI)

and, I get the correct rates and formats in the runtime hardware
parameters. But these values are lost:

- on DMA open (FE pcm open), when the driver calls
  snd_soc_set_runtime_hwparams() (loss of the formats), and

- on dpcm_set_fe_runtime() call (loss of the rates).

The first problem can be fixed in the audio controller by a hack,
saving /restoring the formats on calling snd_soc_set_runtime_hwparams(),
and the second problem is easily fixed moving dpcm_set_fe_runtime() at
the beginning of dpcm_fe_dai_startup(). Are these good solutions?

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 08/19] ASoC: kirkwood: Don't set unused struct snd_pcm_hardware fields

[Lars-Peter Clausen]

On 12/30/2013 08:42 PM, Jean-Francois Moine wrote:
>> On 12/20/2013 08:13 PM, Jean-Francois Moine wrote:
>>> On Fri, 20 Dec 2013 18:18:49 +0100
>>> Lars-Peter Clausen <lars@metafoo.de> wrote:
>>>   
>>>> On 12/20/2013 07:05 PM, Jean-Francois Moine wrote:  
>>>>>> diff --git a/sound/soc/kirkwood/kirkwood-dma.c b/sound/soc/kirkwood/kirkwood-dma.c
>>>>>> index 4af1936..aac22fc 100644
>>>>>> --- a/sound/soc/kirkwood/kirkwood-dma.c
>>>>>> +++ b/sound/soc/kirkwood/kirkwood-dma.c  
>>>>> 	[snip]  
>>>>>> @@ -43,12 +33,6 @@ static struct snd_pcm_hardware kirkwood_dma_snd_hw = {
>>>>>>  		 SNDRV_PCM_INFO_MMAP_VALID |
>>>>>>  		 SNDRV_PCM_INFO_BLOCK_TRANSFER |
>>>>>>  		 SNDRV_PCM_INFO_PAUSE),
>>>>>> -	.formats		= KIRKWOOD_FORMATS,
>>>>>> -	.rates			= KIRKWOOD_RATES,
>>>>>> -	.rate_min		= 8000,
>>>>>> -	.rate_max		= 384000,
>>>>>> -	.channels_min		= 1,
>>>>>> -	.channels_max		= 8,
>>>>>>  	.buffer_bytes_max	= KIRKWOOD_SND_MAX_BUFFER_BYTES,
>>>>>>  	.period_bytes_min	= KIRKWOOD_SND_MIN_PERIOD_BYTES,
>>>>>>  	.period_bytes_max	= KIRKWOOD_SND_MAX_PERIOD_BYTES,  
>>>>>
>>>>> Lars,
>>>>>
>>>>> You removed too many things. The 'formats' field is mandatory.  
>>>>
>>>> No it is not. While snd_soc_set_runtime_hwparams() uses it it is later
>>>> overwritten again in soc_pcm_init_runtime_hw().  
>>>
>>> I have a DPCM system and soc_pcm_init_runtime_hw() is not called
>>> (either 'dynamic' or 'no_pcm' is set in the DAI links).
>>
>> Ok, I see dpcm_set_fe_runtime() does things slightly different.
> 
> Well, I advanced a bit with DPCM, and I have a problem with this field.
> 
> In the driver, the front-end DAI is the audio controller and the
> back-ends DAIs are the HDMI and SPDIF outputs. These back-end DAIs
> have different rates and formats, as have the audio controller outputs
> (I2S and SPDIF). So, I used intermediate DAIs which represent the audio
> controller outputs, and they are described in the DAI links:
> 
> link 0 (FE): audio controller <-> dummy DAI
> link 1 (BE): i2s audio controller output <-> HDMI output
> link 2 (BE): spdif audio controller output <-> HDMI output
> link 3 (BE): spdif audio controller output <-> SPDIF output
> 
> Without any patch in the core, the rates and formats are always the
> rates and formats of the audio controller (FE). This is due to the
> 'goto dynamic' in soc_pcm_open(): as the back-ends are linked to real
> DAIs, the rate and format constraints must be checked. So, as a
> temporary patch, I replaced:
> 
> 	if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm)
> 		goto dynamic;
> by:
> 	if (rtd->dai_link->dynamic)
> 		goto dynamic;
> 
> (indeed, this will not work if the back-end is linked to the dummy DAI)
> 
> and, I get the correct rates and formats in the runtime hardware
> parameters. But these values are lost:
> 
> - on DMA open (FE pcm open), when the driver calls
>   snd_soc_set_runtime_hwparams() (loss of the formats), and
> 
> - on dpcm_set_fe_runtime() call (loss of the rates).
> 
> The first problem can be fixed in the audio controller by a hack,
> saving /restoring the formats on calling snd_soc_set_runtime_hwparams(),
> and the second problem is easily fixed moving dpcm_set_fe_runtime() at
> the beginning of dpcm_fe_dai_startup(). Are these good solutions?

This seems all to be very hackish. We clearly need to fix that DPCM only
considers the constraints of the FE DAI though.

The digital domain of a sound card can be thought of as a pipeline which
mostly operates on one sample at a time. The samples have two main
parameters, the frequency with which they are generated and their width.
Certain components in the pipeline have constraints on which parameters they
can work with. There are also components which can change the parameters,
e.g. a samplerate converter can change the frequency or a DAI might be able
to change the width. What we are interested in is for which parameters on
the PCM side are we able to build up a pipeline that satisfies all
constraints of all the components in the pipeline. I think this can be done
by walking the DAPM graph and collect the constraints associated with the
components in the path (The graph walking only has to be done when
components are added or removed). E.g. build up a list of all the the DAIs
that are reachable from a PCM and then use the constraints of those DAIs for
the PCM.

For starters we probably do not want support components which can change the
parameters for the sample stream. This is currently not supported in ASoC at
all and adding it will makes things more complex. But it should be kept in
mind, so it can be added in the next iteration. When calculating the
constraints we should probably also consider all possible paths and not only
active paths, since otherwise you'll run into problems if you want to change
the active path at runtime and the new configuration has more restrictive
constraints.

While we are at it we should probably also reduce the separation between
DPCM and clasic PCM as clasic PCM is just a special case (static routes) of
DPCM.

- Lars

Re: [PATCH 08/19] ASoC: kirkwood: Don't set unused struct snd_pcm_hardware fields

[Mark Brown]

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

On Wed, Jan 01, 2014 at 09:10:15PM +0100, Lars-Peter Clausen wrote:

> This seems all to be very hackish. We clearly need to fix that DPCM only
> considers the constraints of the FE DAI though.

Yes.  Or otherwise make sure they get fed in anyway.

> The digital domain of a sound card can be thought of as a pipeline which
> mostly operates on one sample at a time. The samples have two main

> to change the width. What we are interested in is for which parameters on
> the PCM side are we able to build up a pipeline that satisfies all
> constraints of all the components in the pipeline. I think this can be done
> by walking the DAPM graph and collect the constraints associated with the
> components in the path (The graph walking only has to be done when
> components are added or removed). E.g. build up a list of all the the DAIs
> that are reachable from a PCM and then use the constraints of those DAIs for
> the PCM.

This is sort of the direction I'd been thinking in but there's a couple
of extra bits to consider here.  The big one is that we have devices
with constraints that aren't connected to the routing so just walking
DAPM isn't sufficient - for example, a constraint like "all the DACs
have to be in the same domain at the same rate".  This makes the
collecting all the constraints harder and routing dependent.

What I'd been thinking of was annotating the DAPM graph with digital
domain objects (registering a list of digital domains each of which has
some DAPM objects anyway) and then using the DAPM graph to flow through
both settings when streams start and constraints when we're trying to
figure out those.  I started implementing this but didn't get much
beyond registration.

> While we are at it we should probably also reduce the separation between
> DPCM and clasic PCM as clasic PCM is just a special case (static routes) of
> DPCM.

I'd been thinking of that in the other direction - moving things out of
DPCM and into DAPM or ASoC specific stuff so that the ALSA level PCM
interface is more hidden, making dynamic PCM less tied to PCM.  The
stuff with handling sample sizes rather than formats is going in that
general direction.  I'm not sure this doesn't boil down to the same
thing as you're suggesting when they're implemented though.

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

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