Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/29/18 8:25 AM, Alexey Skidanov wrote:
>
>
> On 11/29/18 3:30 AM, Laura Abbott wrote:
>> On 11/27/18 12:07 PM, Alexey Skidanov wrote:
>>>
>>>
>>> On 11/27/18 9:20 PM, Laura Abbott wrote:
On 11/26/18 10:43 AM, Alexey Skidanov wrote:
>
>
> On 11/26/18 6:39 PM, Laura Abbott wrote:
>> 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@start" kernel boot parameter.
>>>
>>> Signed-off-by: Alexey Skidanov
>>> ---
>>> 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
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/29/18 3:30 AM, Laura Abbott wrote:
> On 11/27/18 12:07 PM, Alexey Skidanov wrote:
>>
>>
>> On 11/27/18 9:20 PM, Laura Abbott wrote:
>>> On 11/26/18 10:43 AM, Alexey Skidanov wrote:
On 11/26/18 6:39 PM, Laura Abbott wrote:
> 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@start" kernel boot parameter.
>>
>> Signed-off-by: Alexey Skidanov
>> ---
>> 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
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/27/18 12:07 PM, Alexey Skidanov wrote:
On 11/27/18 9:20 PM, Laura Abbott wrote:
On 11/26/18 10:43 AM, Alexey Skidanov wrote:
On 11/26/18 6:39 PM, Laura Abbott wrote:
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@start" kernel boot parameter.
Signed-off-by: Alexey Skidanov
---
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.
I would say that we have the following steps to consider:
1. Memory reservation. The suggested solution doesn't care how it's done.
2. Per-heap information passing to the Kernel. It's different for DT and
non-DT cases.
3. Heap objects instantiation. The DT and non-DT cases have different
ways/formats to pass this per-heap information. But once the parsing is
done, the rest of the code is common.
I think it clearly defines the steps covering both cases. What do you
think?
Yes, that sounds about right.
So, in this patch step #2 - is setup_heap() and step #3 - is
ion_add_chunk_heap(). The only missing part is to support several heap
instances creation, correct?
Mostly yes. I'd like to see struct ion_platform_heap go away since
it really isn't used for anything else but we need another
way to get the reserved memory information into Ion.
Thanks,
Laura
Thanks,
Alexey
Thanks,
Alexey
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
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/27/18 9:20 PM, Laura Abbott wrote:
> On 11/26/18 10:43 AM, Alexey Skidanov wrote:
>>
>>
>> On 11/26/18 6:39 PM, Laura Abbott wrote:
>>> 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@start" kernel boot parameter.
Signed-off-by: Alexey Skidanov
---
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.
>>
>> I would say that we have the following steps to consider:
>>
>> 1. Memory reservation. The suggested solution doesn't care how it's done.
>>
>> 2. Per-heap information passing to the Kernel. It's different for DT and
>> non-DT cases.
>>
>> 3. Heap objects instantiation. The DT and non-DT cases have different
>> ways/formats to pass this per-heap informatio
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/26/18 10:43 AM, Alexey Skidanov wrote:
On 11/26/18 6:39 PM, Laura Abbott wrote:
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@start" kernel boot parameter.
Signed-off-by: Alexey Skidanov
---
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.
I would say that we have the following steps to consider:
1. Memory reservation. The suggested solution doesn't care how it's done.
2. Per-heap information passing to the Kernel. It's different for DT and
non-DT cases.
3. Heap objects instantiation. The DT and non-DT cases have different
ways/formats to pass this per-heap information. But once the parsing is
done, the rest of the code is common.
I think it clearly defines the steps covering both cases. What do you think?
Yes, that sounds about right.
Thanks,
Alexey
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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/26/18 6:39 PM, Laura Abbott wrote:
> 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@start" kernel boot parameter.
>>
>> Signed-off-by: Alexey Skidanov
>> ---
>> 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.
I would say that we have the following steps to consider:
1. Memory reservation. The suggested solution doesn't care how it's done.
2. Per-heap information passing to the Kernel. It's different for DT and
non-DT cases.
3. Heap objects instantiation. The DT and non-DT cases have different
ways/formats to pass this per-heap information. But once the parsing is
done, the rest of the code is common.
I think it clearly defines the steps covering both cases. What do you think?
Thanks,
Alexey
>
> 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.
Re: [PATCH] staging: android: ion: Add chunk heap initialization
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@start" kernel boot parameter.
Signed-off-by: Alexey Skidanov
---
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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heap initialization
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@start" kernel boot parameter.
Signed-off-by: Alexey Skidanov
---
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
?
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
>>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heap initialization
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@start" kernel boot parameter.
Signed-off-by: Alexey Skidanov
---
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
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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heap initialization
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@start" kernel boot parameter.
>>
>> Signed-off-by: Alexey Skidanov
>> ---
>> 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).
>
> 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heap initialization
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@start" kernel boot parameter.
Signed-off-by: Alexey Skidanov
---
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 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.
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.
Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: android: ion: Add chunk heap initialization
Create chunk heap of specified size and base address by adding
"ion_chunk_heap=size@start" kernel boot parameter.
Signed-off-by: Alexey Skidanov
---
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);
+
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
