[PATCH v2 0/8] ARM/MIPS: DTS: add child nodes describing the PVRSGX present in some OMAP SoC and JZ4780

[PATCH v2 0/8] ARM/MIPS: DTS: add child nodes describing the PVRSGX present in some OMAP SoC and JZ4780

[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

[PATCH v2 0/8] ARM/MIPS: DTS: add child nodes describing the PVRSGX present in some OMAP SoC and JZ4780

[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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs

[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

[PATCH v2 1/8] RFC: dt-bindings: add img, pvrsgx.yaml for Imagination GPUs

[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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/8] ARM: DTS: am33xx: add sgx gpu child node

[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

[PATCH v2 2/8] ARM: DTS: am33xx: add sgx gpu child node

[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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 3/8] ARM: DTS: am3517: add sgx gpu child node

[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

[PATCH v2 3/8] ARM: DTS: am3517: add sgx gpu child node

[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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 4/8] ARM: DTS: omap3: add sgx gpu child node

[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

[PATCH v2 4/8] ARM: DTS: omap3: add sgx gpu child node

[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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 5/8] ARM: DTS: omap36xx: add sgx gpu child node

[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

[PATCH v2 5/8] ARM: DTS: omap36xx: add sgx gpu child node

[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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 6/8] ARM: DTS: omap4: add sgx gpu child node

[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

[PATCH v2 6/8] ARM: DTS: omap4: add sgx gpu child node

[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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 7/8] ARM: DTS: omap5: add sgx gpu child node

[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

[PATCH v2 7/8] ARM: DTS: omap5: add sgx gpu child node

[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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 8/8] MIPS: DTS: jz4780: add sgx gpu node

[H. Nikolaus Schaller]

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

[PATCH v2 8/8] MIPS: DTS: jz4780: add sgx gpu node

[H. Nikolaus Schaller]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs

[Rob Herring]

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
>

Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs

[Rob Herring]

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
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs

[Tony Lindgren]

* 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

Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs

[Tony Lindgren]

* 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs

[H. Nikolaus Schaller]

> 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

Re: [PATCH v2 1/8] RFC: dt-bindings: add img, pvrsgx.yaml for Imagination GPUs

[H. Nikolaus Schaller]

> 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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs

[H. Nikolaus Schaller]

> 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

Re: [PATCH v2 1/8] RFC: dt-bindings: add img, pvrsgx.yaml for Imagination GPUs

[H. Nikolaus Schaller]

> 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


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs

[Tony Lindgren]

* 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

Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs

[Tony Lindgren]

* 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs

[Tony Lindgren]

* 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

Re: [PATCH v2 1/8] RFC: dt-bindings: add img,pvrsgx.yaml for Imagination GPUs

[Tony Lindgren]

* 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/8] RFC: dt-bindings: add img, pvrsgx.yaml for Imagination GPUs

[H. Nikolaus Schaller]

Hi Rob,

> 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:
>> I have used the doc2yaml script to get a first version
>> 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.

Yes, the latest version is much more helpful. Thanks!

In addition for getting a better understanding of the YAML syntax I have
writen my own parser and by single-stepping though some sample yaml files,
I now think I understand YAML. And the criticism of YAML on Wikipedia [1]
that it is too ambiguous.. My own toy parser is certainly wrong as well.

Anyways it finally helped me to decide between YAML syntax and DT schemata
which was so far a conglomerate of concepts.

> 
>> * 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

I must admit that I don't even understand completely what you are talking
about... 

Seems to be a complete new set of concepts and terms that is not obvious.
For example: what is a "select" or "custom select" in this context?

> (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.

Unfortunately the dependencies are incomplete (e.g. it depends on pip3 which
itself depends on some python installation).

Now, SCHEMA and CHKDT run fine, but it seems to need a modified DTC because I got:

  DTC     Documentation/devicetree/bindings/arm/actions.example.dt.yaml - due to target missing
FATAL ERROR: Unknown output format "yaml"

Well, I would prefer if that would be self-contained, i.e. that downloading a kernel
tree is sufficient to run the tests and there are only simple dependencies like
make, gcc, flex, yacc/bison? This would mean to include a copy of some libyaml
in the kernel tree.

So, after finding other developers running into the same problem [2], I looked
at scripts/dtc/Makefile which does

	ifeq ($(wildcard /usr/include/yaml.h),)

which is a directory location that is not even writeable on my system...

Fortunately linux-next has improved on that, but

	ifeq ($(shell pkg-config --exists yaml-0.1 2>/dev/null && echo yes),)

still fails on my build host because it runs the pkg-config of the cross-compiler.

So I had to replace the command by a real check if the HOSTCC knows to find
yaml.h and not if the file exists at a certain location:

	ifeq ($(shell echo "#include <yaml.h>" | ${HOSTCC} -E - 2>/dev/null >&2 && echo yes),)

The other issue I have not solved is that

HOSTLDLIBS_dtc := $(shell pkg-config yaml-0.1 --libs)

also runs the pkg-config of the cross-compiler.

IMHO the ideal solution would be if HOSTCFLAGS would be taken care of here
since I already have them include the proper -Lpath/to/lib but it seems to
be ignored. Then, a simple -lyaml would suffice.

> 
>> ---
>> .../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.

Interestingly, this is not checked by the scripts...

> 
>> +%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'.

Done.

...

After digging through all these barriers reported above, I have finally
reworked the bindings file in a way that it passes the dtc checks and
I will submit a v3 now.

BR and thanks,
Nikolaus

[1]: https://en.wikipedia.org/wiki/YAML#Criticism
[2]: https://lkml.org/lkml/2019/8/27/1693

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel