diff mbox series

[PATH,3/4] dt-bindings: display: Add virtual DRM

Message ID 20210621064403.26663-4-etom@igel.co.jp (mailing list archive)
State New, archived
Headers show
Series Support virtual DRM | expand

Commit Message

Esaki Tomohito June 21, 2021, 6:44 a.m. UTC
Add device tree bindings documentation for virtual DRM.

Signed-off-by: Tomohito Esaki <etom@igel.co.jp>
---
 .../devicetree/bindings/display/vdrm.yaml     | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/vdrm.yaml

Comments

Sam Ravnborg June 21, 2021, 4:03 p.m. UTC | #1
Hi Tomohito

> +
> +description:
> +  This document defines device tree properties virtual DRM. The initial
> +  position, size and z-position of the plane used in the virtual DRM is
> +  specified.


> +  The current limitation is that these settings are applied to all crtc.
This comment (I think) refers to the actual implmentation which is
irrelevant for the binding. The implementation may refer to the binding,
but the binding must be implementation agnostic.

> +
> +properties:
> +  compatible:
> +    const: virt-drm
> +
> +patternProperties:
> +  "^plane(@.*)?$":
> +    description: Information of the planes used in virtual DRM
> +    type: object
> +
> +    properties:
> +      x:
> +        type: int
This syntax looks wrong, I had expected something like:
	
	$ref: "/schemas/types.yaml#/definitions/uint32"

> +        description: x-coordinate of the left-top of the plane in pixels
> +
> +      y:
> +        type: int
> +        description: y-coordinate of the left-top of the plane in pixels
> +
> +      width:
> +        type: int
> +        description: width of the plane in pixels
> +
> +      height:
> +        type: int
> +	description: height of the plane in pixels
> +
> +      zpos:
> +        type: int
> +        description: z-position of the plane
> +
> +    required:
> +      - x
> +      - y
> +      - width
> +      - height
> +      - zpos
> +
> +required:
> +  - compatible

> +  - "^plane(@.*)?$"
If there is no node to match this binding does not take effect.
So I think ^plane... do not need to be specified.

> +
> +examples:
> + - |
> +   vdrm@0 {
> +       compatible = "virt-drm";
> +       plane@0 {
> +           x = <200>;
> +	   y = <100>;
> +	   width = <800>;
> +	   height = <600>;
> +	   zpos = <1>;
> +       };
> +   };
Do not mix spaces and tabd, be consistent and use 4 spaces as indent in
all the example.

	Sam
Rob Herring June 21, 2021, 5:40 p.m. UTC | #2
On Mon, 21 Jun 2021 15:44:02 +0900, Tomohito Esaki wrote:
> Add device tree bindings documentation for virtual DRM.
> 
> Signed-off-by: Tomohito Esaki <etom@igel.co.jp>
> ---
>  .../devicetree/bindings/display/vdrm.yaml     | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/vdrm.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/display/vdrm.yaml:39:1: [error] syntax error: found character '\t' that cannot start any token (syntax)

dtschema/dtc warnings/errors:
make[1]: *** Deleting file 'Documentation/devicetree/bindings/display/vdrm.example.dts'
Traceback (most recent call last):
  File "/usr/local/bin/dt-extract-example", line 45, in <module>
    binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
  File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 434, in load
    return constructor.get_single_data()
  File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 120, in get_single_data
    node = self.composer.get_single_node()
  File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
  File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 731, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
ruamel.yaml.scanner.ScannerError: while scanning a plain scalar
  in "<unicode string>", line 38, column 15
found a tab character that violates indentation
  in "<unicode string>", line 39, column 1
