Linux-MIPS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 0/8] ARM/MIPS: DTS: add child nodes describing the PVRSGX present in some OMAP SoC and JZ4780
@ 2019-11-07 11:06 H. Nikolaus Schaller
  2019-11-07 11:06 ` [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs H. Nikolaus Schaller
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: H. Nikolaus Schaller @ 2019-11-07 11:06 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
	Benoît Cousson, Tony Lindgren, Paul Cercueil, Ralf Baechle,
	Paul Burton, James Hogan
  Cc: dri-devel, devicetree, linux-kernel, linux-omap,
	openpvrsgx-devgroup, letux-kernel, kernel, linux-mips,
	H. Nikolaus Schaller

Changes:
* tried convert bindings to YAML format - suggested by Rob Herring
* added JZ4780 DTS node (proven to load the driver)
* removed timer and img,cores properties until we know we really need them - suggested by Rob Herring

PATCH V1 2019-10-18 20:46:35:

This patch set defines child nodes for the SGX5xx interface inside
the OMAP SoC so that a driver can be found and probed by the
compatible strings and can retrieve information about the SGX revision
that is included in a specific SoC. It also defines the interrupt number
to be used by the SGX driver.

There is currently no mainline driver for these GPUs, but a project [1]
is ongoing with the goal to get the open-source part as provided by TI/IMG
into drivers/staging/pvr.

The kernel modules built from this project have successfully demonstrated
to work with the DTS definitions from this patch set on AM335x BeagleBone
Black and OMAP5 Pyra. They partially works on DM3730 and PandaBoard ES but
that is likely a problem in the kernel driver or the (non-free) user-space
blobs.

There is potential to extend this work to JZ4780 (CI20 board) and
BananaPi-M3 (A83) and even some Intel Poulsbo and CedarView devices.

[1]: https://github.com/openpvrsgx-devgroup

H. Nikolaus Schaller (8):
  RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
  ARM: DTS: am33xx: add sgx gpu child node
  ARM: DTS: am3517: add sgx gpu child node
  ARM: DTS: omap3: add sgx gpu child node
  ARM: DTS: omap36xx: add sgx gpu child node
  ARM: DTS: omap4: add sgx gpu child node
  ARM: DTS: omap5: add sgx gpu child node
  MIPS: DTS: jz4780: add sgx gpu node

 .../devicetree/bindings/gpu/img,pvrsgx.yaml   | 128 ++++++++++++++++++
 arch/arm/boot/dts/am33xx.dtsi                 |   9 +-
 arch/arm/boot/dts/am3517.dtsi                 |  11 +-
 arch/arm/boot/dts/omap34xx.dtsi               |  11 +-
 arch/arm/boot/dts/omap36xx.dtsi               |  11 +-
 arch/arm/boot/dts/omap4.dtsi                  |   9 +-
 arch/arm/boot/dts/omap4470.dts                |  15 ++
 arch/arm/boot/dts/omap5.dtsi                  |   9 +-
 arch/mips/boot/dts/ingenic/jz4780.dtsi        |  11 ++
 9 files changed, 187 insertions(+), 27 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
 create mode 100644 arch/arm/boot/dts/omap4470.dts

-- 
2.23.0


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

* [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
  2019-11-07 11:06 [PATCH v2 0/8] ARM/MIPS: DTS: add child nodes describing the PVRSGX present in some OMAP SoC and JZ4780 H. Nikolaus Schaller
@ 2019-11-07 11:06 ` H. Nikolaus Schaller
  2019-11-07 14:35   ` Rob Herring
                     ` (2 more replies)
  2019-11-07 11:06 ` [PATCH v2 2/8] ARM: DTS: am33xx: add sgx gpu child node H. Nikolaus Schaller
                   ` (6 subsequent siblings)
  7 siblings, 3 replies; 15+ messages in thread
From: H. Nikolaus Schaller @ 2019-11-07 11:06 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
	Benoît Cousson, Tony Lindgren, Paul Cercueil, Ralf Baechle,
	Paul Burton, James Hogan
  Cc: dri-devel, devicetree, linux-kernel, linux-omap,
	openpvrsgx-devgroup, letux-kernel, kernel, linux-mips,
	H. Nikolaus Schaller

The Imagination PVR/SGX GPU is part of several SoC from
multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo
and others.

With this binding, we describe how the SGX processor is
interfaced to the SoC (registers, interrupt etc.).

Clock, Reset and power management should be handled
by a parent node or elsewhere.

---

I have used the doc2yaml script to get a first veryion
but I am still stuggling with the yaml thing. My impression
is that while it is human readable, it is not very human
writable... Unfortunately I haven't found a good tutorial
for Dummies (like me) for bindings in YAML.

The big problem is not the YAML syntax but what the schema
should contain and how to correctly formulate ideas in this
new language.

Specific questions for this RFC:

* formatting: is space/tab indentation correct?
* are strings with "" correct or without?
* how do I specify that there is a list of compatible strings required in a specific order?
* but there are multiple such lists, and only one of them is to be chosen?
* how can be described in the binding that there should be certain values in
  the parent node (ranges) to make it work?

I was not able to run

	make dt_binding_check dtbs_check

due to some missing dependencies (which I did not want to
invest time to research them) on my build host, so I could
not get automated help from those.
---
 .../devicetree/bindings/gpu/img,pvrsgx.yaml   | 128 ++++++++++++++++++
 1 file changed, 128 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml

diff --git a/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
new file mode 100644
index 000000000000..b1b021601c47
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
@@ -0,0 +1,128 @@
+# SPDX-License-Identifier: None
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/gpu/img,pvrsgx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Imagination PVR/SGX GPU
+
+maintainers:
+  - H. Nikolaus Schaller <hns@goldelico.com>
+description: |+
+  This binding describes the Imagination SGX5 series of 3D accelerators which
+  are found in several different SoC like TI OMAP, Sitara, Ingenic JZ4780,
+  Allwinner A83, and Intel Poulsbo and CedarView.
+
+  Only the Imagination SGX530, SGX540 and SGX544 GPUs are currently covered by
+  this binding.
+
+  The SGX node is usually a child node of some DT node belonging to the SoC
+  which handles clocks, reset and general address space mapping of the SGX
+  register area.
+
+properties:
+  compatible:
+    oneOf:
+      - item:
+        # BeagleBoard ABC, OpenPandora 600MHz
+        - const: "ti,omap3-sgx530-121", "img,sgx530-121", "img,sgx530", "img,sgx5"
+        # BeagleBoard XM, GTA04, OpenPandora 1GHz
+        - const: "ti,omap3-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"
+        # BeagleBone Black
+        - const: "ti,am335x-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"
+        # Pandaboard (ES)
+        - const: "ti,omap4-sgx540-120", "img,sgx540-120", "img,sgx540", "img,sgx5"
+        - const "ti,omap4-sgx544-112", "img,sgx544-112", "img,sgx544", "img,sgx5"
+        # OMAP5 UEVM, Pyra Handheld
+        "ti,omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5"
+        "ti,dra7-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5"
+        # CI20
+        "ingenic,jz4780-sgx540-120", "img,sgx540-120", "img,sgx540", "img,sgx5";
+
+  reg:
+    items:
+      - description: physical base address and length of the register area
+
+  interrupts:
+     items:
+      - description: interrupt from SGX subsystem to core processor
+
+  clocks:
+     items:
+      - description: optional clocks
+
+  required:
+    - compatible
+    - reg
+    - interrupts
+
+examples: |
+  gpu@fe00 {
+  	compatible = "ti,omap-omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5";
+   	reg = <0xfe00 0x200>;
+   	interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+  };
+
+
+historical: |
+  Imagination PVR/SGX GPU
+
+  Only the Imagination SGX530, SGX540 and SGX544 GPUs are currently covered by this binding.
+
+  Required properties:
+  - compatible:	Should be one of
+  		"ti,omap3-sgx530-121", "img,sgx530-121", "img,sgx530", "img,sgx5"; - BeagleBoard ABC, OpenPandora 600MHz
+  		"ti,omap3-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"; - BeagleBoard XM, GTA04, OpenPandora 1GHz
+  		"ti,am3517-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5";
+  		"ti,am335x-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"; - BeagleBone Black
+  		"ti,omap4-sgx540-120", "img,sgx540-120", "img,sgx540", "img,sgx5"; - Pandaboard (ES)
+  		"ti,omap4-sgx544-112", "img,sgx544-112", "img,sgx544", "img,sgx5";
+  		"ti,omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5"; - OMAP5 UEVM, Pyra Handheld
+  		"ti,dra7-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5";
+  		"ti,am3517-sgx530-?", "img,sgx530-?", "img,sgx530", "img,sgx5";
+  		"ti,am43xx-sgx530-?", "img,sgx530-?", "img,sgx530", "img,sgx5";
+  		"ti,ti81xx-sgx530-?", "img,sgx530-?", "img,sgx530", "img,sgx5";
+  		"img,jz4780-sgx540-?", "img,sgx540-?", "img,sgx540", "img,sgx5"; - CI20
+  		"allwinner,sun8i-a83t-sgx544-?", "img,sgx544-116", "img,sgx544", "img,sgx5"; - Banana-Pi-M3 (Allwinner A83T)
+  		"intel,poulsbo-gma500-sgx535", "img,sgx535-116", "img,sgx535", "img,sgx5"; - Atom Z5xx
+  		"intel,medfield-gma-sgx540", "img,sgx540-116", "img,sgx540", "img,sgx5"; - Atom Z24xx
+  		"intel,cedarview-gma3600-sgx545", "img,sgx545-116", "img,sgx545", "img,sgx5"; - Atom N2600, D2500
+
+  		The "ti,omap..." entries are needed temporarily to handle SoC
+  		specific builds of the kernel module.
+
+  		In the long run, only the "img,sgx..." entry should suffice
+  		to match a generic driver for all architectures and driver
+  		code can dynamically find out on which SoC it is running.
+
+
+  - reg:		Physical base address and length of the register area.
+  - interrupts:	The interrupt numbers.
+
+  / {
+  	ocp {
+  		sgx_module: target-module@56000000 {
+  			compatible = "ti,sysc-omap4", "ti,sysc";
+  			reg = <0x5600fe00 0x4>,
+  			      <0x5600fe10 0x4>;
+  			reg-names = "rev", "sysc";
+  			ti,sysc-midle = <SYSC_IDLE_FORCE>,
+  					<SYSC_IDLE_NO>,
+  					<SYSC_IDLE_SMART>;
+  			ti,sysc-sidle = <SYSC_IDLE_FORCE>,
+  					<SYSC_IDLE_NO>,
+  					<SYSC_IDLE_SMART>;
+  			clocks = <&gpu_clkctrl OMAP5_GPU_CLKCTRL 0>;
+  			clock-names = "fck";
+  			#address-cells = <1>;
+  			#size-cells = <1>;
+  			ranges = <0 0x56000000 0x2000000>;
+
+  			gpu@fe00 {
+  				compatible = "ti,omap-omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5";
+  				reg = <0xfe00 0x200>;
+  				interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+  			};
+  		};
+  	};
+  };
-- 
2.23.0


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

