[PATCH] staging: android: ion: Add chunk heap initialization

Laura Abbott labbott at redhat.com
Mon Nov 26 16:39:38 UTC 2018


On 11/25/18 2:02 PM, Alexey Skidanov wrote:
> 
> 
> On 11/25/18 11:40 PM, Laura Abbott wrote:
>> On 11/25/18 1:22 PM, Alexey Skidanov wrote:
>>>
>>>
>>> On 11/25/18 10:51 PM, Laura Abbott wrote:
>>>> On 11/11/18 11:29 AM, Alexey Skidanov wrote:
>>>>> Create chunk heap of specified size and base address by adding
>>>>> "ion_chunk_heap=size at start" kernel boot parameter.
>>>>>
>>>>> Signed-off-by: Alexey Skidanov <alexey.skidanov at intel.com>
>>>>> ---
>>>>>     drivers/staging/android/ion/ion_chunk_heap.c | 40
>>>>> ++++++++++++++++++++++++++++
>>>>>     1 file changed, 40 insertions(+)
>>>>>
>>>>> diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
>>>>> b/drivers/staging/android/ion/ion_chunk_heap.c
>>>>> index 159d72f..67573aa4 100644
>>>>> --- a/drivers/staging/android/ion/ion_chunk_heap.c
>>>>> +++ b/drivers/staging/android/ion/ion_chunk_heap.c
>>>>> @@ -135,6 +135,7 @@ struct ion_heap *ion_chunk_heap_create(struct
>>>>> ion_platform_heap *heap_data)
>>>>>         }
>>>>>         chunk_heap->base = heap_data->base;
>>>>>         chunk_heap->size = heap_data->size;
>>>>> +    chunk_heap->heap.name = heap_data->name;
>>>>>         chunk_heap->allocated = 0;
>>>>>           gen_pool_add(chunk_heap->pool, chunk_heap->base,
>>>>> heap_data->size, -1);
>>>>> @@ -151,3 +152,42 @@ struct ion_heap *ion_chunk_heap_create(struct
>>>>> ion_platform_heap *heap_data)
>>>>>         return ERR_PTR(ret);
>>>>>     }
>>>>>     +static u64 base;
>>>>> +static u64 size;
>>>>> +
>>>>> +static int __init setup_heap(char *param)
>>>>> +{
>>>>> +    char *p, *pp;
>>>>> +
>>>>> +    size = memparse(param, &p);
>>>>> +    if (param == p)
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    if (*p == '@')
>>>>> +        base = memparse(p + 1, &pp);
>>>>> +    else
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    if (p == pp)
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +__setup("ion_chunk_heap=", setup_heap);
>>>>> +
>>>>> +static int ion_add_chunk_heap(void)
>>>>> +{
>>>>> +    struct ion_heap *heap;
>>>>> +    struct ion_platform_heap plat_heap = {.base = base,
>>>>> +                          .size = size,
>>>>> +                          .name = "chunk_heap",
>>>>> +                          .priv = (void *)PAGE_SIZE};
>>>>> +    heap = ion_chunk_heap_create(&plat_heap);
>>>>> +    if (heap)
>>>>> +        ion_device_add_heap(heap);
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +device_initcall(ion_add_chunk_heap);
>>>>> +
>>>>>
>>>>
>>>> This solves a problem but not enough of the problem.
>>>>
>>>> We need to be able to support more than one chunk/carveout
>>>> heap.
>>> This is easy to support.
>>> This also assumes that the memory has already been
>>>> reserved/placed and that you know the base and size to
>>>> pass on the command line. Part of the issue with the carveout
>>>> heaps is that we need a way to tell the kernel to reserve
>>>> the memory early enough and then get that information to
>>>> Ion. Hard coding memory locations tends to be buggy from
>>>> my past experience with Ion.
>>> memmap= kernel option marks the memory region(s) as reserved (Zone
>>> Allocator doesn't use this memory region(s)). So the heap(s) may manage
>>> this memory region(s).
>>
>> memmap= is x86 only. I really don't like using the command line for
>> specifying the base/size as it seems likely to conflict with platforms
>> that rely on devicetree for reserving memory regions.
>>
>> Thanks,
>> Laura
>>
> I see ... So probably the better way is the one similar to this
> https://elixir.bootlin.com/linux/latest/source/kernel/dma/contiguous.c#L245
> ?
> 

Correct. For platforms that need devicetree, we need a way to specify
that a region should become an Ion heap. I went through a similar
exercise with CMA heaps before I kind of gave up on figuring out a
binding and just had Ion enumerate all CMA heaps. We do still need
a solution to work with non-DT platforms as well so whatever we
come up with needs to plausibly work for both cases. Your solution
would cover the non-DT case but I'd really like to make sure we
at least have a path forward for the devicetree case as well.

Thanks,
Laura

> Thanks,
> Alexey
> 
>>>>
>>>> If you'd like to see about coming up with a complete solution,
>>>> feel free to resubmit but I'm still strongly considering
>>>> removing these heaps.
>>>>
>>> I will add the multiple heaps support and resubmit the patch
>>>> Thanks,
>>>> Laura
>>> Thanks,
>>> Alexey
>>>
>>



More information about the devel mailing list