[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Maxime Ripard]

The ARM Mali Utgard GPU family is embedded into a number of SoCs from
Allwinner, Amlogic, Mediatek or Rockchip.

Add a binding for the GPU of that family.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
 1 file changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
new file mode 100644
index 000000000000..df05ba0ec357
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
@@ -0,0 +1,76 @@
+ARM Mali Utgard GPU
+===================
+
+Required properties:
+  - compatible:
+    * "arm,mali-utgard" and one of the following:
+      + "arm,mali-300"
+      + "arm,mali-400"
+      + "arm,mali-450"
+
+  - reg: Physical base address and length of the GPU registers
+
+  - interrupts: an entry for each entry in interrupt-names.
+    See ../interrupt-controller/interrupts.txt for details.
+
+  - interrupt-names:
+    * ppX: Pixel Processor X interrupt (X from 0 to 7)
+    * ppmmuX: Pixel Processor X MMU interrupt (X from 0 to 7)
+    * pp: Pixel Processor broadcast interrupt (mali-450 only)
+    * gp: Geometry Processor interrupt
+    * gpmmu: Geometry Processor MMU interrupt
+
+
+Optional properties:
+  - interrupt-names:
+    * pmu: Power Management Unit interrupt, if implemented in hardware
+
+Vendor-specific bindings
+------------------------
+
+The Mali GPU is integrated very differently from one SoC to
+another. In order to accommodate those differences, you have the option
+to specify one more vendor-specific compatible, among:
+
+  - allwinner,sun4i-a10-mali
+    Required properties:
+      * clocks: an entry for each entry in clock-names
+      * clock-names:
+        + bus: bus clock for the GPU
+        + core: clock driving the GPU itself
+      * resets: phandle to the reset line for the GPU
+
+  - allwinner,sun7i-a20-mali
+    Required properties:
+      * clocks: an entry for each entry in clock-names
+      * clock-names:
+        + bus: bus clock for the GPU
+        + core: clock driving the GPU itself
+      * resets: phandle to the reset line for the GPU
+
+Example:
+
+mali: gpu at 01c40000 {
+	compatible = "allwinner,sun7i-a20-mali", "arm,mali-400",
+		     "arm,mali-utgard";
+	reg = <0x01c40000 0x10000>;
+	interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
+	interrupt-names = "gp",
+			  "gpmmu",
+			  "pp0",
+			  "ppmmu0",
+			  "pp1",
+			  "ppmmu1",
+			  "pmu";
+	clocks = <&ccu CLK_BUS_GPU>, <&ccu CLK_GPU>;
+	clock-names = "bus", "core";
+	resets = <&ccu RST_BUS_GPU>;
+};
+
+
-- 
2.11.0

[PATCH 2/2] ARM: sun8i: dt: Add mali node

[Maxime Ripard]

The A23 and A33 have an ARM Mali 400 GPU. Now that we have a binding, add
it to our DT.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun8i-a23-a33.dtsi | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index e4991a78ad73..1aaa68ec1880 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -486,6 +486,33 @@
 			#size-cells = <0>;
 		};
 
+		mali: gpu at 01c40000 {
+			compatible = "allwinner,sun8i-a23-mali",
+				     "allwinner,sun7i-a20-mali",
+				     "arm,mali-400", "arm,mali-utgard";
+			reg = <0x01c40000 0x10000>;
+			interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "gp",
+					  "gpmmu",
+					  "pp0",
+					  "ppmmu0",
+					  "pp1",
+					  "ppmmu1",
+					  "pmu";
+			clocks = <&ccu CLK_BUS_GPU>, <&ccu CLK_GPU>;
+			clock-names = "bus", "core";
+			resets = <&ccu RST_BUS_GPU>;
+
+			assigned-clocks = <&ccu CLK_GPU>;
+			assigned-clock-rates = <408000000>;
+		};
+
 		gic: interrupt-controller at 01c81000 {
 			compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
 			reg = <0x01c81000 0x1000>,
-- 
2.11.0

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Krzysztof Kozlowski]

On Mon, Jan 16, 2017 at 02:24:23PM +0100, Maxime Ripard wrote:
> The ARM Mali Utgard GPU family is embedded into a number of SoCs from
> Allwinner, Amlogic, Mediatek or Rockchip.
> 
> Add a binding for the GPU of that family.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> 

Hi,

Do you have a driver in kernel which will implement these bindings?

Defining them for out-of-tree driver does not bring any benefits (3rd
party driver will not respect them anyway).

Best regards,
Krzysztof

> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> new file mode 100644
> index 000000000000..df05ba0ec357
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> @@ -0,0 +1,76 @@
> +ARM Mali Utgard GPU
> +===================
> +
> +Required properties:
> +  - compatible:
> +    * "arm,mali-utgard" and one of the following:
> +      + "arm,mali-300"
> +      + "arm,mali-400"
> +      + "arm,mali-450"
> +
> +  - reg: Physical base address and length of the GPU registers
> +
> +  - interrupts: an entry for each entry in interrupt-names.
> +    See ../interrupt-controller/interrupts.txt for details.
> +
> +  - interrupt-names:
> +    * ppX: Pixel Processor X interrupt (X from 0 to 7)
> +    * ppmmuX: Pixel Processor X MMU interrupt (X from 0 to 7)
> +    * pp: Pixel Processor broadcast interrupt (mali-450 only)
> +    * gp: Geometry Processor interrupt
> +    * gpmmu: Geometry Processor MMU interrupt
> +
> +
> +Optional properties:
> +  - interrupt-names:
> +    * pmu: Power Management Unit interrupt, if implemented in hardware
> +
> +Vendor-specific bindings
> +------------------------
> +
> +The Mali GPU is integrated very differently from one SoC to
> +another. In order to accommodate those differences, you have the option
> +to specify one more vendor-specific compatible, among:
> +
> +  - allwinner,sun4i-a10-mali
> +    Required properties:
> +      * clocks: an entry for each entry in clock-names
> +      * clock-names:
> +        + bus: bus clock for the GPU
> +        + core: clock driving the GPU itself
> +      * resets: phandle to the reset line for the GPU
> +
> +  - allwinner,sun7i-a20-mali
> +    Required properties:
> +      * clocks: an entry for each entry in clock-names
> +      * clock-names:
> +        + bus: bus clock for the GPU
> +        + core: clock driving the GPU itself
> +      * resets: phandle to the reset line for the GPU
> +
> +Example:
> +
> +mali: gpu at 01c40000 {
> +	compatible = "allwinner,sun7i-a20-mali", "arm,mali-400",
> +		     "arm,mali-utgard";
> +	reg = <0x01c40000 0x10000>;
> +	interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
> +	interrupt-names = "gp",
> +			  "gpmmu",
> +			  "pp0",
> +			  "ppmmu0",
> +			  "pp1",
> +			  "ppmmu1",
> +			  "pmu";
> +	clocks = <&ccu CLK_BUS_GPU>, <&ccu CLK_GPU>;
> +	clock-names = "bus", "core";
> +	resets = <&ccu RST_BUS_GPU>;
> +};
> +
> +
> -- 
> 2.11.0
> 

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Maxime Ripard]

Hi,

On Mon, Jan 16, 2017 at 08:49:06PM +0200, Krzysztof Kozlowski wrote:
> On Mon, Jan 16, 2017 at 02:24:23PM +0100, Maxime Ripard wrote:
> > The ARM Mali Utgard GPU family is embedded into a number of SoCs from
> > Allwinner, Amlogic, Mediatek or Rockchip.
> > 
> > Add a binding for the GPU of that family.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
> >  1 file changed, 76 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> 
> Do you have a driver in kernel which will implement these bindings?

No, but we have bindings for out-of-tree drivers already.

> Defining them for out-of-tree driver does not bring any benefits
> (3rd party driver will not respect them anyway).

You could see it the other way around too. The out-of-tree drivers
don't respect it at the moment because there's no binding to respect.

And at least for us, we definitely plan on doing that.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170117/a98cc236/attachment.sig>

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Neil Armstrong]