* [PATCH v2 2/8] ARM: DTS: am33xx: add sgx gpu child node
  2019-11-07 11:06 [PATCH v2 0/8] ARM/MIPS: DTS: add child nodes describing the PVRSGX present in some OMAP SoC and JZ4780 H. Nikolaus Schaller
  2019-11-07 11:06 ` [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs H. Nikolaus Schaller
@ 2019-11-07 11:06 ` H. Nikolaus Schaller
  2019-11-07 11:06 ` [PATCH v2 3/8] ARM: DTS: am3517: " H. Nikolaus Schaller
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: H. Nikolaus Schaller @ 2019-11-07 11:06 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
	Benoît Cousson, Tony Lindgren, Paul Cercueil, Ralf Baechle,
	Paul Burton, James Hogan
  Cc: dri-devel, devicetree, linux-kernel, linux-omap,
	openpvrsgx-devgroup, letux-kernel, kernel, linux-mips,
	H. Nikolaus Schaller

and add interrupt.

Tested on BeagleBone Black.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/am33xx.dtsi | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index a9d848d50b20..dbfb9d5aa915 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -480,10 +480,11 @@
 			#size-cells = <1>;
 			ranges = <0 0x56000000 0x1000000>;
 
-			/*
-			 * Closed source PowerVR driver, no child device
-			 * binding or driver in mainline
-			 */
+			sgx: gpu@0 {
+				compatible = "ti,am335x-sgx530-125", "img,sgx530-125", "img,sgx530";
+				reg = <0x00 0x1000000>;	/* 16 MB */
+				interrupts = <37>;
+			};
 		};
 	};
 };
-- 
2.23.0


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

* [PATCH v2 3/8] ARM: DTS: am3517: add sgx gpu child node
  2019-11-07 11:06 [PATCH v2 0/8] ARM/MIPS: DTS: add child nodes describing the PVRSGX present in some OMAP SoC and JZ4780 H. Nikolaus Schaller
  2019-11-07 11:06 ` [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs H. Nikolaus Schaller
  2019-11-07 11:06 ` [PATCH v2 2/8] ARM: DTS: am33xx: add sgx gpu child node H. Nikolaus Schaller
@ 2019-11-07 11:06 ` " H. Nikolaus Schaller
  2019-11-07 11:06 ` [PATCH v2 4/8] ARM: DTS: omap3: " H. Nikolaus Schaller
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: H. Nikolaus Schaller @ 2019-11-07 11:06 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
	Benoît Cousson, Tony Lindgren, Paul Cercueil, Ralf Baechle,
	Paul Burton, James Hogan
  Cc: dri-devel, devicetree, linux-kernel, linux-omap,
	openpvrsgx-devgroup, letux-kernel, kernel, linux-mips,
	H. Nikolaus Schaller

and add interrupt.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/am3517.dtsi | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/am3517.dtsi b/arch/arm/boot/dts/am3517.dtsi
index bf3002009b00..48d5a250fd40 100644
--- a/arch/arm/boot/dts/am3517.dtsi
+++ b/arch/arm/boot/dts/am3517.dtsi
@@ -97,7 +97,7 @@
 		 * revision register instead of the unreadable OCP revision
 		 * register.
 		 */
-		sgx_module: target-module@50000000 {
+		target-module@50000000 {
 			compatible = "ti,sysc-omap2", "ti,sysc";
 			reg = <0x50000014 0x4>;
 			reg-names = "rev";
@@ -107,10 +107,11 @@
 			#size-cells = <1>;
 			ranges = <0 0x50000000 0x4000>;
 
-			/*
-			 * Closed source PowerVR driver, no child device
-			 * binding or driver in mainline
-			 */
+			sgx: gpu@0 {
+				compatible = "ti,am3517-sgx530-125", "img,sgx530-125", "img,sgx530";
+				reg = <0x0 0x4000>;
+				interrupts = <21>;
+			};
 		};
 	};
 };
-- 
2.23.0


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

* [PATCH v2 4/8] ARM: DTS: omap3: add sgx gpu child node
  2019-11-07 11:06 [PATCH v2 0/8] ARM/MIPS: DTS: add child nodes describing the PVRSGX present in some OMAP SoC and JZ4780 H. Nikolaus Schaller
                   ` (2 preceding siblings ...)
  2019-11-07 11:06 ` [PATCH v2 3/8] ARM: DTS: am3517: " H. Nikolaus Schaller
@ 2019-11-07 11:06 ` " H. Nikolaus Schaller
  2019-11-07 11:06 ` [PATCH v2 5/8] ARM: DTS: omap36xx: " H. Nikolaus Schaller
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: H. Nikolaus Schaller @ 2019-11-07 11:06 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
	Benoît Cousson, Tony Lindgren, Paul Cercueil, Ralf Baechle,
	Paul Burton, James Hogan
  Cc: dri-devel, devicetree, linux-kernel, linux-omap,
	openpvrsgx-devgroup, letux-kernel, kernel, linux-mips,
	H. Nikolaus Schaller

and add interrupt

Tested on OpenPandora 600 MHz.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/omap34xx.dtsi | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/omap34xx.dtsi b/arch/arm/boot/dts/omap34xx.dtsi
index 7b09cbee8bb8..9b050d71849b 100644
--- a/arch/arm/boot/dts/omap34xx.dtsi
+++ b/arch/arm/boot/dts/omap34xx.dtsi
@@ -111,7 +111,7 @@
 		 * are also different clocks, but we do not have any dts users
 		 * for it.
 		 */
-		sgx_module: target-module@50000000 {
+		target-module@50000000 {
 			compatible = "ti,sysc-omap2", "ti,sysc";
 			reg = <0x50000014 0x4>;
 			reg-names = "rev";
@@ -121,10 +121,11 @@
 			#size-cells = <1>;
 			ranges = <0 0x50000000 0x4000>;
 
-			/*
-			 * Closed source PowerVR driver, no child device
-			 * binding or driver in mainline
-			 */
+			sgx: gpu@0 {
+				compatible = "ti,omap3-sgx530-121", "img,sgx530-121", "img,sgx530";
+				reg = <0x0 0x4000>;	/* 64kB */
+				interrupts = <21>;
+			};
 		};
 	};
 
-- 
2.23.0


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

* [PATCH v2 5/8] ARM: DTS: omap36xx: add sgx gpu child node
  2019-11-07 11:06 [PATCH v2 0/8] ARM/MIPS: DTS: add child nodes describing the PVRSGX present in some OMAP SoC and JZ4780 H. Nikolaus Schaller
                   ` (3 preceding siblings ...)
  2019-11-07 11:06 ` [PATCH v2 4/8] ARM: DTS: omap3: " H. Nikolaus Schaller
@ 2019-11-07 11:06 ` " H. Nikolaus Schaller
  2019-11-07 11:06 ` [PATCH v2 6/8] ARM: DTS: omap4: " H. Nikolaus Schaller
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: H. Nikolaus Schaller @ 2019-11-07 11:06 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
	Benoît Cousson, Tony Lindgren, Paul Cercueil, Ralf Baechle,
	Paul Burton, James Hogan
  Cc: dri-devel, devicetree, linux-kernel, linux-omap,
	openpvrsgx-devgroup, letux-kernel, kernel, linux-mips,
	H. Nikolaus Schaller

and add interrupt.

Tested on GTA04 and BeagleBoard XM.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/omap36xx.dtsi | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi
index 1e552f08f120..851d4abb943b 100644
--- a/arch/arm/boot/dts/omap36xx.dtsi
+++ b/arch/arm/boot/dts/omap36xx.dtsi
@@ -145,7 +145,7 @@
 		 * "ti,sysc-omap4" type register with just sidle and midle bits
 		 * available while omap34xx has "ti,sysc-omap2" type sysconfig.
 		 */
-		sgx_module: target-module@50000000 {
+		target-module@50000000 {
 			compatible = "ti,sysc-omap4", "ti,sysc";
 			reg = <0x5000fe00 0x4>,
 			      <0x5000fe10 0x4>;
@@ -162,10 +162,11 @@
 			#size-cells = <1>;
 			ranges = <0 0x50000000 0x2000000>;
 
-			/*
-			 * Closed source PowerVR driver, no child device
-			 * binding or driver in mainline
-			 */
+			sgx: gpu@0 {
+				compatible = "ti,omap3-sgx530-125", "img,sgx530-125", "img,sgx530";
+				reg = <0x0 0x10000>;	/* 64kB */
+				interrupts = <21>;
+			};
 		};
 	};
 
-- 
2.23.0


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

* [PATCH v2 6/8] ARM: DTS: omap4: add sgx gpu child node
  2019-11-07 11:06 [PATCH v2 0/8] ARM/MIPS: DTS: add child nodes describing the PVRSGX present in some OMAP SoC and JZ4780 H. Nikolaus Schaller
                   ` (4 preceding siblings ...)
  2019-11-07 11:06 ` [PATCH v2 5/8] ARM: DTS: omap36xx: " H. Nikolaus Schaller
@ 2019-11-07 11:06 ` " H. Nikolaus Schaller
  2019-11-07 11:06 ` [PATCH v2 7/8] ARM: DTS: omap5: " H. Nikolaus Schaller
  2019-11-07 11:06 ` [PATCH v2 8/8] MIPS: DTS: jz4780: add sgx gpu node H. Nikolaus Schaller
  7 siblings, 0 replies; 15+ messages in thread
From: H. Nikolaus Schaller @ 2019-11-07 11:06 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
	Benoît Cousson, Tony Lindgren, Paul Cercueil, Ralf Baechle,
	Paul Burton, James Hogan
  Cc: dri-devel, devicetree, linux-kernel, linux-omap,
	openpvrsgx-devgroup, letux-kernel, kernel, linux-mips,
	H. Nikolaus Schaller

and add interrupt.

Since omap4420/30/60 and omap4470 come with different SGX variants
we need to introduce a new omap4470.dtsi. If an omap4470 board
does not want to use SGX it is no problem to still include
omap4460.dtsi.

Tested on PandaBoard ES.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/omap4.dtsi   |  9 +++++----
 arch/arm/boot/dts/omap4470.dts | 15 +++++++++++++++
 2 files changed, 20 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/boot/dts/omap4470.dts

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 7cc95bc1598b..4d5958fbe1ef 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -347,10 +347,11 @@
 			#size-cells = <1>;
 			ranges = <0 0x56000000 0x2000000>;
 
-			/*
-			 * Closed source PowerVR driver, no child device
-			 * binding or driver in mainline
-			 */
+			sgx: img@0 {
+				compatible = "ti,omap4-sgx540-120", "img,sgx540-120", "img,sgx540";
+				reg = <0x0 0x2000000>;	/* 32MB */
+				interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+			};
 		};
 
 		dss: dss@58000000 {
diff --git a/arch/arm/boot/dts/omap4470.dts b/arch/arm/boot/dts/omap4470.dts
new file mode 100644
index 000000000000..19b554612401
--- /dev/null
+++ b/arch/arm/boot/dts/omap4470.dts
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Device Tree Source for OMAP4470 SoC
+ *
+ * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+#include "omap4460.dtsi"
+
+&sgx {
+	compatible = "img,sgx544-112", "img,sgx544", "ti,omap-omap4-sgx544-112";
+};
-- 
2.23.0


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

* [PATCH v2 7/8] ARM: DTS: omap5: add sgx gpu child node
  2019-11-07 11:06 [PATCH v2 0/8] ARM/MIPS: DTS: add child nodes describing the PVRSGX present in some OMAP SoC and JZ4780 H. Nikolaus Schaller
                   ` (5 preceding siblings ...)
  2019-11-07 11:06 ` [PATCH v2 6/8] ARM: DTS: omap4: " H. Nikolaus Schaller
@ 2019-11-07 11:06 ` " H. Nikolaus Schaller
  2019-11-07 11:06 ` [PATCH v2 8/8] MIPS: DTS: jz4780: add sgx gpu node H. Nikolaus Schaller
  7 siblings, 0 replies; 15+ messages in thread
From: H. Nikolaus Schaller @ 2019-11-07 11:06 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
	Benoît Cousson, Tony Lindgren, Paul Cercueil, Ralf Baechle,
	Paul Burton, James Hogan
  Cc: dri-devel, devicetree, linux-kernel, linux-omap,
	openpvrsgx-devgroup, letux-kernel, kernel, linux-mips,
	H. Nikolaus Schaller

and add interrupt.

Tested on Pyra-Handheld.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/omap5.dtsi | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 1fb7937638f0..333da4788088 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -274,10 +274,11 @@
 			#size-cells = <1>;
 			ranges = <0 0x56000000 0x2000000>;
 
-			/*
-			 * Closed source PowerVR driver, no child device
-			 * binding or driver in mainline
-			 */
+			sgx: gpu@0 {
+				compatible = "ti,omap5-sgx544-116", "img,sgx544-116", "img,sgx544";
+				reg = <0x0 0x10000>;
+				interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+			};
 		};
 
 		dss: dss@58000000 {
-- 
2.23.0


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

* [PATCH v2 8/8] MIPS: DTS: jz4780: add sgx gpu node
  2019-11-07 11:06 [PATCH v2 0/8] ARM/MIPS: DTS: add child nodes describing the PVRSGX present in some OMAP SoC and JZ4780 H. Nikolaus Schaller
                   ` (6 preceding siblings ...)
  2019-11-07 11:06 ` [PATCH v2 7/8] ARM: DTS: omap5: " H. Nikolaus Schaller
@ 2019-11-07 11:06 ` H. Nikolaus Schaller
  7 siblings, 0 replies; 15+ messages in thread
From: H. Nikolaus Schaller @ 2019-11-07 11:06 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
	Benoît Cousson, Tony Lindgren, Paul Cercueil, Ralf Baechle,
	Paul Burton, James Hogan
  Cc: dri-devel, devicetree, linux-kernel, linux-omap,
	openpvrsgx-devgroup, letux-kernel, kernel, linux-mips,
	H. Nikolaus Schaller, Paul Boddie

and add interrupt and clocks.

Tested to build for CI20 board and load a (non-working) driver.

Suggested-by: Paul Boddie <paul@boddie.org.uk>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
index c54bd7cfec55..21ea5f4a405b 100644
--- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
@@ -46,6 +46,17 @@
 		#clock-cells = <1>;
 	};
 
+	gpu: gpu@13040000 {
+		compatible = "ingenic,jz4780-sgx540-120", "img,sgx540-120", "img,sgx540", "img,sgx5";
+		reg = <0x13040000 0x4000>;
+
+		clocks = <&cgu JZ4780_CLK_GPU>;
+		clock-names = "gpu";
+
+		interrupt-parent = <&intc>;
+		interrupts = <63>;
+	};
+
 	tcu: timer@10002000 {
 		compatible = "ingenic,jz4780-tcu",
 			     "ingenic,jz4770-tcu",
-- 
2.23.0


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

* Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
  2019-11-07 11:06 ` [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs H. Nikolaus Schaller
@ 2019-11-07 14:35   ` Rob Herring
  2019-11-07 16:55     ` H. Nikolaus Schaller
  2019-11-07 15:54   ` Tony Lindgren
  2019-11-08 16:49   ` Tony Lindgren
  2 siblings, 1 reply; 15+ messages in thread
From: Rob Herring @ 2019-11-07 14:35 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: David Airlie, Daniel Vetter, Mark Rutland, Benoît Cousson,
	Tony Lindgren, Paul Cercueil, Ralf Baechle, Paul Burton,
	James Hogan, dri-devel, devicetree, linux-kernel, linux-omap,
	openpvrsgx-devgroup, Discussions about the Letux Kernel, kernel,
	open list:MIPS

On Thu, Nov 7, 2019 at 5:06 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> The Imagination PVR/SGX GPU is part of several SoC from
> multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo
> and others.
>
> With this binding, we describe how the SGX processor is
> interfaced to the SoC (registers, interrupt etc.).
>
> Clock, Reset and power management should be handled
> by a parent node or elsewhere.

That's probably TI specific...

> ---
>
> I have used the doc2yaml script to get a first veryion
> but I am still stuggling with the yaml thing. My impression
> is that while it is human readable, it is not very human
> writable... Unfortunately I haven't found a good tutorial
> for Dummies (like me) for bindings in YAML.

Did you read .../bindings/example-schema.yaml? It explains the common
cases and what schema are doing. I recently added to it, so look at
the version in linux-next.

> The big problem is not the YAML syntax but what the schema
> should contain and how to correctly formulate ideas in this
> new language.
>
> Specific questions for this RFC:
>
> * formatting: is space/tab indentation correct?

YAML requires spaces.

> * are strings with "" correct or without?

Generally only keys or values starting with '#' need quotes. There's
other cases, but we simply don't hit them with DT. We tend to quote
$ref values, but that's not strictly needed.

> * how do I specify that there is a list of compatible strings required in a specific order?

An 'items' list defines the order.

> * but there are multiple such lists, and only one of them is to be chosen?

                                                ^^^^^^
'oneOf' is the schema keyword you are looking for.

> * how can be described in the binding that there should be certain values in
>   the parent node (ranges) to make it work?

You can't. Schemas match on a node and work down from there. So you
can do it, but it's more complicated. You'd need a custom 'select'
select that matches on the parent node having the child node you are
looking for (assuming the parent is something generic like
'simple-bus' which you can't match on). However, based on the example,
I'd say checking 'ranges' is outside the scope of schema checks.
'ranges' doesn't have to be a certain value any more than every case
of 'reg' (except maybe i2c devices with fixed addresses). It's up to
the .dts author how exactly to do address translation.

I would like to have more ranges/reg checks such as bounds checks and
overlapping addresses, but I think we'd do those with code, not
schema.

> I was not able to run
>
>         make dt_binding_check dtbs_check
>
> due to some missing dependencies (which I did not want to
> invest time to research them) on my build host, so I could
> not get automated help from those.

Dependencies are documented in Documentation/devicetree/writing-schema.rst.

> ---
>  .../devicetree/bindings/gpu/img,pvrsgx.yaml   | 128 ++++++++++++++++++
>  1 file changed, 128 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
>
> diff --git a/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
> new file mode 100644
> index 000000000000..b1b021601c47
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
> @@ -0,0 +1,128 @@
> +# SPDX-License-Identifier: None

Obviously not valid.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/gpu/img,pvrsgx.yaml#

This should have been correct with the script, but you need to drop 'bindings'.

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Imagination PVR/SGX GPU
> +
> +maintainers:
> +  - H. Nikolaus Schaller <hns@goldelico.com>
> +description: |+
> +  This binding describes the Imagination SGX5 series of 3D accelerators which
> +  are found in several different SoC like TI OMAP, Sitara, Ingenic JZ4780,
> +  Allwinner A83, and Intel Poulsbo and CedarView.
> +
> +  Only the Imagination SGX530, SGX540 and SGX544 GPUs are currently covered by
> +  this binding.
> +
> +  The SGX node is usually a child node of some DT node belonging to the SoC
> +  which handles clocks, reset and general address space mapping of the SGX
> +  register area.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - item:

'item/items'

> +        # BeagleBoard ABC, OpenPandora 600MHz
> +        - const: "ti,omap3-sgx530-121", "img,sgx530-121", "img,sgx530", "img,sgx5"

Not valid YAML nor json-schema. Each value needs to be list item with 'const:'

Plenty of examples in bindings/arm/ with board/soc bindings.

> +        # BeagleBoard XM, GTA04, OpenPandora 1GHz
> +        - const: "ti,omap3-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"

This needs to be a new 'items' list under 'oneOf'.

> +        # BeagleBone Black
> +        - const: "ti,am335x-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"
> +        # Pandaboard (ES)
> +        - const: "ti,omap4-sgx540-120", "img,sgx540-120", "img,sgx540", "img,sgx5"
> +        - const "ti,omap4-sgx544-112", "img,sgx544-112", "img,sgx544", "img,sgx5"
> +        # OMAP5 UEVM, Pyra Handheld
> +        "ti,omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5"
> +        "ti,dra7-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5"

Just gave up on trying to write a schema here?

> +        # CI20
> +        "ingenic,jz4780-sgx540-120", "img,sgx540-120", "img,sgx540", "img,sgx5";
> +
> +  reg:
> +    items:
> +      - description: physical base address and length of the register area

For single entries, just 'maxItems: 1' is enough. Unless you have
something special about this device, you don't need a description
here.

> +
> +  interrupts:
> +     items:
> +      - description: interrupt from SGX subsystem to core processor
> +
> +  clocks:
> +     items:
> +      - description: optional clocks
> +
> +  required:
> +    - compatible
> +    - reg
> +    - interrupts
> +
> +examples: |
> +  gpu@fe00 {
> +       compatible = "ti,omap-omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5";
> +       reg = <0xfe00 0x200>;
> +       interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
> +  };
> +
> +
> +historical: |

This should be dropped. It's just for reference as you write the schema.

> +  Imagination PVR/SGX GPU
> +
> +  Only the Imagination SGX530, SGX540 and SGX544 GPUs are currently covered by this binding.
> +
> +  Required properties:
> +  - compatible:        Should be one of
> +               "ti,omap3-sgx530-121", "img,sgx530-121", "img,sgx530", "img,sgx5"; - BeagleBoard ABC, OpenPandora 600MHz
> +               "ti,omap3-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"; - BeagleBoard XM, GTA04, OpenPandora 1GHz
> +               "ti,am3517-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5";
> +               "ti,am335x-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"; - BeagleBone Black
> +               "ti,omap4-sgx540-120", "img,sgx540-120", "img,sgx540", "img,sgx5"; - Pandaboard (ES)
> +               "ti,omap4-sgx544-112", "img,sgx544-112", "img,sgx544", "img,sgx5";
> +               "ti,omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5"; - OMAP5 UEVM, Pyra Handheld
> +               "ti,dra7-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5";
> +               "ti,am3517-sgx530-?", "img,sgx530-?", "img,sgx530", "img,sgx5";
> +               "ti,am43xx-sgx530-?", "img,sgx530-?", "img,sgx530", "img,sgx5";
> +               "ti,ti81xx-sgx530-?", "img,sgx530-?", "img,sgx530", "img,sgx5";
> +               "img,jz4780-sgx540-?", "img,sgx540-?", "img,sgx540", "img,sgx5"; - CI20
> +               "allwinner,sun8i-a83t-sgx544-?", "img,sgx544-116", "img,sgx544", "img,sgx5"; - Banana-Pi-M3 (Allwinner A83T)
> +               "intel,poulsbo-gma500-sgx535", "img,sgx535-116", "img,sgx535", "img,sgx5"; - Atom Z5xx
> +               "intel,medfield-gma-sgx540", "img,sgx540-116", "img,sgx540", "img,sgx5"; - Atom Z24xx
> +               "intel,cedarview-gma3600-sgx545", "img,sgx545-116", "img,sgx545", "img,sgx5"; - Atom N2600, D2500
> +
> +               The "ti,omap..." entries are needed temporarily to handle SoC
> +               specific builds of the kernel module.
> +
> +               In the long run, only the "img,sgx..." entry should suffice
> +               to match a generic driver for all architectures and driver
> +               code can dynamically find out on which SoC it is running.
> +
> +
> +  - reg:               Physical base address and length of the register area.
> +  - interrupts:        The interrupt numbers.
> +
> +  / {
> +       ocp {
> +               sgx_module: target-module@56000000 {
> +                       compatible = "ti,sysc-omap4", "ti,sysc";
> +                       reg = <0x5600fe00 0x4>,
> +                             <0x5600fe10 0x4>;
> +                       reg-names = "rev", "sysc";
> +                       ti,sysc-midle = <SYSC_IDLE_FORCE>,
> +                                       <SYSC_IDLE_NO>,
> +                                       <SYSC_IDLE_SMART>;
> +                       ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> +                                       <SYSC_IDLE_NO>,
> +                                       <SYSC_IDLE_SMART>;
> +                       clocks = <&gpu_clkctrl OMAP5_GPU_CLKCTRL 0>;
> +                       clock-names = "fck";
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +                       ranges = <0 0x56000000 0x2000000>;
> +
> +                       gpu@fe00 {
> +                               compatible = "ti,omap-omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5";
> +                               reg = <0xfe00 0x200>;
> +                               interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
> +                       };
> +               };
> +       };
> +  };
> --
> 2.23.0
>

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

* Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
  2019-11-07 11:06 ` [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs H. Nikolaus Schaller
  2019-11-07 14:35   ` Rob Herring
@ 2019-11-07 15:54   ` Tony Lindgren
  2019-11-07 16:37     ` H. Nikolaus Schaller
  2019-11-08 16:49   ` Tony Lindgren
  2 siblings, 1 reply; 15+ messages in thread
From: Tony Lindgren @ 2019-11-07 15:54 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
	Benoît Cousson, Paul Cercueil, Ralf Baechle, Paul Burton,
	James Hogan, dri-devel, devicetree, linux-kernel, linux-omap,
	openpvrsgx-devgroup, letux-kernel, kernel, linux-mips

* H. Nikolaus Schaller <hns@goldelico.com> [191107 11:07]:
> +        - const: "ti,am335x-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"

This should be without the x, maybe use the earliest one here
for "ti,am3352-sgx530-125" like we have for "ti,am3352-uart".

We could use "ti,am3-sgx530-125" but that can get confused
then with am3517 then.

Regards,

Tony

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

* Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
  2019-11-07 15:54   ` Tony Lindgren
@ 2019-11-07 16:37     ` H. Nikolaus Schaller
  0 siblings, 0 replies; 15+ messages in thread
From: H. Nikolaus Schaller @ 2019-11-07 16:37 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
	Benoît Cousson, Paul Cercueil, Ralf Baechle, Paul Burton,
	James Hogan, dri-devel, devicetree, linux-kernel, linux-omap,
	openpvrsgx-devgroup, letux-kernel, kernel, linux-mips


> Am 07.11.2019 um 16:54 schrieb Tony Lindgren <tony@atomide.com>:
> 
> * H. Nikolaus Schaller <hns@goldelico.com> [191107 11:07]:
>> +        - const: "ti,am335x-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"
> 
> This should be without the x, maybe use the earliest one here
> for "ti,am3352-sgx530-125" like we have for "ti,am3352-uart".

Ok, fine. Will update accordingly.

> We could use "ti,am3-sgx530-125" but that can get confused
> then with am3517 then.

Indeed.

BR and thanks,
Nikolaus


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

* Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
  2019-11-07 14:35   ` Rob Herring
@ 2019-11-07 16:55     ` H. Nikolaus Schaller
  2019-11-08 16:45       ` Tony Lindgren
  0 siblings, 1 reply; 15+ messages in thread
From: H. Nikolaus Schaller @ 2019-11-07 16:55 UTC (permalink / raw)
  To: Rob Herring
  Cc: David Airlie, Daniel Vetter, Mark Rutland, Benoît Cousson,
	Tony Lindgren, Paul Cercueil, Ralf Baechle, Paul Burton,
	James Hogan, dri-devel, devicetree, linux-kernel, linux-omap,
	OpenPVRSGX Linux Driver Group,
	Discussions about the Letux Kernel, kernel, open list:MIPS


> Am 07.11.2019 um 15:35 schrieb Rob Herring <robh+dt@kernel.org>:
> 
> On Thu, Nov 7, 2019 at 5:06 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> 
>> The Imagination PVR/SGX GPU is part of several SoC from
>> multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo
>> and others.
>> 
>> With this binding, we describe how the SGX processor is
>> interfaced to the SoC (registers, interrupt etc.).
>> 
>> Clock, Reset and power management should be handled
>> by a parent node or elsewhere.
> 
> That's probably TI specific...

Yes and no.

For example the img4780 seems to need a clock reference in the
gpu node. But it could maybe connected in a parent node like recent
TI SoC do with the target-module approach.

And our goal is to end up with a common driver for all SoC and architectures
in far future. Then, probably clock, reset and power management should
be handled in the same way.

> 
>> ---
>> 
>> I have used the doc2yaml script to get a first veryion
>> but I am still stuggling with the yaml thing. My impression
>> is that while it is human readable, it is not very human
>> writable... Unfortunately I haven't found a good tutorial
>> for Dummies (like me) for bindings in YAML.
> 
> Did you read .../bindings/example-schema.yaml? It explains the common
> cases and what schema are doing.

Yes.

> I recently added to it, so look at
> the version in linux-next.

Ah, ok. I haven't read that one.

> 
>> The big problem is not the YAML syntax but what the schema
>> should contain and how to correctly formulate ideas in this
>> new language.
>> 
>> Specific questions for this RFC:
>> 
>> * formatting: is space/tab indentation correct?
> 
> YAML requires spaces.

Which is quite uncommon if you aren't a python programmer...

>> * are strings with "" correct or without?
> 
> Generally only keys or values starting with '#' need quotes. There's
> other cases, but we simply don't hit them with DT. We tend to quote
> $ref values, but that's not strictly needed.

Ok. Good.

> 
>> * how do I specify that there is a list of compatible strings required in a specific order?
> 
> An 'items' list defines the order.

I see.

> 
>> * but there are multiple such lists, and only one of them is to be chosen?
> 
>                                                ^^^^^^
> 'oneOf' is the schema keyword you are looking for.

Ok.

> 
>> * how can be described in the binding that there should be certain values in
>>  the parent node (ranges) to make it work?
> 
> You can't. Schemas match on a node and work down from there. So you
> can do it, but it's more complicated. You'd need a custom 'select'
> select that matches on the parent node having the child node you are
> looking for (assuming the parent is something generic like
> 'simple-bus' which you can't match on). However, based on the example,
> I'd say checking 'ranges' is outside the scope of schema checks.
> 'ranges' doesn't have to be a certain value any more than every case
> of 'reg' (except maybe i2c devices with fixed addresses).

Ok.

> It's up to
> the .dts author how exactly to do address translation.

Well, my concern as a regular .dts author is that I usually treat
bindings as documentation and giving hints how to write a .dts and
what to take care of. If it is not complete, I get into big trouble.

> I would like to have more ranges/reg checks such as bounds checks and
> overlapping addresses, but I think we'd do those with code, not
> schema.
> 
>> I was not able to run
>> 
>>        make dt_binding_check dtbs_check
>> 
>> due to some missing dependencies (which I did not want to
>> invest time to research them) on my build host, so I could
>> not get automated help from those.
> 
> Dependencies are documented in Documentation/devicetree/writing-schema.rst.

One says it needs a libyaml but after installing one my HOSTCC didn't find
it. The other asks for another script which seems to be missing.


> 
>> ---
>> .../devicetree/bindings/gpu/img,pvrsgx.yaml   | 128 ++++++++++++++++++
>> 1 file changed, 128 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
>> new file mode 100644
>> index 000000000000..b1b021601c47
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
>> @@ -0,0 +1,128 @@
>> +# SPDX-License-Identifier: None
> 
> Obviously not valid.

:)

> 
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/bindings/gpu/img,pvrsgx.yaml#
> 
> This should have been correct with the script, but you need to drop 'bindings'.

Ok.

> 
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Imagination PVR/SGX GPU
>> +
>> +maintainers:
>> +  - H. Nikolaus Schaller <hns@goldelico.com>
>> +description: |+
>> +  This binding describes the Imagination SGX5 series of 3D accelerators which
>> +  are found in several different SoC like TI OMAP, Sitara, Ingenic JZ4780,
>> +  Allwinner A83, and Intel Poulsbo and CedarView.
>> +
>> +  Only the Imagination SGX530, SGX540 and SGX544 GPUs are currently covered by
>> +  this binding.
>> +
>> +  The SGX node is usually a child node of some DT node belonging to the SoC
>> +  which handles clocks, reset and general address space mapping of the SGX
>> +  register area.
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - item:
> 
> 'item/items'


Ok, as you described above we need "items".

> 
>> +        # BeagleBoard ABC, OpenPandora 600MHz
>> +        - const: "ti,omap3-sgx530-121", "img,sgx530-121", "img,sgx530", "img,sgx5"
> 
> Not valid YAML nor json-schema. Each value needs to be list item with 'const:'

Have to look up how that syntax is.

> Plenty of examples in bindings/arm/ with board/soc bindings.

Ok.

> 
>> +        # BeagleBoard XM, GTA04, OpenPandora 1GHz
>> +        - const: "ti,omap3-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"
> 
> This needs to be a new 'items' list under 'oneOf'.

Ok!

> 
>> +        # BeagleBone Black
>> +        - const: "ti,am335x-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"
>> +        # Pandaboard (ES)
>> +        - const: "ti,omap4-sgx540-120", "img,sgx540-120", "img,sgx540", "img,sgx5"
>> +        - const "ti,omap4-sgx544-112", "img,sgx544-112", "img,sgx544", "img,sgx5"
>> +        # OMAP5 UEVM, Pyra Handheld
>> +        "ti,omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5"
>> +        "ti,dra7-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5"
> 
> Just gave up on trying to write a schema here?

Yes...

You see into what issues a first time YAML/schema writer with 35 years C but no
YAML, Python or JSON experience runs into...

Writing bindings as .txt was easy :)

> 
>> +        # CI20
>> +        "ingenic,jz4780-sgx540-120", "img,sgx540-120", "img,sgx540", "img,sgx5";
>> +
>> +  reg:
>> +    items:
>> +      - description: physical base address and length of the register area
> 
> For single entries, just 'maxItems: 1' is enough. Unless you have
> something special about this device, you don't need a description
> here.

I am not sure if I understand that yet.

> 
>> +
>> +  interrupts:
>> +     items:
>> +      - description: interrupt from SGX subsystem to core processor
>> +
>> +  clocks:
>> +     items:
>> +      - description: optional clocks
>> +
>> +  required:
>> +    - compatible
>> +    - reg
>> +    - interrupts
>> +
>> +examples: |
>> +  gpu@fe00 {
>> +       compatible = "ti,omap-omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5";
>> +       reg = <0xfe00 0x200>;
>> +       interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
>> +  };
>> +
>> +
>> +historical: |
> 
> This should be dropped. It's just for reference as you write the schema.

Yes that is clear. I kept it for reference what I intended to translate from.

> 
>> +  Imagination PVR/SGX GPU
>> +
>> +  Only the Imagination SGX530, SGX540 and SGX544 GPUs are currently covered by this binding.
>> +
>> +  Required properties:
>> +  - compatible:        Should be one of
>> +               "ti,omap3-sgx530-121", "img,sgx530-121", "img,sgx530", "img,sgx5"; - BeagleBoard ABC, OpenPandora 600MHz
>> +               "ti,omap3-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"; - BeagleBoard XM, GTA04, OpenPandora 1GHz
>> +               "ti,am3517-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5";
>> +               "ti,am335x-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"; - BeagleBone Black
>> +               "ti,omap4-sgx540-120", "img,sgx540-120", "img,sgx540", "img,sgx5"; - Pandaboard (ES)
>> +               "ti,omap4-sgx544-112", "img,sgx544-112", "img,sgx544", "img,sgx5";
>> +               "ti,omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5"; - OMAP5 UEVM, Pyra Handheld
>> +               "ti,dra7-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5";
>> +               "ti,am3517-sgx530-?", "img,sgx530-?", "img,sgx530", "img,sgx5";
>> +               "ti,am43xx-sgx530-?", "img,sgx530-?", "img,sgx530", "img,sgx5";
>> +               "ti,ti81xx-sgx530-?", "img,sgx530-?", "img,sgx530", "img,sgx5";
>> +               "img,jz4780-sgx540-?", "img,sgx540-?", "img,sgx540", "img,sgx5"; - CI20
>> +               "allwinner,sun8i-a83t-sgx544-?", "img,sgx544-116", "img,sgx544", "img,sgx5"; - Banana-Pi-M3 (Allwinner A83T)
>> +               "intel,poulsbo-gma500-sgx535", "img,sgx535-116", "img,sgx535", "img,sgx5"; - Atom Z5xx
>> +               "intel,medfield-gma-sgx540", "img,sgx540-116", "img,sgx540", "img,sgx5"; - Atom Z24xx
>> +               "intel,cedarview-gma3600-sgx545", "img,sgx545-116", "img,sgx545", "img,sgx5"; - Atom N2600, D2500
>> +
>> +               The "ti,omap..." entries are needed temporarily to handle SoC
>> +               specific builds of the kernel module.
>> +
>> +               In the long run, only the "img,sgx..." entry should suffice
>> +               to match a generic driver for all architectures and driver
>> +               code can dynamically find out on which SoC it is running.
>> +
>> +
>> +  - reg:               Physical base address and length of the register area.
>> +  - interrupts:        The interrupt numbers.
>> +
>> +  / {
>> +       ocp {
>> +               sgx_module: target-module@56000000 {
>> +                       compatible = "ti,sysc-omap4", "ti,sysc";
>> +                       reg = <0x5600fe00 0x4>,
>> +                             <0x5600fe10 0x4>;
>> +                       reg-names = "rev", "sysc";
>> +                       ti,sysc-midle = <SYSC_IDLE_FORCE>,
>> +                                       <SYSC_IDLE_NO>,
>> +                                       <SYSC_IDLE_SMART>;
>> +                       ti,sysc-sidle = <SYSC_IDLE_FORCE>,
>> +                                       <SYSC_IDLE_NO>,
>> +                                       <SYSC_IDLE_SMART>;
>> +                       clocks = <&gpu_clkctrl OMAP5_GPU_CLKCTRL 0>;
>> +                       clock-names = "fck";
>> +                       #address-cells = <1>;
>> +                       #size-cells = <1>;
>> +                       ranges = <0 0x56000000 0x2000000>;
>> +
>> +                       gpu@fe00 {
>> +                               compatible = "ti,omap-omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5";
>> +                               reg = <0xfe00 0x200>;
>> +                               interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
>> +                       };
>> +               };
>> +       };
>> +  };
>> --
>> 2.23.0
>> 

BR and thanks for the help towards a PATCH v3,
Nikolaus



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

* Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
  2019-11-07 16:55     ` H. Nikolaus Schaller
@ 2019-11-08 16:45       ` Tony Lindgren
  0 siblings, 0 replies; 15+ messages in thread
From: Tony Lindgren @ 2019-11-08 16:45 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Rob Herring, David Airlie, Daniel Vetter, Mark Rutland,
	Benoît Cousson, Paul Cercueil, Ralf Baechle, Paul Burton,
	James Hogan, dri-devel, devicetree, linux-kernel, linux-omap,
	OpenPVRSGX Linux Driver Group,
	Discussions about the Letux Kernel, kernel, open list:MIPS

* H. Nikolaus Schaller <hns@goldelico.com> [191107 16:56]:
> > Am 07.11.2019 um 15:35 schrieb Rob Herring <robh+dt@kernel.org>:
> > On Thu, Nov 7, 2019 at 5:06 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> >> Clock, Reset and power management should be handled
> >> by a parent node or elsewhere.
> > 
> > That's probably TI specific...
> 
> Yes and no.
> 
> For example the img4780 seems to need a clock reference in the
> gpu node. But it could maybe connected in a parent node like recent
> TI SoC do with the target-module approach.

The clocks are implemented at the SoC glue layer and/or the
interconnect layer, and then the device probably has it's
own clock gate controls.

> And our goal is to end up with a common driver for all SoC and architectures
> in far future. Then, probably clock, reset and power management should
> be handled in the same way.

Yeah so that's standard Linux features such as PM runtime
and genpd basically :)

So you can just leave out the clocks paragraph from the
binding. Then if clocks are really needed beyond PM runtime
and genpd, those can always be added later.

We just need a super minimal binding to start with that
only uses standard properties, then more can be added
later if needed.

Regards,

Tony

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

* Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
  2019-11-07 11:06 ` [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs H. Nikolaus Schaller
  2019-11-07 14:35   ` Rob Herring
  2019-11-07 15:54   ` Tony Lindgren
@ 2019-11-08 16:49   ` Tony Lindgren
  2 siblings, 0 replies; 15+ messages in thread
From: Tony Lindgren @ 2019-11-08 16:49 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
	Benoît Cousson, Paul Cercueil, Ralf Baechle, Paul Burton,
	James Hogan, dri-devel, devicetree, linux-kernel, linux-omap,
	openpvrsgx-devgroup, letux-kernel, kernel, linux-mips

* H. Nikolaus Schaller <hns@goldelico.com> [191107 11:07]:
> +        - const: "ti,am335x-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"

I did a quick probe test for the module and am4 is the same
as am335x, so please also add this for the next version:

"ti,am4-sgx530-125"

Regards,

Tony

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

end of thread, back to index

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-07 11:06 [PATCH v2 0/8] ARM/MIPS: DTS: add child nodes describing the PVRSGX present in some OMAP SoC and JZ4780 H. Nikolaus Schaller
2019-11-07 11:06 ` [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs H. Nikolaus Schaller
2019-11-07 14:35   ` Rob Herring
2019-11-07 16:55     ` H. Nikolaus Schaller
2019-11-08 16:45       ` Tony Lindgren
2019-11-07 15:54   ` Tony Lindgren
2019-11-07 16:37     ` H. Nikolaus Schaller
2019-11-08 16:49   ` Tony Lindgren
2019-11-07 11:06 ` [PATCH v2 2/8] ARM: DTS: am33xx: add sgx gpu child node H. Nikolaus Schaller
2019-11-07 11:06 ` [PATCH v2 3/8] ARM: DTS: am3517: " H. Nikolaus Schaller
2019-11-07 11:06 ` [PATCH v2 4/8] ARM: DTS: omap3: " H. Nikolaus Schaller
2019-11-07 11:06 ` [PATCH v2 5/8] ARM: DTS: omap36xx: " H. Nikolaus Schaller
2019-11-07 11:06 ` [PATCH v2 6/8] ARM: DTS: omap4: " H. Nikolaus Schaller
2019-11-07 11:06 ` [PATCH v2 7/8] ARM: DTS: omap5: " H. Nikolaus Schaller
2019-11-07 11:06 ` [PATCH v2 8/8] MIPS: DTS: jz4780: add sgx gpu node H. Nikolaus Schaller

Linux-MIPS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mips/0 linux-mips/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mips linux-mips/ https://lore.kernel.org/linux-mips \
		linux-mips@vger.kernel.org
	public-inbox-index linux-mips

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-mips


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git