On Wed, 25 Aug 2010 22:32:37 +0200, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> wrote:
On Fri, Aug 20, 2010 at 11:50:42AM +0200, Michal Nazarewicz wrote:
The Contiguous Memory Allocator framework is a set of APIs for
allocating physically contiguous chunks of memory.
Various chips require contiguous blocks of memory to operate. Those
chips include devices such as cameras, hardware video decoders and
encoders, etc.
I am not that familiar with how StrongARM works, and I took a bit look
at the arch/arm/mach-s* and then some of the
drivers/video/media/video/cx88 to get an idea how the hardware video
decoders would work this.
What I got from this patch review is that you are writting an IOMMU
No. CMA's designed for systems without IOMMU. If system has IOMMU then
there is no need for contiguous memory blocks since all discontiguousnesses
can be hidden by the IOMMU.
that is on steroids. It essentially knows that this device and that
device can both share the same region, and it has fancy plugin system
to deal with fragmentation and offers an simple API for other to
write their own "allocators".
Dunno if the plugin system is "fancy" but essentially the above is true. ;)
Even better, during init, the sub-platform can use
cma_early_regions_reserve(<func>) to register their own function
for reserving large regions of memory. Which from my experience (with
Xen) means that there is a mechanism in place to have it setup
contingous regions using sub-platform code.
Essentially that's the idea. Platform init code adds early regions and later
on reserves memory for all of the early regions. For the former some
additional helper functions are provided which can be used.
This is how I think it works, but I am not sure if I got it right. From
looking at 'cma_alloc' and 'cma_alloc_from_region' - both return
an dma_addr_t, which is what is usually feed in the DMA API. And looking
at the cx88 driver I see it using that API..
I do understand that under ARM platform you might not have a need for
DMA at all, and you use the 'dma_addr_t' just as handles, but for
other platforms this would be used.
In the first version I've used unsigned long as return type but then it
was suggested that maybe dma_addr_t would be better. This is easily
changed at this stage so I'd be more then happy to hear any comments.
So here is the bit where I am confused. Why not have this
as Software IOMMU that would utilize the IOMMU API? There would be some
technical questions to be answered (such as, what to do when you have
another IOMMU and can you stack them on top of each other).
If I understood you correctly this is something I'm thinking about. I'm
actually thinking of ways to integrate CMA with Zach's IOMMU proposal posted
some time ago. The idea would be to define a subset of functionalities
of the IOMMU API that would work on systems with and without hardware IOMMU.
If platform had no IOMMU CMA would be used.
I'm currently trying to fully understand Zach's proposal to see how such an
approach could be pursued.
A light review below:
Thanks! Greatly appreciated.
+ * cma_alloc_from - allocates contiguous chunk of memory from named regions.
+ * @regions: Comma separated list of region names. Terminated by NUL
I think you mean 'NULL'
No, a NUL byte, ie. '\0'.
+ * byte or a semicolon.
Uh, really? Why? Why not just simplify your life and make it \0?
This is a consequence of how map is stored. It's stored as a single string
with entries separated by semicolons.
+ * The cma_allocator::alloc() operation need to set only the @start
^^- C++, eh?
Well, I'm unaware of a C way to reference "methods" so I just borrowed C++ style.
+int __init cma_early_region_reserve(struct cma_region *reg)
+{
[...]
+#ifndef CONFIG_NO_BOOTMEM
[...]
+#endif
+
+#ifdef CONFIG_HAVE_MEMBLOCK
[...]
+#endif
Those two #ifdefs are pretty ugly. What if you defined in a header
something along this:
#ifdef CONFIG_HAVE_MEMBLOCK
int __init default_early_region_reserve(struct cma_region *reg) {
.. do it using memblock
}
#endif
#ifdef CONFIG_NO_BOOTMEM
int __init default_early_region_reserve(struct cma_region *reg) {
.. do it using bootmem
}
#endif
I wanted the function to try all possible allocators. As a matter of fact,
both APIs (memblock and bootmem) can be supported at the same time.
and you would cut the API by one function, the
cma_early_regions_reserve(struct cma_region *reg)
Actually, I would prefer to leave it. It may be useful for platform
initialisation code. Especially if platform has some special regions
which are allocated in a different but for the rest wants to use the
default CMA's reserve call.
--
Best regards, _ _
| Humble Liege of Serenely Enlightened Majesty of o' \,=./ `o
| Computer Science, Michał "mina86" Nazarewicz (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--
--
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