On 01/17/2017 10:38 AM, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Jan 16, 2017 at 08:49:06PM +0200, Krzysztof Kozlowski wrote:
>> On Mon, Jan 16, 2017 at 02:24:23PM +0100, Maxime Ripard wrote:
>>> The ARM Mali Utgard GPU family is embedded into a number of SoCs from
>>> Allwinner, Amlogic, Mediatek or Rockchip.
>>>
>>> Add a binding for the GPU of that family.
>>>
>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>> ---
>>>  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
>>>  1 file changed, 76 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>>
>> Do you have a driver in kernel which will implement these bindings?
> 
> No, but we have bindings for out-of-tree drivers already.
> 
>> Defining them for out-of-tree driver does not bring any benefits
>> (3rd party driver will not respect them anyway).
> 
> You could see it the other way around too. The out-of-tree drivers
> don't respect it at the moment because there's no binding to respect.
> 
> And at least for us, we definitely plan on doing that.
> 
> Maxime

Hi Maxime, Krzysztof,

We hope this will be accepted so it will solve the same issue we have on Amlogic SoCs
and all the other mali powered SoCs.

Having mainline bindings will forcre out-of-tree driver to respect those bindings
and remove a dts out-of-tree patch aswell.

Neil

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170117/4be3a61c/attachment.sig>

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Krzysztof Kozlowski]

On Tue, Jan 17, 2017 at 11:38 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Mon, Jan 16, 2017 at 08:49:06PM +0200, Krzysztof Kozlowski wrote:
>> On Mon, Jan 16, 2017 at 02:24:23PM +0100, Maxime Ripard wrote:
>> > The ARM Mali Utgard GPU family is embedded into a number of SoCs from
>> > Allwinner, Amlogic, Mediatek or Rockchip.
>> >
>> > Add a binding for the GPU of that family.
>> >
>> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> > ---
>> >  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
>> >  1 file changed, 76 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>>
>> Do you have a driver in kernel which will implement these bindings?
>
> No, but we have bindings for out-of-tree drivers already.
>
>> Defining them for out-of-tree driver does not bring any benefits
>> (3rd party driver will not respect them anyway).
>
> You could see it the other way around too. The out-of-tree drivers
> don't respect it at the moment because there's no binding to respect.

Indeed, that's a point. However valid only when the out-of-tree driver
will respect them, for example do not break them on next release.

Best regards,
Krzysztof

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Krzysztof Kozlowski]

On Tue, Jan 17, 2017 at 12:22 PM, Neil Armstrong
<narmstrong@baylibre.com> wrote:
> On 01/17/2017 10:38 AM, Maxime Ripard wrote:
>> Hi,
>>
>> On Mon, Jan 16, 2017 at 08:49:06PM +0200, Krzysztof Kozlowski wrote:
>>> On Mon, Jan 16, 2017 at 02:24:23PM +0100, Maxime Ripard wrote:
>>>> The ARM Mali Utgard GPU family is embedded into a number of SoCs from
>>>> Allwinner, Amlogic, Mediatek or Rockchip.
>>>>
>>>> Add a binding for the GPU of that family.
>>>>
>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>>> ---
>>>>  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
>>>>  1 file changed, 76 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>>>
>>> Do you have a driver in kernel which will implement these bindings?
>>
>> No, but we have bindings for out-of-tree drivers already.
>>
>>> Defining them for out-of-tree driver does not bring any benefits
>>> (3rd party driver will not respect them anyway).
>>
>> You could see it the other way around too. The out-of-tree drivers
>> don't respect it at the moment because there's no binding to respect.
>>
>> And at least for us, we definitely plan on doing that.
>>
>> Maxime
>
> Hi Maxime, Krzysztof,
>
> We hope this will be accepted so it will solve the same issue we have on Amlogic SoCs
> and all the other mali powered SoCs.

It will be helpful also for other SoCs using Mali 400 (e.g.
Exynos3250, Exynos4412).

> Having mainline bindings will forcre out-of-tree driver to respect those bindings
> and remove a dts out-of-tree patch aswell.

I would argue here over the word "force". Having bindings defined here
does not force anyone into anything. The out-of-tree can do whatever
they want. It is a wish from kernel side - it might be respected but
it might not.

Just to be sure - I am not opposed against. Some time ago I wanted
Mali400 to be upstreamed but with current policy about user-space side
it is not possible.

Best regards,
Krzysztof

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Maxime Ripard]

On Tue, Jan 17, 2017 at 01:33:57PM +0200, Krzysztof Kozlowski wrote:
> On Tue, Jan 17, 2017 at 12:22 PM, Neil Armstrong
> <narmstrong@baylibre.com> wrote:
> > On 01/17/2017 10:38 AM, Maxime Ripard wrote:
> >> Hi,
> >>
> >> On Mon, Jan 16, 2017 at 08:49:06PM +0200, Krzysztof Kozlowski wrote:
> >>> On Mon, Jan 16, 2017 at 02:24:23PM +0100, Maxime Ripard wrote:
> >>>> The ARM Mali Utgard GPU family is embedded into a number of SoCs from
> >>>> Allwinner, Amlogic, Mediatek or Rockchip.
> >>>>
> >>>> Add a binding for the GPU of that family.
> >>>>
> >>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >>>> ---
> >>>>  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
> >>>>  1 file changed, 76 insertions(+)
> >>>>  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> >>>
> >>> Do you have a driver in kernel which will implement these bindings?
> >>
> >> No, but we have bindings for out-of-tree drivers already.
> >>
> >>> Defining them for out-of-tree driver does not bring any benefits
> >>> (3rd party driver will not respect them anyway).
> >>
> >> You could see it the other way around too. The out-of-tree drivers
> >> don't respect it at the moment because there's no binding to respect.
> >>
> >> And at least for us, we definitely plan on doing that.
> >>
> >> Maxime
> >
> > Hi Maxime, Krzysztof,
> >
> > We hope this will be accepted so it will solve the same issue we have on Amlogic SoCs
> > and all the other mali powered SoCs.
> 
> It will be helpful also for other SoCs using Mali 400 (e.g.
> Exynos3250, Exynos4412).

Yep, hence why you were in Cc :)

> > Having mainline bindings will forcre out-of-tree driver to respect those bindings
> > and remove a dts out-of-tree patch aswell.
> 
> I would argue here over the word "force". Having bindings defined here
> does not force anyone into anything. The out-of-tree can do whatever
> they want. It is a wish from kernel side - it might be respected but
> it might not.

Well, that statement can be made for each line of the kernel itself,
nothing forces you to use the code as is, or to make any kind of
modifications, including to its ABI. You just have an incentive to
respect the various frameworks and ABI because it's the path of least
resistance and all the tools, libraries and applications already know
how to use those interfaces.

But that's just it, an incentive, that anyone is free or not to comply
to.

> Just to be sure - I am not opposed against. Some time ago I wanted
> Mali400 to be upstreamed but with current policy about user-space side
> it is not possible.

While I would also very much like to see the kernel driver upstreamed,
this is unreasonable at this point. But this patch is here only to add
a binding to it, so that the out-of-tree drivers can comply to it, and
this is something we already have done.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170117/9e0c63ec/attachment-0001.sig>

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Maxime Ripard]

On Tue, Jan 17, 2017 at 01:31:19PM +0200, Krzysztof Kozlowski wrote:
> On Tue, Jan 17, 2017 at 11:38 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Hi,
> >
> > On Mon, Jan 16, 2017 at 08:49:06PM +0200, Krzysztof Kozlowski wrote:
> >> On Mon, Jan 16, 2017 at 02:24:23PM +0100, Maxime Ripard wrote:
> >> > The ARM Mali Utgard GPU family is embedded into a number of SoCs from
> >> > Allwinner, Amlogic, Mediatek or Rockchip.
> >> >
> >> > Add a binding for the GPU of that family.
> >> >
> >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >> > ---
> >> >  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
> >> >  1 file changed, 76 insertions(+)
> >> >  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> >>
> >> Do you have a driver in kernel which will implement these bindings?
> >
> > No, but we have bindings for out-of-tree drivers already.
> >
> >> Defining them for out-of-tree driver does not bring any benefits
> >> (3rd party driver will not respect them anyway).
> >
> > You could see it the other way around too. The out-of-tree drivers
> > don't respect it at the moment because there's no binding to respect.
> 
> Indeed, that's a point. However valid only when the out-of-tree driver
> will respect them, for example do not break them on next release.

