On 1/3/19 12:37 AM, Laura Abbott wrote: > On 12/20/18 1:29 PM, Alexey Skidanov wrote: >> >> >> On 12/20/18 10:36 PM, Laura Abbott wrote: >>> On 12/16/18 2:46 AM, Alexey Skidanov wrote: >>>> Chunk heap instantiation should be supported for device tree platforms >>>> and >>>> non device tree platforms. For device tree platforms, it's a platform >>>> specific code responsibility to retrieve the heap configuration data >>>> and to call the appropriate API for heap creation. For non device tree >>>> platforms, there is no defined way to create the heaps. >>>> >>>> This patch provides the way of chunk heaps creation using >>>> "ion_chunk_heap=name:size@start" kernel boot parameter. >>>> >>> >>> I've been thinking about this and I think it works but >>> I'm still kind of torn about not having devicetree bindings. >>> It doesn't _preclude_ devicetree bindings but I'd hate to >>> merge this and then end up with something incompatible. >>> I do want to support non-Android use cases too. >> Sorry, what do you mean by non-Android cases? > > Any user of Ion that isn't tied to Android. This includes other > userspace frameworks as well as non-devicetree targets. > Sorry, don't follow you ... I tested the patch on Ubuntu machine - so the non-Android and non-devicetree case is obviously supported :) >>> >>> I'm also curious about the value of this heap with just PAGE_SIZE. >>> The original purpose of the chunk heap was a carveout where >>> you could easily get allocations large than PAGE_SIZE to >>> reduce TLB pressure. Do you have another use case for the >>> chunk heap? >> It need to be fixed ... Obviously ... The minimum allocation size should >> be parametrized >>> >>> Sumit, do you have any thoughts? >>> >>> Thanks, >>> Laura >>> >>>> Link: >>>> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/128495.html >>>> >>>> >>>> Signed-off-by: Alexey Skidanov <alexey.skidanov@xxxxxxxxx> >>>> --- >>>> drivers/staging/android/ion/ion_chunk_heap.c | 96 >>>> +++++++++++++++++++++++++--- >>>> include/linux/ion.h | 18 ++++++ >>>> 2 files changed, 105 insertions(+), 9 deletions(-) >>>> create mode 100644 include/linux/ion.h >>>> >>>> diff --git a/drivers/staging/android/ion/ion_chunk_heap.c >>>> b/drivers/staging/android/ion/ion_chunk_heap.c >>>> index 102c093..1a8e3ad 100644 >>>> --- a/drivers/staging/android/ion/ion_chunk_heap.c >>>> +++ b/drivers/staging/android/ion/ion_chunk_heap.c >>>> @@ -21,8 +21,12 @@ >>>> #include <linux/scatterlist.h> >>>> #include <linux/slab.h> >>>> #include <linux/vmalloc.h> >>>> +#include <linux/ion.h> >>>> #include "ion.h" >>>> +static struct ion_chunk_heap_cfg >>>> chunk_heap_cfg[MAX_NUM_OF_CHUNK_HEAPS]; >>>> +static unsigned int num_of_req_chunk_heaps; >>>> + >>>> struct ion_chunk_heap { >>>> struct ion_heap heap; >>>> struct gen_pool *pool; >>>> @@ -117,15 +121,15 @@ static struct ion_heap_ops chunk_heap_ops = { >>>> .unmap_kernel = ion_heap_unmap_kernel, >>>> }; >>>> -struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap >>>> *heap_data) >>>> +static struct ion_heap *ion_chunk_heap_create(struct >>>> ion_chunk_heap_cfg *heap_cfg) >>>> { >>>> struct ion_chunk_heap *chunk_heap; >>>> int ret; >>>> struct page *page; >>>> size_t size; >>>> - page = pfn_to_page(PFN_DOWN(heap_data->base)); >>>> - size = heap_data->size; >>>> + page = pfn_to_page(PFN_DOWN(heap_cfg->base)); >>>> + size = heap_cfg->size; >>>> ret = ion_heap_pages_zero(page, size, >>>> pgprot_writecombine(PAGE_KERNEL)); >>>> if (ret) >>>> @@ -135,23 +139,27 @@ struct ion_heap *ion_chunk_heap_create(struct >>>> ion_platform_heap *heap_data) >>>> if (!chunk_heap) >>>> return ERR_PTR(-ENOMEM); >>>> - chunk_heap->chunk_size = (unsigned long)heap_data->priv; >>>> + chunk_heap->chunk_size = heap_cfg->chunk_size; >>>> chunk_heap->pool = >>>> gen_pool_create(get_order(chunk_heap->chunk_size) + >>>> PAGE_SHIFT, -1); >>>> if (!chunk_heap->pool) { >>>> ret = -ENOMEM; >>>> goto error_gen_pool_create; >>>> } >>>> - chunk_heap->base = heap_data->base; >>>> - chunk_heap->size = heap_data->size; >>>> + chunk_heap->base = heap_cfg->base; >>>> + chunk_heap->size = heap_cfg->size; >>>> + chunk_heap->heap.name = heap_cfg->heap_name; >>>> chunk_heap->allocated = 0; >>>> - gen_pool_add(chunk_heap->pool, chunk_heap->base, >>>> heap_data->size, -1); >>>> + gen_pool_add(chunk_heap->pool, chunk_heap->base, heap_cfg->size, >>>> -1); >>>> chunk_heap->heap.ops = &chunk_heap_ops; >>>> chunk_heap->heap.type = ION_HEAP_TYPE_CHUNK; >>>> chunk_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE; >>>> - pr_debug("%s: base %pa size %zu\n", __func__, >>>> - &chunk_heap->base, heap_data->size); >>>> + >>>> + pr_info("%s: name %s base %pa size %zu\n", __func__, >>>> + heap_cfg->heap_name, >>>> + &heap_cfg->base, >>>> + heap_cfg->size); >>>> return &chunk_heap->heap; >>>> @@ -160,3 +168,73 @@ struct ion_heap *ion_chunk_heap_create(struct >>>> ion_platform_heap *heap_data) >>>> return ERR_PTR(ret); >>>> } >>>> +static int __init setup_heap(char *param) >>>> +{ >>>> + char *at_sign, *coma, *colon; >>>> + size_t size_to_copy; >>>> + struct ion_chunk_heap_cfg *cfg; >>>> + >>>> + do { >>>> + cfg = &chunk_heap_cfg[num_of_req_chunk_heaps]; >>>> + >>>> + /* heap name */ >>>> + colon = strchr(param, ':'); >>>> + if (!colon) >>>> + return -EINVAL; >>>> + >>>> + size_to_copy = min_t(size_t, MAX_CHUNK_HEAP_NAME_SIZE - 1, >>>> + (colon - param)); >>>> + strncpy(cfg->heap_name, param, size_to_copy); >>>> + cfg->heap_name[size_to_copy] = '\0'; >>>> + >>>> + /* heap size */ >>>> + cfg->size = memparse((colon + 1), &at_sign); >>>> + if ((colon + 1) == at_sign) >>>> + return -EINVAL; >>>> + >>>> + /* heap base addr */ >>>> + if (*at_sign == '@') >>>> + cfg->base = memparse(at_sign + 1, &coma); >>>> + else >>>> + return -EINVAL; >>>> + >>>> + if (at_sign == coma) >>>> + return -EINVAL; >>>> + >>>> + /* Chunk size */ >>>> + cfg->chunk_size = PAGE_SIZE; >>>> + >>>> + num_of_req_chunk_heaps++; >>>> + >>>> + /* if one more heap configuration exists */ >>>> + if (*coma == ',') >>>> + param = coma + 1; >>>> + else >>>> + param = NULL; >>>> + } while (num_of_req_chunk_heaps < MAX_NUM_OF_CHUNK_HEAPS && >>>> param); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +__setup("ion_chunk_heap=", setup_heap); >>>> + >>>> +int ion_add_chunk_heaps(struct ion_chunk_heap_cfg *cfg, >>>> + unsigned int num_of_heaps) >>>> +{ >>>> + unsigned int i; >>>> + struct ion_heap *heap; >>>> + >>>> + for (i = 0; i < num_of_heaps; i++) { >>>> + heap = ion_chunk_heap_create(&cfg[i]); >>>> + if (heap) >>>> + ion_device_add_heap(heap); >>>> + } >>>> + return 0; >>>> +} >>>> + >>>> +static int ion_add_chunk_heaps_from_boot_param(void) >>>> +{ >>>> + return ion_add_chunk_heaps(chunk_heap_cfg, >>>> num_of_req_chunk_heaps); >>>> +} >>>> + >>>> +device_initcall(ion_add_chunk_heaps_from_boot_param); >>>> diff --git a/include/linux/ion.h b/include/linux/ion.h >>>> new file mode 100644 >>>> index 0000000..60296c9 >>>> --- /dev/null >>>> +++ b/include/linux/ion.h >>>> @@ -0,0 +1,18 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>> +#ifndef _LINUX_ION_H >>>> +#define _LINUX_ION_H >>>> + >>>> +#define MAX_NUM_OF_CHUNK_HEAPS 32 >>>> +#define MAX_CHUNK_HEAP_NAME_SIZE 32 >>>> + >>>> +struct ion_chunk_heap_cfg { >>>> + char heap_name[MAX_CHUNK_HEAP_NAME_SIZE]; >>>> + phys_addr_t base; >>>> + size_t size; >>>> + size_t chunk_size; >>>> +}; >>>> + >>>> +int ion_add_chunk_heaps(struct ion_chunk_heap_cfg *cfg, >>>> + unsigned int num_of_heaps); >>>> + >>>> +#endif /* _LINUX_ION_H */ >>>> >>> > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel