Re: [PATCH v4 1/8] dt-bindings: add img, pvrsgx.yaml for Imagination GPUs
Hi Rob, > Am 18.12.2019 um 22:16 schrieb Rob Herring : > > On Tue, Dec 17, 2019 at 07:01:59PM +0100, H. Nikolaus Schaller 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.). >> >> In most cases, Clock, Reset and power management is handled >> by a parent node or elsewhere. >> >> Tested by make dt_binding_check dtbs_check > > I'm surprised that worked... (Not for long if it did). AFAIR, it did not fail but emit thousands of warnings for other areas so I may have missed some warning but there was no error that did stop compilation. > >> >> Signed-off-by: H. Nikolaus Schaller >> --- >> .../devicetree/bindings/gpu/img,pvrsgx.yaml | 80 +++ >> 1 file changed, 80 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 ..44799774e34d >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml >> @@ -0,0 +1,80 @@ >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/gpu/img,pvrsgx.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Imagination PVR/SGX GPU >> + >> +maintainers: >> + - H. Nikolaus Schaller >> + >> +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 and more. >> + >> + For an almost complete list see: >> https://en.wikipedia.org/wiki/PowerVR#Implementations >> + >> + Only the Imagination SGX530, SGX540 and SGX544 GPUs are currently covered >> by >> + this binding but the extension of the pattern is straightforward. >> + >> + 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: >> +enum: >> +# Example: BeagleBoard A/B/C, OpenPandora 600MHz >> + - ti,omap3-sgx530-121, img,sgx530-121, img,sgx530 > > Didn't I comment before this is not valid. Not that I am aware of. Your comment was: <<< > +properties: > + compatible: > +enum: > +# BeagleBoard ABC, OpenPandora 600MHz I'd expect compatibles to be per SoC, not per board. > + - ti,omap3-sgx530-121, img,sgx530-121, img,sgx530, img,sgx5 4 compatibles is probably a bit much. Are there not any version or feature registers that some of this could be detected? If there are, I'd assume the middle 2 strings could be dropped. If not, drop the last one and just match on the 3rd string. It's not a long list. >>> So I interpreted your comment is about the # comment focussing too much on boards instead of SoC and about dropping img,sgx5 > You are defining the > compatible string is: 'ti,omap3-sgx530-121, img,sgx530-121, img,sgx530' Yes. > > You need: > > compatible: > oneOf: >- description: BeagleBoard A/B/C, OpenPandora 600MHz > items: >- const: ti,omap3-sgx530-121 >- const: img,sgx530-121 >- const: img,sgx530 > > And so on for each of the rest. Hm. As far as I understand YAML, it has multiple ways to define the same structure but this manifold nature has its drawbacks. If it is wrong I have to admit that I still do not understand how to write correct schemes... Why has it been made so difficult for us developers? > >> +# Example: BeagleBoard XM, GTA04, OpenPandora 1GHz >> + - ti,omap3-sgx530-125, img,sgx530-125, img,sgx530 >> +# Example: BeagleBone Black >> + - ti,am3352-sgx530-125, img,sgx530-125, img,sgx530 > > These 2 could be combined using 'enum' for the first item. Basically, > you can group ones where the last 2 strings are the same. Is this better readable? IMHO, this would be a nice coding trick that would make this even more unreadable and does not add any new information. Generally, in my opinion, a bindings document should not only satisfy the dtbs_check but should be human readable (and writeable!) so that a DTS developer can still understand and ideally copy fragments. With such enum tricks and -items const: constructions this IMHO becomes more difficult. Therefore I think the linear list is better readable and can be directly copied. If the test rig has problems with that statement, I would suggest that the parser should be modified to understand what we can easily write and read. The same is for # comment vs. description: . The # comment is for a human reader of this document to get a hint what board the next line is intended for. It is not
Re: [PATCH v4 1/8] dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
On Tue, Dec 17, 2019 at 07:01:59PM +0100, H. Nikolaus Schaller 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.).
>
> In most cases, Clock, Reset and power management is handled
> by a parent node or elsewhere.
>
> Tested by make dt_binding_check dtbs_check
I'm surprised that worked... (Not for long if it did).
>
> Signed-off-by: H. Nikolaus Schaller
> ---
> .../devicetree/bindings/gpu/img,pvrsgx.yaml | 80 +++
> 1 file changed, 80 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 ..44799774e34d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpu/img,pvrsgx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Imagination PVR/SGX GPU
> +
> +maintainers:
> + - H. Nikolaus Schaller
> +
> +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 and more.
> +
> + For an almost complete list see:
> https://en.wikipedia.org/wiki/PowerVR#Implementations
> +
> + Only the Imagination SGX530, SGX540 and SGX544 GPUs are currently covered
> by
> + this binding but the extension of the pattern is straightforward.
> +
> + 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:
> +enum:
> +# Example: BeagleBoard A/B/C, OpenPandora 600MHz
> + - ti,omap3-sgx530-121, img,sgx530-121, img,sgx530
Didn't I comment before this is not valid. You are defining the
compatible string is: 'ti,omap3-sgx530-121, img,sgx530-121, img,sgx530'
You need:
compatible:
oneOf:
- description: BeagleBoard A/B/C, OpenPandora 600MHz
items:
- const: ti,omap3-sgx530-121
- const: img,sgx530-121
- const: img,sgx530
And so on for each of the rest.
> +# Example: BeagleBoard XM, GTA04, OpenPandora 1GHz
> + - ti,omap3-sgx530-125, img,sgx530-125, img,sgx530
> +# Example: BeagleBone Black
> + - ti,am3352-sgx530-125, img,sgx530-125, img,sgx530
These 2 could be combined using 'enum' for the first item. Basically,
you can group ones where the last 2 strings are the same.
> +# Example: Pandaboard, Pandaboard ES
> + - ti,omap4-sgx540-120, img,sgx540-120, img,sgx540
> + - ti,omap4-sgx544-112, img,sgx544-112, img,sgx544
> +# Example: OMAP5 UEVM, Pyra Handheld
> + - ti,omap5-sgx544-116, img,sgx544-116, img,sgx544
> + - ti,dra7-sgx544-116, img,sgx544-116, img,sgx544
> +# Example: CI20
> + - ingenic,jz4780-sgx540-120, img,sgx540-120, img,sgx540
> +# the following entries are not validated with real hardware
What am I supposed to do with that? You're just defining some strings.
If you're not sure they are okay, then don't define them.
> +# more TI SoC
> + - ti,am3517-sgx530-125, img,sgx530-125, img,sgx530
> + - ti,am4-sgx530-125, img,sgx530-125, img,sgx530
> + - ti,ti81xx-sgx530-125, img,sgx530-125, img,sgx530
> +# Example: Banana-Pi-M3 (Allwinner A83T)
> + - allwinner,sun8i-a83t-sgx544-116, img,sgx544-116, img,sgx544
> +# Example: Atom Z5xx
> + - intel,poulsbo-gma500-sgx535, img,sgx535-116, img,sgx535
> +# Example: Atom Z24xx
> + - intel,medfield-gma-sgx540, img,sgx540-116, img,sgx540
> +# Example: Atom N2600, D2500
> + - intel,cedarview-gma3600-sgx545, img,sgx545-116, img,sgx545
> +
> + reg:
> +maxItems: 1
> +
> + interrupts:
> +maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> + - |+
> +#include
> +
> +gpu@fe00 {
> + compatible = "ti,omap-omap5-sgx544-116", "img,sgx544-116",
> "img,sgx544", "img,sgx5";
Doesn't match the schema.
> + reg = <0xfe00 0x200>;
> + interrupts = ;
> +};
> +
> +...
> --
> 2.23.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 1/8] dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
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.).
In most cases, Clock, Reset and power management is handled
by a parent node or elsewhere.
Tested by make dt_binding_check dtbs_check
Signed-off-by: H. Nikolaus Schaller
---
.../devicetree/bindings/gpu/img,pvrsgx.yaml | 80 +++
1 file changed, 80 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 ..44799774e34d
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpu/img,pvrsgx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Imagination PVR/SGX GPU
+
+maintainers:
+ - H. Nikolaus Schaller
+
+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 and more.
+
+ For an almost complete list see:
https://en.wikipedia.org/wiki/PowerVR#Implementations
+
+ Only the Imagination SGX530, SGX540 and SGX544 GPUs are currently covered by
+ this binding but the extension of the pattern is straightforward.
+
+ 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:
+enum:
+# Example: BeagleBoard A/B/C, OpenPandora 600MHz
+ - ti,omap3-sgx530-121, img,sgx530-121, img,sgx530
+# Example: BeagleBoard XM, GTA04, OpenPandora 1GHz
+ - ti,omap3-sgx530-125, img,sgx530-125, img,sgx530
+# Example: BeagleBone Black
+ - ti,am3352-sgx530-125, img,sgx530-125, img,sgx530
+# Example: Pandaboard, Pandaboard ES
+ - ti,omap4-sgx540-120, img,sgx540-120, img,sgx540
+ - ti,omap4-sgx544-112, img,sgx544-112, img,sgx544
+# Example: OMAP5 UEVM, Pyra Handheld
+ - ti,omap5-sgx544-116, img,sgx544-116, img,sgx544
+ - ti,dra7-sgx544-116, img,sgx544-116, img,sgx544
+# Example: CI20
+ - ingenic,jz4780-sgx540-120, img,sgx540-120, img,sgx540
+# the following entries are not validated with real hardware
+# more TI SoC
+ - ti,am3517-sgx530-125, img,sgx530-125, img,sgx530
+ - ti,am4-sgx530-125, img,sgx530-125, img,sgx530
+ - ti,ti81xx-sgx530-125, img,sgx530-125, img,sgx530
+# Example: Banana-Pi-M3 (Allwinner A83T)
+ - allwinner,sun8i-a83t-sgx544-116, img,sgx544-116, img,sgx544
+# Example: Atom Z5xx
+ - intel,poulsbo-gma500-sgx535, img,sgx535-116, img,sgx535
+# Example: Atom Z24xx
+ - intel,medfield-gma-sgx540, img,sgx540-116, img,sgx540
+# Example: Atom N2600, D2500
+ - intel,cedarview-gma3600-sgx545, img,sgx545-116, img,sgx545
+
+ reg:
+maxItems: 1
+
+ interrupts:
+maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |+
+#include
+
+gpu@fe00 {
+ compatible = "ti,omap-omap5-sgx544-116", "img,sgx544-116", "img,sgx544",
"img,sgx5";
+ reg = <0xfe00 0x200>;
+ interrupts = ;
+};
+
+...
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