If you're talking about the default platform provided by ARM, you also
have the option of creating your own. That's what we did, and it
worked like a charm.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170117/9fc96800/attachment.sig>

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Linus Walleij]

On Mon, Jan 16, 2017 at 2:24 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:

> The ARM Mali Utgard GPU family is embedded into a number of SoCs from
> Allwinner, Amlogic, Mediatek or Rockchip.
>
> Add a binding for the GPU of that family.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>
> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> new file mode 100644
> index 000000000000..df05ba0ec357
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> @@ -0,0 +1,76 @@
> +ARM Mali Utgard GPU
> +===================
> +
> +Required properties:
> +  - compatible:
> +    * "arm,mali-utgard" and one of the following:
> +      + "arm,mali-300"
> +      + "arm,mali-400"
> +      + "arm,mali-450"
> +
> +  - reg: Physical base address and length of the GPU registers
> +
> +  - interrupts: an entry for each entry in interrupt-names.
> +    See ../interrupt-controller/interrupts.txt for details.
> +
> +  - interrupt-names:
> +    * ppX: Pixel Processor X interrupt (X from 0 to 7)
> +    * ppmmuX: Pixel Processor X MMU interrupt (X from 0 to 7)
> +    * pp: Pixel Processor broadcast interrupt (mali-450 only)
> +    * gp: Geometry Processor interrupt
> +    * gpmmu: Geometry Processor MMU interrupt
> +
> +
> +Optional properties:
> +  - interrupt-names:
> +    * pmu: Power Management Unit interrupt, if implemented in hardware

On the MALI-400 MP in the ST-Ericsson DB8500 we have an additional interrupt
called "Mali400 combined". This is simply the HW designer's
doing an OR over all the 4 IRQ lines. Is this useful to define in the
bindings? Then it should be an optional

 * combined: all lines OR:ed together (if available)

Also you are defining "resets" below in the examples, should
this be listed as an optional property?

> +The Mali GPU is integrated very differently from one SoC to
> +another. In order to accommodate those differences, you have the option
> +to specify one more vendor-specific compatible, among:
> +
> +  - allwinner,sun4i-a10-mali
> +    Required properties:
> +      * clocks: an entry for each entry in clock-names
> +      * clock-names:
> +        + bus: bus clock for the GPU
> +        + core: clock driving the GPU itself
> +      * resets: phandle to the reset line for the GPU
> +
> +  - allwinner,sun7i-a20-mali
> +    Required properties:
> +      * clocks: an entry for each entry in clock-names
> +      * clock-names:
> +        + bus: bus clock for the GPU
> +        + core: clock driving the GPU itself
> +      * resets: phandle to the reset line for the GPU

Please add:

- stericsson,db8500-mali: also known as the "Smart Graphics
Accelerator" (SGA500)
   Required properties:
    * clocks: an entry for each entry in clock-names
    * clock-names:
      + bus: bus clock for the GPU (ICNCLK a.k.a. PRCMU_ACLK)
      + core: clock driving the GPU itself (PRCMU_SGACLK)

(It has no explicit reset line.)

With these:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Maxime Ripard]

Hi Linus,

On Thu, Jan 19, 2017 at 12:09:37AM +0100, Linus Walleij wrote:
> On Mon, Jan 16, 2017 at 2:24 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> 
> > The ARM Mali Utgard GPU family is embedded into a number of SoCs from
> > Allwinner, Amlogic, Mediatek or Rockchip.
> >
> > Add a binding for the GPU of that family.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
> >  1 file changed, 76 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> >
> > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> > new file mode 100644
> > index 000000000000..df05ba0ec357
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> > @@ -0,0 +1,76 @@
> > +ARM Mali Utgard GPU
> > +===================
> > +
> > +Required properties:
> > +  - compatible:
> > +    * "arm,mali-utgard" and one of the following:
> > +      + "arm,mali-300"
> > +      + "arm,mali-400"
> > +      + "arm,mali-450"
> > +
> > +  - reg: Physical base address and length of the GPU registers
> > +
> > +  - interrupts: an entry for each entry in interrupt-names.
> > +    See ../interrupt-controller/interrupts.txt for details.
> > +
> > +  - interrupt-names:
> > +    * ppX: Pixel Processor X interrupt (X from 0 to 7)
> > +    * ppmmuX: Pixel Processor X MMU interrupt (X from 0 to 7)
> > +    * pp: Pixel Processor broadcast interrupt (mali-450 only)
> > +    * gp: Geometry Processor interrupt
> > +    * gpmmu: Geometry Processor MMU interrupt
> > +
> > +
> > +Optional properties:
> > +  - interrupt-names:
> > +    * pmu: Power Management Unit interrupt, if implemented in hardware
> 
> On the MALI-400 MP in the ST-Ericsson DB8500 we have an additional interrupt
> called "Mali400 combined". This is simply the HW designer's
> doing an OR over all the 4 IRQ lines. Is this useful to define in the
> bindings? Then it should be an optional

Do you still have all the other interrupts, or just this combined interrupt?

Either way, that should definitely be part of the binding, but maybe
as part of the vendor specific binding below?

I didn't encounter any other platform doing so when I gave it a
(quick) look.

>  * combined: all lines OR:ed together (if available)
> 
> Also you are defining "resets" below in the examples, should
> this be listed as an optional property?

In my mind, this is not optional. For some platforms, it's mandatory,
and for some, it's not there at all. IMO, this should really be a
mandatory property, but only for the compatibles that use it (just
like the clocks are).

> > +The Mali GPU is integrated very differently from one SoC to
> > +another. In order to accommodate those differences, you have the option
> > +to specify one more vendor-specific compatible, among:
> > +
> > +  - allwinner,sun4i-a10-mali
> > +    Required properties:
> > +      * clocks: an entry for each entry in clock-names
> > +      * clock-names:
> > +        + bus: bus clock for the GPU
> > +        + core: clock driving the GPU itself
> > +      * resets: phandle to the reset line for the GPU
> > +
> > +  - allwinner,sun7i-a20-mali
> > +    Required properties:
> > +      * clocks: an entry for each entry in clock-names
> > +      * clock-names:
> > +        + bus: bus clock for the GPU
> > +        + core: clock driving the GPU itself
> > +      * resets: phandle to the reset line for the GPU
> 
> Please add:
> 
> - stericsson,db8500-mali: also known as the "Smart Graphics
> Accelerator" (SGA500)
>    Required properties:
>     * clocks: an entry for each entry in clock-names
>     * clock-names:
>       + bus: bus clock for the GPU (ICNCLK a.k.a. PRCMU_ACLK)
>       + core: clock driving the GPU itself (PRCMU_SGACLK)
>
> (It has no explicit reset line.)

Ack.

> With these:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170119/57898ac2/attachment-0001.sig>

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Maxime Ripard]

Hi Neil,

On Tue, Jan 17, 2017 at 11:22:22AM +0100, Neil Armstrong wrote:
> On 01/17/2017 10:38 AM, Maxime Ripard wrote:
> > Hi,
> > 
> > On Mon, Jan 16, 2017 at 08:49:06PM +0200, Krzysztof Kozlowski wrote:
> >> On Mon, Jan 16, 2017 at 02:24:23PM +0100, Maxime Ripard wrote:
> >>> The ARM Mali Utgard GPU family is embedded into a number of SoCs from
> >>> Allwinner, Amlogic, Mediatek or Rockchip.
> >>>
> >>> Add a binding for the GPU of that family.
> >>>
> >>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >>> ---
> >>>  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
> >>>  1 file changed, 76 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> >>
> >> Do you have a driver in kernel which will implement these bindings?
> > 
> > No, but we have bindings for out-of-tree drivers already.
> > 
> >> Defining them for out-of-tree driver does not bring any benefits
> >> (3rd party driver will not respect them anyway).
> > 
> > You could see it the other way around too. The out-of-tree drivers
> > don't respect it at the moment because there's no binding to respect.
> > 
> > And at least for us, we definitely plan on doing that.
> > 
> > Maxime
> 
> Hi Maxime, Krzysztof,
> 
> We hope this will be accepted so it will solve the same issue we
> have on Amlogic SoCs and all the other mali powered SoCs.

