On Tue, Aug 10, 2021 at 06:10:43PM +0200, Thierry Reding wrote:
On Tue, Aug 10, 2021 at 06:50:26PM +0300, Mikko Perttunen wrote:
On 10.8.2021 18.43, Thierry Reding wrote:
On Fri, Aug 06, 2021 at 03:34:48PM +0300, Mikko Perttunen wrote:
Convert the original Host1x bindings to YAML and add new bindings for NVDEC, now in a more appropriate location. The old text bindings for Host1x and engines are still kept at display/tegra/ since they encompass a lot more engines that haven't been converted over yet.
Signed-off-by: Mikko Perttunen mperttunen@nvidia.com
v2:
- Fix issues pointed out in v1
- Add T194 nvidia,instance property
.../gpu/host1x/nvidia,tegra20-host1x.yaml | 131 ++++++++++++++++++ .../gpu/host1x/nvidia,tegra210-nvdec.yaml | 109 +++++++++++++++ MAINTAINERS | 1 + 3 files changed, 241 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra20-host1x.yaml create mode 100644 Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
Can we split off the NVDEC bindings addition into a separate patch? I've been working on converting the existing host1x bindings in full to json- schema and this partial conversion would conflict with that effort.
I assume that NVDEC itself validates properly even if host1x hasn't been converted yet?
Sure. I thought I had some problems with this before but can't see any now.
diff --git a/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml new file mode 100644 index 000000000000..fc535bb7aee0 --- /dev/null +++ b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml @@ -0,0 +1,109 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/gpu/host1x/nvidia,tegra210-nvdec.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+title: Device tree binding for NVIDIA Tegra NVDEC
+description: |
- NVDEC is the hardware video decoder present on NVIDIA Tegra210
- and newer chips. It is located on the Host1x bus and typically
- programmed through Host1x channels.
+maintainers:
- Thierry Reding treding@gmail.com
- Mikko Perttunen mperttunen@nvidia.com
+properties:
- $nodename:
- pattern: "^nvdec@[0-9a-f]*$"
- compatible:
- enum:
- nvidia,tegra210-nvdec- nvidia,tegra186-nvdec- nvidia,tegra194-nvdec- reg:
- maxItems: 1
- clocks:
- maxItems: 1
- clock-names:
- items:
- const: nvdec- resets:
- maxItems: 1
- reset-names:
- items:
- const: nvdec- power-domains:
- maxItems: 1
- iommus:
- maxItems: 1
- interconnects:
- items:
- description: DMA read memory client- description: DMA read 2 memory client- description: DMA write memory client- interconnect-names:
- items:
- const: dma-mem- const: read2The convention that we've used so far has been to start numbering these at 0 and use a dash, so this would be "read-1".
Will fix.
- const: write+required:
- compatible
- reg
- clocks
- clock-names
- resets
- reset-names
- power-domains
+if:
- properties:
- compatible:
contains:const: nvidia,tegra194-host1x+then:
- properties:
- nvidia,instance:
items:- description: 0 for NVDEC0, or 1 for NVDEC1I know we had discussed this before, but looking at the driver patch, I don't actually see this being used now, so I wonder if we still need it.
+additionalProperties: true
Maybe this should have a comment noting that this should really be unevaluatedProperties: false, but we can't use that because the tooling doesn't support it yet?
Use 'unevaluatedProperties: false'. There's support in jsonschema pkg master now, so we should have support working soon. I've run it with the kernel tree as well. I was surprised there weren't many issues...
'additionalProperties: true' should only ever be used for incomplete collections of properties (i.e. common bindings). I haven't come up with a meta-schema to check this. We'd probably need to point to different meta-schemas.
I can add such a comment if desired. Honestly, I don't really know what 'unevaluatedProperties' means or does -- the explanation in example-schema.yaml doesn't seem like it's relevant here and I cannot find any other documentation.
It's basically like additionalProperties, except that it applies to properties evaluated via if: blocks.
So with additionalProperties: false, the presence of the nvidia,instance property in a Tegra194 DTS file would cause a validation failure because it's a property that was not defined in the properties: block.
With unevaluatedProperties: false, on the other hand, the properties that are defined in the if: block above will become a evaluated properties and therefore a Tegra194 DTS with the nvidia,instance property present would succeed validation. Unless, of course, if it contained additional properties that are not defined in any of the properties: blocks (either unconditional or conditional ones).
So in other words, the additionalProperties schema applies to all unconditionally defined properties, whereas unevaluatedProperties applies to all (conditionally and unconditionally) defined properties.
I generally discourage only defining the properties in an if/then. Partly because unevaluatedProperties wasn't implemented. I guess if a property is only allowed in the 'then' case, it makes sense. But if the property is just different constraints for then and else, then I'd define it in the main section.
Rob