linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] add ION driver for STIh4xx SoC
@ 2016-10-26 13:32 Benjamin Gaignard
  2016-10-26 13:32 ` [PATCH 1/3] add binding for STIh4xx ION driver Benjamin Gaignard
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Benjamin Gaignard @ 2016-10-26 13:32 UTC (permalink / raw)
  To: labbott, sumit.semwal, gregkh, yudongbin, puck.chen, linux-kernel
  Cc: linaro-kernel, kernel, Benjamin Gaignard

It is more or less a copy of Hisilicon driver but with a heap definition
fitting with STIH4xx SoC needs.
I have just chnage the some function prefix from "hi6220" to "sti".

Benjamin Gaignard (3):
  add binding for STIh4xx ION driver
  add STIH4xx ION driver
  add STIH4xx ION driver in DT

 .../devicetree/bindings/staging/ion/st,sti-ion.txt |  14 +++
 arch/arm/boot/dts/stih407-family.dtsi              |   7 ++
 drivers/staging/android/ion/Kconfig                |   7 ++
 drivers/staging/android/ion/Makefile               |   2 +
 drivers/staging/android/ion/sti/Makefile           |   1 +
 drivers/staging/android/ion/sti/sti_ion.c          | 103 +++++++++++++++++++++
 6 files changed, 134 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/staging/ion/st,sti-ion.txt
 create mode 100644 drivers/staging/android/ion/sti/Makefile
 create mode 100644 drivers/staging/android/ion/sti/sti_ion.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/3] add binding for STIh4xx ION driver
  2016-10-26 13:32 [PATCH 0/3] add ION driver for STIh4xx SoC Benjamin Gaignard
@ 2016-10-26 13:32 ` Benjamin Gaignard
  2016-10-26 13:32 ` [PATCH 2/3] add STIH4xx " Benjamin Gaignard
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Benjamin Gaignard @ 2016-10-26 13:32 UTC (permalink / raw)
  To: labbott, sumit.semwal, gregkh, yudongbin, puck.chen, linux-kernel
  Cc: linaro-kernel, kernel, Benjamin Gaignard

Describe bindings for STIH4xx ION driver.
They are limited to have is used by ion_of.c

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 .../devicetree/bindings/staging/ion/st,sti-ion.txt         | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/staging/ion/st,sti-ion.txt

diff --git a/Documentation/devicetree/bindings/staging/ion/st,sti-ion.txt b/Documentation/devicetree/bindings/staging/ion/st,sti-ion.txt
new file mode 100644
index 0000000..0fde5ae
--- /dev/null
+++ b/Documentation/devicetree/bindings/staging/ion/st,sti-ion.txt
@@ -0,0 +1,14 @@
+STIH4xxSoC ION
+
+Required properties:
+- compatible: "st,sti-ion"
+- list of ION heaps:
+	- compatible: "st,cma"
+
+Example:
+	sti_ion {
+		compatible = "st,sti-ion";
+		ion_cma {
+			compatible = "st,cma";
+		};
+	};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/3] add STIH4xx ION driver
  2016-10-26 13:32 [PATCH 0/3] add ION driver for STIh4xx SoC Benjamin Gaignard
  2016-10-26 13:32 ` [PATCH 1/3] add binding for STIh4xx ION driver Benjamin Gaignard
