[PATCH] ASoC: omap-pcm: Allow only formats with 1, 2, and 4 byte physical size

[PATCH] ASoC: omap-pcm: Allow only formats with 1, 2, and 4 byte physical size

[Jyri Sarha]

sDMA support only transfer elements with 1, 2, and 4 byte physical
size. Initialize the pcm driver accordingly.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 sound/soc/omap/omap-pcm.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c
index f4b05bc..e49ee23 100644
--- a/sound/soc/omap/omap-pcm.c
+++ b/sound/soc/omap/omap-pcm.c
@@ -39,7 +39,7 @@
 #define pcm_omap1510()	0
 #endif
 
-static const struct snd_pcm_hardware omap_pcm_hardware = {
+static struct snd_pcm_hardware omap_pcm_hardware = {
 	.info			= SNDRV_PCM_INFO_MMAP |
 				  SNDRV_PCM_INFO_MMAP_VALID |
 				  SNDRV_PCM_INFO_INTERLEAVED |
@@ -53,6 +53,24 @@ static const struct snd_pcm_hardware omap_pcm_hardware = {
 	.buffer_bytes_max	= 128 * 1024,
 };
 
+/* sDMA supports only 1, 2, and 4 byte transfer elements. */
+static void omap_pcm_limit_supported_formats(void)
+{
+	int i;
+
+	for (i = 0; i < SNDRV_PCM_FORMAT_LAST; i++) {
+		switch (snd_pcm_format_physical_width(i)) {
+		case 8:
+		case 16:
+		case 32:
+			omap_pcm_hardware.formats |= (1LL << i);
+			break;
+		default:
+			break;
+		}
+	}
+}
+
 /* this may get called several times by oss emulation */
 static int omap_pcm_hw_params(struct snd_pcm_substream *substream,
 			      struct snd_pcm_hw_params *params)
@@ -235,6 +253,7 @@ static struct snd_soc_platform_driver omap_soc_platform = {
 
 int omap_pcm_platform_register(struct device *dev)
 {
+	omap_pcm_limit_supported_formats();
 	return devm_snd_soc_register_platform(dev, &omap_soc_platform);
 }
 EXPORT_SYMBOL_GPL(omap_pcm_platform_register);
-- 
1.9.1

Re: [PATCH] ASoC: omap-pcm: Allow only formats with 1, 2, and 4 byte physical size

[Peter Ujfalusi]

On 03/03/2015 01:28 PM, Jyri Sarha wrote:
> sDMA support only transfer elements with 1, 2, and 4 byte physical
> size. Initialize the pcm driver accordingly.

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

> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  sound/soc/omap/omap-pcm.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c
> index f4b05bc..e49ee23 100644
> --- a/sound/soc/omap/omap-pcm.c
> +++ b/sound/soc/omap/omap-pcm.c
> @@ -39,7 +39,7 @@
>  #define pcm_omap1510()	0
>  #endif
>  
> -static const struct snd_pcm_hardware omap_pcm_hardware = {
> +static struct snd_pcm_hardware omap_pcm_hardware = {
>  	.info			= SNDRV_PCM_INFO_MMAP |
>  				  SNDRV_PCM_INFO_MMAP_VALID |
>  				  SNDRV_PCM_INFO_INTERLEAVED |
> @@ -53,6 +53,24 @@ static const struct snd_pcm_hardware omap_pcm_hardware = {
>  	.buffer_bytes_max	= 128 * 1024,
>  };
>  
> +/* sDMA supports only 1, 2, and 4 byte transfer elements. */
> +static void omap_pcm_limit_supported_formats(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < SNDRV_PCM_FORMAT_LAST; i++) {
> +		switch (snd_pcm_format_physical_width(i)) {
> +		case 8:
> +		case 16:
> +		case 32:
> +			omap_pcm_hardware.formats |= (1LL << i);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +}
> +
>  /* this may get called several times by oss emulation */
>  static int omap_pcm_hw_params(struct snd_pcm_substream *substream,
>  			      struct snd_pcm_hw_params *params)
> @@ -235,6 +253,7 @@ static struct snd_soc_platform_driver omap_soc_platform = {
>  
>  int omap_pcm_platform_register(struct device *dev)
>  {
> +	omap_pcm_limit_supported_formats();
>  	return devm_snd_soc_register_platform(dev, &omap_soc_platform);
>  }
>  EXPORT_SYMBOL_GPL(omap_pcm_platform_register);
> 


-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ASoC: omap-pcm: Allow only formats with 1, 2, and 4 byte physical size

[Jarkko Nikula]

On Tue, Mar 03, 2015 at 01:41:21PM +0200, Peter Ujfalusi wrote:
> On 03/03/2015 01:28 PM, Jyri Sarha wrote:
> > sDMA support only transfer elements with 1, 2, and 4 byte physical
> > size. Initialize the pcm driver accordingly.
> 
> Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> 
> > 
> > Signed-off-by: Jyri Sarha <jsarha@ti.com>
> > ---
> >  sound/soc/omap/omap-pcm.c | 21 ++++++++++++++++++++-
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> > 
> > diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c
> > index f4b05bc..e49ee23 100644
> > --- a/sound/soc/omap/omap-pcm.c
> > +++ b/sound/soc/omap/omap-pcm.c
> > @@ -39,7 +39,7 @@
> >  #define pcm_omap1510()	0
> >  #endif
> >  
> > -static const struct snd_pcm_hardware omap_pcm_hardware = {
> > +static struct snd_pcm_hardware omap_pcm_hardware = {
> >  	.info			= SNDRV_PCM_INFO_MMAP |
> >  				  SNDRV_PCM_INFO_MMAP_VALID |
> >  				  SNDRV_PCM_INFO_INTERLEAVED |
> > @@ -53,6 +53,24 @@ static const struct snd_pcm_hardware omap_pcm_hardware = {
> >  	.buffer_bytes_max	= 128 * 1024,
> >  };
> >  
> > +/* sDMA supports only 1, 2, and 4 byte transfer elements. */
> > +static void omap_pcm_limit_supported_formats(void)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < SNDRV_PCM_FORMAT_LAST; i++) {
> > +		switch (snd_pcm_format_physical_width(i)) {
> > +		case 8:
> > +		case 16:
> > +		case 32:
> > +			omap_pcm_hardware.formats |= (1LL << i);
> > +			break;
> > +		default:
> > +			break;
> > +		}
> > +	}
> > +}
> > +

I wonder why these are set in a loop instead of setting formats statically
and why this is needed after commit 60e21d287344 ("ASoC: omap: Don't set
unused struct snd_pcm_hardware fields")?

-- 
Jarkko

Re: [PATCH] ASoC: omap-pcm: Allow only formats with 1, 2, and 4 byte physical size

[Peter Ujfalusi]

On 03/03/2015 02:22 PM, Jarkko Nikula wrote:
> On Tue, Mar 03, 2015 at 01:41:21PM +0200, Peter Ujfalusi wrote:
>> On 03/03/2015 01:28 PM, Jyri Sarha wrote:
>>> sDMA support only transfer elements with 1, 2, and 4 byte physical
>>> size. Initialize the pcm driver accordingly.
>>
>> Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>>
>>>
>>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>>> ---
>>>  sound/soc/omap/omap-pcm.c | 21 ++++++++++++++++++++-
>>>  1 file changed, 20 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c
>>> index f4b05bc..e49ee23 100644
>>> --- a/sound/soc/omap/omap-pcm.c
>>> +++ b/sound/soc/omap/omap-pcm.c
>>> @@ -39,7 +39,7 @@
>>>  #define pcm_omap1510()	0
>>>  #endif
>>>  
>>> -static const struct snd_pcm_hardware omap_pcm_hardware = {
>>> +static struct snd_pcm_hardware omap_pcm_hardware = {
>>>  	.info			= SNDRV_PCM_INFO_MMAP |
>>>  				  SNDRV_PCM_INFO_MMAP_VALID |
>>>  				  SNDRV_PCM_INFO_INTERLEAVED |
>>> @@ -53,6 +53,24 @@ static const struct snd_pcm_hardware omap_pcm_hardware = {
>>>  	.buffer_bytes_max	= 128 * 1024,
>>>  };
>>>  
>>> +/* sDMA supports only 1, 2, and 4 byte transfer elements. */
>>> +static void omap_pcm_limit_supported_formats(void)
>>> +{
>>> +	int i;
>>> +
>>> +	for (i = 0; i < SNDRV_PCM_FORMAT_LAST; i++) {
>>> +		switch (snd_pcm_format_physical_width(i)) {
>>> +		case 8:
>>> +		case 16:
>>> +		case 32:
>>> +			omap_pcm_hardware.formats |= (1LL << i);
>>> +			break;
>>> +		default:
>>> +			break;
>>> +		}
>>> +	}
>>> +}
>>> +
> 
> I wonder why these are set in a loop instead of setting formats statically

omap-pcm as such can work with any formats which has 1, 2 or 4 bytes layout in
memory (and not only S16_LE, S32_LE).
sDMA can not handle 3byte words so we can not handle those with omap-pcm. In
contrast eDMA for example can handle 1, 2, 3 and 4 w/o issue.

> and why this is needed after commit 60e21d287344 ("ASoC: omap: Don't set
> unused struct snd_pcm_hardware fields")?

Hrm, interesting. We also have:
2d38df128321 (ASoC: generic-dmaengine-pcm: Prepare formats mask for valid
physical sample sizes)

Which does the same thing for the generic dmaengine pcm users.

Actually the information from the omap_pcm_hardware is copied to runtime.hw
via snd_soc_set_runtime_hwparams() call, so it is not ignored.

We need this masking to be done here since the omap-pcm is still mostly custom
code and do not plug easily to the generic code (ie, not at all).

-- 
Péter

Re: [PATCH] ASoC: omap-pcm: Allow only formats with 1, 2, and 4 byte physical size

[Jyri Sarha]

On 03/03/2015 02:22 PM, Jarkko Nikula wrote:
> On Tue, Mar 03, 2015 at 01:41:21PM +0200, Peter Ujfalusi wrote:
>> On 03/03/2015 01:28 PM, Jyri Sarha wrote:
>>> sDMA support only transfer elements with 1, 2, and 4 byte physical
>>> size. Initialize the pcm driver accordingly.
>>
>> Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>>
>>>
>>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>>> ---
>>>   sound/soc/omap/omap-pcm.c | 21 ++++++++++++++++++++-
>>>   1 file changed, 20 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c
>>> index f4b05bc..e49ee23 100644
>>> --- a/sound/soc/omap/omap-pcm.c
>>> +++ b/sound/soc/omap/omap-pcm.c
>>> @@ -39,7 +39,7 @@
>>>   #define pcm_omap1510()	0
>>>   #endif
>>>
>>> -static const struct snd_pcm_hardware omap_pcm_hardware = {
>>> +static struct snd_pcm_hardware omap_pcm_hardware = {
>>>   	.info			= SNDRV_PCM_INFO_MMAP |
>>>   				  SNDRV_PCM_INFO_MMAP_VALID |
>>>   				  SNDRV_PCM_INFO_INTERLEAVED |
>>> @@ -53,6 +53,24 @@ static const struct snd_pcm_hardware omap_pcm_hardware = {
>>>   	.buffer_bytes_max	= 128 * 1024,
>>>   };
>>>
>>> +/* sDMA supports only 1, 2, and 4 byte transfer elements. */
>>> +static void omap_pcm_limit_supported_formats(void)
>>> +{
>>> +	int i;
>>> +
>>> +	for (i = 0; i < SNDRV_PCM_FORMAT_LAST; i++) {
>>> +		switch (snd_pcm_format_physical_width(i)) {
>>> +		case 8:
>>> +		case 16:
>>> +		case 32:
>>> +			omap_pcm_hardware.formats |= (1LL << i);
>>> +			break;
>>> +		default:
>>> +			break;
>>> +		}
>>> +	}
>>> +}
>>> +
>
> I wonder why these are set in a loop instead of setting formats statically
> and why this is needed after commit 60e21d287344 ("ASoC: omap: Don't set
> unused struct snd_pcm_hardware fields")?
>


Hmmm, interesting. The the problem I was trying to solve was "read 
error: Cannot allocate memory" when trying to play S24_3LE format. With 
this patch the error is more informative:

aplay: set_params:1233: Sample format non available
Available formats:
- S16_LE
- S24_LE
- S32_LE

So the comment of commit 60e21d287344 is not entirely true, as far as I 
understand.

Best regards,
Jyri

Re: [PATCH] ASoC: omap-pcm: Allow only formats with 1, 2, and 4 byte physical size

[Mark Brown]

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

On Tue, Mar 03, 2015 at 01:28:53PM +0200, Jyri Sarha wrote:
> sDMA support only transfer elements with 1, 2, and 4 byte physical
> size. Initialize the pcm driver accordingly.

Applied, thanks.

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

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