Do you have any requirements (or weird case) that is not covered by
the current doc?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170119/c102004c/attachment.sig>

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Neil Armstrong]

On 01/19/2017 04:51 PM, Maxime Ripard wrote:
> Hi Neil,
> 
> On Tue, Jan 17, 2017 at 11:22:22AM +0100, Neil Armstrong wrote:
>> On 01/17/2017 10:38 AM, Maxime Ripard wrote:
>>> Hi,
>>>
>>> On Mon, Jan 16, 2017 at 08:49:06PM +0200, Krzysztof Kozlowski wrote:
>>>> On Mon, Jan 16, 2017 at 02:24:23PM +0100, Maxime Ripard wrote:
>>>>> The ARM Mali Utgard GPU family is embedded into a number of SoCs from
>>>>> Allwinner, Amlogic, Mediatek or Rockchip.
>>>>>
>>>>> Add a binding for the GPU of that family.
>>>>>
>>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>>>> ---
>>>>>  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
>>>>>  1 file changed, 76 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>>>>
>>>> Do you have a driver in kernel which will implement these bindings?
>>>
>>> No, but we have bindings for out-of-tree drivers already.
>>>
>>>> Defining them for out-of-tree driver does not bring any benefits
>>>> (3rd party driver will not respect them anyway).
>>>
>>> You could see it the other way around too. The out-of-tree drivers
>>> don't respect it at the moment because there's no binding to respect.
>>>
>>> And at least for us, we definitely plan on doing that.
>>>
>>> Maxime
>>
>> Hi Maxime, Krzysztof,
>>
>> We hope this will be accepted so it will solve the same issue we
>> have on Amlogic SoCs and all the other mali powered SoCs.
> 
> Do you have any requirements (or weird case) that is not covered by
> the current doc?
> 
> Thanks,
> Maxime
> 

Hi Maxime,

Apart the GPU DVFS, all is covered by the current doc for the Amlogic Meson GXBB and GXL SoCs.

Neil

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170119/869ba77a/attachment.sig>

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Rob Herring]

On Tue, Jan 17, 2017 at 10:38:13AM +0100, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Jan 16, 2017 at 08:49:06PM +0200, Krzysztof Kozlowski wrote:
> > On Mon, Jan 16, 2017 at 02:24:23PM +0100, Maxime Ripard wrote:
> > > The ARM Mali Utgard GPU family is embedded into a number of SoCs from
> > > Allwinner, Amlogic, Mediatek or Rockchip.
> > > 
> > > Add a binding for the GPU of that family.
> > > 
> > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > ---
> > >  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
> > >  1 file changed, 76 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> > 
> > Do you have a driver in kernel which will implement these bindings?
> 
> No, but we have bindings for out-of-tree drivers already.
> 
> > Defining them for out-of-tree driver does not bring any benefits
> > (3rd party driver will not respect them anyway).
> 
> You could see it the other way around too. The out-of-tree drivers
> don't respect it at the moment because there's no binding to respect.
> 
> And at least for us, we definitely plan on doing that.

I have no issue taking this. Bindings are in the kernel for 
convenience. Having a driver can help make sure they are complete and 
sound, but otherwise isn't strictly required.

In fact, the ARM folks already asked me about this, but you beat them to 
it (plus it's hard to get them to care about mali400 much).

Rob

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Rob Herring]

On Mon, Jan 16, 2017 at 02:24:23PM +0100, Maxime Ripard wrote:
> The ARM Mali Utgard GPU family is embedded into a number of SoCs from
> Allwinner, Amlogic, Mediatek or Rockchip.
> 
> Add a binding for the GPU of that family.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> 
> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> new file mode 100644
> index 000000000000..df05ba0ec357
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> @@ -0,0 +1,76 @@
> +ARM Mali Utgard GPU
> +===================
> +
> +Required properties:
> +  - compatible:
> +    * "arm,mali-utgard" and one of the following:

Drop this. It's meaningless and 3 compatibles to match is the kernel is 
not a big deal.

> +      + "arm,mali-300"
> +      + "arm,mali-400"
> +      + "arm,mali-450"
> +
> +  - reg: Physical base address and length of the GPU registers
> +
> +  - interrupts: an entry for each entry in interrupt-names.
> +    See ../interrupt-controller/interrupts.txt for details.
> +
> +  - interrupt-names:
> +    * ppX: Pixel Processor X interrupt (X from 0 to 7)
> +    * ppmmuX: Pixel Processor X MMU interrupt (X from 0 to 7)

Is the number of pixel processors probe-able? If not, then it needs to 
be implied by the vendor compatible string or a property.

> +    * pp: Pixel Processor broadcast interrupt (mali-450 only)
> +    * gp: Geometry Processor interrupt
> +    * gpmmu: Geometry Processor MMU interrupt

That's a lot of interrupts. Was going to ask about combining, but 
I see Linus raised that.

> +
> +
> +Optional properties:
> +  - interrupt-names:
> +    * pmu: Power Management Unit interrupt, if implemented in hardware
> +
> +Vendor-specific bindings
> +------------------------
> +
> +The Mali GPU is integrated very differently from one SoC to
> +another. In order to accommodate those differences, you have the option
> +to specify one more vendor-specific compatible, among:
> +
> +  - allwinner,sun4i-a10-mali

List this with the compatible strings. I assume one of the arm,mali-??? 
strings applies too.

> +    Required properties:
> +      * clocks: an entry for each entry in clock-names
> +      * clock-names:
> +        + bus: bus clock for the GPU
> +        + core: clock driving the GPU itself

This should be in the core binding. The number of clocks should not 
vary. We often get this wrong because the IP blocks get integrated and 
connected to the same clock source. But this should be equal to the 
number of clock inputs.

> +      * resets: phandle to the reset line for the GPU
> +
> +  - allwinner,sun7i-a20-mali
> +    Required properties:
> +      * clocks: an entry for each entry in clock-names
> +      * clock-names:
> +        + bus: bus clock for the GPU
> +        + core: clock driving the GPU itself
> +      * resets: phandle to the reset line for the GPU
> +
> +Example:
> +
> +mali: gpu at 01c40000 {
> +	compatible = "allwinner,sun7i-a20-mali", "arm,mali-400",
> +		     "arm,mali-utgard";
> +	reg = <0x01c40000 0x10000>;
> +	interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> +		     <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
> +	interrupt-names = "gp",
> +			  "gpmmu",
> +			  "pp0",
> +			  "ppmmu0",
> +			  "pp1",
> +			  "ppmmu1",
> +			  "pmu";
> +	clocks = <&ccu CLK_BUS_GPU>, <&ccu CLK_GPU>;
> +	clock-names = "bus", "core";
> +	resets = <&ccu RST_BUS_GPU>;
> +};
> +
> +
> -- 
> 2.11.0
> 

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[John Stultz]

On Mon, Jan 16, 2017 at 5:24 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The ARM Mali Utgard GPU family is embedded into a number of SoCs from
> Allwinner, Amlogic, Mediatek or Rockchip.
>
> Add a binding for the GPU of that family.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>
> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> new file mode 100644
> index 000000000000..df05ba0ec357
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> @@ -0,0 +1,76 @@
> +ARM Mali Utgard GPU
> +===================
> +
> +Required properties:
> +  - compatible:
> +    * "arm,mali-utgard" and one of the following:
> +      + "arm,mali-300"
> +      + "arm,mali-400"
> +      + "arm,mali-450"
> +
> +  - reg: Physical base address and length of the GPU registers
> +
> +  - interrupts: an entry for each entry in interrupt-names.
> +    See ../interrupt-controller/interrupts.txt for details.
> +
> +  - interrupt-names:
> +    * ppX: Pixel Processor X interrupt (X from 0 to 7)
> +    * ppmmuX: Pixel Processor X MMU interrupt (X from 0 to 7)
> +    * pp: Pixel Processor broadcast interrupt (mali-450 only)
> +    * gp: Geometry Processor interrupt
> +    * gpmmu: Geometry Processor MMU interrupt
> +
> +
> +Optional properties:
> +  - interrupt-names:
> +    * pmu: Power Management Unit interrupt, if implemented in hardware
> +
> +Vendor-specific bindings
> +------------------------
> +
> +The Mali GPU is integrated very differently from one SoC to
> +another. In order to accommodate those differences, you have the option
> +to specify one more vendor-specific compatible, among:
> +
> +  - allwinner,sun4i-a10-mali
> +    Required properties:
> +      * clocks: an entry for each entry in clock-names
> +      * clock-names:
> +        + bus: bus clock for the GPU
> +        + core: clock driving the GPU itself
> +      * resets: phandle to the reset line for the GPU
> +
> +  - allwinner,sun7i-a20-mali
> +    Required properties:
> +      * clocks: an entry for each entry in clock-names
> +      * clock-names:
> +        + bus: bus clock for the GPU
> +        + core: clock driving the GPU itself
> +      * resets: phandle to the reset line for the GPU
> +
> +Example:
> +
> +mali: gpu at 01c40000 {
> +       compatible = "allwinner,sun7i-a20-mali", "arm,mali-400",
> +                    "arm,mali-utgard";
> +       reg = <0x01c40000 0x10000>;
> +       interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
> +                    <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
> +                    <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
> +                    <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
> +                    <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
> +                    <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> +                    <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
> +       interrupt-names = "gp",
> +                         "gpmmu",
> +                         "pp0",
> +                         "ppmmu0",
> +                         "pp1",
> +                         "ppmmu1",
> +                         "pmu";
> +       clocks = <&ccu CLK_BUS_GPU>, <&ccu CLK_GPU>;
> +       clock-names = "bus", "core";
> +       resets = <&ccu RST_BUS_GPU>;
> +};
> +
> +


Having a mali utgard binding upstream would be great. However I'm a
little worried that the mali driver I've used sort of only half way
uses DT, and still requires a custom built in platform driver to setup
numerous other things.  Curious if you have a pointer to the kernel
driver you've been using with the vendor specific bindings above?  I'd
like to try to adapt what we have to your method to validate the above
as generic.

And, just for context, here's the node we've been using with hikey:

                mali:mali at f4080000 {
                        compatible = "arm,mali-450", "arm,mali-utgard";
                        reg = <0x0 0x3f100000 0x0 0x00708000>;
                        clocks = <&media_ctrl HI6220_G3D_CLK>,
                                 <&media_ctrl HI6220_G3D_PCLK>;
                        clock-names = "clk_g3d", "pclk_g3d";
                        mali_def_freq = <500>;
                        pclk_freq = <144>;
                        dfs_steps = <2>;
                        dfs_lockprf = <1>;
                        dfs_limit_max_prf = <1>;
                        dfs_profile_num = <2>;
                        dfs_profiles = <250 3 0>, <500 1 0>;
                        mali_type = <2>;

                        interrupt-parent = <&gic>;
                        interrupts =    <1 126 4>, /*gp*/
                                        <1 126 4>, /*gp mmu*/
                                        <1 126 4>, /*pp bc*/
                                        <1 126 4>, /*pmu*/
                                        <1 126 4>, /*pp0*/
                                        <1 126 4>,
                                        <1 126 4>, /*pp1*/
                                        <1 126 4>,
                                        <1 126 4>, /*pp2*/
                                        <1 126 4>,
                                        <1 126 4>, /*pp4*/
                                        <1 126 4>,
                                        <1 126 4>, /*pp5*/
                                        <1 126 4>,
                                        <1 126 4>, /*pp6*/
                                        <1 126 4>;
                        interrupt-names = "IRQGP", "IRQGPMMU",
"IRQPP", "IRQPMU",
                                        "IRQPP0", "IRQPPMMU0",
"IRQPP1", "IRQPPMMU1",
                                        "IRQPP2",
"IRQPPMMU2","IRQPP4", "IRQPPMMU4",
                                        "IRQPP5", "IRQPPMMU5",
"IRQPP6", "IRQPPMMU6";
                };



thanks
-john

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Maxime Ripard]

Hi Rob,

On Thu, Jan 19, 2017 at 10:16:48AM -0600, Rob Herring wrote:
> On Mon, Jan 16, 2017 at 02:24:23PM +0100, Maxime Ripard wrote:
> > The ARM Mali Utgard GPU family is embedded into a number of SoCs from
> > Allwinner, Amlogic, Mediatek or Rockchip.
> > 
> > Add a binding for the GPU of that family.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
> >  1 file changed, 76 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> > new file mode 100644
> > index 000000000000..df05ba0ec357
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> > @@ -0,0 +1,76 @@
> > +ARM Mali Utgard GPU
> > +===================
> > +
> > +Required properties:
> > +  - compatible:
> > +    * "arm,mali-utgard" and one of the following:
> 
> Drop this. It's meaningless and 3 compatibles to match is the kernel is 
> not a big deal.

Ok.

> > +      + "arm,mali-300"
> > +      + "arm,mali-400"
> > +      + "arm,mali-450"
> > +
> > +  - reg: Physical base address and length of the GPU registers
> > +
> > +  - interrupts: an entry for each entry in interrupt-names.
> > +    See ../interrupt-controller/interrupts.txt for details.
> > +
> > +  - interrupt-names:
> > +    * ppX: Pixel Processor X interrupt (X from 0 to 7)
> > +    * ppmmuX: Pixel Processor X MMU interrupt (X from 0 to 7)
> 
> Is the number of pixel processors probe-able? If not, then it needs to 
> be implied by the vendor compatible string or a property.

Yes, this is something that can be discovered by reading the pixel
processors version register. If the value is not 0, the processor is
there, otherwise it's not.

> > +    * pp: Pixel Processor broadcast interrupt (mali-450 only)
> > +    * gp: Geometry Processor interrupt
> > +    * gpmmu: Geometry Processor MMU interrupt
> 
> That's a lot of interrupts. Was going to ask about combining, but 
> I see Linus raised that.

Yes, apparently, some SoCs use that, but that doesn't seem to be a
standard feature (or at least, it's not used on our SoCs). So we're
stuck with those :/

> > +Optional properties:
> > +  - interrupt-names:
> > +    * pmu: Power Management Unit interrupt, if implemented in hardware
> > +
> > +Vendor-specific bindings
> > +------------------------
> > +
> > +The Mali GPU is integrated very differently from one SoC to
> > +another. In order to accommodate those differences, you have the option
> > +to specify one more vendor-specific compatible, among:
> > +
> > +  - allwinner,sun4i-a10-mali
> 
> List this with the compatible strings. I assume one of the arm,mali-??? 
> strings applies too.

Yep.

> > +    Required properties:
> > +      * clocks: an entry for each entry in clock-names
> > +      * clock-names:
> > +        + bus: bus clock for the GPU
> > +        + core: clock driving the GPU itself
> 
> This should be in the core binding. The number of clocks should not 
> vary. We often get this wrong because the IP blocks get integrated and 
> connected to the same clock source. But this should be equal to the 
> number of clock inputs.

Ok. So far, the Allwinner one and the examples from Linus and John
have been using two clocks, but apparently on rockchip it's using a
single one, which is why I made it that way. I'll move that to the
core binding.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170120/c89b2246/attachment.sig>

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Linus Walleij]

On Thu, Jan 19, 2017 at 4:49 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:

>> > +Optional properties:
>> > +  - interrupt-names:
>> > +    * pmu: Power Management Unit interrupt, if implemented in hardware
>>
>> On the MALI-400 MP in the ST-Ericsson DB8500 we have an additional interrupt
>> called "Mali400 combined". This is simply the HW designer's
>> doing an OR over all the 4 IRQ lines. Is this useful to define in the
>> bindings? Then it should be an optional
>
> Do you still have all the other interrupts, or just this combined interrupt?

We have the others too, I guess some HW engineer just thought it'd
be nice to have the combo.

> Either way, that should definitely be part of the binding, but maybe
> as part of the vendor specific binding below?

OK.

>>  * combined: all lines OR:ed together (if available)
>>
>> Also you are defining "resets" below in the examples, should
>> this be listed as an optional property?
>
> In my mind, this is not optional. For some platforms, it's mandatory,
> and for some, it's not there at all. IMO, this should really be a
> mandatory property, but only for the compatibles that use it (just
> like the clocks are).