@ 2016-10-26 13:32 ` Benjamin Gaignard
  2016-10-26 13:32 ` [PATCH 3/3] add STIH4xx ION driver in DT Benjamin Gaignard
  2016-10-26 13:51 ` [PATCH 0/3] add ION driver for STIh4xx SoC Sumit Semwal
  3 siblings, 0 replies; 13+ messages in thread
From: Benjamin Gaignard @ 2016-10-26 13:32 UTC (permalink / raw)
  To: labbott, sumit.semwal, gregkh, yudongbin, puck.chen, linux-kernel
  Cc: linaro-kernel, kernel, Benjamin Gaignard

This the same driver that Hisilicon one.
Change the list of heap to fit with sti SoC needs.
Rename/prefix some functions and structure with sti

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 drivers/staging/android/ion/Kconfig       |   7 ++
 drivers/staging/android/ion/Makefile      |   2 +
 drivers/staging/android/ion/sti/Makefile  |   1 +
 drivers/staging/android/ion/sti/sti_ion.c | 103 ++++++++++++++++++++++++++++++
 4 files changed, 113 insertions(+)
 create mode 100644 drivers/staging/android/ion/sti/Makefile
 create mode 100644 drivers/staging/android/ion/sti/sti_ion.c

diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig
index c8fb413..eb47230 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -42,6 +42,13 @@ config ION_HISI
 
 source "drivers/staging/android/ion/hisilicon/Kconfig"
 
+config ION_STI
+        bool "stih4xx ION Driver"
+        depends on ARCH_STI && ION
+	select ION_OF
+        help
+          Choose this option if you wish to use ion on STIH4xx SoC.
+
 config ION_OF
 	bool "Devicetree support for Ion"
 	depends on ION && OF_ADDRESS
diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile
index 5d630a0..bc02ea4 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -9,5 +9,7 @@ endif
 obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o
 obj-$(CONFIG_ION_TEGRA) += tegra/
 obj-$(CONFIG_ION_HISI) += hisilicon/
+obj-$(CONFIG_ION_STI) += sti/
+
 obj-$(CONFIG_ION_OF) += ion_of.o
 
diff --git a/drivers/staging/android/ion/sti/Makefile b/drivers/staging/android/ion/sti/Makefile
new file mode 100644
index 0000000..5690431
--- /dev/null
+++ b/drivers/staging/android/ion/sti/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_ION_STI) += sti_ion.o
diff --git a/drivers/staging/android/ion/sti/sti_ion.c b/drivers/staging/android/ion/sti/sti_ion.c
new file mode 100644
index 0000000..4c0f52b
--- /dev/null
+++ b/drivers/staging/android/ion/sti/sti_ion.c
@@ -0,0 +1,103 @@
+/*
+ * STIH4xx ION Driver
+ *
+ * Copyright (c) 2016 STMicroelectronics
+ *
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/err.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include "../ion_priv.h"
+#include "../ion.h"
+#include "../ion_of.h"
+
+struct sti_ion_dev {
+	struct ion_heap	**heaps;
+	struct ion_device *idev;
+	struct ion_platform_data *data;
+};
+
+static struct ion_of_heap sti_heaps[] = {
+	PLATFORM_HEAP("st,cma", 0, ION_HEAP_TYPE_DMA, "cma"),
+	{}
+};
+
+static int sti_ion_probe(struct platform_device *pdev)
+{
+	struct sti_ion_dev *ipdev;
+	int i;
+
+	ipdev = devm_kzalloc(&pdev->dev, sizeof(*ipdev), GFP_KERNEL);
+	if (!ipdev)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, ipdev);
+
+	ipdev->idev = ion_device_create(NULL);
+	if (IS_ERR(ipdev->idev))
+		return PTR_ERR(ipdev->idev);
+
+	ipdev->data = ion_parse_dt(pdev, sti_heaps);
+	if (IS_ERR(ipdev->data))
+		return PTR_ERR(ipdev->data);
+
+	ipdev->heaps = devm_kzalloc(&pdev->dev,
+				sizeof(struct ion_heap) * ipdev->data->nr,
+				GFP_KERNEL);
+	if (!ipdev->heaps) {
+		ion_destroy_platform_data(ipdev->data);
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < ipdev->data->nr; i++) {
+		ipdev->heaps[i] = ion_heap_create(&ipdev->data->heaps[i]);
+		if (!ipdev->heaps) {
+			ion_destroy_platform_data(ipdev->data);
+			return -ENOMEM;
+		}
+		ion_device_add_heap(ipdev->idev, ipdev->heaps[i]);
+	}
+	return 0;
+}
+
+static int sti_ion_remove(struct platform_device *pdev)
+{
+	struct sti_ion_dev *ipdev;
+	int i;
+
+	ipdev = platform_get_drvdata(pdev);
+
+	for (i = 0; i < ipdev->data->nr; i++)
+		ion_heap_destroy(ipdev->heaps[i]);
+
+	ion_destroy_platform_data(ipdev->data);
+	ion_device_destroy(ipdev->idev);
+
+	return 0;
+}
+
+static const struct of_device_id sti_ion_match_table[] = {
+	{.compatible = "st,sti-ion"},
+	{},
+};
+
+static struct platform_driver sti_ion_driver = {
+	.probe = sti_ion_probe,
+	.remove = sti_ion_remove,
+	.driver = {
+		.name = "ion-sti",
+		.of_match_table = sti_ion_match_table,
+	},
+};
+
+module_platform_driver(sti_ion_driver);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/3] add STIH4xx ION driver in DT
  2016-10-26 13:32 [PATCH 0/3] add ION driver for STIh4xx SoC Benjamin Gaignard
  2016-10-26 13:32 ` [PATCH 1/3] add binding for STIh4xx ION driver Benjamin Gaignard
  2016-10-26 13:32 ` [PATCH 2/3] add STIH4xx " Benjamin Gaignard
@ 2016-10-26 13:32 ` Benjamin Gaignard
  2016-10-26 13:51 ` [PATCH 0/3] add ION driver for STIh4xx SoC Sumit Semwal
  3 siblings, 0 replies; 13+ messages in thread
From: Benjamin Gaignard @ 2016-10-26 13:32 UTC (permalink / raw)
  To: labbott, sumit.semwal, gregkh, yudongbin, puck.chen, linux-kernel
  Cc: linaro-kernel, kernel, Benjamin Gaignard

Very simple and limited to one heap for contiguous memory allocation

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 arch/arm/boot/dts/stih407-family.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index 91096a4..7bc1f17 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -1007,4 +1007,11 @@
 			status = "disabled";
 		};
 	};
+
+	sti_ion {
+		compatible = "st,sti-ion";
+		ion_cma {
+			compatible = "st,cma";
+		};
+	};
 };
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] add ION driver for STIh4xx SoC
  2016-10-26 13:32 [PATCH 0/3] add ION driver for STIh4xx SoC Benjamin Gaignard
                   ` (2 preceding siblings ...)
  2016-10-26 13:32 ` [PATCH 3/3] add STIH4xx ION driver in DT Benjamin Gaignard
@ 2016-10-26 13:51 ` Sumit Semwal
  2016-10-26 14:41   ` Benjamin Gaignard
  3 siblings, 1 reply; 13+ messages in thread
From: Sumit Semwal @ 2016-10-26 13:51 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Laura Abbott, Greg Kroah-Hartman, yudongbin, Chen Feng, LKML,
	linaro-kernel, kernel

Hello Benjamin,

On 26 October 2016 at 19:02, Benjamin Gaignard
<benjamin.gaignard@linaro.org> wrote:
> It is more or less a copy of Hisilicon driver but with a heap definition
> fitting with STIH4xx SoC needs.
> I have just chnage the some function prefix from "hi6220" to "sti".
>
Thanks for your patches!

I was just wondering if you couldn't convert the HiSilicon driver into
something like a 'simple-ion' driver, and have just the DT definitions
as specifics? This would save a lot of code duplication, and keep it
as a simple interface for common heaps like cma.

If there are any ST-specific requirements that are incompatible with
the existing driver, it should be clearly documented out here I think.

> Benjamin Gaignard (3):
>   add binding for STIh4xx ION driver
>   add STIH4xx ION driver
>   add STIH4xx ION driver in DT
>
>  .../devicetree/bindings/staging/ion/st,sti-ion.txt |  14 +++
>  arch/arm/boot/dts/stih407-family.dtsi              |   7 ++
>  drivers/staging/android/ion/Kconfig                |   7 ++
>  drivers/staging/android/ion/Makefile               |   2 +
>  drivers/staging/android/ion/sti/Makefile           |   1 +
>  drivers/staging/android/ion/sti/sti_ion.c          | 103 +++++++++++++++++++++
>  6 files changed, 134 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/staging/ion/st,sti-ion.txt
>  create mode 100644 drivers/staging/android/ion/sti/Makefile
>  create mode 100644 drivers/staging/android/ion/sti/sti_ion.c
>
> --
> 1.9.1
>

Best regards,
Sumit.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] add ION driver for STIh4xx SoC
  2016-10-26 13:51 ` [PATCH 0/3] add ION driver for STIh4xx SoC Sumit Semwal
