[PATCH v5 0/5] Add ARM Mali Midgard device tree bindings and gpu node for rk3288

[PATCH v5 0/5] Add ARM Mali Midgard device tree bindings and gpu node for rk3288

[Guillaume Tucker]

The ARM Mali Midgard GPU kernel driver is only available
out-of-tree and is not going to be merged in its current form.
However, it would be useful to have its device tree bindings
merged.  In particular, this would enable distributions to create
working driver packages (dkms...) without having to patch the
kernel.

The bindings for the earlier Mali Utgard GPU family have already
been merged, so this is essentially the same scenario but for
newer GPUs (Mali-T604 ~ Mali-T880).

This series of patches first imports the bindings from the latest
driver release with some clean-up then adds a gpu node for the
rk3288 SoC.  This was successfully tested on Radxa Rock2 Square,
Firefly, Veyron Minnie and Jerry boards using Mali kernel driver
r16p0 and r12p0 user-space binary.


Changes since v1:
- enabled gpu on rk3288-veyron boards

Changes since v2:
- removed "clk-names" property and "clk_mali" name
- converted values of "interrupt-names" property to
  lower-case: "job", "mmu" and "gpu"
- replaced dt compatible strings with list of all Midgard GPU variants and
  optional vendors
- cleaned up gpu node example

Changes since v3:
- add "rockchip,rk3288-mali" vendor compatible string
- move gpu node at the right location in rk3288.dtsi
- use operating-points-v2 in documentation and rk3288.dtsi

Changes since v4:
- removed wildcards (mali-t60x, -t62x)
- vendor compatible strings are not optional any more
- example updated accordingly, based on rk3288


Enric Balletbo i Serra (1):
  ARM: dts: rockchip: enable ARM Mali GPU on rk3288-veyron

Guillaume Tucker (4):
  dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU
  ARM: dts: rockchip: add ARM Mali GPU node for rk3288
  ARM: dts: rockchip: enable ARM Mali GPU on rk3288-rock2-som
  ARM: dts: rockchip: enable ARM Mali GPU on rk3288-firefly

 .../devicetree/bindings/gpu/arm,mali-midgard.txt   | 86 ++++++++++++++++++++++
 arch/arm/boot/dts/rk3288-firefly.dtsi              |  5 ++
 arch/arm/boot/dts/rk3288-rock2-som.dtsi            |  5 ++
 arch/arm/boot/dts/rk3288-veyron.dtsi               |  5 ++
 arch/arm/boot/dts/rk3288.dtsi                      | 43 +++++++++++
 5 files changed, 144 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt

--
2.11.0

[PATCH v5 1/5] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU

[Guillaume Tucker]

The ARM Mali Midgard GPU family is present in a number of SoCs
from many different vendors such as Samsung Exynos and Rockchip.

Import the device tree bindings documentation from the r16p0
release of the Mali Midgard GPU kernel driver:

  https://developer.arm.com/-/media/Files/downloads/mali-drivers/kernel/mali-midgard-gpu/TX011-SW-99002-r16p0-00rel0.tgz

Remove the copyright and GPL licence header as deemed not necessary.

Redesign the "compatible" property strings to list all the Mali
Midgard GPU types and add vendor specific ones.

Drop the "clock-names" property as the Mali Midgard GPU uses only one
clock (the driver now needs to call clk_get with NULL).

Convert the "interrupt-names" property values to lower-case: "job",
"mmu" and "gpu".

Replace the deprecated "operating-points" optional property with
"operating-points-v2".

Omit the following optional properties in this initial version as they
are only used in very specific cases:

  * snoop_enable_smc
  * snoop_disable_smc
  * jm_config
  * power_model
  * system-coherency
  * ipa-model

Update the example accordingly to reflect all these changes, based on
rk3288 mali-t760.

CC: John Reitan <john.reitan@arm.com>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
 .../devicetree/bindings/gpu/arm,mali-midgard.txt   | 86 ++++++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
new file mode 100644
index 000000000000..d3b6e1a4713a
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
@@ -0,0 +1,86 @@
+ARM Mali Midgard GPU
+====================
+
+Required properties:
+
+- compatible :
+  * Must contain one of the following:
+    + "arm,mali-t604"
+    + "arm,mali-t624"
+    + "arm,mali-t628"
+    + "arm,mali-t720"
+    + "arm,mali-t760"
+    + "arm,mali-t820"
+    + "arm,mali-t830"
+    + "arm,mali-t860"
+    + "arm,mali-t880"
+  * which must be preceded by one of the following vendor specifics:
+    + "amlogic,meson-gxm-mali"
+    + "rockchip,rk3288-mali"
+
+- reg : Physical base address of the device and length of the register area.
+
+- interrupts : Contains the three IRQ lines required by Mali Midgard devices.
+
+- interrupt-names : Contains the names of IRQ resources in the order they were
+  provided in the interrupts property. Must contain: "job", "mmu", "gpu".
+
+
+Optional properties:
+
+- clocks : Phandle to clock for the Mali Midgard device.
+
+- mali-supply : Phandle to regulator for the Mali device. Refer to
+  Documentation/devicetree/bindings/regulator/regulator.txt for details.
+
+- operating-points-v2 : Refer to Documentation/devicetree/bindings/power/opp.txt
+  for details.
+
+
+Example for a Mali-T760:
+
+gpu@ffa30000 {
+	compatible = "rockchip,rk3288-mali", "arm,mali-t760", "arm,mali-midgard";
+	reg = <0xffa30000 0x10000>;
+	interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
+	interrupt-names = "job", "mmu", "gpu";
+	clocks = <&cru ACLK_GPU>;
+	mali-supply = <&vdd_gpu>;
+	operating-points-v2 = <&gpu_opp_table>;
+	power-domains = <&power RK3288_PD_GPU>;
+};
+
+gpu_opp_table: opp_table0 {
+	compatible = "operating-points-v2";
+
+	opp@533000000 {
+		opp-hz = /bits/ 64 <533000000>;
+		opp-microvolt = <1250000>;
+	};
+	opp@450000000 {
+		opp-hz = /bits/ 64 <450000000>;
+		opp-microvolt = <1150000>;
+	};
+	opp@400000000 {
+		opp-hz = /bits/ 64 <400000000>;
+		opp-microvolt = <1125000>;
+	};
+	opp@350000000 {
+		opp-hz = /bits/ 64 <350000000>;
+		opp-microvolt = <1075000>;
+	};
+	opp@266000000 {
+		opp-hz = /bits/ 64 <266000000>;
+		opp-microvolt = <1025000>;
+	};
+	opp@160000000 {
+		opp-hz = /bits/ 64 <160000000>;
+		opp-microvolt = <925000>;
+	};
+	opp@100000000 {
+		opp-hz = /bits/ 64 <100000000>;
+		opp-microvolt = <912500>;
+	};
+};
-- 
2.11.0

