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

[PATCH v4 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


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   | 82 ++++++++++++++++++++++
 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, 140 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt

--
2.11.0

[PATCH v4 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 include optional vendor ones.

Drop the "clock-names" property as only one clock is used by the Mali
Midgard driver (which 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.

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   | 82 ++++++++++++++++++++++
 1 file changed, 82 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..547ddeceb498
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
@@ -0,0 +1,82 @@
+ARM Mali Midgard GPU
+====================
+
+Required properties:
+
+- compatible :
+  * Must be one of the following:
+    + "arm,mali-t60x"
+    + "arm,mali-t62x"
+    + "arm,mali-t720"
+    + "arm,mali-t760"
+    + "arm,mali-t820"
+    + "arm,mali-t830"
+    + "arm,mali-t860"
+    + "arm,mali-t880"
+  * And, optionally, one of the vendor specific compatible:
+    + "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-T602:
+
+gpu@fc010000 {
+	compatible = "arm,mali-t60x", "arm,mali-midgard";
+	reg = <0xfc010000 0x4000>;
+	interrupts = <0 36 4>, <0 37 4>, <0 38 4>;
+	interrupt-names = "job", "mmu", "gpu";
+	clocks = <&pclk_mali>;
+	mali-supply = <&vdd_mali>;
+	operating-points-v2 = <&gpu_opp_table>;
+};
+
+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 v4 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 v4 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 v4 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 v4 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 v4 1/5] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU

[Rob Herring]

On Tue, Apr 25, 2017 at 02:16:16PM +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 include optional vendor ones.
> 
> Drop the "clock-names" property as only one clock is used by the Mali
> Midgard driver (which 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.
> 
> 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   | 82 ++++++++++++++++++++++
>  1 file changed, 82 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..547ddeceb498
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
> @@ -0,0 +1,82 @@
> +ARM Mali Midgard GPU
> +====================
> +
> +Required properties:
> +
> +- compatible :
> +  * Must be one of the following:
> +    + "arm,mali-t60x"
> +    + "arm,mali-t62x"

Don't use wildcards.

> +    + "arm,mali-t720"
> +    + "arm,mali-t760"
> +    + "arm,mali-t820"
> +    + "arm,mali-t830"
> +    + "arm,mali-t860"
> +    + "arm,mali-t880"
> +  * And, optionally, one of the vendor specific compatible:

IMO, these should not be optional.

> +    + "amlogic,meson-gxm-mali"
> +    + "rockchip,rk3288-mali"

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

[Guillaume Tucker]

Hi Rob,

On 28/04/17 20:27, Rob Herring wrote:
> On Tue, Apr 25, 2017 at 02:16:16PM +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..547ddeceb498
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
>> @@ -0,0 +1,82 @@
>> +ARM Mali Midgard GPU
>> +====================
>> +
>> +Required properties:
>> +
>> +- compatible :
>> +  * Must be one of the following:
>> +    + "arm,mali-t60x"
>> +    + "arm,mali-t62x"
>
> Don't use wildcards.

Sure, old habits die hard...  I'll fix it in patch v5.

>> +    + "arm,mali-t720"
>> +    + "arm,mali-t760"
>> +    + "arm,mali-t820"
>> +    + "arm,mali-t830"
>> +    + "arm,mali-t860"
>> +    + "arm,mali-t880"
>> +  * And, optionally, one of the vendor specific compatible:
>
> IMO, these should not be optional.

Well, vendor compatible strings are clearly optional for the
Utgard GPU series for which the bindings docs were recently
merged.  It seems that whether these should be optional or not,
the documentation should be consistent between at least all
similar types of devices like Midgard and Utgard GPUs.  They have
different architectures but from a device tree point of view,
they both have the same kind of SoC-specific integration (clocks,
irqs, regulators...).

So was this was overlooked in the Utgard case and should it
ideally be fixed there as well as non-optional?  Or, is it OK to
keep these optional on a second thought?

>> +    + "amlogic,meson-gxm-mali"
>> +    + "rockchip,rk3288-mali"

Guillaume

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

[Rob Herring]

On Tue, May 2, 2017 at 6:23 AM, Guillaume Tucker
<guillaume.tucker@collabora.com> wrote:
> Hi Rob,
>
> On 28/04/17 20:27, Rob Herring wrote:
>>
>> On Tue, Apr 25, 2017 at 02:16:16PM +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..547ddeceb498
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
>>> @@ -0,0 +1,82 @@
>>> +ARM Mali Midgard GPU
>>> +====================
>>> +
>>> +Required properties:
>>> +
>>> +- compatible :
>>> +  * Must be one of the following:
>>> +    + "arm,mali-t60x"
>>> +    + "arm,mali-t62x"
>>
>>
>> Don't use wildcards.
>
>
> Sure, old habits die hard...  I'll fix it in patch v5.
>
>>> +    + "arm,mali-t720"
>>> +    + "arm,mali-t760"
>>> +    + "arm,mali-t820"
>>> +    + "arm,mali-t830"
>>> +    + "arm,mali-t860"
>>> +    + "arm,mali-t880"
>>> +  * And, optionally, one of the vendor specific compatible:
>>
>>
>> IMO, these should not be optional.
>
>
> Well, vendor compatible strings are clearly optional for the
> Utgard GPU series for which the bindings docs were recently
> merged.  It seems that whether these should be optional or not,
> the documentation should be consistent between at least all
> similar types of devices like Midgard and Utgard GPUs.  They have
> different architectures but from a device tree point of view,
> they both have the same kind of SoC-specific integration (clocks,
> irqs, regulators...).

Clocks should not vary by SoC. There is often variation because clocks
get driven by same source or are not s/w controlled, but really there
should not be that variation. I noticed Utgard has 2 clocks. So is
Midgard really just 1 clock? The DT should have all the clocks listed
in the TRMs.

> So was this was overlooked in the Utgard case and should it
> ideally be fixed there as well as non-optional?  Or, is it OK to
> keep these optional on a second thought?

Probably should be required in the Utgard case as well.

Rob

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

[Guillaume Tucker]

On 02/05/17 15:13, Rob Herring wrote:
> On Tue, May 2, 2017 at 6:23 AM, Guillaume Tucker
> <guillaume.tucker@collabora.com> wrote:
>> Hi Rob,
>>
>> On 28/04/17 20:27, Rob Herring wrote:
>>>
>>> On Tue, Apr 25, 2017 at 02:16:16PM +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..547ddeceb498
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
>>>> @@ -0,0 +1,82 @@
>>>> +ARM Mali Midgard GPU
>>>> +====================
>>>> +
>>>> +Required properties:
>>>> +
>>>> +- compatible :
>>>> +  * Must be one of the following:
>>>> +    + "arm,mali-t60x"
>>>> +    + "arm,mali-t62x"
>>>
>>>
>>> Don't use wildcards.
>>
>>
>> Sure, old habits die hard...  I'll fix it in patch v5.
>>
>>>> +    + "arm,mali-t720"
>>>> +    + "arm,mali-t760"
>>>> +    + "arm,mali-t820"
>>>> +    + "arm,mali-t830"
>>>> +    + "arm,mali-t860"
>>>> +    + "arm,mali-t880"
>>>> +  * And, optionally, one of the vendor specific compatible:
>>>
>>>
>>> IMO, these should not be optional.
>>
>>
>> Well, vendor compatible strings are clearly optional for the
>> Utgard GPU series for which the bindings docs were recently
>> merged.  It seems that whether these should be optional or not,
>> the documentation should be consistent between at least all
>> similar types of devices like Midgard and Utgard GPUs.  They have
>> different architectures but from a device tree point of view,
>> they both have the same kind of SoC-specific integration (clocks,
>> irqs, regulators...).
>
> Clocks should not vary by SoC. There is often variation because clocks
> get driven by same source or are not s/w controlled, but really there
> should not be that variation. I noticed Utgard has 2 clocks. So is
> Midgard really just 1 clock? The DT should have all the clocks listed
> in the TRMs.

I meant to say that the clock sources are different in each SoC,
but yes the same clock input is always needed by the GPU.

The TRM is confidential but to the best of my knowledge and based
on existing device trees and the out-of-tree kernel driver, the
Midgard GPU has only one clock input.

>> So was this was overlooked in the Utgard case and should it
>> ideally be fixed there as well as non-optional?  Or, is it OK to
>> keep these optional on a second thought?
>
> Probably should be required in the Utgard case as well.

OK, so I'll make the vendor compatible strings required (for
Midgard) in patch v5.

Guillaume