@ 2016-10-26 14:41   ` Benjamin Gaignard
  2016-10-26 14:44     ` Sumit Semwal
  0 siblings, 1 reply; 13+ messages in thread
From: Benjamin Gaignard @ 2016-10-26 14:41 UTC (permalink / raw)
  To: Sumit Semwal
  Cc: Laura Abbott, Greg Kroah-Hartman, yudongbin, Chen Feng, LKML,
	linaro-kernel, kernel

2016-10-26 15:51 GMT+02:00 Sumit Semwal <sumit.semwal@linaro.org>:
> Hello Benjamin,
>
> On 26 October 2016 at 19:02, Benjamin Gaignard
> <benjamin.gaignard@linaro.org> wrote:
>> It is more or less a copy of Hisilicon driver but with a heap definition
>> fitting with STIH4xx SoC needs.
>> I have just chnage the some function prefix from "hi6220" to "sti".
>>
> Thanks for your patches!
>
> I was just wondering if you couldn't convert the HiSilicon driver into
> something like a 'simple-ion' driver, and have just the DT definitions
> as specifics? This would save a lot of code duplication, and keep it
> as a simple interface for common heaps like cma.

Create a simple-ion driver is a good idea but it means that heaps
(configuration, name, etc..)
will have to be describe into devicetree. I'm not sure if that will is
acceptable.

