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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