[PATCH v5 2/5] ARM: dts: rockchip: add ARM Mali GPU node for rk3288

[Guillaume Tucker]

Add Mali GPU device tree node for the rk3288 SoC, with devfreq
opp table.

Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
 arch/arm/boot/dts/rk3288.dtsi | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index df8a0dbe9d91..35969041eae2 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -43,6 +43,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/pinctrl/rockchip.h>
 #include <dt-bindings/clock/rk3288-cru.h>
+#include <dt-bindings/power/rk3288-power.h>
 #include <dt-bindings/thermal/thermal.h>
 #include <dt-bindings/power/rk3288-power.h>
 #include <dt-bindings/soc/rockchip,boot-mode.h>
@@ -1117,6 +1118,48 @@
 		};
 	};
 
+	gpu: mali@ffa30000 {
+		compatible = "rockchip,rk3288-mali", "arm,mali-t760", "arm,mali-midgard";
+		reg = <0xffa30000 0x10000>;
+		interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "job", "mmu", "gpu";
+		clocks = <&cru ACLK_GPU>;
+		operating-points-v2 = <&gpu_opp_table>;
+		power-domains = <&power RK3288_PD_GPU>;
+		status = "disabled";
+	};
+
+	gpu_opp_table: opp_table0 {
+		compatible = "operating-points-v2";
+
+		opp@100000000 {
+			opp-hz = /bits/ 64 <100000000>;
+			opp-microvolt = <950000>;
+		};
+		opp@200000000 {
+			opp-hz = /bits/ 64 <200000000>;
+			opp-microvolt = <950000>;
+		};
+		opp@300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			opp-microvolt = <1000000>;
+		};
+		opp@400000000 {
+			opp-hz = /bits/ 64 <400000000>;
+			opp-microvolt = <1100000>;
+		};
+		opp@500000000 {
+			opp-hz = /bits/ 64 <500000000>;
+			opp-microvolt = <1200000>;
+		};
+		opp@600000000 {
+			opp-hz = /bits/ 64 <600000000>;
+			opp-microvolt = <1250000>;
+		};
+	};
+
 	qos_gpu_r: qos@ffaa0000 {
 		compatible = "syscon";
 		reg = <0xffaa0000 0x20>;
-- 
2.11.0

[PATCH v5 3/5] ARM: dts: rockchip: enable ARM Mali GPU on rk3288-rock2-som

[Guillaume Tucker]

Add reference to the Mali GPU device tree node on the
rk3288-rock2-som platform.  Tested on a Radxa Rock2 Square board.

Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
 arch/arm/boot/dts/rk3288-rock2-som.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-rock2-som.dtsi b/arch/arm/boot/dts/rk3288-rock2-som.dtsi
index 1c0bbc9b928b..f694867fa46a 100644
--- a/arch/arm/boot/dts/rk3288-rock2-som.dtsi
+++ b/arch/arm/boot/dts/rk3288-rock2-som.dtsi
@@ -301,3 +301,8 @@
 &wdt {
 	status = "okay";
 };
+
+&gpu {
+	mali-supply = <&vdd_gpu>;
+	status = "okay";
+};
-- 
2.11.0

[PATCH v5 4/5] ARM: dts: rockchip: enable ARM Mali GPU on rk3288-firefly

[Guillaume Tucker]

Add reference to the Mali GPU device tree node on rk3288-firefly.
Tested on Firefly board.

Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
 arch/arm/boot/dts/rk3288-firefly.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-firefly.dtsi b/arch/arm/boot/dts/rk3288-firefly.dtsi
index 10793ac18599..f520589493b4 100644
--- a/arch/arm/boot/dts/rk3288-firefly.dtsi
+++ b/arch/arm/boot/dts/rk3288-firefly.dtsi
@@ -594,3 +594,8 @@
 &wdt {
 	status = "okay";
 };
+
+&gpu {
+	mali-supply = <&vdd_gpu>;
+	status = "okay";
+};
-- 
2.11.0

[PATCH v5 5/5] ARM: dts: rockchip: enable ARM Mali GPU on rk3288-veyron

[Guillaume Tucker]

From: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Add reference to the Mali GPU device tree node on rk3288-veyron.
Tested on Minnie and Jerry boards.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
 arch/arm/boot/dts/rk3288-veyron.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
index 5d1eb0a25827..9847d5c6db3b 100644
--- a/arch/arm/boot/dts/rk3288-veyron.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
@@ -447,6 +447,11 @@
 	status = "okay";
 };
 
+&gpu {
+	mali-supply = <&vdd_gpu>;
+	status = "okay";
+};
+
 &wdt {
 	status = "okay";
 };
-- 
2.11.0

Re: [PATCH v5 1/5] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU

[Rob Herring]

On Wed, May 03, 2017 at 10:56:25AM +0100, Guillaume Tucker wrote:
> The ARM Mali Midgard GPU family is present in a number of SoCs
> from many different vendors such as Samsung Exynos and Rockchip.
> 
> Import the device tree bindings documentation from the r16p0
> release of the Mali Midgard GPU kernel driver:
> 
>   https://developer.arm.com/-/media/Files/downloads/mali-drivers/kernel/mali-midgard-gpu/TX011-SW-99002-r16p0-00rel0.tgz
> 
> Remove the copyright and GPL licence header as deemed not necessary.
> 
> Redesign the "compatible" property strings to list all the Mali
> Midgard GPU types and add vendor specific ones.
> 
> Drop the "clock-names" property as the Mali Midgard GPU uses only one
> clock (the driver now needs to call clk_get with NULL).
> 
> Convert the "interrupt-names" property values to lower-case: "job",
> "mmu" and "gpu".
> 
> Replace the deprecated "operating-points" optional property with
> "operating-points-v2".
> 
> Omit the following optional properties in this initial version as they
> are only used in very specific cases:
> 
>   * snoop_enable_smc
>   * snoop_disable_smc
>   * jm_config
>   * power_model
>   * system-coherency
>   * ipa-model
> 
> Update the example accordingly to reflect all these changes, based on
> rk3288 mali-t760.
> 
> CC: John Reitan <john.reitan@arm.com>
> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---
>  .../devicetree/bindings/gpu/arm,mali-midgard.txt   | 86 ++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 0/5] Add ARM Mali Midgard device tree bindings and gpu node for rk3288