>
> If there are any ST-specific requirements that are incompatible with
> the existing driver, it should be clearly documented out here I think.

heaps names and Ids aren't the same so I can't reuse hisilicon driver.


-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] add ION driver for STIh4xx SoC
  2016-10-26 14:41   ` Benjamin Gaignard
@ 2016-10-26 14:44     ` Sumit Semwal
  2016-10-26 15:05       ` Benjamin Gaignard
  0 siblings, 1 reply; 13+ messages in thread
From: Sumit Semwal @ 2016-10-26 14:44 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Laura Abbott, Greg Kroah-Hartman, yudongbin, Chen Feng, LKML,
	linaro-kernel, kernel

On 26 October 2016 at 20:11, Benjamin Gaignard
<benjamin.gaignard@linaro.org> wrote:
> 2016-10-26 15:51 GMT+02:00 Sumit Semwal <sumit.semwal@linaro.org>:
>> Hello Benjamin,
>>
>> On 26 October 2016 at 19:02, Benjamin Gaignard
>> <benjamin.gaignard@linaro.org> wrote:
>>> It is more or less a copy of Hisilicon driver but with a heap definition
>>> fitting with STIH4xx SoC needs.
>>> I have just chnage the some function prefix from "hi6220" to "sti".
>>>
>> Thanks for your patches!
>>
>> I was just wondering if you couldn't convert the HiSilicon driver into
>> something like a 'simple-ion' driver, and have just the DT definitions
>> as specifics? This would save a lot of code duplication, and keep it
>> as a simple interface for common heaps like cma.
>
> Create a simple-ion driver is a good idea but it means that heaps
> (configuration, name, etc..)
> will have to be describe into devicetree. I'm not sure if that will is
> acceptable.
>
>>
>> If there are any ST-specific requirements that are incompatible with
>> the existing driver, it should be clearly documented out here I think.
>
> heaps names and Ids aren't the same so I can't reuse hisilicon driver.
>
But I'd suspect both these are solvable with using something like
'generic,cma' instead of 'hisi,cma' or 'st,cma'?
>
Best,
Sumit.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] add ION driver for STIh4xx SoC
  2016-10-26 14:44     ` Sumit Semwal
@ 2016-10-26 15:05       ` Benjamin Gaignard
  2016-10-26 23:25         ` Laura Abbott
  0 siblings, 1 reply; 13+ messages in thread
From: Benjamin Gaignard @ 2016-10-26 15:05 UTC (permalink / raw)
  To: Sumit Semwal
  Cc: Laura Abbott, Greg Kroah-Hartman, yudongbin, Chen Feng, LKML,
	linaro-kernel, kernel

2016-10-26 16:44 GMT+02:00 Sumit Semwal <sumit.semwal@linaro.org>:
> On 26 October 2016 at 20:11, Benjamin Gaignard
> <benjamin.gaignard@linaro.org> wrote:
>> 2016-10-26 15:51 GMT+02:00 Sumit Semwal <sumit.semwal@linaro.org>:
>>> Hello Benjamin,
>>>
>>> On 26 October 2016 at 19:02, Benjamin Gaignard
>>> <benjamin.gaignard@linaro.org> wrote:
>>>> It is more or less a copy of Hisilicon driver but with a heap definition
>>>> fitting with STIH4xx SoC needs.
>>>> I have just chnage the some function prefix from "hi6220" to "sti".
>>>>
>>> Thanks for your patches!
>>>
>>> I was just wondering if you couldn't convert the HiSilicon driver into
>>> something like a 'simple-ion' driver, and have just the DT definitions
>>> as specifics? This would save a lot of code duplication, and keep it
>>> as a simple interface for common heaps like cma.
>>
>> Create a simple-ion driver is a good idea but it means that heaps
>> (configuration, name, etc..)
>> will have to be describe into devicetree. I'm not sure if that will is
>> acceptable.
>>
>>>
>>> If there are any ST-specific requirements that are incompatible with
>>> the existing driver, it should be clearly documented out here I think.
>>
>> heaps names and Ids aren't the same so I can't reuse hisilicon driver.
>>
> But I'd suspect both these are solvable with using something like
> 'generic,cma' instead of 'hisi,cma' or 'st,cma'?

yes, but it requires to describe the heaps in devicetree.
Hisilicon driver was doing like that until last month but Laura
convert it to common platform:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/staging/android/ion/hisilicon?id=b6e336dbeda85585c3ba6d935753d8240e18baf1

I bet she got good reasons do to that so I have implemented sti ION
driver in this mindset.

