Messages in this thread |  | | | Date | Tue, 06 Dec 2005 19:41:03 +0100 | | From | Takashi Iwai <> | | Subject | Re: [Alsa-devel] Re: [PATCH 2.6.13.1 1/1] CS5535 AUDIO ALSA driver |
| |
At Tue, 6 Dec 2005 17:42:03 +0000, Christoph Hellwig wrote: > On Thu, Sep 29, 2005 at 03:21:43PM +0200, Takashi Iwai wrote: > > At Wed, 21 Sep 2005 09:31:09 +0100, > > Christoph Hellwig wrote: > > > > > > On Tue, Sep 20, 2005 at 05:23:09PM -0700, Andrew Morton wrote: > > > > > I'm not sure what to do. I'd be happy to take them out. But I woudn't > > > > > mind leaving them in if that's what alsa convention is. > > > > > > > > I'd be inclined to stick with the alsa style. That's just an fyi if you > > > > plan on working in other places. > > > > > > I'd _really_ prefer to fix all of alsa. Alsa is so full of wrappers > > > and multiple names for the same thing that's it's almost impossibly > > > for a normal kernel developer to fix anythign in there. > > > > Oh I guess you're referring to the messy memory stuff in ALSA code? > > > > (Or you mean about the foo_t style? Then it would be easy to fix, and > > I'd appreciate if someone takes this job :) > > > > Basically I agree with all your suggestions - hacks have been there > > simply as workarounds. They should be removed. > > > > The following are in WIP on my local tree: > > > > - Merge of dma_alloc_coherent() hacks for pages <32bit to kernel core > > (for i386 and ppc) > > Did this get out?
Not yet. It's pending because I had other big rewrites beforehand. I'll prepare a patch for this soon later.
> > - PageReserve and mmaps: > > dma_mmap_coherent() will be used in all architectures instead of > > vma nopage callback. Of course, dma_mmap_coherent() should be > > ported to all archs. > > What about just making the i386 one Russell posted generic? It shouldn't > be any less broken than what we do now and every architecture that needs > something more specific can implements it.
Yes, it's a good starting point, indeed.
> > - Removal of the common allocator in sound/core/memalloc.c > > Instead, each driver has alloc_buffer() and free_buffer() > > callbacks. > > this isn't in mainline yet, is it?
Not yet, too. I'm waiting for stabilization of the recent changes on mm before this clean-up action. Also, still considering the best solution...
> > - The "right" way to allocate/mmap coherent SG-buffers. > > How do you get coherent SG-buffers? The only way to get coherent dma > allocations is dma_alloc_coheren (and the pci variant), but that's not > handing back an SG list. But even if you build one yourself it should > be handled by dma_mmap_coherent.
I thought of creating an API for coherent SG-buffers, but it looks ineffective on some architectures to have large number of non-contiguous coherent pages. Each call of dma_alloc_coherent() involves kmalloc of another record for a consistent VM region.
So, currently I gave up it and rewrote the stuff in an easier style, using dma_map_sg() + dma_sync_sg_*() for pages allocated via __vmalloc(). The question is whether mmap of non-coherent buffer may influence on the behavior of user-space apps on architectures like ARM or SPARC. If it really matters, we can disable mmap for such architectures, as a safe solution :)
> > - A common API for mmap vmalloc buffers. > > What API do you think about? If you do alloc_page yourself and use > vmap everything gets much much easier. And with the new vm_install_page > this becomes pretty easy: > > init_routine() > { > > ... > for (i = 0; i < NR_PAGES; i++) { > pages[i] = alloc_page(GFP_KERNEL); > ... > } > > dma_buffer = vmap(pages, ...); > } > > dma_map() > { > for (i = 0; i < NR_PAGES; i++) { > dma_map_page(pages[i, ....); > ... > } > } > > mmap() > { > for (i = 0; i < NR_PAGES; i++) { > vm_insert_page(..., pages[i], ..); > ... > } > } > > maybe some tiny helpers that encapsulate the loop over the pages would > be nice, but we shouldn't need much of a new API here.
My current version has the helper functions as below. This is intended for the generic SG-buffers, and uses struct scatterlist. It looks a bit more complicated than your example above as consequence.
Any comments are appreciated.
Thanks,
Takashi
================ struct snd_sg_buf { struct scatterlist *sglist; int nelems; int nmaps; int direction; }; static void *do_alloc_sgbuf(struct device *dev, struct snd_sg_buf *sg, gfp_t gfp) { void *vptr; struct scatterlist *sgl; int i; vptr = __vmalloc(sg->nelems << PAGE_SHIFT, gfp, PAGE_KERNEL); if (! vptr) return NULL; /* set up sg list */ sgl = sg->sglist; memset(sgl, 0, sg->nelems * sizeof(*sgl)); for (i = 0; i < sg->nelems; i++, sgl++) { sgl->page = vmalloc_to_page(vptr + (i << PAGE_SHIFT)); sgl->offset = 0; sgl->length = PAGE_SIZE; } if (! dev) return vptr; sg->nmaps = dma_map_sg(dev, sg->sglist, sg->nelems, sg->direction); if (! sg->nmaps) { vfree(vptr); return NULL; } if (dev->dma_mask && *dev->dma_mask && *dev->dma_mask < DMA_32BIT_MASK) { /* check whether all pages are in the given dma mask */ dma_addr_t rmask = ~(*dev->dma_mask); sgl = sg->sglist; for (i = 0; i < sg->nmaps; i++, sgl++) { if (sg_dma_address(sgl) & rmask) { dma_unmap_sg(dev, sg->sglist, sg->nelems, sg->direction); vfree(vptr); return NULL; } } } return vptr; } /* * allocate an sg buffer and map it */ void *snd_dma_alloc_sgbuf(struct device *dev, size_t size, int direction, struct snd_sg_buf **sg_ret) { struct snd_sg_buf *sg; void *vptr; sg = kzalloc(sizeof(*sg), GFP_KERNEL); if (! sg) return NULL; size = PAGE_ALIGN(size); sg->nelems = size >> PAGE_SHIFT; sg->direction = direction; sg->sglist = kmalloc(sg->nelems * sizeof(struct scatterlist), GFP_KERNEL); if (! sg->sglist) { kfree(sg); return NULL; } if ((vptr = do_alloc_sgbuf(dev, sg, GFP_KERNEL)) == NULL && (vptr = do_alloc_sgbuf(dev, sg, GFP_DMA)) == NULL) { kfree(sg->sglist); kfree(sg); return NULL; } *sg_ret = sg; return vptr; } /* * unmap and release the sg buffer */ void snd_dma_free_sgbuf(struct device *dev, void *vptr, struct snd_sg_buf *sg) { if (sg->nmaps) dma_unmap_sg(dev, sg->sglist, sg->nelems, sg->direction); vfree(vptr); kfree(sg->sglist); kfree(sg); } /* * return the bus address at the corresponding offset */ dma_addr_t snd_sgbuf_get_addr(struct snd_sg_buf *sg, size_t offset) { struct scatterlist *sgl; int i; if (offset >= (sg->nelems << PAGE_SHIFT)) return 0; /* short path - no compounds */ if (sg->nelems == sg->nmaps) return sg_dma_address(&sg->sglist[offset >> PAGE_SHIFT]) + offset % PAGE_SIZE; /* look for the corresponding entry... should be optimized */ sgl = sg->sglist; for (i = 0; i < sg->nmaps; i++, sgl++) { if (sg_dma_len(sgl) > offset) return sg_dma_address(sgl) + offset; offset -= sg_dma_len(sgl); } return 0; } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |