Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap
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
On 06/28/2012 03:51 PM, Takashi Iwai wrote:
At Thu, 28 Jun 2012 19:06:07 +0530, Laxman Dewangan wrote: [...]
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.
If writecombine is not supported it will fallback to a "normal" mmap. Here's an excerpt from the DMA mapping documentation:
DMA_ATTR_WRITE_COMBINE ----------------------
DMA_ATTR_WRITE_COMBINE specifies that writes to the mapping may be buffered to improve performance.
Since it is optional for platforms to implement DMA_ATTR_WRITE_COMBINE, those that do not will simply ignore the attribute and exhibit default behavior.
- Lars
participants (2)
-
Lars-Peter Clausen -
Takashi Iwai