>>
> Best,
> Sumit.



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] add ION driver for STIh4xx SoC
  2016-10-26 15:05       ` Benjamin Gaignard
@ 2016-10-26 23:25         ` Laura Abbott
  2016-11-07 10:17           ` Benjamin Gaignard
  0 siblings, 1 reply; 13+ messages in thread
From: Laura Abbott @ 2016-10-26 23:25 UTC (permalink / raw)
  To: Benjamin Gaignard, Sumit Semwal
  Cc: Greg Kroah-Hartman, yudongbin, Chen Feng, LKML, linaro-kernel, kernel

On 10/26/2016 08:05 AM, Benjamin Gaignard wrote:
> 2016-10-26 16:44 GMT+02:00 Sumit Semwal <sumit.semwal@linaro.org>:
>> On 26 October 2016 at 20:11, Benjamin Gaignard
>> <benjamin.gaignard@linaro.org> wrote:
>>> 2016-10-26 15:51 GMT+02:00 Sumit Semwal <sumit.semwal@linaro.org>:
>>>> Hello Benjamin,
>>>>
>>>> On 26 October 2016 at 19:02, Benjamin Gaignard
>>>> <benjamin.gaignard@linaro.org> wrote:
>>>>> It is more or less a copy of Hisilicon driver but with a heap definition
>>>>> fitting with STIH4xx SoC needs.
>>>>> I have just chnage the some function prefix from "hi6220" to "sti".
>>>>>
>>>> Thanks for your patches!
>>>>
>>>> I was just wondering if you couldn't convert the HiSilicon driver into
>>>> something like a 'simple-ion' driver, and have just the DT definitions
>>>> as specifics? This would save a lot of code duplication, and keep it
>>>> as a simple interface for common heaps like cma.
>>>
>>> Create a simple-ion driver is a good idea but it means that heaps
>>> (configuration, name, etc..)
>>> will have to be describe into devicetree. I'm not sure if that will is
>>> acceptable.
>>>
>>>>
>>>> If there are any ST-specific requirements that are incompatible with
>>>> the existing driver, it should be clearly documented out here I think.
>>>
>>> heaps names and Ids aren't the same so I can't reuse hisilicon driver.
>>>
>> But I'd suspect both these are solvable with using something like
>> 'generic,cma' instead of 'hisi,cma' or 'st,cma'?
>
> yes, but it requires to describe the heaps in devicetree.
> Hisilicon driver was doing like that until last month but Laura
> convert it to common platform:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/staging/android/ion/hisilicon?id=b6e336dbeda85585c3ba6d935753d8240e18baf1
>
> I bet she got good reasons do to that so I have implemented sti ION
> driver in this mindset.
>

I agree that having common code would be useful. There are some
generic bindings listed in drivers/staging/android/ion/devicetree.txt
so we could go with linux,ion-heap-dma.

The changes got merged and there were never mailing list objections
but that's because the devicetree maintainers got busy and never
actually looked at them and Arnd at least still didn't like the
idea of Ion in devicetree. I'd like to wait until after plumbers
next week to decide what to do.



>>>
>> Best,
>> Sumit.
>
>
>

Thanks,
Laura

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] add ION driver for STIh4xx SoC
  2016-10-26 23:25         ` Laura Abbott
@ 2016-11-07 10:17           ` Benjamin Gaignard
  2016-11-07 22:17             ` Laura Abbott
  0 siblings, 1 reply; 13+ messages in thread
From: Benjamin Gaignard @ 2016-11-07 10:17 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Sumit Semwal, Greg Kroah-Hartman, yudongbin, Chen Feng, LKML,
	linaro-kernel, kernel

Hello Laura,

What are plumbers outputs for ION after your talk ?

Regards,
Benjamin