[Heiko Stübner]

Am Mittwoch, 3. Mai 2017, 10:56:24 CEST schrieb Guillaume Tucker:
> The ARM Mali Midgard GPU kernel driver is only available
> out-of-tree and is not going to be merged in its current form.
> However, it would be useful to have its device tree bindings
> merged.  In particular, this would enable distributions to create
> working driver packages (dkms...) without having to patch the
> kernel.
> 
> The bindings for the earlier Mali Utgard GPU family have already
> been merged, so this is essentially the same scenario but for
> newer GPUs (Mali-T604 ~ Mali-T880).
> 
> This series of patches first imports the bindings from the latest
> driver release with some clean-up then adds a gpu node for the
> rk3288 SoC.  This was successfully tested on Radxa Rock2 Square,
> Firefly, Veyron Minnie and Jerry boards using Mali kernel driver
> r16p0 and r12p0 user-space binary.

The actual devicetree parts are all Rockchip-specific, so I guess I'll just
pick up the whole series, including the binding doc, after the merge
window if nobody complains before that :-)


Heiko

Re: [PATCH v5 0/5] Add ARM Mali Midgard device tree bindings and gpu node for rk3288

[Randy Li]

On 05/09/2017 12:27 AM, Heiko Stübner wrote:
> Am Mittwoch, 3. Mai 2017, 10:56:24 CEST schrieb Guillaume Tucker:
>> The ARM Mali Midgard GPU kernel driver is only available
>> out-of-tree and is not going to be merged in its current form.
>> However, it would be useful to have its device tree bindings
>> merged.  In particular, this would enable distributions to create
>> working driver packages (dkms...) without having to patch the
>> kernel.
>>
>> The bindings for the earlier Mali Utgard GPU family have already
>> been merged, so this is essentially the same scenario but for
>> newer GPUs (Mali-T604 ~ Mali-T880).
>>
>> This series of patches first imports the bindings from the latest
>> driver release with some clean-up then adds a gpu node for the
>> rk3288 SoC.  This was successfully tested on Radxa Rock2 Square,
>> Firefly, Veyron Minnie and Jerry boards using Mali kernel driver
>> r16p0 and r12p0 user-space binary.
I won't suggest such combine. We meet some problems at mali 400 serial.
I would suggest the kernel version would match the user library.
Also please notice there is rk3288w, the hardware version becomes r1p0.
> 
> The actual devicetree parts are all Rockchip-specific, so I guess I'll just
> pick up the whole series, including the binding doc, after the merge
> window if nobody complains before that :-)
> 
> 
> Heiko
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 

-- 
Randy Li

Re: [PATCH v5 0/5] Add ARM Mali Midgard device tree bindings and gpu node for rk3288

[Guillaume Tucker]

On 09/05/17 02:16, Randy Li wrote:
>
>
> On 05/09/2017 12:27 AM, Heiko Stübner wrote:
>> Am Mittwoch, 3. Mai 2017, 10:56:24 CEST schrieb Guillaume Tucker:
>>> The ARM Mali Midgard GPU kernel driver is only available
>>> out-of-tree and is not going to be merged in its current form.
>>> However, it would be useful to have its device tree bindings
>>> merged.  In particular, this would enable distributions to create
>>> working driver packages (dkms...) without having to patch the
>>> kernel.
>>>
>>> The bindings for the earlier Mali Utgard GPU family have already
>>> been merged, so this is essentially the same scenario but for
>>> newer GPUs (Mali-T604 ~ Mali-T880).
>>>
>>> This series of patches first imports the bindings from the latest
>>> driver release with some clean-up then adds a gpu node for the
>>> rk3288 SoC.  This was successfully tested on Radxa Rock2 Square,
>>> Firefly, Veyron Minnie and Jerry boards using Mali kernel driver
>>> r16p0 and r12p0 user-space binary.
> I won't suggest such combine. We meet some problems at mali 400 serial.
> I would suggest the kernel version would match the user library.

Well, I can test it again with r12p0 kernel driver (out-of-tree)
if you want.  The user-space driver checks the version of the
kernel driver and gives up if it's not compatible.  With Midgard,
there's a range of versions that maintain kernel/userspace
compatibility unlike Utgard and older Midgard releases where they
had to exactly match.  Again, if there was a mismatch then the
user-space would fail to initialise and report an error.

> Also please notice there is rk3288w, the hardware version becomes r1p0.

Sounds like a new SoC?  Does rk3288w affect rk3288 in any way?

Unless it's a special case, it seems to me that any new SoC with
a Midgard GPU would need an extra vendor compatible string in the
binding documentation and maybe a separate gpu dt node.

>> The actual devicetree parts are all Rockchip-specific, so I guess I'll just
>> pick up the whole series, including the binding doc, after the merge
>> window if nobody complains before that :-)

Thanks!


Guillaume

Re: [PATCH v5 0/5] Add ARM Mali Midgard device tree bindings and gpu node for rk3288

[Randy Li]

On 05/09/2017 03:40 PM, Guillaume Tucker wrote:
> On 09/05/17 02:16, Randy Li wrote:
>>
>>
>> On 05/09/2017 12:27 AM, Heiko Stübner wrote:
>>> Am Mittwoch, 3. Mai 2017, 10:56:24 CEST schrieb Guillaume Tucker:
>>>> The ARM Mali Midgard GPU kernel driver is only available
>>>> out-of-tree and is not going to be merged in its current form.
>>>> However, it would be useful to have its device tree bindings
>>>> merged.  In particular, this would enable distributions to create
>>>> working driver packages (dkms...) without having to patch the
>>>> kernel.
>>>>
>>>> The bindings for the earlier Mali Utgard GPU family have already
>>>> been merged, so this is essentially the same scenario but for
>>>> newer GPUs (Mali-T604 ~ Mali-T880).
>>>>
>>>> This series of patches first imports the bindings from the latest
>>>> driver release with some clean-up then adds a gpu node for the
>>>> rk3288 SoC.  This was successfully tested on Radxa Rock2 Square,
>>>> Firefly, Veyron Minnie and Jerry boards using Mali kernel driver
>>>> r16p0 and r12p0 user-space binary.
>> I won't suggest such combine. We meet some problems at mali 400 serial.
>> I would suggest the kernel version would match the user library.
> 
> Well, I can test it again with r12p0 kernel driver (out-of-tree)
> if you want.  The user-space driver checks the version of the
> kernel driver and gives up if it's not compatible.  With Midgard,
> there's a range of versions that maintain kernel/userspace
> compatibility unlike Utgard and older Midgard releases where they
> had to exactly match.  Again, if there was a mismatch then the
> user-space would fail to initialise and report an error.
Anyway, could you verify the mali userspace library r13p0?
Rockchip would use this version to offer the support of X11, wayland and 
gbm.
> 
>> Also please notice there is rk3288w, the hardware version becomes r1p0.
> 
> Sounds like a new SoC?  Does rk3288w affect rk3288 in any way?
It is not a new SoC, just a new version of rk3288. It fixes a various of 
the chip bug of the rk3288.
> 
> Unless it's a special case, it seems to me that any new SoC with
> a Midgard GPU would need an extra vendor compatible string in the
> binding documentation and maybe a separate gpu dt node.
> 
>>> The actual devicetree parts are all Rockchip-specific, so I guess 
>>> I'll just
>>> pick up the whole series, including the binding doc, after the merge
>>> window if nobody complains before that :-)
> 
> Thanks!
> 
> 
> Guillaume
> 
> 

-- 
Randy Li

Re: [PATCH v5 0/5] Add ARM Mali Midgard device tree bindings and gpu node for rk3288

[Guillaume Tucker]

Hi Randi,

On 09/05/17 08:45, Randy Li wrote:
>
>
> On 05/09/2017 03:40 PM, Guillaume Tucker wrote:
>> On 09/05/17 02:16, Randy Li wrote:
>>>
>>>
>>> On 05/09/2017 12:27 AM, Heiko Stübner wrote:
>>>> Am Mittwoch, 3. Mai 2017, 10:56:24 CEST schrieb Guillaume Tucker:
>>>>> The ARM Mali Midgard GPU kernel driver is only available
>>>>> out-of-tree and is not going to be merged in its current form.
>>>>> However, it would be useful to have its device tree bindings
>>>>> merged.  In particular, this would enable distributions to create
>>>>> working driver packages (dkms...) without having to patch the
>>>>> kernel.
>>>>>
>>>>> The bindings for the earlier Mali Utgard GPU family have already
>>>>> been merged, so this is essentially the same scenario but for
>>>>> newer GPUs (Mali-T604 ~ Mali-T880).
>>>>>
>>>>> This series of patches first imports the bindings from the latest
>>>>> driver release with some clean-up then adds a gpu node for the
>>>>> rk3288 SoC.  This was successfully tested on Radxa Rock2 Square,
>>>>> Firefly, Veyron Minnie and Jerry boards using Mali kernel driver
>>>>> r16p0 and r12p0 user-space binary.
>>> I won't suggest such combine. We meet some problems at mali 400 serial.
>>> I would suggest the kernel version would match the user library.
>>
>> Well, I can test it again with r12p0 kernel driver (out-of-tree)
>> if you want.  The user-space driver checks the version of the
>> kernel driver and gives up if it's not compatible.  With Midgard,
>> there's a range of versions that maintain kernel/userspace
>> compatibility unlike Utgard and older Midgard releases where they
>> had to exactly match.  Again, if there was a mismatch then the
>> user-space would fail to initialise and report an error.

> Anyway, could you verify the mali userspace library r13p0?

The latest user-space binary available from developer.arm.com for
rk3288 is 12p0.  Maybe Rockchip could make some new binaries
available to the public?

> Rockchip would use this version to offer the support of X11, wayland and gbm.

Sounds great, although windowing systems are quite far away from
GPU device tree bindings so I don't think this is too relevant
for this current patch series.

On a side note, it would be fantastic to get all this available
in Debian packages :)

>>> Also please notice there is rk3288w, the hardware version becomes r1p0.
>>
>> Sounds like a new SoC?  Does rk3288w affect rk3288 in any way?
> It is not a new SoC, just a new version of rk3288. It fixes a various of the chip bug of the rk3288.

I see.  I don't have access to any rk3288w platform but if you do
then I guess it would be nice if you could test these patches on
it :)  Also I'm not sure if all rk3288 device trees changes must
be tested on rk3288w, and this SoC is not mentioned anywhere in
the kernel (as far as I can tell).

Is there anything you think should block these current patches
which only claim to support rk3288 from being merged?

>> Unless it's a special case, it seems to me that any new SoC with
>> a Midgard GPU would need an extra vendor compatible string in the
>> binding documentation and maybe a separate gpu dt node.
>>
>>>> The actual devicetree parts are all Rockchip-specific, so I guess I'll just
>>>> pick up the whole series, including the binding doc, after the merge
>>>> window if nobody complains before that :-)
>>
>> Thanks!

Guillaume


P.S. kernel branch with a Mali driver I used for testing:
https://git.collabora.com/cgit/user/gtucker/linux.git/log/?h=linux-4.11-mali-dt-rk3288

Re: [PATCH v5 0/5] Add ARM Mali Midgard device tree bindings and gpu node for rk3288

[Heiko Stübner]

