Re: [PATCH 0/3] ASoC: Move pcm writecombine dma buffer allocation to core

[Takashi Iwai <tiwai@suse.de>]

At Fri, 29 Jun 2012 20:34:24 +0530,
Laxman Dewangan wrote:
> 
> Hi Takashi,
> Thanks for sample code. It helps lot.
> 
> On Friday 29 June 2012 05:43 PM, Takashi Iwai wrote:
> > At Fri, 29 Jun 2012 15:53:15 +0530,
> > Laxman Dewangan wrote:
> >> Some of the ARM based soc allocate the writecombine dma buffer for
> >> pcm substreams. They have the same codes for managing this buffer.
> >> Moving this to the core/pcm files so that they can use that directly.
> >>
> >> Remove the code from Tegra PCM and use these new library function.
> >>
> >> This is enabled only for ARM specific and can be extended to other
> >> architecture if they support the writecombine dma buffer.
> >>
> >> This patch is based on detail discussion on patch:
> >> [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap
> >> And suggestion from Lars and Takashi.
> > Looking through your patch, I think an easier integration is just to
> > add writecombine option to memalloc.c which calls
> > dma_alloc_writecombine() instead of dma_alloc_coherent().
> >
> > The addition for mmap is still an open question.  Again, an easier
> > option so far looks like just add the call of dma_alloc_writecombine()
> > in snd_pcm_lib_default_mmap().  Alternatively, we can create an
> > individual mmap pcm_ops as we discussed.
> >
> > Below is a totally untested patch, but you can imagine what I meant.
> >
> > After this change, replace with snd_pcm_lib_malloc_pages with
> > SNDRV_DMA_TYPE_DEV_WC, and that's all you need in the driver side.
> 
> We can not use the snd_pcm_lib_malloc_pages() in the pcm_new callback 
> because at this time the substream->runtime is not initialized and it 
> leads to the kernel crash.
> 
> I used the apis as
> 
> int snd_soc_pcm_new_wc_dma_buffer(struct snd_soc_pcm_runtime *rtd,
>                  size_t max_bytes)
> {
>       :::::::::::::
>          substream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream;
>          if (substream) {
>                  substream->dma_buffer.dev.type = SNDRV_DMA_TYPE_DEV_WC;
>                  ret = snd_pcm_lib_malloc_pages(substream, max_bytes);
>                  if (ret)
>                          goto err;
>          }
> ::::::::
> }
> 
> 
> int snd_pcm_lib_malloc_pages(struct snd_pcm_substream *substream, size_t 
> size)
> {
>          struct snd_pcm_runtime *runtime;
>          struct snd_dma_buffer *dmab = NULL;
> 
>          if (PCM_RUNTIME_CHECK(substream))
>                  return -EINVAL;
> 
>          if (snd_BUG_ON(substream->dma_buffer.dev.type ==
>                         SNDRV_DMA_TYPE_UNKNOWN))
>                  return -EINVAL;
> 
>          runtime = substream->runtime;
> 
>          if (runtime->dma_buffer_p) {
> ---------------Kernel crash at this point ----------
> 
> So I used the snd_dma_alloc_pages() from the driver to allocate WC memory.

snd_pcm_lib_malloc_pages() should be called only from hw_params.
Prior to that, you need to set up the dam_buffer type by calling
snd_pcm_lib_preallocate_pages() or
snd_pcm_lib_preallocate_pages_for_all() at the time to create a PCM
instance.  The size can be 0 if not necessary to allocate there but
later on demand.

> 
> > +/* allocate the coherent DMA pages */
> > +static void *snd_malloc_dev_pages(struct device *dev, size_t size, dma_addr_t *dma)
> > +{
> > +	return __snd_malloc_dev_pages(dev, size, dma, dma_alloc_coherent);
> 
> This does not get compiled in ARM because dma_alloc_coherant is macro 
> defined as
> 
> #define dma_alloc_coherent(d, s, h, f) dma_alloc_attrs(d, s, h, f, NULL)

Ah, OK.


thanks,

Takashi

> static inline void *dma_alloc_attrs(struct device *dev, size_t size,
>                                         dma_addr_t *dma_handle, gfp_t flag,
>                                         struct dma_attrs *attrs)
> 
> 
> I fixed this by
> static void *_dma_alloc_coherent(struct device *dev, size_t size,
>                  dma_addr_t *dma_handle, gfp_t flag)
> {
>          return dma_alloc_coherent(dev, size, dma_handle, flag);
> }
> 
> /* allocate the coherent DMA pages */
> static void *snd_malloc_dev_pages(struct device *dev, size_t size, 
> dma_addr_t *dma)
> {
>          return _snd_malloc_dev_pages(dev, size, dma, _dma_alloc_coherent);
> }
>