OK.

Yours,
LInus Walleij

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Maxime Ripard]

Hi John,

On Thu, Jan 19, 2017 at 11:24:38AM -0800, John Stultz wrote:
> On Mon, Jan 16, 2017 at 5:24 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > The ARM Mali Utgard GPU family is embedded into a number of SoCs from
> > Allwinner, Amlogic, Mediatek or Rockchip.
> >
> > Add a binding for the GPU of that family.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
> >  1 file changed, 76 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> >
> > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> > new file mode 100644
> > index 000000000000..df05ba0ec357
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> > @@ -0,0 +1,76 @@
> > +ARM Mali Utgard GPU
> > +===================
> > +
> > +Required properties:
> > +  - compatible:
> > +    * "arm,mali-utgard" and one of the following:
> > +      + "arm,mali-300"
> > +      + "arm,mali-400"
> > +      + "arm,mali-450"
> > +
> > +  - reg: Physical base address and length of the GPU registers
> > +
> > +  - interrupts: an entry for each entry in interrupt-names.
> > +    See ../interrupt-controller/interrupts.txt for details.
> > +
> > +  - interrupt-names:
> > +    * ppX: Pixel Processor X interrupt (X from 0 to 7)
> > +    * ppmmuX: Pixel Processor X MMU interrupt (X from 0 to 7)
> > +    * pp: Pixel Processor broadcast interrupt (mali-450 only)
> > +    * gp: Geometry Processor interrupt
> > +    * gpmmu: Geometry Processor MMU interrupt
> > +
> > +
> > +Optional properties:
> > +  - interrupt-names:
> > +    * pmu: Power Management Unit interrupt, if implemented in hardware
> > +
> > +Vendor-specific bindings
> > +------------------------
> > +
> > +The Mali GPU is integrated very differently from one SoC to
> > +another. In order to accommodate those differences, you have the option
> > +to specify one more vendor-specific compatible, among:
> > +
> > +  - allwinner,sun4i-a10-mali
> > +    Required properties:
> > +      * clocks: an entry for each entry in clock-names
> > +      * clock-names:
> > +        + bus: bus clock for the GPU
> > +        + core: clock driving the GPU itself
> > +      * resets: phandle to the reset line for the GPU
> > +
> > +  - allwinner,sun7i-a20-mali
> > +    Required properties:
> > +      * clocks: an entry for each entry in clock-names
> > +      * clock-names:
> > +        + bus: bus clock for the GPU
> > +        + core: clock driving the GPU itself
> > +      * resets: phandle to the reset line for the GPU
> > +
> > +Example:
> > +
> > +mali: gpu at 01c40000 {
> > +       compatible = "allwinner,sun7i-a20-mali", "arm,mali-400",
> > +                    "arm,mali-utgard";
> > +       reg = <0x01c40000 0x10000>;
> > +       interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
> > +                    <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
> > +                    <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
> > +                    <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
> > +                    <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
> > +                    <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> > +                    <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
> > +       interrupt-names = "gp",
> > +                         "gpmmu",
> > +                         "pp0",
> > +                         "ppmmu0",
> > +                         "pp1",
> > +                         "ppmmu1",
> > +                         "pmu";
> > +       clocks = <&ccu CLK_BUS_GPU>, <&ccu CLK_GPU>;
> > +       clock-names = "bus", "core";
> > +       resets = <&ccu RST_BUS_GPU>;
> > +};
> 
> Having a mali utgard binding upstream would be great. However I'm a
> little worried that the mali driver I've used sort of only half way
> uses DT, and still requires a custom built in platform driver to setup
> numerous other things.  Curious if you have a pointer to the kernel
> driver you've been using with the vendor specific bindings above?  I'd
> like to try to adapt what we have to your method to validate the above
> as generic.

I've created a custom platform driver, so just like you it seems,
because I've not managed to make ARM's DT support work.

https://github.com/mripard/sunxi-mali/blob/master/driver/src/devicedrv/mali/platform/sunxi/sunxi.c

I haven't updated it yet with the bindings suggested above, but only
the interrupt and clock names have changed. The rest very much
applies.

The only thing that might be vendor specific would be the (optional)
declaration of the mali_gpu_device_data structure. As far as I know,
there's two things of importance there:
  - the list of the valid OPPs in order to do DVFS, but that could be
    made generic too using the operating-points binding

  - And the valid area for the buffers for the fbdev blobs (fb_start,
    fb_size and shared_mem_size). I'm not entirely happy with this one
    so far (which is also why I've not pushed it). We'd need to come
    up with a way to get the base address and size of the CMA region
    backing the framebuffer allocation, but I haven't find any trivial
    way to do so, so for now I just hardcoded it. Worst case scenario,
    we can hardcode values based on the compatible.

> And, just for context, here's the node we've been using with hikey:
> 
>                 mali:mali at f4080000 {
>                         compatible = "arm,mali-450", "arm,mali-utgard";
>                         reg = <0x0 0x3f100000 0x0 0x00708000>;

This is because the hikey is using a 64 bits CPU, right?

>                         clocks = <&media_ctrl HI6220_G3D_CLK>,
>                                  <&media_ctrl HI6220_G3D_PCLK>;
>                         clock-names = "clk_g3d", "pclk_g3d";
>                         mali_def_freq = <500>;
>                         pclk_freq = <144>;
>                         dfs_steps = <2>;
>                         dfs_lockprf = <1>;
>                         dfs_limit_max_prf = <1>;
>                         dfs_profile_num = <2>;
>                         dfs_profiles = <250 3 0>, <500 1 0>;
>                         mali_type = <2>;
> 
>                         interrupt-parent = <&gic>;
>                         interrupts =    <1 126 4>, /*gp*/
>                                         <1 126 4>, /*gp mmu*/
>                                         <1 126 4>, /*pp bc*/
>                                         <1 126 4>, /*pmu*/
>                                         <1 126 4>, /*pp0*/
>                                         <1 126 4>,
>                                         <1 126 4>, /*pp1*/
>                                         <1 126 4>,
>                                         <1 126 4>, /*pp2*/
>                                         <1 126 4>,
>                                         <1 126 4>, /*pp4*/
>                                         <1 126 4>,
>                                         <1 126 4>, /*pp5*/
>                                         <1 126 4>,
>                                         <1 126 4>, /*pp6*/
>                                         <1 126 4>;
>                         interrupt-names = "IRQGP", "IRQGPMMU",
> "IRQPP", "IRQPMU",
>                                         "IRQPP0", "IRQPPMMU0",
> "IRQPP1", "IRQPPMMU1",
>                                         "IRQPP2",
> "IRQPPMMU2","IRQPP4", "IRQPPMMU4",
>                                         "IRQPP5", "IRQPPMMU5",
> "IRQPP6", "IRQPPMMU6";
>                 };

So all your interrupts are shared?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170120/22b35a87/attachment.sig>

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Rob Herring]

