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

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

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