[PATCH v3 1/3] dt-bindings: Add YAML bindings for NVDEC
Rob Herring
robh at kernel.org
Fri Aug 20 18:00:08 UTC 2021
On Wed, Aug 18, 2021 at 8:18 AM Thierry Reding <thierry.reding at gmail.com> wrote:
>
> On Wed, Aug 18, 2021 at 11:24:28AM +0300, Mikko Perttunen wrote:
> > On 8/18/21 12:20 AM, Rob Herring wrote:
> > > On Wed, Aug 11, 2021 at 01:50:28PM +0300, Mikko Perttunen wrote:
> > > > Add YAML device tree bindings for NVDEC, now in a more appropriate
> > > > place compared to the old textual Host1x bindings.
> > > >
> > > > Signed-off-by: Mikko Perttunen <mperttunen at nvidia.com>
> > > > ---
> > > > v3:
> > > > * Drop host1x bindings
> > > > * Change read2 to read-1 in interconnect names
> > > > v2:
> > > > * Fix issues pointed out in v1
> > > > * Add T194 nvidia,instance property
> > > > ---
> > > > .../gpu/host1x/nvidia,tegra210-nvdec.yaml | 109 ++++++++++++++++++
> > > > MAINTAINERS | 1 +
> > > > 2 files changed, 110 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
> > > >
> > > > 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..571849625da8
> > > > --- /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 at gmail.com>
> > > > + - Mikko Perttunen <mperttunen at 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: read-1
> > > > + - const: write
> > > > +
> > > > +required:
> > > > + - compatible
> > > > + - reg
> > > > + - clocks
> > > > + - clock-names
> > > > + - resets
> > > > + - reset-names
> > > > + - power-domains
> > > > +
> > > > +if:
> > > > + properties:
> > > > + compatible:
> > > > + contains:
> > > > + const: nvidia,tegra194-host1x
> > >
> > > host1x? This will never be true as the schema is only applied to nodes
> > > with the nvdec compatible.
> >
> > Argh, it's a typo indeed. Should be nvidia,tegra194-nvdec.
> >
> > >
> > > > +then:
> > > > + properties:
> > > > + nvidia,instance:
> > > > + items:
> > > > + - description: 0 for NVDEC0, or 1 for NVDEC1
> > >
> > > What's this for? We generally don't do indices in DT.
> >
> > When programming the hardware through Host1x, we need to know the "class ID"
> > of the hardware, specific to each instance. So we need to know which
> > instance it is. Technically of course we could derive this from the MMIO
> > address but that seems more confusing.
> >
> > >
> > > > +
> > > > +additionalProperties: true
> > >
> > > This should be false or 'unevaluatedProperties: false'
> >
> > I tried that but it resulted in validation failures; please see the
> > discussion in v2.
>
> Rob mentioned that there is now support for unevaluatedProperties in
> dt-schema. I was able to test this, though with only limited success. I
> made the following changes on top of this patch:
Here's a branch that works with current jsonschema master:
https://github.com/robherring/dt-schema/tree/draft2020-12
> --- >8 ---
> diff --git a/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
> index d2681c98db7e..0bdf05fc8fc7 100644
> --- a/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
> +++ b/Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
> @@ -73,14 +73,18 @@ if:
> properties:
> compatible:
> contains:
> - const: nvidia,tegra194-host1x
> + const: nvidia,tegra194-nvdec
> then:
> properties:
> nvidia,instance:
> + $ref: /schemas/types.yaml#/definitions/uint32
> items:
> - description: 0 for NVDEC0, or 1 for NVDEC1
>
> -additionalProperties: true
> + required:
> + - nvidia,instance
> +
> +unevaluatedProperties: false
>
> examples:
> - |
> @@ -105,3 +109,28 @@ examples:
> interconnect-names = "dma-mem", "read-1", "write";
> iommus = <&smmu TEGRA186_SID_NVDEC>;
> };
> +
> + - |
> + #include <dt-bindings/clock/tegra194-clock.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/memory/tegra194-mc.h>
> + #include <dt-bindings/power/tegra194-powergate.h>
> + #include <dt-bindings/reset/tegra194-reset.h>
> +
> + nvdec at 15480000 {
> + compatible = "nvidia,tegra194-nvdec";
> + reg = <0x15480000 0x40000>;
> + clocks = <&bpmp TEGRA194_CLK_NVDEC>;
> + clock-names = "nvdec";
> + resets = <&bpmp TEGRA194_RESET_NVDEC>;
> + reset-names = "nvdec";
> +
> + nvidia,instance = <0>;
> +
> + power-domains = <&bpmp TEGRA194_POWER_DOMAIN_NVDECA>;
> + interconnects = <&mc TEGRA194_MEMORY_CLIENT_NVDECSRD &emc>,
> + <&mc TEGRA194_MEMORY_CLIENT_NVDECSRD1 &emc>,
> + <&mc TEGRA194_MEMORY_CLIENT_NVDECSWR &emc>;
> + interconnect-names = "dma-mem", "read-1", "write";
> + iommus = <&smmu TEGRA194_SID_NVDEC>;
> + };
> --- >8 ---
>
> As I said, this only works partially. One thing I have to do is comment
> out the whole "if:" block in meta-schemas/base.yaml in order to get rid
> of this error:
>
> Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml: 'additionalProperties' is a required property
> hint: A schema without a "$ref" to another schema must define all properties and use "additionalProperties"
> from schema $id: http://devicetree.org/meta-schemas/base.yaml#
>
> which basically makes the whole file invalid. The reason seems to be
> that dt-schema will only allow unevaluatedProperties if there are any
> $ref references in the schema. Although I'm not sure I understand how
> exactly that works because I tried to inject a dummy $ref but that
> didn't fix the above error.
I think we'll end up relaxing this with 'unevaluatedProperties'
supported. Primarily for just this case with a conditionally defined
property.
> Once that's worked around, though, I do get the examples to validate
> with just a small caveat: nvidia,instance is recognized as being
> required in the Tegra194 case (if I remove it from the example, I do get
> an error, as expected), but if I add nvidia,instance to the Tegra186
> example, it doesn't actually produce an error and will just accept it as
> valid, even though the compatible is not nvidia,tegra194-nvdec.
>
> I don't have time at the moment to investigate why that is, but just
> thought I'd report this here in the meantime. Perhaps it's already a
> known issue?
>
> We could potentially side-step this by getting rid of the custom
> nvidia,instance property altogether. Unfortunately of_device_id table
> matching only supports matching by name, but not by unit-address, which
> would've been nice in this case. Matching by base address manually is a
> bit suboptimal, but it's not that bad.
>
> In any case, there are other examples I know of which need this type of
> functionality (a bunch of devices where the number and names of power
> supplies change from one generation to another), so this problem isn't
> going entirely away anyway.
That's pretty common (though I think we get more variation than we
should), but why would you need the instance or base address for this?
Rob
More information about the dri-devel
mailing list