Re: [PATCH v2 1/3] dt-bindings: Add YAML bindings for Host1x and NVDEC
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 > > > > --- > > > > 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 ..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 > > > > + - Mikko Perttunen > > > > + > > > > +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: read2 > > > > > > The 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 NVDEC1 > > > > > > I 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
Re: [PATCH v2 1/3] dt-bindings: Add YAML bindings for Host1x and NVDEC
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 > > > --- > > > 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 ..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 > > > + - Mikko Perttunen > > > + > > > +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: read2 > > > > The 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 NVDEC1 > > > > I 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? > > 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
Re: [PATCH v2 1/3] dt-bindings: Add YAML bindings for Host1x and NVDEC
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 --- 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 ..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 + - Mikko Perttunen + +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: read2 The 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 NVDEC1 I 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? 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. Thanks, Mikko Rob, what's the current best practice for that? I see that there are quite a few bindings that use unevaluatedProperties, so I wonder if we just ignore errors from that for now? Or do we have some development branch of the tooling somewhere that supports this now? I vaguely recall reading about work in progress patches for this, but I can't find the link now to see if there's been an update since I last looked. Thierry
Re: [PATCH v2 1/3] dt-bindings: Add YAML bindings for Host1x and NVDEC
On Tue, Aug 10, 2021 at 05:43:26PM +0200, Thierry Reding wrote: > On Fri, Aug 06, 2021 at 03:34:48PM +0300, Mikko Perttunen wrote: [...] > > diff --git > > a/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml > > b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml [...] > > +if: > > + properties: > > +compatible: > > + contains: > > +const: nvidia,tegra194-host1x > > +then: > > + properties: > > +nvidia,instance: > > + items: > > +- description: 0 for NVDEC0, or 1 for NVDEC1 > > I 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. Oh, nevermind, upon closer inspection, I do see it used in the driver. Thierry signature.asc Description: PGP signature
Re: [PATCH v2 1/3] dt-bindings: Add YAML bindings for Host1x and NVDEC
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 > --- > 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? > 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 ..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 > + - Mikko Perttunen > + > +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: read2 The 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". > + - 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 NVDEC1 I 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? Rob, what's the current best practice for that? I see that there are quite a few bindings that use unevaluatedProperties, so I wonder if we just ignore errors from that for now? Or do we have some development branch of the tooling somewhere that supports this now? I vaguely recall reading about work in progress patches for this, but I can't find the link now to see if there's been an update since I last looked. Thierry signature.asc Description: PGP signature