2016-10-27 1:25 GMT+02:00 Laura Abbott <labbott@redhat.com>:
> On 10/26/2016 08:05 AM, Benjamin Gaignard wrote:
>>
>> 2016-10-26 16:44 GMT+02:00 Sumit Semwal <sumit.semwal@linaro.org>:
>>>
>>> On 26 October 2016 at 20:11, Benjamin Gaignard
>>> <benjamin.gaignard@linaro.org> wrote:
>>>>
>>>> 2016-10-26 15:51 GMT+02:00 Sumit Semwal <sumit.semwal@linaro.org>:
>>>>>
>>>>> Hello Benjamin,
>>>>>
>>>>> On 26 October 2016 at 19:02, Benjamin Gaignard
>>>>> <benjamin.gaignard@linaro.org> wrote:
>>>>>>
>>>>>> It is more or less a copy of Hisilicon driver but with a heap
>>>>>> definition
>>>>>> fitting with STIH4xx SoC needs.
>>>>>> I have just chnage the some function prefix from "hi6220" to "sti".
>>>>>>
>>>>> Thanks for your patches!
>>>>>
>>>>> I was just wondering if you couldn't convert the HiSilicon driver into
>>>>> something like a 'simple-ion' driver, and have just the DT definitions
>>>>> as specifics? This would save a lot of code duplication, and keep it
>>>>> as a simple interface for common heaps like cma.
>>>>
>>>>
>>>> Create a simple-ion driver is a good idea but it means that heaps
>>>> (configuration, name, etc..)
>>>> will have to be describe into devicetree. I'm not sure if that will is
>>>> acceptable.
>>>>
>>>>>
>>>>> If there are any ST-specific requirements that are incompatible with
>>>>> the existing driver, it should be clearly documented out here I think.
>>>>
>>>>
>>>> heaps names and Ids aren't the same so I can't reuse hisilicon driver.
>>>>
>>> But I'd suspect both these are solvable with using something like
>>> 'generic,cma' instead of 'hisi,cma' or 'st,cma'?
>>
>>
>> yes, but it requires to describe the heaps in devicetree.
>> Hisilicon driver was doing like that until last month but Laura
>> convert it to common platform:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/staging/android/ion/hisilicon?id=b6e336dbeda85585c3ba6d935753d8240e18baf1
>>
>> I bet she got good reasons do to that so I have implemented sti ION
>> driver in this mindset.
>>
>
> I agree that having common code would be useful. There are some
> generic bindings listed in drivers/staging/android/ion/devicetree.txt
> so we could go with linux,ion-heap-dma.
>
> The changes got merged and there were never mailing list objections
> but that's because the devicetree maintainers got busy and never
> actually looked at them and Arnd at least still didn't like the
> idea of Ion in devicetree. I'd like to wait until after plumbers
> next week to decide what to do.
>
>
>
>>>>
>>> Best,
>>> Sumit.
>>
>>
>>
>>
>
> Thanks,
> Laura



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] add ION driver for STIh4xx SoC
  2016-11-07 10:17           ` Benjamin Gaignard
@ 2016-11-07 22:17             ` Laura Abbott
  2016-11-08  9:18               ` Benjamin Gaignard
  0 siblings, 1 reply; 13+ messages in thread
From: Laura Abbott @ 2016-11-07 22:17 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Sumit Semwal, Greg Kroah-Hartman, yudongbin, Chen Feng, LKML,
	linaro-kernel, kernel

On 11/07/2016 02:17 AM, Benjamin Gaignard wrote:
> Hello Laura,
>
> What are plumbers outputs for ION after your talk ?
>
> Regards,
> Benjamin
>

We agreed to suspend work on new major Ion features and work
towards a more generic design being pushed by the graphics
team (see https://github.com/cubanismo/allocator). Platform/
devicetree support is more interesting because it will still
be needed for a more generic design but the existing
bindings still aren't acceptable to the DT maintainers. I
don't see much value in trying to convert another board to
a set of bindings that aren't actually acceptable or even
refactor existing code.

The objections to the bindings are that they are still too
linux specific and not platform specific enough. Most of
the reason why I went with this proposal had to do with
getting a device for CMA, otherwise there isn't much of a need
for anything in devicetree. I'll be giving this more thought,
if you have your own ideas, please feel free to submit them
for review.

Thanks,
Laura


> 2016-10-27 1:25 GMT+02:00 Laura Abbott <labbott@redhat.com>:
>> On 10/26/2016 08:05 AM, Benjamin Gaignard wrote:
>>>
>>> 2016-10-26 16:44 GMT+02:00 Sumit Semwal <sumit.semwal@linaro.org>:
>>>>
>>>> On 26 October 2016 at 20:11, Benjamin Gaignard
>>>> <benjamin.gaignard@linaro.org> wrote:
>>>>>
>>>>> 2016-10-26 15:51 GMT+02:00 Sumit Semwal <sumit.semwal@linaro.org>:
>>>>>>
>>>>>> Hello Benjamin,
>>>>>>
>>>>>> On 26 October 2016 at 19:02, Benjamin Gaignard
>>>>>> <benjamin.gaignard@linaro.org> wrote:
>>>>>>>
>>>>>>> It is more or less a copy of Hisilicon driver but with a heap
>>>>>>> definition
>>>>>>> fitting with STIH4xx SoC needs.
>>>>>>> I have just chnage the some function prefix from "hi6220" to "sti".
>>>>>>>
>>>>>> Thanks for your patches!
>>>>>>
>>>>>> I was just wondering if you couldn't convert the HiSilicon driver into
>>>>>> something like a 'simple-ion' driver, and have just the DT definitions
>>>>>> as specifics? This would save a lot of code duplication, and keep it
>>>>>> as a simple interface for common heaps like cma.
>>>>>
>>>>>
>>>>> Create a simple-ion driver is a good idea but it means that heaps
>>>>> (configuration, name, etc..)
>>>>> will have to be describe into devicetree. I'm not sure if that will is
>>>>> acceptable.
>>>>>
>>>>>>
>>>>>> If there are any ST-specific requirements that are incompatible with
>>>>>> the existing driver, it should be clearly documented out here I think.
>>>>>
>>>>>
>>>>> heaps names and Ids aren't the same so I can't reuse hisilicon driver.
>>>>>
>>>> But I'd suspect both these are solvable with using something like
>>>> 'generic,cma' instead of 'hisi,cma' or 'st,cma'?
>>>
>>>
>>> yes, but it requires to describe the heaps in devicetree.
>>> Hisilicon driver was doing like that until last month but Laura
>>> convert it to common platform:
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/staging/android/ion/hisilicon?id=b6e336dbeda85585c3ba6d935753d8240e18baf1
>>>
>>> I bet she got good reasons do to that so I have implemented sti ION
>>> driver in this mindset.
>>>
>>
>> I agree that having common code would be useful. There are some
>> generic bindings listed in drivers/staging/android/ion/devicetree.txt
>> so we could go with linux,ion-heap-dma.
>>
>> The changes got merged and there were never mailing list objections
>> but that's because the devicetree maintainers got busy and never
>> actually looked at them and Arnd at least still didn't like the
>> idea of Ion in devicetree. I'd like to wait until after plumbers
>> next week to decide what to do.
>>
>>
>>
>>>>>
>>>> Best,
>>>> Sumit.
>>>
>>>
>>>
>>>
>>
>> Thanks,
>> Laura
>
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] add ION driver for STIh4xx SoC
  2016-11-07 22:17             ` Laura Abbott
