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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



Relevant Pages

  • Re: [Alsa-devel] Re: [PATCH 2.6.13.1 1/1] CS5535 AUDIO ALSA driver
    ... >>> I'd be inclined to stick with the alsa style. ... How do you get coherent SG-buffers? ... > - A common API for mmap vmalloc buffers. ... What API do you think about? ...
    (Linux-Kernel)
  • Re: read vs. mmap (or io vs. page faults)
    ... I think you forgot my earlier explanation regarding buffer ... This could explain, why using mmap is not faster than read, but it does ... your vast knowledge of the internals of the kernel workings ... But because the CPU is underutilized, ...
    (freebsd-current)
  • Re: read vs. mmap (or io vs. page faults)
    ... I think you forgot my earlier explanation regarding buffer ... This could explain, why using mmap is not faster than read, but it does ... your vast knowledge of the internals of the kernel workings ... But because the CPU is underutilized, ...
    (freebsd-questions)
  • Re: Making existing process efficient
    ... using mmap() instead, with madviseto tell the virtual ... pipe with the child. ... The child would inherit the buffer from ... input buffer to an output buffer and thence to the kernel, ...
    (comp.unix.programmer)
  • Re: How to develop a random number generation device
    ... Many operating systens are by design, immune to buffer over-runs modifying unrelated code. ... randomising memory locations) rather than actually preventing the overrun. ... If you try to run a 5-year old Linux binary on a current distribution, ... Linux, and the API is very stable - new API's and system calls are added, but existing ones are seldom changed or removed, and never without very good reason. ...
    (sci.electronics.design)