make[1]: *** [Documentation/devicetree/bindings/Makefile:20: Documentation/devicetree/bindings/display/vdrm.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/display/vdrm.yaml:  while scanning a plain scalar
  in "<unicode string>", line 38, column 15
found a tab character that violates indentation
  in "<unicode string>", line 39, column 1
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/vdrm.yaml: ignoring, error parsing file
warning: no schema found in file: ./Documentation/devicetree/bindings/display/vdrm.yaml
make: *** [Makefile:1416: dt_binding_check] Error 2
\ndoc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1494913

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Esaki Tomohito June 22, 2021, 4:17 a.m. UTC | #3
Hi, Rob

Thank you for the error report and advice.
I will recheck DT binding.

Best regards
Tomohito Esaki

On 2021/06/22 2:40, Rob Herring wrote:
> On Mon, 21 Jun 2021 15:44:02 +0900, Tomohito Esaki wrote:
>> Add device tree bindings documentation for virtual DRM.
>>
>> Signed-off-by: Tomohito Esaki <etom@igel.co.jp>
>> ---
>>  .../devicetree/bindings/display/vdrm.yaml     | 67 +++++++++++++++++++
>>  1 file changed, 67 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/display/vdrm.yaml
>>
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/display/vdrm.yaml:39:1: [error] syntax error: found character '\t' that cannot start any token (syntax)
> 
> dtschema/dtc warnings/errors:
> make[1]: *** Deleting file 'Documentation/devicetree/bindings/display/vdrm.example.dts'
> Traceback (most recent call last):
>   File "/usr/local/bin/dt-extract-example", line 45, in <module>
>     binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
>   File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 434, in load
>     return constructor.get_single_data()
>   File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 120, in get_single_data
>     node = self.composer.get_single_node()
>   File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
>   File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
>   File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
>   File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
>   File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
>   File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
>   File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
>   File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
>   File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
>   File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
>   File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
>   File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
>   File "_ruamel_yaml.pyx", line 731, in _ruamel_yaml.CParser._compose_node
>   File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
> ruamel.yaml.scanner.ScannerError: while scanning a plain scalar
>   in "<unicode string>", line 38, column 15
> found a tab character that violates indentation
>   in "<unicode string>", line 39, column 1
> make[1]: *** [Documentation/devicetree/bindings/Makefile:20: Documentation/devicetree/bindings/display/vdrm.example.dts] Error 1
> make[1]: *** Waiting for unfinished jobs....
> ./Documentation/devicetree/bindings/display/vdrm.yaml:  while scanning a plain scalar
>   in "<unicode string>", line 38, column 15
> found a tab character that violates indentation
>   in "<unicode string>", line 39, column 1
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/vdrm.yaml: ignoring, error parsing file
> warning: no schema found in file: ./Documentation/devicetree/bindings/display/vdrm.yaml
> make: *** [Makefile:1416: dt_binding_check] Error 2
> \ndoc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/patch/1494913
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.
>
Rob Herring June 22, 2021, 4:54 p.m. UTC | #4
On Mon, Jun 21, 2021 at 03:44:02PM +0900, Tomohito Esaki wrote:
> Add device tree bindings documentation for virtual DRM.

DRM is a Linuxism. What's virtual DRM? Why does it need to be in DT? 
What's the usecase? You're going to need a lot more reasoning to justify 
this for DT.

> 
> Signed-off-by: Tomohito Esaki <etom@igel.co.jp>
> ---
>  .../devicetree/bindings/display/vdrm.yaml     | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/vdrm.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/vdrm.yaml b/Documentation/devicetree/bindings/display/vdrm.yaml
> new file mode 100644
> index 000000000000..6493bb0fc09f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/vdrm.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/vdrm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Virtual DRM Device Tree Bindings
> +
> +description:
> +  This document defines device tree properties virtual DRM. The initial
> +  position, size and z-position of the plane used in the virtual DRM is
> +  specified.
> +  The current limitation is that these settings are applied to all crtc.
> +
> +properties:
> +  compatible:
> +    const: virt-drm
> +
> +patternProperties:
> +  "^plane(@.*)?$":
> +    description: Information of the planes used in virtual DRM
> +    type: object
> +
> +    properties:
> +      x:
> +        type: int
> +        description: x-coordinate of the left-top of the plane in pixels
> +
> +      y:
> +        type: int
> +        description: y-coordinate of the left-top of the plane in pixels
> +
> +      width:
> +        type: int
> +        description: width of the plane in pixels
> +
> +      height:
> +        type: int
> +	description: height of the plane in pixels
> +
> +      zpos:
> +        type: int
> +        description: z-position of the plane
> +
> +    required:
> +      - x
> +      - y
> +      - width
> +      - height
> +      - zpos
> +
> +required:
> +  - compatible
> +  - "^plane(@.*)?$"
> +
> +examples:
> + - |
> +   vdrm@0 {
> +       compatible = "virt-drm";
> +       plane@0 {
> +           x = <200>;
> +	   y = <100>;
> +	   width = <800>;
> +	   height = <600>;
> +	   zpos = <1>;
> +       };
> +   };
> -- 
> 2.25.1
> 
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/vdrm.yaml b/Documentation/devicetree/bindings/display/vdrm.yaml
new file mode 100644
index 000000000000..6493bb0fc09f
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/vdrm.yaml
@@ -0,0 +1,67 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/vdrm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtual DRM Device Tree Bindings
+
+description:
+  This document defines device tree properties virtual DRM. The initial
+  position, size and z-position of the plane used in the virtual DRM is
+  specified.
+  The current limitation is that these settings are applied to all crtc.
+
+properties:
+  compatible:
+    const: virt-drm
+
+patternProperties:
+  "^plane(@.*)?$":
+    description: Information of the planes used in virtual DRM
+    type: object
+
+    properties:
+      x:
+        type: int
+        description: x-coordinate of the left-top of the plane in pixels
+
+      y:
+        type: int
+        description: y-coordinate of the left-top of the plane in pixels
+
+      width:
+        type: int
+        description: width of the plane in pixels
+
+      height:
+        type: int
+	description: height of the plane in pixels
+
+      zpos:
+        type: int
+        description: z-position of the plane
+
+    required:
+      - x
+      - y
+      - width
+      - height
+      - zpos
+
+required:
+  - compatible
+  - "^plane(@.*)?$"
+
+examples:
+ - |
+   vdrm@0 {
+       compatible = "virt-drm";
+       plane@0 {
+           x = <200>;
+	   y = <100>;
+	   width = <800>;
+	   height = <600>;
+	   zpos = <1>;
+       };
+   };