Hello,

On Friday, August 12, 2011 5:01 PM Arnd Bergmann wrote:

> On Friday 12 August 2011, Marek Szyprowski wrote:
> > @@ -82,16 +103,16 @@ static struct page *__dma_alloc_buffer(struct device
*dev,
> size_t size, gfp_t gf
> >     if (mask < 0xffffffffULL)
> >             gfp |= GFP_DMA;
> >
> > -   page = alloc_pages(gfp, order);
> > -   if (!page)
> > -           return NULL;
> > -
> >     /*
> > -    * Now split the huge page and free the excess pages
> > +    * Allocate contiguous memory
> >      */
> > -   split_page(page, order);
> > -   for (p = page + (size >> PAGE_SHIFT), e = page + (1 << order); p < e;
p++)
> > -           __free_page(p);
> > +   if (cma_available())
> > +           page = dma_alloc_from_contiguous(dev, count, order);
> > +   else
> > +           page = __dma_alloc_system_pages(count, gfp, order);
> > +
> > +   if (!page)
> > +           return NULL;
> 
> Why do you need the fallback here? I would assume that CMA now has to be
available
> on ARMv6 and up to work at all. When you allocate from
__dma_alloc_system_pages(),
> wouldn't that necessarily fail in the dma_remap_area() stage?

It is not a fallback - I've just merged 2 cases together (CMA case and
coheren/nommu
arch). I agree that such mixed code might be confusing.

> >
> > -   if (arch_is_coherent() || nommu()) {
> > +   if (arch_is_coherent() || nommu() ||
> > +      (cma_available() && !(gfp & GFP_ATOMIC))) {
> > +           /*
> > +            * Allocate from system or CMA pages
> > +            */
> >             struct page *page = __dma_alloc_buffer(dev, size, gfp);
> >             if (!page)
> >                     return NULL;
> > +           dma_remap_area(page, size, area->prot);
> >             pfn = page_to_pfn(page);
> >             ret = page_address(page);
> 
> Similarly with coherent and nommu. It seems to me that lumping too
> many cases together creates extra complexity here.
> 
> How about something like
> 
>       if (arch_is_coherent() || nommu())
>               ret = alloc_simple_buffer();
>       else if (arch_is_v4_v5())
>               ret = alloc_remap();
>       else if (gfp & GFP_ATOMIC)
>               ret = alloc_from_pool();
>       else
>               ret = alloc_from_contiguous();
> 
> This also allows a natural conversion to dma_map_ops when we get there.

Ok. Is it ok to enable CMA permanently for ARMv6+? If CMA is left conditional
the dma pool code will be much more complicated, because it will need to support
both CMA and non-CMA cases.

> >     /* reserve any platform specific memblock areas */
> >     if (mdesc->reserve)
> >             mdesc->reserve();
> >
> > +   dma_coherent_reserve();
> > +   dma_contiguous_reserve();
> > +
> >     memblock_analyze();
> >     memblock_dump_all();
> >  }
> 
> Since we can handle most allocations using CMA on ARMv6+, I would think
> that we can have a much smaller reserved area. Have you tried changing
> dma_coherent_reserve() to allocate out of the contiguous area instead of
> wasting a full 2MB section of memory?

I will move the reserved pool directly into CMA area, so it can be shrunk below
2MiB.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to