On Fri, Aug 24, 2012 at 12:42 AM, Haojian Zhuang haojian.zhuang@gmail.com wrote:
On Fri, Aug 24, 2012 at 5:53 AM, Russell King - ARM Linux linux@arm.linux.org.uk wrote:
On Thu, Aug 23, 2012 at 07:49:42AM -0700, Laura Abbott wrote:
The problem is more fundamental than that. In setting up the sg_list, Ion calls phys_to_page on the carved out memory. There's no guarantee in general that the page returned is valid at all so if addr is a physical address of memory carved out with memblock_remove, page_to_phys(phys_to_page(addr)) != addr.
Internally, we've gotten around this by using the dma_address field in the sg_list to indicate carved out memory. Obviously most APIs rely on sg_lists having a page so this isn't very generic.
Really this problem boils down to the question of should the dma APIs support memory removed with memblock_remove and if so, how?
They should not, plain and simple. They're designed to be used with bits of memory which have valid struct page structures associated with them, and that's pretty fundamental to them.
However, this raises one question. You say you're using memblock_remove(). How? Show the code, I want to know.
I don't know whether the below code is helpful. And you could get kernel with ION from git://android.git.kernel.org/kernel/common.git. The branch is android-3.4. ION driver is in drivers/gpu/ion.
static void __init brownstone_ion_reserve(void) { /* reserve memory for ION */ struct ion_platform_data *p = &ion_data; int i;
for (i = 0; i < p->nr; i++) { p->heaps[i].base = memblock_alloc(p->heaps[i].size, PAGE_SIZE); if (!p->heaps[i].base) { pr_err("%s: failed to reserve %x bytes\n", __func__, p->heaps[i].size); goto err_alloc; } memblock_remove(p->heaps[i].base, p->heaps[i].size); } return;err_alloc: for (; i > 0; i--) memblock_free(p->heaps[i].base, p->heaps[i].size);
return;}
FYI I helpfully included ion_reserve in gpu/ion/ion.c so you don't have to code this yourself. Note that that function doesn't call memblock_remove, it only reserves the memory.
Now, the thing is, using memblock_remove, you are removing the memory from the system memory map, taking it away from the system. You're taking full responsibility for managing that memory. The system won't map it for you in any way.
.map_kernel() in ION carveout heap calls arm_ioremap() directly. Since we can't map memory with different mapping in ARM V6+, ioremap() blocks the behavior. If we use memblock_remove() also, ioremap() can work.
Now, since you have full control over that memory, why would you want the overhead of using the DMA API with it?
Moreover, you have one bloody big problem with this if you want to map the memory into userspace _and_ have it cacheable there, and do DMA off it. There is _no_ _way_ for userspace to issue the cache maintanence instructions (not that userspace _should_ be doing that anyway.) The cacheflush system call is also _NOT_ _DESIGNED_ for DMA purposes (it's there to support self-modifying code ONLY.)
dmabuf can be used to hide cache flush operation. ION integrates dmabuf.
While mmap() is inovked, the vma area is allocated but mapping doesn't take effect.
While user application access memory buffer, it triggers vm_fault. And the new mapping takes effect. If DMA is ready to go, .map_dma() is called that also triggers cache flushing. Once cache flush is finished, the mapping is destroyed.
So user space application needn't to maintain the cache flush operation.
The key issue is that those cache flushing is trying to use virtual address from page, like page_to_virt(). But the address is abandoned while memblock_remove() is called.
So I suggest to use memblock_alloc() only to reserve memory. And use vmap() in .map_kernel(). Otherwise, we can't avoid to use memblock_remove().
Now, next question, what's this large buffer for?
Large buffer is used for graphics driver, camera driver, graphics user application. In some silicons, DMA is required continuous physical address. We have to reserve memory by ION carveout heap.