From: benjamin gaignard benjamin.gaignard@linaro.org
The goal of those patches is to allow ION clients (drivers or userland applications) to use Contiguous Memory Allocator (CMA).
Benjamin Gaignard (3): make struct ion_device available for other heap fix ion_platform_data definition add CMA heap
drivers/gpu/ion/Kconfig | 5 + drivers/gpu/ion/Makefile | 1 + drivers/gpu/ion/cma/Makefile | 1 + drivers/gpu/ion/cma/ion_cma_heap.c | 214 ++++++++++++++++++++++++++++++++++++ drivers/gpu/ion/ion.c | 20 ---- drivers/gpu/ion/ion_priv.h | 22 ++++ include/linux/ion.h | 2 +- 7 files changed, 244 insertions(+), 21 deletions(-) create mode 100644 drivers/gpu/ion/cma/Makefile create mode 100644 drivers/gpu/ion/cma/ion_cma_heap.c
From: Benjamin Gaignard benjamin.gaignard@linaro.org
CMA heap needs to know misc device to be able to do the link between ION heap and CMA area reserved in board configuration file.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org --- drivers/gpu/ion/ion.c | 20 -------------------- drivers/gpu/ion/ion_priv.h | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/ion/ion.c b/drivers/gpu/ion/ion.c index 37b23af..dbfdd7e 100644 --- a/drivers/gpu/ion/ion.c +++ b/drivers/gpu/ion/ion.c @@ -34,26 +34,6 @@ #define DEBUG
/** - * struct ion_device - the metadata of the ion device node - * @dev: the actual misc device - * @buffers: an rb tree of all the existing buffers - * @lock: lock protecting the buffers & heaps trees - * @heaps: list of all the heaps in the system - * @user_clients: list of all the clients created from userspace - */ -struct ion_device { - struct miscdevice dev; - struct rb_root buffers; - struct mutex lock; - struct rb_root heaps; - long (*custom_ioctl) (struct ion_client *client, unsigned int cmd, - unsigned long arg); - struct rb_root user_clients; - struct rb_root kernel_clients; - struct dentry *debug_root; -}; - -/** * struct ion_client - a process/hw block local address space * @ref: for reference counting the client * @node: node in the tree of all clients diff --git a/drivers/gpu/ion/ion_priv.h b/drivers/gpu/ion/ion_priv.h index 3323954..82e44ea 100644 --- a/drivers/gpu/ion/ion_priv.h +++ b/drivers/gpu/ion/ion_priv.h @@ -22,6 +22,8 @@ #include <linux/mutex.h> #include <linux/rbtree.h> #include <linux/ion.h> +#include <linux/miscdevice.h> +#include <linux/rbtree.h>
struct ion_mapping;
@@ -38,6 +40,26 @@ struct ion_kernel_mapping { struct ion_buffer *ion_handle_buffer(struct ion_handle *handle);
/** + * struct ion_device - the metadata of the ion device node + * @dev: the actual misc device + * @buffers: an rb tree of all the existing buffers + * @lock: lock protecting the buffers & heaps trees + * @heaps: list of all the heaps in the system + * @user_clients: list of all the clients created from userspace + */ +struct ion_device { + struct miscdevice dev; + struct rb_root buffers; + struct mutex lock; + struct rb_root heaps; + long (*custom_ioctl) (struct ion_client *client, unsigned int cmd, + unsigned long arg); + struct rb_root user_clients; + struct rb_root kernel_clients; + struct dentry *debug_root; +}; + +/** * struct ion_buffer - metadata for a particular buffer * @ref: refernce count * @node: node in the ion_device buffers tree
From: Benjamin Gaignard benjamin.gaignard@linaro.org
fix ion_platform_heap to make is use an usual way in board configuration file.
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org --- include/linux/ion.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/ion.h b/include/linux/ion.h index aed8349..1468cb3 100644 --- a/include/linux/ion.h +++ b/include/linux/ion.h @@ -83,7 +83,7 @@ struct ion_platform_heap { */ struct ion_platform_data { int nr; - struct ion_platform_heap heaps[]; + struct ion_platform_heap *heaps; };
/**
From: benjamin gaignard benjamin.gaignard@linaro.org
New heap, allocation is done with dma_alloc_coherent API. device coherent_dma_mask is set to DMA_BIT_MASK(32) Heap type isn't checked but we can think that ION_HEAP_TYPE_SYSTEM_CONTIG is the correct choice
Signed-off-by: Benjamin Gaignard benjamin.gaignard@linaro.org --- drivers/gpu/ion/Kconfig | 5 + drivers/gpu/ion/Makefile | 1 + drivers/gpu/ion/cma/Makefile | 1 + drivers/gpu/ion/cma/ion_cma_heap.c | 214 ++++++++++++++++++++++++++++++++++++ 4 files changed, 221 insertions(+), 0 deletions(-) create mode 100644 drivers/gpu/ion/cma/Makefile create mode 100644 drivers/gpu/ion/cma/ion_cma_heap.c
diff --git a/drivers/gpu/ion/Kconfig b/drivers/gpu/ion/Kconfig index 5b48b4e..231dbb1 100644 --- a/drivers/gpu/ion/Kconfig +++ b/drivers/gpu/ion/Kconfig @@ -10,3 +10,8 @@ config ION_TEGRA help Choose this option if you wish to use ion on an nVidia Tegra.
+config ION_CMA + tristate "Ion CMA heap" + depends on ION && CMA + help + Choose this option to enable ION CMA heap. diff --git a/drivers/gpu/ion/Makefile b/drivers/gpu/ion/Makefile index 73fe3fa..05f174a 100644 --- a/drivers/gpu/ion/Makefile +++ b/drivers/gpu/ion/Makefile @@ -1,2 +1,3 @@ obj-$(CONFIG_ION) += ion.o ion_heap.o ion_system_heap.o ion_carveout_heap.o obj-$(CONFIG_ION_TEGRA) += tegra/ +obj-$(CONFIG_ION_CMA) += cma/ diff --git a/drivers/gpu/ion/cma/Makefile b/drivers/gpu/ion/cma/Makefile new file mode 100644 index 0000000..673508d --- /dev/null +++ b/drivers/gpu/ion/cma/Makefile @@ -0,0 +1 @@ +obj-y += ion_cma_heap.o diff --git a/drivers/gpu/ion/cma/ion_cma_heap.c b/drivers/gpu/ion/cma/ion_cma_heap.c new file mode 100644 index 0000000..79c68af --- /dev/null +++ b/drivers/gpu/ion/cma/ion_cma_heap.c @@ -0,0 +1,214 @@ +/* + * drivers/gpu/ion/ion_cma_heap.c + * + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <linux/device.h> +#include <linux/ion.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/errno.h> +#include <linux/err.h> +#include <linux/miscdevice.h> + +/* for ion_heap_ops structure */ +#include "../ion_priv.h" + +#include <linux/dma-mapping.h> + +#define ION_CMA_ALLOCATE_FAILED -1 + +static u64 dma_mask = 0xffffffffULL; + +struct ion_heap **heaps; +struct ion_device *ion_cma_device; + +struct ion_cma_buffer_info { + void *cpu_addr; + dma_addr_t handle; +}; + +/* ION CMA heap operations functions */ +int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer, + unsigned long len, unsigned long align, + unsigned long flags) +{ + struct ion_device *idev = heap->dev; + struct device *dev = idev->dev.parent; + struct ion_cma_buffer_info *info; + + dev_dbg(dev, "Request buffer allocation len %ld\n", len); + + info = kzalloc(sizeof(struct ion_cma_buffer_info), GFP_KERNEL); + if (!info) { + dev_err(dev, "Can't allocate buffer info\n"); + return ION_CMA_ALLOCATE_FAILED; + } + + info->cpu_addr = dma_alloc_coherent(dev, len, &(info->handle), 0); + + if (!info->cpu_addr) { + dev_err(dev, "Fail to allocate buffer\n"); + kfree(info); + return ION_CMA_ALLOCATE_FAILED; + } + + /* keep this for memory release */ + buffer->priv_virt = info; + dev_dbg(dev, "Allocate buffer %p\n", buffer); + return 0; +} + +void ion_cma_free(struct ion_buffer *buffer) +{ + struct ion_device *idev = buffer->dev; + struct device *dev = idev->dev.parent; + struct ion_cma_buffer_info *info = buffer->priv_virt; + + dev_dbg(dev, "Release buffer %p\n", buffer); + dma_free_coherent(dev, buffer->size, info->cpu_addr, info->handle); + kfree(info); +} + +/* return physical address in addr */ +int ion_cma_phys(struct ion_heap *heap, struct ion_buffer *buffer, + ion_phys_addr_t *addr, size_t *len) +{ + struct ion_device *idev = buffer->dev; + struct device *dev = idev->dev.parent; + struct ion_cma_buffer_info *info = buffer->priv_virt; + + dev_dbg(dev, "Return buffer %p physical address 0x%x\n", buffer, + virt_to_phys(info->cpu_addr)); + *addr = virt_to_phys(info->cpu_addr); + *len = buffer->size; + return 0; +} + +int ion_cma_mmap(struct ion_heap *mapper, struct ion_buffer *buffer, + struct vm_area_struct *vma) +{ + struct ion_device *idev = buffer->dev; + struct device *dev = idev->dev.parent; + struct ion_cma_buffer_info *info = buffer->priv_virt; + + return dma_mmap_coherent(dev, vma, info->cpu_addr, info->handle, + buffer->size); +} + +static struct ion_heap_ops ion_cma_ops = { + .allocate = ion_cma_allocate, + .free = ion_cma_free, + .phys = ion_cma_phys, + .map_user = ion_cma_mmap, +}; + +struct ion_heap *ion_cma_heap_create(struct ion_platform_heap *data) +{ + struct ion_heap *heap; + + heap = ion_heap_create(data); + if (!heap) + return NULL; + heap->ops = &ion_cma_ops; + return heap; +} + +/* ION CMA heap platform driver functions */ +int ion_cma_probe(struct platform_device *pdev) +{ + struct ion_platform_data *pdata = pdev->dev.platform_data; + int num_heaps = pdata->nr; + int i, err; + + heaps = kzalloc(sizeof(struct ion_heap *) * num_heaps, GFP_KERNEL); + + dev_info(&pdev->dev, "Create ION CMA device %p\n", &pdev->dev); + ion_cma_device = ion_device_create(NULL); + if (IS_ERR_OR_NULL(ion_cma_device)) { + kfree(heaps); + dev_err(&pdev->dev, "Can't create ION CMA device\n"); + return PTR_ERR(ion_cma_device); + } + + ion_cma_device->dev.parent = &(pdev->dev); + + /* set dma mask for this device */ + pdev->dev.dma_mask = &dma_mask; + pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32); + + /* create all the heaps specified in board configuration file */ + for (i = 0; i < num_heaps; i++) { + struct ion_platform_heap *heap_data = &pdata->heaps[i]; + + heaps[i] = ion_cma_heap_create(heap_data); + if (IS_ERR_OR_NULL(heaps[i])) { + dev_err(&pdev->dev, "Can't create heap %s\n", + heap_data->name); + err = PTR_ERR(heaps[i]); + goto err; + } + ion_device_add_heap(ion_cma_device, heaps[i]); + dev_dbg(&pdev->dev, "Add heap %s\n", heap_data->name); + } + + platform_set_drvdata(pdev, ion_cma_device); + return 0; + +err: + for (i = 0; i < num_heaps; i++) { + if (heaps[i]) + ion_heap_destroy(heaps[i]); + } + kfree(heaps); + return err; +} + +int ion_cma_remove(struct platform_device *pdev) +{ + struct ion_device *idev = platform_get_drvdata(pdev); + struct ion_platform_data *pdata = pdev->dev.platform_data; + int i, num_heaps; + + num_heaps = pdata->nr; + + /* remove ION device */ + ion_device_destroy(idev); + + /* remove all heaps create by the driver */ + for (i = 0; i < num_heaps; i++) { + if (heaps[i]) + ion_heap_destroy(heaps[i]); + } + return 0; +} + +static struct platform_driver ion_cma_driver = { + .probe = ion_cma_probe, + .remove = ion_cma_remove, + .driver = { + .name = "ion-cma"} +}; + +static int __init ion_cma_init(void) +{ + return platform_driver_register(&ion_cma_driver); +} + +static void __exit ion_cma_exit(void) +{ + platform_driver_unregister(&ion_cma_driver); +} + +module_init(ion_cma_init); +module_exit(ion_cma_exit);
On 03/01/2012 10:39 PM, Somebody in the thread at some point said:
From: benjamin gaignard benjamin.gaignard@linaro.org
New heap, allocation is done with dma_alloc_coherent API. device coherent_dma_mask is set to DMA_BIT_MASK(32)
Hi -
I don't know much about ion, but just some small style comments. However I think it's great to bind it to the standardized cma.
It might be better to repost on linaro-dev / linaro-kernel to get interest of people who do know about ion.
+#include <linux/device.h> +#include <linux/ion.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/errno.h> +#include <linux/err.h> +#include <linux/miscdevice.h>
+/* for ion_heap_ops structure */ +#include "../ion_priv.h"
+#include <linux/dma-mapping.h>
Group all the same subpath includes together (ie, put with other <linux/... )
+#define ION_CMA_ALLOCATE_FAILED -1
+static u64 dma_mask = 0xffffffffULL;
You used DMA_BIT_MASK(32) in the code, maybe here too.
+/* return physical address in addr */ +int ion_cma_phys(struct ion_heap *heap, struct ion_buffer *buffer,
ion_phys_addr_t *addr, size_t *len)
+{
- struct ion_device *idev = buffer->dev;
- struct device *dev = idev->dev.parent;
- struct ion_cma_buffer_info *info = buffer->priv_virt;
- dev_dbg(dev, "Return buffer %p physical address 0x%x\n", buffer,
virt_to_phys(info->cpu_addr));
- *addr = virt_to_phys(info->cpu_addr);
- *len = buffer->size;
I'm not sure if you're planning on sending this to mainline, if you are it's worth running it through checkpatch.pl which will tell you to add a blank line here, etc.
- return 0;
+}
+int ion_cma_mmap(struct ion_heap *mapper, struct ion_buffer *buffer,
struct vm_area_struct *vma)
...
+static struct ion_heap_ops ion_cma_ops = {
- .allocate = ion_cma_allocate,
- .free = ion_cma_free,
- .phys = ion_cma_phys,
- .map_user = ion_cma_mmap,
+};
If these functions are only used by this api table, they can be marked up as static scope at their definitions.
+struct ion_heap *ion_cma_heap_create(struct ion_platform_heap *data) +{
- struct ion_heap *heap;
- heap = ion_heap_create(data);
- if (!heap)
return NULL;
- heap->ops = &ion_cma_ops;
- return heap;
+}
Can be simplified to
if (heap) heap->ops = &ion_cma_ops;
return heap;
+/* ION CMA heap platform driver functions */ +int ion_cma_probe(struct platform_device *pdev) +{
- struct ion_platform_data *pdata = pdev->dev.platform_data;
- int num_heaps = pdata->nr;
- int i, err;
- heaps = kzalloc(sizeof(struct ion_heap *) * num_heaps, GFP_KERNEL);
- dev_info(&pdev->dev, "Create ION CMA device %p\n", &pdev->dev);
- ion_cma_device = ion_device_create(NULL);
- if (IS_ERR_OR_NULL(ion_cma_device)) {
kfree(heaps);
dev_err(&pdev->dev, "Can't create ION CMA device\n");
return PTR_ERR(ion_cma_device);
- }
- ion_cma_device->dev.parent = &(pdev->dev);
() not needed
+err:
- for (i = 0; i < num_heaps; i++) {
if (heaps[i])
ion_heap_destroy(heaps[i]);
- }
{ } are not needed
- kfree(heaps);
- return err;
+}
+int ion_cma_remove(struct platform_device *pdev) +{
- struct ion_device *idev = platform_get_drvdata(pdev);
- struct ion_platform_data *pdata = pdev->dev.platform_data;
- int i, num_heaps;
- num_heaps = pdata->nr;
- /* remove ION device */
- ion_device_destroy(idev);
- /* remove all heaps create by the driver */
- for (i = 0; i < num_heaps; i++) {
if (heaps[i])
ion_heap_destroy(heaps[i]);
- }
- return 0;
+}
{ } not needed
+static struct platform_driver ion_cma_driver = {
- .probe = ion_cma_probe,
- .remove = ion_cma_remove,
- .driver = {
.name = "ion-cma"}
} should be on next line indented
-Andy
linaro-android@lists.linaro.org