From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: An driver error when I using aplay! Date: Tue, 08 Jun 2004 17:48:45 +0200 Sender: alsa-devel-admin@lists.sourceforge.net Message-ID: References: <20040607140817.A28526@flint.arm.linux.org.uk> <20040607145113.B28526@flint.arm.linux.org.uk> <20040607160442.D28526@flint.arm.linux.org.uk> <20040607161812.F28526@flint.arm.linux.org.uk> <20040607164401.G28526@flint.arm.linux.org.uk> Mime-Version: 1.0 (generated by SEMI 1.14.5 - "Awara-Onsen") Content-Type: text/plain; charset=US-ASCII Return-path: In-Reply-To: <20040607190416.K28526@flint.arm.linux.org.uk> Errors-To: alsa-devel-admin@lists.sourceforge.net List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , List-Archive: To: Russell King Cc: Jaroslav Kysela , Roc Wu , Clemens Ladisch , Alsa-devel@lists.sourceforge.net List-Id: alsa-devel@alsa-project.org At Mon, 7 Jun 2004 19:04:16 +0100, Russell King wrote: > > > > snd_pcm_ops_t should have a mmap() method just like everything else > > > in the kernel, which allows drivers to select the appropriate mmap() > > > implementation. > > > > I thought of this, too. This may help some cases, but we still need a > > generic solution, because a PCI driver can be used in different > > architectures. > > Yes, but at least if you have a common function you won't have to > modify thousands of drivers - and you'll need that function to > get at the struct device. Eg, something like: > > static int aaci_pcm_mmap(snd_pcm_substream_t *substream, struct vm_area_struct *vma) > { > struct aaci *aaci = substream->private_data; > return devdma_mmap(&aaci->dev->dev, substream, vma); > } > > int devdma_mmap(struct device *dev, snd_pcm_substream_t *substream, struct vm_area_struct *vma) > { > snd_pcm_runtime_t *runtime = substream->runtime; > return dma_mmap_coherent(dev, vma, runtime->dma_area, runtime->dma_addr, runtime->dma_bytes); > } This looks nice. I'd love to have such one, too. > And devdma_mmap contains whatever is necessary for each architecture > until they implement dma_mmap_coherent - which would be something akin > to the code currently in pcm_native.c for handling this itself. IOW, > snd_pcm_mmap_data_open, snd_pcm_mmap_data_close, snd_pcm_mmap_data_nopage > and the bulk of snd_pcm_mmap_data in devdma_mmap(). Well, in practice, there are some hurdles. - For using nopage vma_ops, we need to set an appropriate vma_private_data to pass the base DMA pointer and address. If this is a kmalloc data created in dev_mmap(), how can we release it? - Instead of using nopage, we need mark pages as reserved so that mmap callback can simply use remap_page_addr(), since dma_alloc_coherent() doesn't do this on all architectures. The question is who should mark this reseved hack. Ideally, dma_alloc_coherent() assures that the pages are marked as reserved. Otherwise, dev_mmap() itself would need to handle this marking/clearing. - Handling of SG coherent DMA case? If nopage is not used, the caller can call dev_mmap() on each page. If nopage is used, the SG-implementation must be in nopage, too. If we assumed that the passed DMA buffer is already marked as reserved, using remap_page_addr() in mmap would be the easiest solution. On the other hand, when we just fix the ALSA case, providing the DMA -> page conversion (let's call dma_map_to_page) would be the easiest. (Except for the cache stuff of control/status pages.) > > > > - adding pgprot_noncached() in fop->mmap callback assures that the > > > > mmaped page is accessed without cache side effects. > > > > > > Correct, but as it has been already decided elsewhere, interfaces > > > which expose pgprot tweaking are broken and new ones should not be > > > introduced. > > > > Hmm, is there a standard interface for this purpose? > > No - because it's completely non-portable. Eg, not all architectures > are capable of turning off the cache on a per-page basis. Ok. This can be hidden in dev_mmap(), too, if we do it right... > > > They also _may_ only affect the userspace mapping of > > > that memory and not kernel space, so this doesn't help the control > > > or status mmaped pages. > > > > Oh yes, then how about to use a DMA page for the control/status mmap, > > too? > > Oddly that's what I did on VIVT caches to get Alsa working a few months > ago, but its very unclean and the way I did it, it exposes the "dma" > address in the actual page - which would be a major security problem > if I let the code out. As I said back then, I didn't have a clean > solution for this. Well, i386 and x86-64 provide a function change_page_attr() for this purpose. But it's also a horrible arch-spec hack. How about to introduce a function like alloc_pages_noncached()? -- Takashi Iwai ALSA Developer - www.alsa-project.org ------------------------------------------------------- This SF.Net email is sponsored by: GNOME Foundation Hackers Unite! GUADEC: The world's #1 Open Source Desktop Event. GNOME Users and Developers European Conference, 28-30th June in Norway http://2004/guadec.org