On Thu, Jan 19, 2017 at 1:24 PM, John Stultz <john.stultz@linaro.org> wrote:
> On Mon, Jan 16, 2017 at 5:24 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> The ARM Mali Utgard GPU family is embedded into a number of SoCs from
>> Allwinner, Amlogic, Mediatek or Rockchip.
>>
>> Add a binding for the GPU of that family.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> ---
>>  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
>>  1 file changed, 76 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>>
>> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>> new file mode 100644
>> index 000000000000..df05ba0ec357
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>> @@ -0,0 +1,76 @@
>> +ARM Mali Utgard GPU
>> +===================
>> +
>> +Required properties:
>> +  - compatible:
>> +    * "arm,mali-utgard" and one of the following:
>> +      + "arm,mali-300"
>> +      + "arm,mali-400"
>> +      + "arm,mali-450"
>> +
>> +  - reg: Physical base address and length of the GPU registers
>> +
>> +  - interrupts: an entry for each entry in interrupt-names.
>> +    See ../interrupt-controller/interrupts.txt for details.
>> +
>> +  - interrupt-names:
>> +    * ppX: Pixel Processor X interrupt (X from 0 to 7)
>> +    * ppmmuX: Pixel Processor X MMU interrupt (X from 0 to 7)
>> +    * pp: Pixel Processor broadcast interrupt (mali-450 only)
>> +    * gp: Geometry Processor interrupt
>> +    * gpmmu: Geometry Processor MMU interrupt
>> +
>> +
>> +Optional properties:
>> +  - interrupt-names:
>> +    * pmu: Power Management Unit interrupt, if implemented in hardware
>> +
>> +Vendor-specific bindings
>> +------------------------
>> +
>> +The Mali GPU is integrated very differently from one SoC to
>> +another. In order to accommodate those differences, you have the option
>> +to specify one more vendor-specific compatible, among:
>> +
>> +  - allwinner,sun4i-a10-mali
>> +    Required properties:
>> +      * clocks: an entry for each entry in clock-names
>> +      * clock-names:
>> +        + bus: bus clock for the GPU
>> +        + core: clock driving the GPU itself
>> +      * resets: phandle to the reset line for the GPU
>> +
>> +  - allwinner,sun7i-a20-mali
>> +    Required properties:
>> +      * clocks: an entry for each entry in clock-names
>> +      * clock-names:
>> +        + bus: bus clock for the GPU
>> +        + core: clock driving the GPU itself
>> +      * resets: phandle to the reset line for the GPU
>> +
>> +Example:
>> +
>> +mali: gpu at 01c40000 {
>> +       compatible = "allwinner,sun7i-a20-mali", "arm,mali-400",
>> +                    "arm,mali-utgard";
>> +       reg = <0x01c40000 0x10000>;
>> +       interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
>> +                    <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
>> +                    <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
>> +                    <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
>> +                    <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
>> +                    <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
>> +                    <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
>> +       interrupt-names = "gp",
>> +                         "gpmmu",
>> +                         "pp0",
>> +                         "ppmmu0",
>> +                         "pp1",
>> +                         "ppmmu1",
>> +                         "pmu";
>> +       clocks = <&ccu CLK_BUS_GPU>, <&ccu CLK_GPU>;
>> +       clock-names = "bus", "core";
>> +       resets = <&ccu RST_BUS_GPU>;
>> +};
>> +
>> +
>
>
> Having a mali utgard binding upstream would be great. However I'm a
> little worried that the mali driver I've used sort of only half way
> uses DT, and still requires a custom built in platform driver to setup
> numerous other things.  Curious if you have a pointer to the kernel
> driver you've been using with the vendor specific bindings above?  I'd
> like to try to adapt what we have to your method to validate the above
> as generic.
>
> And, just for context, here's the node we've been using with hikey:
>
>                 mali:mali at f4080000 {
>                         compatible = "arm,mali-450", "arm,mali-utgard";
>                         reg = <0x0 0x3f100000 0x0 0x00708000>;

Why's the size 7MB?

>                         clocks = <&media_ctrl HI6220_G3D_CLK>,
>                                  <&media_ctrl HI6220_G3D_PCLK>;
>                         clock-names = "clk_g3d", "pclk_g3d";

Seems like these are swapped from Maxime's order. Based on the pclk
frequency, that's the bus clock.

>                         mali_def_freq = <500>;
>                         pclk_freq = <144>;

There's a standard property for these with assigned-clocks.

>                         dfs_steps = <2>;
>                         dfs_lockprf = <1>;
>                         dfs_limit_max_prf = <1>;
>                         dfs_profile_num = <2>;
>                         dfs_profiles = <250 3 0>, <500 1 0>;

This looks like a OPP table. It can all be driver data associated with
the compatible string for now.

>                         mali_type = <2>;

Not sure about this one.

Rob

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Rob Herring]

