| Message ID | 1425382133-17120-1-git-send-email-jsarha@ti.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6742e15cf92a8dc3065843a627952ed518e08267 |
| Headers | show |
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); >
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")?
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).
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
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.
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);
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(-)