Am Mittwoch, 10. Mai 2017, 11:10:55 CEST schrieb Guillaume Tucker:
> Hi Randi,
> 
> On 09/05/17 08:45, Randy Li wrote:
> > On 05/09/2017 03:40 PM, Guillaume Tucker wrote:
> >> On 09/05/17 02:16, Randy Li wrote:
> >>> On 05/09/2017 12:27 AM, Heiko Stübner wrote:
> >>>> Am Mittwoch, 3. Mai 2017, 10:56:24 CEST schrieb Guillaume Tucker:
> >>>>> The ARM Mali Midgard GPU kernel driver is only available
> >>>>> out-of-tree and is not going to be merged in its current form.
> >>>>> However, it would be useful to have its device tree bindings
> >>>>> merged.  In particular, this would enable distributions to create
> >>>>> working driver packages (dkms...) without having to patch the
> >>>>> kernel.
> >>>>> 
> >>>>> The bindings for the earlier Mali Utgard GPU family have already
> >>>>> been merged, so this is essentially the same scenario but for
> >>>>> newer GPUs (Mali-T604 ~ Mali-T880).
> >>>>> 
> >>>>> This series of patches first imports the bindings from the latest
> >>>>> driver release with some clean-up then adds a gpu node for the
> >>>>> rk3288 SoC.  This was successfully tested on Radxa Rock2 Square,
> >>>>> Firefly, Veyron Minnie and Jerry boards using Mali kernel driver
> >>>>> r16p0 and r12p0 user-space binary.
> >>> 
> >>> I won't suggest such combine. We meet some problems at mali 400 serial.
> >>> I would suggest the kernel version would match the user library.
> >> 
> >> Well, I can test it again with r12p0 kernel driver (out-of-tree)
> >> if you want.  The user-space driver checks the version of the
> >> kernel driver and gives up if it's not compatible.  With Midgard,
> >> there's a range of versions that maintain kernel/userspace
> >> compatibility unlike Utgard and older Midgard releases where they
> >> had to exactly match.  Again, if there was a mismatch then the
> >> user-space would fail to initialise and report an error.
> > 
> > Anyway, could you verify the mali userspace library r13p0?
> 
> The latest user-space binary available from developer.arm.com for
> rk3288 is 12p0.  Maybe Rockchip could make some new binaries
> available to the public?
> 
> > Rockchip would use this version to offer the support of X11, wayland and
> > gbm.
> Sounds great, although windowing systems are quite far away from
> GPU device tree bindings so I don't think this is too relevant
> for this current patch series.
> 
> On a side note, it would be fantastic to get all this available
> in Debian packages :)
> 
> >>> Also please notice there is rk3288w, the hardware version becomes r1p0.
> >> 
> >> Sounds like a new SoC?  Does rk3288w affect rk3288 in any way?
> > 
> > It is not a new SoC, just a new version of rk3288. It fixes a various of
> > the chip bug of the rk3288.
> I see.  I don't have access to any rk3288w platform but if you do
> then I guess it would be nice if you could test these patches on
> it :)  Also I'm not sure if all rk3288 device trees changes must
> be tested on rk3288w, and this SoC is not mentioned anywhere in
> the kernel (as far as I can tell).
> 
> Is there anything you think should block these current patches
> which only claim to support rk3288 from being merged?

>From what I've read on #linux-rockchip:

<wzyy2> rk3288w have some differences:  USB HOST : add ohci, fix ehci bug  GPU : update rev
<wzyy2> VOP, video codec, rga : some bug fix
<wzyy2> hdmi :  HDCP2.2

So from, yes a new gpu-revision, but that shouldn't affect this series.
>From the above I guess the hooks into the rest of the soc haven't changed
so rk3288-mali should still be enough and the driver normally can detect
and handle revision differences.

And if there are really severe changes appearing later on _and_ we actually
get special support for the rk3288w in the kernel (googling for rk3288w
mentions it in conjunction with windows 10) there is nothing speaking
against having a rk3288w-mali compatible at that time.


Heiko

> 
> >> Unless it's a special case, it seems to me that any new SoC with
> >> a Midgard GPU would need an extra vendor compatible string in the
> >> binding documentation and maybe a separate gpu dt node.
> >> 
> >>>> The actual devicetree parts are all Rockchip-specific, so I guess I'll
> >>>> just
> >>>> pick up the whole series, including the binding doc, after the merge
> >>>> window if nobody complains before that :-)
> >> 
> >> Thanks!
> 
> Guillaume
> 
> 
> P.S. kernel branch with a Mali driver I used for testing:
> https://git.collabora.com/cgit/user/gtucker/linux.git/log/?h=linux-4.11-mali
> -dt-rk3288

Re: [PATCH v5 0/5] Add ARM Mali Midgard device tree bindings and gpu node for rk3288

[Wookey]

[-- Attachment #1: Type: text/plain, Size: 591 bytes --]

On 2017-05-10 11:10 +0100, Guillaume Tucker wrote:
> 
> On a side note, it would be fantastic to get all this available
> in Debian packages :)

There are prototype debian packages here:
https://git.linaro.org/people/wookey/mali/mali-dkms.git/
https://git.linaro.org/people/wookey/mali/mali-drivers.git/

Which I'm very happy to get feedback on if people try them with
guillaume's DTB patches.

Quite hacky at the moment, but I want to prove they work at all before
tarting up the packaging.

Wookey
-- 
Principal hats:  Linaro, Debian, Wookware, ARM
http://wookware.org/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v5 0/5] Add ARM Mali Midgard device tree bindings and gpu node for rk3288

[Heiko Stuebner]

Hi Guillaume,

Am Mittwoch, 3. Mai 2017, 10:56:24 CEST schrieb Guillaume Tucker:
> The ARM Mali Midgard GPU kernel driver is only available
> out-of-tree and is not going to be merged in its current form.
> However, it would be useful to have its device tree bindings
> merged.  In particular, this would enable distributions to create
> working driver packages (dkms...) without having to patch the
> kernel.
> 
> The bindings for the earlier Mali Utgard GPU family have already
> been merged, so this is essentially the same scenario but for
> newer GPUs (Mali-T604 ~ Mali-T880).
> 
> This series of patches first imports the bindings from the latest
> driver release with some clean-up then adds a gpu node for the
> rk3288 SoC.  This was successfully tested on Radxa Rock2 Square,
> Firefly, Veyron Minnie and Jerry boards using Mali kernel driver
> r16p0 and r12p0 user-space binary.

as nobody complained anymore, I've applied all patches for 4.13
with the following small changes:
- patch2: node name to
	gpu_opp_table: gpu-opp-table
  from the original
	gpu_opp_table: opp_table0
- patches3-5: moved &gpu{} to its alphabetical correct position
  (below &gmac and above &hdmi)


Heiko

Re: [PATCH v5 1/5] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU

[Brian Norris]

Hi Guillaume,

I know this has already been merged, but I figured here was an OK place
to note (inline):

On Wed, May 03, 2017 at 10:56:25AM +0100, Guillaume Tucker wrote:
> The ARM Mali Midgard GPU family is present in a number of SoCs
> from many different vendors such as Samsung Exynos and Rockchip.
> 
> Import the device tree bindings documentation from the r16p0
> release of the Mali Midgard GPU kernel driver:
> 
>   https://developer.arm.com/-/media/Files/downloads/mali-drivers/kernel/mali-midgard-gpu/TX011-SW-99002-r16p0-00rel0.tgz
> 
> Remove the copyright and GPL licence header as deemed not necessary.
> 
> Redesign the "compatible" property strings to list all the Mali
> Midgard GPU types and add vendor specific ones.
> 
> Drop the "clock-names" property as the Mali Midgard GPU uses only one
> clock (the driver now needs to call clk_get with NULL).
> 
> Convert the "interrupt-names" property values to lower-case: "job",
> "mmu" and "gpu".
> 
> Replace the deprecated "operating-points" optional property with
> "operating-points-v2".
> 
> Omit the following optional properties in this initial version as they
> are only used in very specific cases:
> 
>   * snoop_enable_smc
>   * snoop_disable_smc
>   * jm_config
>   * power_model
>   * system-coherency
>   * ipa-model
> 
> Update the example accordingly to reflect all these changes, based on
> rk3288 mali-t760.
> 
> CC: John Reitan <john.reitan@arm.com>
> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---
>  .../devicetree/bindings/gpu/arm,mali-midgard.txt   | 86 ++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
> 
> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
> new file mode 100644
> index 000000000000..d3b6e1a4713a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt

