Heap ids need not be a power of 2. The ids need to be unique and maximum 16 heaps can co-exist. From the source code, my inference is that heap_mask passed at allocation time is a bitmask of heap ids.
You are right that the heap id and heap type gets mixed up. The ion_debug_heap_total, client create heap_mask relies on heap type. The ion_debug_client_show and allocation heap_mask relies on heap id. I would prefer all of them using heap id instead of heap type. I had earlier submitted a patch to update ion_debug_heap_total() to use heap id instead of heap type. But, did not see any response. :)
On Tue, Dec 11, 2012 at 6:20 PM, Johan Mossberg johan.mossberg@stericsson.com wrote:
I see what you mean. In my opinion it is very hard to understand that it works this way. heap_mask means one thing in ion_client_create and another thing in ion_alloc. I can't find any information about heap ids being a power of two either. We should probably consider updating the documentation and variable names to make it more clear how this works.
On 12/11/2012 12:50 PM, Nishanth Peethambaran wrote:
Consider having two carveout/cma heaps partitioned into different banks of SDRAM. Say, the heap->id iof carveout0 is 0 and heap->id of carveout1 is 1. But both would have the heap->type as ION_HEAP_TYPE_CARVEOUT (2)
Userspace ion client would be created with a heap_mask of -1 (any heap type is ok). At allocation time, user process can pass heap_mask as 1 to allocate from carveout0; heap_mask as 2 to allocate from carveout1; heap_mask as 3 to try allocating from carveout0 first and then carveout1 next;. If the check is based on heap->type, you would miss that flexibiility.
On Tue, Dec 11, 2012 at 4:34 PM, Johan Mossberg johan.mossberg@stericsson.com wrote:
On 12/11/2012 10:55 AM, Nishanth Peethambaran wrote:
Johan,
It should be heap->id only. You can always have two heaps with different ids, but of the same type. For eg: two CMA heaps or two carveout heaps. User may control which heap should be used for buffer allocation by setting appropriate bits in the heap_mask.
I don't understand, please elaborate. Using the heap id here makes no sense to me. We are trying to check if the heap matches the supplied heap type mask, just like we checked if the heap matched the client's heap type mask in the line before.
- Nishanth Peethambaran +91-9448074166
On Tue, Dec 11, 2012 at 2:21 PM, Johan Mossberg johan.mossberg@stericsson.com wrote:
The heap id is compared against the heap mask in ion_alloc which is incorrect, the heap type should be used.
Signed-off-by: Johan Mossberg johan.mossberg@stericsson.com
drivers/gpu/ion/ion.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/ion/ion.c b/drivers/gpu/ion/ion.c index fc152b9..c811380f 100644 --- a/drivers/gpu/ion/ion.c +++ b/drivers/gpu/ion/ion.c @@ -414,7 +414,7 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len, if (!((1 << heap->type) & client->heap_mask)) continue; /* if the caller didn't specify this heap type */
if (!((1 << heap->id) & heap_mask))
if (!((1 << heap->type) & heap_mask)) continue; buffer = ion_buffer_create(heap, dev, len, align, flags); if (!IS_ERR_OR_NULL(buffer))-- 1.8.0
Linaro-mm-sig mailing list Linaro-mm-sig@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-mm-sig
- Nishanth Peethambaran
- Nishanth Peethambaran