@ 2016-11-08  9:18               ` Benjamin Gaignard
  2016-11-09  4:49                 ` Laura Abbott
  0 siblings, 1 reply; 13+ messages in thread
From: Benjamin Gaignard @ 2016-11-08  9:18 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Sumit Semwal, Greg Kroah-Hartman, yudongbin, Chen Feng, LKML,
	linaro-kernel, kernel

Ok so no more dev on ION but can we add ION drivers  like hisilicon does ?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] add ION driver for STIh4xx SoC
  2016-11-08  9:18               ` Benjamin Gaignard
@ 2016-11-09  4:49                 ` Laura Abbott
  0 siblings, 0 replies; 13+ messages in thread
From: Laura Abbott @ 2016-11-09  4:49 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Sumit Semwal, Greg Kroah-Hartman, yudongbin, Chen Feng, LKML,
	linaro-kernel, kernel

On 11/08/2016 01:18 AM, Benjamin Gaignard wrote:
> Ok so no more dev on ION but can we add ION drivers  like hisilicon does ?
> 

If we can agree upon bindings, yes I think it would make sense
to have more platform support. Ideally, it would translate over
to newer features as well.

Thanks,
Laura

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-11-09  4:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-26 13:32 [PATCH 0/3] add ION driver for STIh4xx SoC Benjamin Gaignard
2016-10-26 13:32 ` [PATCH 1/3] add binding for STIh4xx ION driver Benjamin Gaignard
2016-10-26 13:32 ` [PATCH 2/3] add STIH4xx " Benjamin Gaignard
2016-10-26 13:32 ` [PATCH 3/3] add STIH4xx ION driver in DT Benjamin Gaignard
2016-10-26 13:51 ` [PATCH 0/3] add ION driver for STIh4xx SoC Sumit Semwal
2016-10-26 14:41   ` Benjamin Gaignard
2016-10-26 14:44     ` Sumit Semwal
2016-10-26 15:05       ` Benjamin Gaignard
2016-10-26 23:25         ` Laura Abbott
2016-11-07 10:17           ` Benjamin Gaignard
2016-11-07 22:17             ` Laura Abbott
2016-11-08  9:18               ` Benjamin Gaignard
2016-11-09  4:49                 ` Laura Abbott

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).