^^ This file name implies we're talking about a base "arm,mali-midgard"
compatible property.

> @@ -0,0 +1,86 @@
> +ARM Mali Midgard GPU
> +====================
> +
> +Required properties:
> +
> +- compatible :
> +  * Must contain one of the following:
> +    + "arm,mali-t604"
> +    + "arm,mali-t624"
> +    + "arm,mali-t628"
> +    + "arm,mali-t720"
> +    + "arm,mali-t760"
> +    + "arm,mali-t820"
> +    + "arm,mali-t830"
> +    + "arm,mali-t860"
> +    + "arm,mali-t880"
> +  * which must be preceded by one of the following vendor specifics:
> +    + "amlogic,meson-gxm-mali"
> +    + "rockchip,rk3288-mali"

But it's not listed here.

> +
> +- reg : Physical base address of the device and length of the register area.
> +
> +- interrupts : Contains the three IRQ lines required by Mali Midgard devices.
> +
> +- interrupt-names : Contains the names of IRQ resources in the order they were
> +  provided in the interrupts property. Must contain: "job", "mmu", "gpu".
> +
> +
> +Optional properties:
> +
> +- clocks : Phandle to clock for the Mali Midgard device.
> +
> +- mali-supply : Phandle to regulator for the Mali device. Refer to
> +  Documentation/devicetree/bindings/regulator/regulator.txt for details.
> +
> +- operating-points-v2 : Refer to Documentation/devicetree/bindings/power/opp.txt
> +  for details.
> +
> +
> +Example for a Mali-T760:
> +
> +gpu@ffa30000 {
> +	compatible = "rockchip,rk3288-mali", "arm,mali-t760", "arm,mali-midgard";

And it *is* used here in this example.

So, should it be used/documented or not?

> +	reg = <0xffa30000 0x10000>;
> +	interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> +	interrupt-names = "job", "mmu", "gpu";
> +	clocks = <&cru ACLK_GPU>;
> +	mali-supply = <&vdd_gpu>;
> +	operating-points-v2 = <&gpu_opp_table>;
> +	power-domains = <&power RK3288_PD_GPU>;
> +};

[...]

Brian

Re: [PATCH v5 1/5] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU

[Heiko Stübner]

Hi Brian,

Am Mittwoch, 12. Juli 2017, 10:26:39 CEST schrieb Brian Norris:
> Hi Guillaume,
> 
> I know this has already been merged, but I figured here was an OK place
> to note (inline):
> 
> On Wed, May 03, 2017 at 10:56:25AM +0100, Guillaume Tucker wrote:
> > The ARM Mali Midgard GPU family is present in a number of SoCs
> > from many different vendors such as Samsung Exynos and Rockchip.
> > 
> > Import the device tree bindings documentation from the r16p0
> > 
> > release of the Mali Midgard GPU kernel driver:
> >   https://developer.arm.com/-/media/Files/downloads/mali-drivers/kernel/ma
> >   li-midgard-gpu/TX011-SW-99002-r16p0-00rel0.tgz> 
> > Remove the copyright and GPL licence header as deemed not necessary.
> > 
> > Redesign the "compatible" property strings to list all the Mali
> > Midgard GPU types and add vendor specific ones.
> > 
> > Drop the "clock-names" property as the Mali Midgard GPU uses only one
> > clock (the driver now needs to call clk_get with NULL).
> > 
> > Convert the "interrupt-names" property values to lower-case: "job",
> > "mmu" and "gpu".
> > 
> > Replace the deprecated "operating-points" optional property with
> > "operating-points-v2".
> > 
> > Omit the following optional properties in this initial version as they
> > 
> > are only used in very specific cases:
> >   * snoop_enable_smc
> >   * snoop_disable_smc
> >   * jm_config
> >   * power_model
> >   * system-coherency
> >   * ipa-model
> > 
> > Update the example accordingly to reflect all these changes, based on
> > rk3288 mali-t760.
> > 
> > CC: John Reitan <john.reitan@arm.com>
> > Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> > ---
> > 
> >  .../devicetree/bindings/gpu/arm,mali-midgard.txt   | 86
> >  ++++++++++++++++++++++ 1 file changed, 86 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt> 
> > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
> > b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt new file
> > mode 100644
> > index 000000000000..d3b6e1a4713a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
> 
> ^^ This file name implies we're talking about a base "arm,mali-midgard"
> compatible property.

it follows the precedent of arm,mali-utgard.txt and while the file-name
is not part of the compatible list, it feels way better collecting all the 
variants in one file, compared to spawning multiple ones for each soc
vendor using a mali gpu.

Also in other places the naming of binding descriptions does not follow
any compatible at all, so the filename = main-compatible does not seem
to be the rule.


