Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Takashi Iwai <tiwai@xxxxxxx>]

At Thu, 28 Jun 2012 19:06:07 +0530,
Laxman Dewangan wrote:
> 
> On Thursday 28 June 2012 06:58 PM, Takashi Iwai wrote:
> > At Thu, 28 Jun 2012 18:34:02 +0530,
> > Laxman Dewangan wrote:
> >> On Thursday 28 June 2012 06:27 PM, Takashi Iwai wrote:
> >>> At Thu, 28 Jun 2012 18:05:53 +0530,
> >>> Laxman Dewangan wrote:
> >>>> On Thursday 28 June 2012 05:58 PM, Mark Brown wrote:
> >>>>> * PGP Signed by an unknown key
> >>>>>
> >>>>> On Thu, Jun 28, 2012 at 02:30:26PM +0200, Lars-Peter Clausen wrote:
> >>>>>> On 06/28/2012 02:18 PM, Mark Brown wrote:
> >>>>>>> Oh, dear - this means we don't actually have a generic API at all.  Is
> >>>>>>> there any effort being made to make this generally available?
> >>>>>> Yes there is, and in fact in next/master there is generic
> >>>>>> dma_mmap_writecombine. I'm not quite sure how it behaves on non ARM archs
> >>>>>> though. But in theory it should fallback to a normal coherent mapping. Marek
> >>>>>> did a lot of work on this, I've put him on Cc.
> >>>> So can we put the function snd_pcm_lib_writecombine_mmap() in the
> >>>> pcm_native.c and only export this api for ARM i.e.  under macro  #ifdef
> >>>> CONFIG_ARM so that ARM based SOCs like Tegra/epa3xx/mxs/spear can
> >>>> directly use that one?
> >>>> Or, wait for this common API until all ARCH support it?
> >>> I think it's fine to put it first in ALSA side with some ifdef.
> >>> A similar trick is already found for snd_pcm_lib_mmap_iomem.
> >>> See include/sound/pcm.h.
> >>>
> >>> But, actually it's a still question what if an architecture doesn't
> >>> support the mmap of writecombine at all.  The proposed patch doesn't
> >>> allow you to know whether writecombine-mmap is possible or not on the
> >>> running architecture until you really try to call it and fail.
> >>> It's a missing piece, IMO.
> >> Lars wanted to move the buffer allocation also to common place.
> > Which common place?
> 
> He wanted to refactor allocation part also at same time and so I guessed 
> that it will be in some common place and though about the sound/core.

But the memory allocation rework is basically irrelevant from what's
needed for pcm_mmap_writecombine() now.  It'd need a lot more reworks
than the simple addition of dma_mmap_writecombine() wrapper.

> >> Then how about this?
> >> create new file and header for snd-pcm-writecombine-buffer.c/.h and put
> >> this in sound/core.
> >> Select this file compilation through config SND_PCM_WRITECOMBINE_BUFFER
> >> which will be select this config through sound/soc/xxx/Kconfig if they
> >> want to use.
> >> This will provide three apis: new, free and mmap.
> > Way too much hustles than necessary...
> Ooh no. :-(  I had taken the idea from snd_dmaengine_pcm driver.
> Bit I like to hear simple way which solves the purpose.

As Lars suggested, a simple ifdef should suffice for now.
With the upcoming generic dma_mmap_writecombine() stuff, it'd be even
simpler in future.

But, still we need to be careful about this.  As mentioned, there is
no flag to know the possibility of writecombine mmap beforehand.
It'd be nice if we have either a compile-time or a run-time flag /
function to check that.  Then the driver can also expose the mmap
capability to user-space depending on the flag.


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/