On Fri, Jan 20, 2017 at 3:16 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi John,
>
> On Thu, Jan 19, 2017 at 11:24:38AM -0800, John Stultz wrote:
>> On Mon, Jan 16, 2017 at 5:24 AM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > The ARM Mali Utgard GPU family is embedded into a number of SoCs from
>> > Allwinner, Amlogic, Mediatek or Rockchip.
>> >
>> > Add a binding for the GPU of that family.
>> >
>> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> > ---
>> >  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
>> >  1 file changed, 76 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>> > new file mode 100644
>> > index 000000000000..df05ba0ec357
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>> > @@ -0,0 +1,76 @@
>> > +ARM Mali Utgard GPU
>> > +===================
>> > +
>> > +Required properties:
>> > +  - compatible:
>> > +    * "arm,mali-utgard" and one of the following:
>> > +      + "arm,mali-300"
>> > +      + "arm,mali-400"
>> > +      + "arm,mali-450"
>> > +
>> > +  - reg: Physical base address and length of the GPU registers
>> > +
>> > +  - interrupts: an entry for each entry in interrupt-names.
>> > +    See ../interrupt-controller/interrupts.txt for details.
>> > +
>> > +  - interrupt-names:
>> > +    * ppX: Pixel Processor X interrupt (X from 0 to 7)
>> > +    * ppmmuX: Pixel Processor X MMU interrupt (X from 0 to 7)
>> > +    * pp: Pixel Processor broadcast interrupt (mali-450 only)
>> > +    * gp: Geometry Processor interrupt
>> > +    * gpmmu: Geometry Processor MMU interrupt
>> > +
>> > +
>> > +Optional properties:
>> > +  - interrupt-names:
>> > +    * pmu: Power Management Unit interrupt, if implemented in hardware
>> > +
>> > +Vendor-specific bindings
>> > +------------------------
>> > +
>> > +The Mali GPU is integrated very differently from one SoC to
>> > +another. In order to accommodate those differences, you have the option
>> > +to specify one more vendor-specific compatible, among:
>> > +
>> > +  - allwinner,sun4i-a10-mali
>> > +    Required properties:
>> > +      * clocks: an entry for each entry in clock-names
>> > +      * clock-names:
>> > +        + bus: bus clock for the GPU
>> > +        + core: clock driving the GPU itself
>> > +      * resets: phandle to the reset line for the GPU
>> > +
>> > +  - allwinner,sun7i-a20-mali
>> > +    Required properties:
>> > +      * clocks: an entry for each entry in clock-names
>> > +      * clock-names:
>> > +        + bus: bus clock for the GPU
>> > +        + core: clock driving the GPU itself
>> > +      * resets: phandle to the reset line for the GPU
>> > +
>> > +Example:
>> > +
>> > +mali: gpu at 01c40000 {
>> > +       compatible = "allwinner,sun7i-a20-mali", "arm,mali-400",
>> > +                    "arm,mali-utgard";
>> > +       reg = <0x01c40000 0x10000>;
>> > +       interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
>> > +                    <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
>> > +                    <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
>> > +                    <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
>> > +                    <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
>> > +                    <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
>> > +                    <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
>> > +       interrupt-names = "gp",
>> > +                         "gpmmu",
>> > +                         "pp0",
>> > +                         "ppmmu0",
>> > +                         "pp1",
>> > +                         "ppmmu1",
>> > +                         "pmu";
>> > +       clocks = <&ccu CLK_BUS_GPU>, <&ccu CLK_GPU>;
>> > +       clock-names = "bus", "core";
>> > +       resets = <&ccu RST_BUS_GPU>;
>> > +};
>>
>> Having a mali utgard binding upstream would be great. However I'm a
>> little worried that the mali driver I've used sort of only half way
>> uses DT, and still requires a custom built in platform driver to setup
>> numerous other things.  Curious if you have a pointer to the kernel
>> driver you've been using with the vendor specific bindings above?  I'd
>> like to try to adapt what we have to your method to validate the above
>> as generic.
>
> I've created a custom platform driver, so just like you it seems,
> because I've not managed to make ARM's DT support work.
>
> https://github.com/mripard/sunxi-mali/blob/master/driver/src/devicedrv/mali/platform/sunxi/sunxi.c
>
> I haven't updated it yet with the bindings suggested above, but only
> the interrupt and clock names have changed. The rest very much
> applies.
>
> The only thing that might be vendor specific would be the (optional)
> declaration of the mali_gpu_device_data structure. As far as I know,
> there's two things of importance there:
>   - the list of the valid OPPs in order to do DVFS, but that could be
>     made generic too using the operating-points binding
>
>   - And the valid area for the buffers for the fbdev blobs (fb_start,
>     fb_size and shared_mem_size). I'm not entirely happy with this one
>     so far (which is also why I've not pushed it). We'd need to come
>     up with a way to get the base address and size of the CMA region
>     backing the framebuffer allocation, but I haven't find any trivial
>     way to do so, so for now I just hardcoded it. Worst case scenario,
>     we can hardcode values based on the compatible.

memory-region property?

>
>> And, just for context, here's the node we've been using with hikey:
>>
>>                 mali:mali at f4080000 {
>>                         compatible = "arm,mali-450", "arm,mali-utgard";
>>                         reg = <0x0 0x3f100000 0x0 0x00708000>;
>
> This is because the hikey is using a 64 bits CPU, right?

Having 2 cells for address and size is generally entirely pointless.
The peripheral regions generally don't need 64-bits of address space
or size. ranges should be used.

Rob

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Maxime Ripard]

Hi Rob,

On Fri, Jan 20, 2017 at 08:15:21AM -0600, Rob Herring wrote:
> > The only thing that might be vendor specific would be the (optional)
> > declaration of the mali_gpu_device_data structure. As far as I know,
> > there's two things of importance there:
> >   - the list of the valid OPPs in order to do DVFS, but that could be
> >     made generic too using the operating-points binding
> >
> >   - And the valid area for the buffers for the fbdev blobs (fb_start,
> >     fb_size and shared_mem_size). I'm not entirely happy with this one
> >     so far (which is also why I've not pushed it). We'd need to come
> >     up with a way to get the base address and size of the CMA region
> >     backing the framebuffer allocation, but I haven't find any trivial
> >     way to do so, so for now I just hardcoded it. Worst case scenario,
> >     we can hardcode values based on the compatible.
> 
> memory-region property?

That might do it, however those buffers usually are allocated by the
display engine, and not the GPU device. Even if we declare a reserved
memory region and use memory-region in the GPU node, chances are that
buffers allocated for the display may not be in that region (for
example if it's using the default CMA pool).

Unless we create a memory region for the default CMA pool?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170123/baa0c2b3/attachment.sig>

[PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings

[Mason]

On 19/01/2017 20:24, John Stultz wrote:

> On Mon, Jan 16, 2017 at 5:24 AM, Maxime Ripard wrote:
>
>> The ARM Mali Utgard GPU family is embedded into a number of SoCs from
>> Allwinner, Amlogic, Mediatek or Rockchip.
>>
>> Add a binding for the GPU of that family.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> ---
>>  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
>>  1 file changed, 76 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>>
>> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>> new file mode 100644
>> index 000000000000..df05ba0ec357
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>> @@ -0,0 +1,76 @@
>> +ARM Mali Utgard GPU
>> +===================
>> +
>> +Required properties:
>> +  - compatible:
>> +    * "arm,mali-utgard" and one of the following:
>> +      + "arm,mali-300"
>> +      + "arm,mali-400"
>> +      + "arm,mali-450"
>> +
>> +  - reg: Physical base address and length of the GPU registers
>> +
>> +  - interrupts: an entry for each entry in interrupt-names.
>> +    See ../interrupt-controller/interrupts.txt for details.
>> +
>> +  - interrupt-names:
>> +    * ppX: Pixel Processor X interrupt (X from 0 to 7)
>> +    * ppmmuX: Pixel Processor X MMU interrupt (X from 0 to 7)
>> +    * pp: Pixel Processor broadcast interrupt (mali-450 only)
>> +    * gp: Geometry Processor interrupt
>> +    * gpmmu: Geometry Processor MMU interrupt
>> +
>> +
>> +Optional properties:
>> +  - interrupt-names:
>> +    * pmu: Power Management Unit interrupt, if implemented in hardware
>> +
>> +Vendor-specific bindings
>> +------------------------
>> +
>> +The Mali GPU is integrated very differently from one SoC to
>> +another. In order to accommodate those differences, you have the option
>> +to specify one more vendor-specific compatible, among:
>> +
>> +  - allwinner,sun4i-a10-mali
>> +    Required properties:
>> +      * clocks: an entry for each entry in clock-names
>> +      * clock-names:
>> +        + bus: bus clock for the GPU
>> +        + core: clock driving the GPU itself
>> +      * resets: phandle to the reset line for the GPU
>> +
>> +  - allwinner,sun7i-a20-mali
>> +    Required properties:
>> +      * clocks: an entry for each entry in clock-names
>> +      * clock-names:
>> +        + bus: bus clock for the GPU
>> +        + core: clock driving the GPU itself
>> +      * resets: phandle to the reset line for the GPU
>> +
>> +Example:
>> +
>> +mali: gpu at 01c40000 {
>> +       compatible = "allwinner,sun7i-a20-mali", "arm,mali-400",
>> +                    "arm,mali-utgard";
>> +       reg = <0x01c40000 0x10000>;
>> +       interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
>> +                    <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
>> +                    <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
>> +                    <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
>> +                    <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
>> +                    <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
>> +                    <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
>> +       interrupt-names = "gp",
>> +                         "gpmmu",
>> +                         "pp0",
>> +                         "ppmmu0",
>> +                         "pp1",
>> +                         "ppmmu1",
>> +                         "pmu";
>> +       clocks = <&ccu CLK_BUS_GPU>, <&ccu CLK_GPU>;
>> +       clock-names = "bus", "core";
>> +       resets = <&ccu RST_BUS_GPU>;
>> +};
>> +
>> +
> 
> 
> Having a mali utgard binding upstream would be great. However I'm a
> little worried that the mali driver I've used sort of only half way
> uses DT, and still requires a custom built in platform driver to setup
> numerous other things.  Curious if you have a pointer to the kernel
> driver you've been using with the vendor specific bindings above?  I'd
> like to try to adapt what we have to your method to validate the above
> as generic.
> 
> And, just for context, here's the node we've been using with hikey:
> 
>                 mali:mali at f4080000 {
>                         compatible = "arm,mali-450", "arm,mali-utgard";
>                         reg = <0x0 0x3f100000 0x0 0x00708000>;
>                         clocks = <&media_ctrl HI6220_G3D_CLK>,
>                                  <&media_ctrl HI6220_G3D_PCLK>;
>                         clock-names = "clk_g3d", "pclk_g3d";
>                         mali_def_freq = <500>;
>                         pclk_freq = <144>;
>                         dfs_steps = <2>;
>                         dfs_lockprf = <1>;
>                         dfs_limit_max_prf = <1>;
>                         dfs_profile_num = <2>;
>                         dfs_profiles = <250 3 0>, <500 1 0>;
>                         mali_type = <2>;
> 
>                         interrupt-parent = <&gic>;
>                         interrupts =    <1 126 4>, /*gp*/
>                                         <1 126 4>, /*gp mmu*/
>                                         <1 126 4>, /*pp bc*/
>                                         <1 126 4>, /*pmu*/
>                                         <1 126 4>, /*pp0*/
>                                         <1 126 4>,
>                                         <1 126 4>, /*pp1*/
>                                         <1 126 4>,
>                                         <1 126 4>, /*pp2*/
>                                         <1 126 4>,
>                                         <1 126 4>, /*pp4*/
>                                         <1 126 4>,
>                                         <1 126 4>, /*pp5*/
>                                         <1 126 4>,
>                                         <1 126 4>, /*pp6*/
>                                         <1 126 4>;
>                         interrupt-names = "IRQGP", "IRQGPMMU", "IRQPP", "IRQPMU",
>                                         "IRQPP0", "IRQPPMMU0", "IRQPP1", "IRQPPMMU1",
>                                         "IRQPP2", "IRQPPMMU2", "IRQPP4", "IRQPPMMU4",
>                                         "IRQPP5", "IRQPPMMU5", "IRQPP6", "IRQPPMMU6";
>                 };

Interesting. Thanks for sharing.

The SoC I work on (tango4 smp8758) also embeds a Mali-400 MP4.
(IIUC, MP4 means 4 shader cores?)

I have seen
	https://github.com/mripard/sunxi-mali/blob/r6p2/driver/src/devicedrv/mali/platform/sunxi/sunxi.c
in the past, but I suppose it's time to take a much closer look.

Regards.