> > @@ -0,0 +1,86 @@
> > +ARM Mali Midgard GPU
> > +====================
> > +
> > +Required properties:
> > +
> > +- compatible :
> > +  * Must contain one of the following:
> > +    + "arm,mali-t604"
> > +    + "arm,mali-t624"
> > +    + "arm,mali-t628"
> > +    + "arm,mali-t720"
> > +    + "arm,mali-t760"
> > +    + "arm,mali-t820"
> > +    + "arm,mali-t830"
> > +    + "arm,mali-t860"
> > +    + "arm,mali-t880"
> > +  * which must be preceded by one of the following vendor specifics:
> > +    + "amlogic,meson-gxm-mali"
> > +    + "rockchip,rk3288-mali"
> 
> But it's not listed here.
> 
> > +
> > +- reg : Physical base address of the device and length of the register
> > area. +
> > +- interrupts : Contains the three IRQ lines required by Mali Midgard
> > devices. +
> > +- interrupt-names : Contains the names of IRQ resources in the order they
> > were +  provided in the interrupts property. Must contain: "job", "mmu",
> > "gpu". +
> > +
> > +Optional properties:
> > +
> > +- clocks : Phandle to clock for the Mali Midgard device.
> > +
> > +- mali-supply : Phandle to regulator for the Mali device. Refer to
> > +  Documentation/devicetree/bindings/regulator/regulator.txt for details.
> > +
> > +- operating-points-v2 : Refer to
> > Documentation/devicetree/bindings/power/opp.txt +  for details.
> > +
> > +
> > +Example for a Mali-T760:
> > +
> > +gpu@ffa30000 {
> > +	compatible = "rockchip,rk3288-mali", "arm,mali-t760",
> > "arm,mali-midgard";
> 
> And it *is* used here in this example.
> 
> So, should it be used/documented or not?

No I don't think it should and have patches that drop the mali-midgard from
the example + rk3288 dtsi to not confuse people:
- https://patchwork.kernel.org/patch/9832811/
- https://patchwork.kernel.org/patch/9832807/


Devicetree supposedly should not contain wildcards and without knowing
for real that all of then in fact are compatible we should not assume so
[and that's hard to check with all the secrecy :-) ] . Using the real name 
(tXXX) sounds nice enough.


Heiko

> > +	reg = <0xffa30000 0x10000>;
> > +	interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> > +		     <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
> > +		     <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> > +	interrupt-names = "job", "mmu", "gpu";
> > +	clocks = <&cru ACLK_GPU>;
> > +	mali-supply = <&vdd_gpu>;
> > +	operating-points-v2 = <&gpu_opp_table>;
> > +	power-domains = <&power RK3288_PD_GPU>;
> > +};
> 
> [...]
> 
> Brian

Re: [PATCH v5 1/5] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU

[Brian Norris]

Hi Heiko,

On Wed, Jul 12, 2017 at 08:30:55PM +0200, Heiko Stuebner wrote:
> Am Mittwoch, 12. Juli 2017, 10:26:39 CEST schrieb Brian Norris:
> > On Wed, May 03, 2017 at 10:56:25AM +0100, Guillaume Tucker wrote:
> > > The ARM Mali Midgard GPU family is present in a number of SoCs
> > > from many different vendors such as Samsung Exynos and Rockchip.
> > > 
> > > Import the device tree bindings documentation from the r16p0
> > > 
> > > release of the Mali Midgard GPU kernel driver:
> > >   https://developer.arm.com/-/media/Files/downloads/mali-drivers/kernel/ma
> > >   li-midgard-gpu/TX011-SW-99002-r16p0-00rel0.tgz> 
> > > Remove the copyright and GPL licence header as deemed not necessary.
> > > 
> > > Redesign the "compatible" property strings to list all the Mali
> > > Midgard GPU types and add vendor specific ones.
> > > 
> > > Drop the "clock-names" property as the Mali Midgard GPU uses only one
> > > clock (the driver now needs to call clk_get with NULL).
> > > 
> > > Convert the "interrupt-names" property values to lower-case: "job",
> > > "mmu" and "gpu".
> > > 
> > > Replace the deprecated "operating-points" optional property with
> > > "operating-points-v2".
> > > 
> > > Omit the following optional properties in this initial version as they
> > > 
> > > are only used in very specific cases:
> > >   * snoop_enable_smc
> > >   * snoop_disable_smc
> > >   * jm_config
> > >   * power_model
> > >   * system-coherency
> > >   * ipa-model
> > > 
> > > Update the example accordingly to reflect all these changes, based on
> > > rk3288 mali-t760.
> > > 
> > > CC: John Reitan <john.reitan@arm.com>
> > > Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > > Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> > > ---
> > > 
> > >  .../devicetree/bindings/gpu/arm,mali-midgard.txt   | 86
> > >  ++++++++++++++++++++++ 1 file changed, 86 insertions(+)
> > >  create mode 100644
> > >  Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt> 
> > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
> > > b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt new file
> > > mode 100644
> > > index 000000000000..d3b6e1a4713a
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
> > 
> > ^^ This file name implies we're talking about a base "arm,mali-midgard"
> > compatible property.
> 
> it follows the precedent of arm,mali-utgard.txt and while the file-name
> is not part of the compatible list, it feels way better collecting all the 
> variants in one file, compared to spawning multiple ones for each soc
> vendor using a mali gpu.
> 
> Also in other places the naming of binding descriptions does not follow
> any compatible at all, so the filename = main-compatible does not seem
> to be the rule.

Well, when they do get a name in the form of "<vendor>,<foo>.txt", the
"<vendor>,<foo>" is usually at least *a* valid compatible string. For
example, "nvidia,gk20a.txt" has several other strings, but its filename
is one of the supported ones.

Anyway, that wasn't my main worry. I was mostly curious about the body
of the text.

> > > @@ -0,0 +1,86 @@
> > > +ARM Mali Midgard GPU
> > > +====================
> > > +
> > > +Required properties:
> > > +
> > > +- compatible :
> > > +  * Must contain one of the following:
> > > +    + "arm,mali-t604"
> > > +    + "arm,mali-t624"
> > > +    + "arm,mali-t628"
> > > +    + "arm,mali-t720"
> > > +    + "arm,mali-t760"
> > > +    + "arm,mali-t820"
> > > +    + "arm,mali-t830"
> > > +    + "arm,mali-t860"
> > > +    + "arm,mali-t880"
> > > +  * which must be preceded by one of the following vendor specifics:
> > > +    + "amlogic,meson-gxm-mali"
> > > +    + "rockchip,rk3288-mali"
> > 
> > But it's not listed here.
> > 
> > > +
> > > +- reg : Physical base address of the device and length of the register
> > > area. +
> > > +- interrupts : Contains the three IRQ lines required by Mali Midgard
> > > devices. +
> > > +- interrupt-names : Contains the names of IRQ resources in the order they
> > > were +  provided in the interrupts property. Must contain: "job", "mmu",
> > > "gpu". +
> > > +
> > > +Optional properties:
> > > +
> > > +- clocks : Phandle to clock for the Mali Midgard device.
> > > +
> > > +- mali-supply : Phandle to regulator for the Mali device. Refer to
> > > +  Documentation/devicetree/bindings/regulator/regulator.txt for details.
> > > +
> > > +- operating-points-v2 : Refer to
> > > Documentation/devicetree/bindings/power/opp.txt +  for details.
> > > +
> > > +
> > > +Example for a Mali-T760:
> > > +
> > > +gpu@ffa30000 {
> > > +	compatible = "rockchip,rk3288-mali", "arm,mali-t760",
> > > "arm,mali-midgard";
> > 
> > And it *is* used here in this example.
> > 
> > So, should it be used/documented or not?
> 
> No I don't think it should and have patches that drop the mali-midgard from
> the example + rk3288 dtsi to not confuse people:
> - https://patchwork.kernel.org/patch/9832811/
> - https://patchwork.kernel.org/patch/9832807/

I didn't notice those. Thanks.

> Devicetree supposedly should not contain wildcards and without knowing
> for real that all of then in fact are compatible we should not assume so
> [and that's hard to check with all the secrecy :-) ] . Using the real name 
> (tXXX) sounds nice enough.

Fine with me.

Brian

Re: [PATCH v5 1/5] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU

[Guillaume Tucker]

Hi Brian,

On 12/07/17 18:26, Brian Norris wrote:
> Hi Guillaume,
>
> I know this has already been merged, but I figured here was an OK place
> to note (inline):
>
> On Wed, May 03, 2017 at 10:56:25AM +0100, Guillaume Tucker wrote:

[...]

>> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
>> new file mode 100644
>> index 000000000000..d3b6e1a4713a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
>
> ^^ This file name implies we're talking about a base "arm,mali-midgard"
> compatible property.
>
>> @@ -0,0 +1,86 @@
>> +ARM Mali Midgard GPU
>> +====================
>> +
>> +Required properties:
>> +
>> +- compatible :
>> +  * Must contain one of the following:
>> +    + "arm,mali-t604"
>> +    + "arm,mali-t624"
>> +    + "arm,mali-t628"
>> +    + "arm,mali-t720"
>> +    + "arm,mali-t760"
>> +    + "arm,mali-t820"
>> +    + "arm,mali-t830"
>> +    + "arm,mali-t860"
>> +    + "arm,mali-t880"
>> +  * which must be preceded by one of the following vendor specifics:
>> +    + "amlogic,meson-gxm-mali"
>> +    + "rockchip,rk3288-mali"
>
> But it's not listed here.
>
>> +
>> +- reg : Physical base address of the device and length of the register area.
>> +
>> +- interrupts : Contains the three IRQ lines required by Mali Midgard devices.
>> +
>> +- interrupt-names : Contains the names of IRQ resources in the order they were
>> +  provided in the interrupts property. Must contain: "job", "mmu", "gpu".
>> +
>> +
>> +Optional properties:
>> +
>> +- clocks : Phandle to clock for the Mali Midgard device.
>> +
>> +- mali-supply : Phandle to regulator for the Mali device. Refer to
>> +  Documentation/devicetree/bindings/regulator/regulator.txt for details.
>> +
>> +- operating-points-v2 : Refer to Documentation/devicetree/bindings/power/opp.txt
>> +  for details.
>> +
>> +
>> +Example for a Mali-T760:
>> +
>> +gpu@ffa30000 {
>> +	compatible = "rockchip,rk3288-mali", "arm,mali-t760", "arm,mali-midgard";
>
> And it *is* used here in this example.
>
> So, should it be used/documented or not?

The important part is that having only arm,mali-midgard in the
gpu node is not enough to accurately describe the hardware, which
is why it is not listed in the required compatible strings.  On
the other hand, it does help with describing the hardware, so
it's fine to use it.  If people find this confusing then I guess
we could add something to say that arm,mali-midgard can be used
optionally in addition to the specific ones.

Also, if you look at the 64-bit ARM CPUs for example, they
typically have armv8 in their node although armv8 is not
mentioned anywhere in the bindings documentation...

Guillaume

Re: [PATCH v5 1/5] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU

[Guillaume Tucker]

On 14/07/17 07:41, Guillaume Tucker wrote:
> Hi Brian,
>
> On 12/07/17 18:26, Brian Norris wrote:
>> Hi Guillaume,
>>
>> I know this has already been merged, but I figured here was an OK place
>> to note (inline):
>>
>> On Wed, May 03, 2017 at 10:56:25AM +0100, Guillaume Tucker wrote:
>
> [...]
>
>>> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
>>> new file mode 100644
>>> index 000000000000..d3b6e1a4713a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
>>
>> ^^ This file name implies we're talking about a base "arm,mali-midgard"
>> compatible property.
>>
>>> @@ -0,0 +1,86 @@
>>> +ARM Mali Midgard GPU
>>> +====================
>>> +
>>> +Required properties:
>>> +
>>> +- compatible :
>>> +  * Must contain one of the following:
>>> +    + "arm,mali-t604"
>>> +    + "arm,mali-t624"
>>> +    + "arm,mali-t628"
>>> +    + "arm,mali-t720"
>>> +    + "arm,mali-t760"
>>> +    + "arm,mali-t820"
>>> +    + "arm,mali-t830"
>>> +    + "arm,mali-t860"
>>> +    + "arm,mali-t880"
>>> +  * which must be preceded by one of the following vendor specifics:
>>> +    + "amlogic,meson-gxm-mali"
>>> +    + "rockchip,rk3288-mali"
>>
>> But it's not listed here.
>>
>>> +
>>> +- reg : Physical base address of the device and length of the register area.
>>> +
>>> +- interrupts : Contains the three IRQ lines required by Mali Midgard devices.
>>> +
>>> +- interrupt-names : Contains the names of IRQ resources in the order they were
>>> +  provided in the interrupts property. Must contain: "job", "mmu", "gpu".
>>> +
>>> +
>>> +Optional properties:
>>> +
>>> +- clocks : Phandle to clock for the Mali Midgard device.
>>> +
>>> +- mali-supply : Phandle to regulator for the Mali device. Refer to
>>> +  Documentation/devicetree/bindings/regulator/regulator.txt for details.
>>> +
>>> +- operating-points-v2 : Refer to Documentation/devicetree/bindings/power/opp.txt
>>> +  for details.
>>> +
>>> +
>>> +Example for a Mali-T760:
>>> +
>>> +gpu@ffa30000 {
>>> +    compatible = "rockchip,rk3288-mali", "arm,mali-t760", "arm,mali-midgard";
>>
>> And it *is* used here in this example.
>>
>> So, should it be used/documented or not?
>
> The important part is that having only arm,mali-midgard in the
> gpu node is not enough to accurately describe the hardware, which
> is why it is not listed in the required compatible strings.  On
> the other hand, it does help with describing the hardware, so
> it's fine to use it.  If people find this confusing then I guess
> we could add something to say that arm,mali-midgard can be used
> optionally in addition to the specific ones.
>
> Also, if you look at the 64-bit ARM CPUs for example, they
> typically have armv8 in their node although armv8 is not
> mentioned anywhere in the bindings documentation...

Sorry I've only just seen Heiko's other emails and patches about
removing arm,mali-midgard from the example.

Guillaume