All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] NVIDIA Tegra NVDEC support
@ 2021-08-11 10:50 Mikko Perttunen
  2021-08-11 10:50 ` [PATCH v3 1/3] dt-bindings: Add YAML bindings for NVDEC Mikko Perttunen
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Mikko Perttunen @ 2021-08-11 10:50 UTC (permalink / raw)
  To: thierry.reding, jonathanh, airlied, daniel, robh+dt
  Cc: dri-devel, linux-tegra, devicetree, Mikko Perttunen

Here's the v3 of the NVDEC support series, containing minor fixes
compared to v2.

NVDEC hardware documentation can be found at
https://github.com/NVIDIA/open-gpu-doc/tree/master/classes/video

and example userspace can be found at
https://github.com/cyndis/vaapi-tegra-driver

Thanks,
Mikko

Mikko Perttunen (3):
  dt-bindings: Add YAML bindings for NVDEC
  arm64: tegra: Add NVDEC to Tegra186/194 device trees
  drm/tegra: Add NVDEC driver

 .../gpu/host1x/nvidia,tegra210-nvdec.yaml     | 109 ++++
 MAINTAINERS                                   |   1 +
 arch/arm64/boot/dts/nvidia/tegra186.dtsi      |  16 +
 arch/arm64/boot/dts/nvidia/tegra194.dtsi      |  36 ++
 drivers/gpu/drm/tegra/Makefile                |   3 +-
 drivers/gpu/drm/tegra/drm.c                   |   4 +
 drivers/gpu/drm/tegra/drm.h                   |   1 +
 drivers/gpu/drm/tegra/nvdec.c                 | 474 ++++++++++++++++++
 drivers/gpu/host1x/dev.c                      |  18 +
 include/linux/host1x.h                        |   2 +
 10 files changed, 663 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/gpu/host1x/nvidia,tegra210-nvdec.yaml
 create mode 100644 drivers/gpu/drm/tegra/nvdec.c

-- 
2.32.0


^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH v3 1/3] dt-bindings: Add YAML bindings for NVDEC
  2021-08-11 10:50 [PATCH v3 0/3] NVIDIA Tegra NVDEC support Mikko Perttunen
@ 2021-08-11 10:50 ` Mikko Perttunen
  2021-08-17 21:20   ` Rob Herring
  2021-08-11 10:50 ` [PATCH v3 2/3] arm64: tegra: Add NVDEC to Tegra186/194 device trees Mikko Perttunen
  2021-08-11 10:50 ` [PATCH v3 3/3] drm/tegra: Add NVDEC driver Mikko Perttunen
  2 siblings, 1 reply; 9+ messages in thread
From: Mikko Perttunen @ 2021-08-11 10:50 UTC (permalink / raw)
  To: thierry.reding, jonathanh, airlied, daniel, robh+dt
  Cc: dri-devel, linux-tegra, devicetree, Mikko Perttunen

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@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@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: 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
+
+additionalProperties: true
+
+examples:
+  - |
+    #include <dt-bindings/clock/tegra186-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/memory/tegra186-mc.h>
+    #include <dt-bindings/power/tegra186-powergate.h>
+    #include <dt-bindings/reset/tegra186-reset.h>
+
+    nvdec@15480000 {
+            compatible = "nvidia,tegra186-nvdec";
+            reg = <0x15480000 0x40000>;
+            clocks = <&bpmp TEGRA186_CLK_NVDEC>;
+            clock-names = "nvdec";
+            resets = <&bpmp TEGRA186_RESET_NVDEC>;
+            reset-names = "nvdec";
+
+            power-domains = <&bpmp TEGRA186_POWER_DOMAIN_NVDEC>;
+            interconnects = <&mc TEGRA186_MEMORY_CLIENT_NVDECSRD &emc>,
+                            <&mc TEGRA186_MEMORY_CLIENT_NVDECSRD1 &emc>,
+                            <&mc TEGRA186_MEMORY_CLIENT_NVDECSWR &emc>;
+            interconnect-names = "dma-mem", "read-1", "write";
+            iommus = <&smmu TEGRA186_SID_NVDEC>;
+    };
+
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 69932194e1ba..ce9e360639d5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6230,6 +6230,7 @@ L:	linux-tegra@vger.kernel.org
 S:	Supported
 T:	git git://anongit.freedesktop.org/tegra/linux.git
 F:	Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
+F:	Documentation/devicetree/bindings/gpu/host1x/
 F:	drivers/gpu/drm/tegra/
 F:	drivers/gpu/host1x/
 F:	include/linux/host1x.h
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH v3 2/3] arm64: tegra: Add NVDEC to Tegra186/194 device trees
  2021-08-11 10:50 [PATCH v3 0/3] NVIDIA Tegra NVDEC support Mikko Perttunen
  2021-08-11 10:50 ` [PATCH v3 1/3] dt-bindings: Add YAML bindings for NVDEC Mikko Perttunen
@ 2021-08-11 10:50 ` Mikko Perttunen
  2021-08-11 10:50 ` [PATCH v3 3/3] drm/tegra: Add NVDEC driver Mikko Perttunen
  2 siblings, 0 replies; 9+ messages in thread
From: Mikko Perttunen @ 2021-08-11 10:50 UTC (permalink / raw)
  To: thierry.reding, jonathanh, airlied, daniel, robh+dt
  Cc: dri-devel, linux-tegra, devicetree, Mikko Perttunen

Add a device tree node for NVDEC on Tegra186, and
device tree nodes for NVDEC and NVDEC1 on Tegra194.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
v3:
* Change read2 to read-1
v2:
* Add NVDECSRD1 memory client
* Add also to T194 (both NVDEC0/1)
---
 arch/arm64/boot/dts/nvidia/tegra186.dtsi | 16 +++++++++++
 arch/arm64/boot/dts/nvidia/tegra194.dtsi | 36 ++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index d02f6bf3e2ca..4f2f21242b2c 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1342,6 +1342,22 @@ dsib: dsi@15400000 {
 			power-domains = <&bpmp TEGRA186_POWER_DOMAIN_DISP>;
 		};
 
+		nvdec@15480000 {
+			compatible = "nvidia,tegra186-nvdec";
+			reg = <0x15480000 0x40000>;
+			clocks = <&bpmp TEGRA186_CLK_NVDEC>;
+			clock-names = "nvdec";
+			resets = <&bpmp TEGRA186_RESET_NVDEC>;
+			reset-names = "nvdec";
+
+			power-domains = <&bpmp TEGRA186_POWER_DOMAIN_NVDEC>;
+			interconnects = <&mc TEGRA186_MEMORY_CLIENT_NVDECSRD &emc>,
+					<&mc TEGRA186_MEMORY_CLIENT_NVDECSRD1 &emc>,
+					<&mc TEGRA186_MEMORY_CLIENT_NVDECSWR &emc>;
+			interconnect-names = "dma-mem", "read-1", "write";
+			iommus = <&smmu TEGRA186_SID_NVDEC>;
+		};
+
 		sor0: sor@15540000 {
 			compatible = "nvidia,tegra186-sor";
 			reg = <0x15540000 0x10000>;
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 5ba7a4519b95..9119aa4f28c1 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -1412,6 +1412,24 @@ host1x@13e00000 {
 			interconnect-names = "dma-mem";
 			iommus = <&smmu TEGRA194_SID_HOST1X>;
 
+			nvdec@15140000 {
+				compatible = "nvidia,tegra194-nvdec";
+				reg = <0x15140000 0x00040000>;
+				clocks = <&bpmp TEGRA194_CLK_NVDEC1>;
+				clock-names = "nvdec";
+				resets = <&bpmp TEGRA194_RESET_NVDEC1>;
+				reset-names = "nvdec";
+
+				power-domains = <&bpmp TEGRA194_POWER_DOMAIN_NVDECB>;
+				interconnects = <&mc TEGRA194_MEMORY_CLIENT_NVDEC1SRD &emc>,
+						<&mc TEGRA194_MEMORY_CLIENT_NVDEC1SRD1 &emc>,
+						<&mc TEGRA194_MEMORY_CLIENT_NVDEC1SWR &emc>;
+				interconnect-names = "dma-mem", "read-1", "write";
+				iommus = <&smmu TEGRA194_SID_NVDEC1>;
+
+				nvidia,instance = <1>;
+			};
+
 			display-hub@15200000 {
 				compatible = "nvidia,tegra194-display";
 				reg = <0x15200000 0x00040000>;
@@ -1525,6 +1543,24 @@ vic@15340000 {
 				iommus = <&smmu TEGRA194_SID_VIC>;
 			};
 
+			nvdec@15480000 {
+				compatible = "nvidia,tegra194-nvdec";
+				reg = <0x15480000 0x00040000>;
+				clocks = <&bpmp TEGRA194_CLK_NVDEC>;
+				clock-names = "nvdec";
+				resets = <&bpmp TEGRA194_RESET_NVDEC>;
+				reset-names = "nvdec";
+
+				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>;
+
+				nvidia,instance = <0>;
+			};
+
 			dpaux0: dpaux@155c0000 {
 				compatible = "nvidia,tegra194-dpaux";
 				reg = <0x155c0000 0x10000>;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH v3 3/3] drm/tegra: Add NVDEC driver
  2021-08-11 10:50 [PATCH v3 0/3] NVIDIA Tegra NVDEC support Mikko Perttunen
  2021-08-11 10:50 ` [PATCH v3 1/3] dt-bindings: Add YAML bindings for NVDEC Mikko Perttunen
  2021-08-11 10:50 ` [PATCH v3 2/3] arm64: tegra: Add NVDEC to Tegra186/194 device trees Mikko Perttunen
@ 2021-08-11 10:50 ` Mikko Perttunen
  2 siblings, 0 replies; 9+ messages in thread
From: Mikko Perttunen @ 2021-08-11 10:50 UTC (permalink / raw)
  To: thierry.reding, jonathanh, airlied, daniel, robh+dt
  Cc: dri-devel, linux-tegra, devicetree, Mikko Perttunen

Add support for booting and using NVDEC on Tegra210, Tegra186
and Tegra194 to the Host1x and TegraDRM drivers. Booting in
secure mode is not currently supported.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
v3:
* Change num_instances to unsigned int
* Remove unnecessary '= 0' initializer
* Populate num_instances data
* Fix instance number check
v2:
* Use devm_platform_get_and_ioremap_resource
* Remove reset handling, done by power domain code
* Assume runtime PM is enabled
---
 drivers/gpu/drm/tegra/Makefile |   3 +-
 drivers/gpu/drm/tegra/drm.c    |   4 +
 drivers/gpu/drm/tegra/drm.h    |   1 +
 drivers/gpu/drm/tegra/nvdec.c  | 474 +++++++++++++++++++++++++++++++++
 drivers/gpu/host1x/dev.c       |  18 ++
 include/linux/host1x.h         |   2 +
 6 files changed, 501 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/tegra/nvdec.c

diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
index 5d2039f0c734..b248c631f790 100644
--- a/drivers/gpu/drm/tegra/Makefile
+++ b/drivers/gpu/drm/tegra/Makefile
@@ -24,7 +24,8 @@ tegra-drm-y := \
 	gr2d.o \
 	gr3d.o \
 	falcon.o \
-	vic.o
+	vic.o \
+	nvdec.o
 
 tegra-drm-y += trace.o
 
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index b20fd0833661..5f5afd7ba37e 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1337,15 +1337,18 @@ static const struct of_device_id host1x_drm_subdevs[] = {
 	{ .compatible = "nvidia,tegra210-sor", },
 	{ .compatible = "nvidia,tegra210-sor1", },
 	{ .compatible = "nvidia,tegra210-vic", },
+	{ .compatible = "nvidia,tegra210-nvdec", },
 	{ .compatible = "nvidia,tegra186-display", },
 	{ .compatible = "nvidia,tegra186-dc", },
 	{ .compatible = "nvidia,tegra186-sor", },
 	{ .compatible = "nvidia,tegra186-sor1", },
 	{ .compatible = "nvidia,tegra186-vic", },
+	{ .compatible = "nvidia,tegra186-nvdec", },
 	{ .compatible = "nvidia,tegra194-display", },
 	{ .compatible = "nvidia,tegra194-dc", },
 	{ .compatible = "nvidia,tegra194-sor", },
 	{ .compatible = "nvidia,tegra194-vic", },
+	{ .compatible = "nvidia,tegra194-nvdec", },
 	{ /* sentinel */ }
 };
 
@@ -1369,6 +1372,7 @@ static struct platform_driver * const drivers[] = {
 	&tegra_gr2d_driver,
 	&tegra_gr3d_driver,
 	&tegra_vic_driver,
+	&tegra_nvdec_driver,
 };
 
 static int __init host1x_drm_init(void)
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 8b28327c931c..fc0a19554eac 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -202,5 +202,6 @@ extern struct platform_driver tegra_sor_driver;
 extern struct platform_driver tegra_gr2d_driver;
 extern struct platform_driver tegra_gr3d_driver;
 extern struct platform_driver tegra_vic_driver;
+extern struct platform_driver tegra_nvdec_driver;
 
 #endif /* HOST1X_DRM_H */
diff --git a/drivers/gpu/drm/tegra/nvdec.c b/drivers/gpu/drm/tegra/nvdec.c
new file mode 100644
index 000000000000..e8e7ebbea53e
--- /dev/null
+++ b/drivers/gpu/drm/tegra/nvdec.c
@@ -0,0 +1,474 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2015-2021, NVIDIA Corporation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/host1x.h>
+#include <linux/iommu.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+
+#include <soc/tegra/pmc.h>
+
+#include "drm.h"
+#include "falcon.h"
+#include "vic.h"
+
+struct nvdec_config {
+	const char *firmware;
+	unsigned int version;
+	bool supports_sid;
+	unsigned int num_instances;
+};
+
+struct nvdec {
+	struct falcon falcon;
+
+	void __iomem *regs;
+	struct tegra_drm_client client;
+	struct host1x_channel *channel;
+	struct device *dev;
+	struct clk *clk;
+
+	/* Platform configuration */
+	const struct nvdec_config *config;
+};
+
+static inline struct nvdec *to_nvdec(struct tegra_drm_client *client)
+{
+	return container_of(client, struct nvdec, client);
+}
+
+static void nvdec_writel(struct nvdec *nvdec, u32 value, unsigned int offset)
+{
+	writel(value, nvdec->regs + offset);
+}
+
+static int nvdec_boot(struct nvdec *nvdec)
+{
+#ifdef CONFIG_IOMMU_API
+	struct iommu_fwspec *spec = dev_iommu_fwspec_get(nvdec->dev);
+#endif
+	int err;
+
+#ifdef CONFIG_IOMMU_API
+	if (nvdec->config->supports_sid && spec) {
+		u32 value;
+
+		value = TRANSCFG_ATT(1, TRANSCFG_SID_FALCON) | TRANSCFG_ATT(0, TRANSCFG_SID_HW);
+		nvdec_writel(nvdec, value, VIC_TFBIF_TRANSCFG);
+
+		if (spec->num_ids > 0) {
+			value = spec->ids[0] & 0xffff;
+
+			nvdec_writel(nvdec, value, VIC_THI_STREAMID0);
+			nvdec_writel(nvdec, value, VIC_THI_STREAMID1);
+		}
+	}
+#endif
+
+	err = falcon_boot(&nvdec->falcon);
+	if (err < 0)
+		return err;
+
+	err = falcon_wait_idle(&nvdec->falcon);
+	if (err < 0) {
+		dev_err(nvdec->dev, "falcon boot timed out\n");
+		return err;
+	}
+
+	return 0;
+}
+
+static int nvdec_init(struct host1x_client *client)
+{
+	struct tegra_drm_client *drm = host1x_to_drm_client(client);
+	struct drm_device *dev = dev_get_drvdata(client->host);
+	struct tegra_drm *tegra = dev->dev_private;
+	struct nvdec *nvdec = to_nvdec(drm);
+	int err;
+
+	err = host1x_client_iommu_attach(client);
+	if (err < 0 && err != -ENODEV) {
+		dev_err(nvdec->dev, "failed to attach to domain: %d\n", err);
+		return err;
+	}
+
+	nvdec->channel = host1x_channel_request(client);
+	if (!nvdec->channel) {
+		err = -ENOMEM;
+		goto detach;
+	}
+
+	client->syncpts[0] = host1x_syncpt_request(client, 0);
+	if (!client->syncpts[0]) {
+		err = -ENOMEM;
+		goto free_channel;
+	}
+
+	err = tegra_drm_register_client(tegra, drm);
+	if (err < 0)
+		goto free_syncpt;
+
+	/*
+	 * Inherit the DMA parameters (such as maximum segment size) from the
+	 * parent host1x device.
+	 */
+	client->dev->dma_parms = client->host->dma_parms;
+
+	return 0;
+
+free_syncpt:
+	host1x_syncpt_put(client->syncpts[0]);
+free_channel:
+	host1x_channel_put(nvdec->channel);
+detach:
+	host1x_client_iommu_detach(client);
+
+	return err;
+}
+
+static int nvdec_exit(struct host1x_client *client)
+{
+	struct tegra_drm_client *drm = host1x_to_drm_client(client);
+	struct drm_device *dev = dev_get_drvdata(client->host);
+	struct tegra_drm *tegra = dev->dev_private;
+	struct nvdec *nvdec = to_nvdec(drm);
+	int err;
+
+	/* avoid a dangling pointer just in case this disappears */
+	client->dev->dma_parms = NULL;
+
+	err = tegra_drm_unregister_client(tegra, drm);
+	if (err < 0)
+		return err;
+
+	host1x_syncpt_put(client->syncpts[0]);
+	host1x_channel_put(nvdec->channel);
+	host1x_client_iommu_detach(client);
+
+	if (client->group) {
+		dma_unmap_single(nvdec->dev, nvdec->falcon.firmware.phys,
+				 nvdec->falcon.firmware.size, DMA_TO_DEVICE);
+		tegra_drm_free(tegra, nvdec->falcon.firmware.size,
+			       nvdec->falcon.firmware.virt,
+			       nvdec->falcon.firmware.iova);
+	} else {
+		dma_free_coherent(nvdec->dev, nvdec->falcon.firmware.size,
+				  nvdec->falcon.firmware.virt,
+				  nvdec->falcon.firmware.iova);
+	}
+
+	return 0;
+}
+
+static const struct host1x_client_ops nvdec_client_ops = {
+	.init = nvdec_init,
+	.exit = nvdec_exit,
+};
+
+static int nvdec_load_firmware(struct nvdec *nvdec)
+{
+	struct host1x_client *client = &nvdec->client.base;
+	struct tegra_drm *tegra = nvdec->client.drm;
+	dma_addr_t iova;
+	size_t size;
+	void *virt;
+	int err;
+
+	if (nvdec->falcon.firmware.virt)
+		return 0;
+
+	err = falcon_read_firmware(&nvdec->falcon, nvdec->config->firmware);
+	if (err < 0)
+		return err;
+
+	size = nvdec->falcon.firmware.size;
+
+	if (!client->group) {
+		virt = dma_alloc_coherent(nvdec->dev, size, &iova, GFP_KERNEL);
+
+		err = dma_mapping_error(nvdec->dev, iova);
+		if (err < 0)
+			return err;
+	} else {
+		virt = tegra_drm_alloc(tegra, size, &iova);
+	}
+
+	nvdec->falcon.firmware.virt = virt;
+	nvdec->falcon.firmware.iova = iova;
+
+	err = falcon_load_firmware(&nvdec->falcon);
+	if (err < 0)
+		goto cleanup;
+
+	/*
+	 * In this case we have received an IOVA from the shared domain, so we
+	 * need to make sure to get the physical address so that the DMA API
+	 * knows what memory pages to flush the cache for.
+	 */
+	if (client->group) {
+		dma_addr_t phys;
+
+		phys = dma_map_single(nvdec->dev, virt, size, DMA_TO_DEVICE);
+
+		err = dma_mapping_error(nvdec->dev, phys);
+		if (err < 0)
+			goto cleanup;
+
+		nvdec->falcon.firmware.phys = phys;
+	}
+
+	return 0;
+
+cleanup:
+	if (!client->group)
+		dma_free_coherent(nvdec->dev, size, virt, iova);
+	else
+		tegra_drm_free(tegra, size, virt, iova);
+
+	return err;
+}
+
+
+static int nvdec_runtime_resume(struct device *dev)
+{
+	struct nvdec *nvdec = dev_get_drvdata(dev);
+	int err;
+
+	err = clk_prepare_enable(nvdec->clk);
+	if (err < 0)
+		return err;
+
+	usleep_range(10, 20);
+
+	err = nvdec_load_firmware(nvdec);
+	if (err < 0)
+		goto disable;
+
+	err = nvdec_boot(nvdec);
+	if (err < 0)
+		goto disable;
+
+	return 0;
+
+disable:
+	clk_disable_unprepare(nvdec->clk);
+	return err;
+}
+
+static int nvdec_runtime_suspend(struct device *dev)
+{
+	struct nvdec *nvdec = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(nvdec->clk);
+
+	return 0;
+}
+
+static int nvdec_open_channel(struct tegra_drm_client *client,
+			    struct tegra_drm_context *context)
+{
+	struct nvdec *nvdec = to_nvdec(client);
+	int err;
+
+	err = pm_runtime_get_sync(nvdec->dev);
+	if (err < 0) {
+		pm_runtime_put(nvdec->dev);
+		return err;
+	}
+
+	context->channel = host1x_channel_get(nvdec->channel);
+	if (!context->channel) {
+		pm_runtime_put(nvdec->dev);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static void nvdec_close_channel(struct tegra_drm_context *context)
+{
+	struct nvdec *nvdec = to_nvdec(context->client);
+
+	host1x_channel_put(context->channel);
+	pm_runtime_put(nvdec->dev);
+}
+
+static const struct tegra_drm_client_ops nvdec_ops = {
+	.open_channel = nvdec_open_channel,
+	.close_channel = nvdec_close_channel,
+	.submit = tegra_drm_submit,
+};
+
+#define NVIDIA_TEGRA_210_NVDEC_FIRMWARE "nvidia/tegra210/nvdec.bin"
+
+static const struct nvdec_config nvdec_t210_config = {
+	.firmware = NVIDIA_TEGRA_210_NVDEC_FIRMWARE,
+	.version = 0x21,
+	.supports_sid = false,
+	.num_instances = 1,
+};
+
+#define NVIDIA_TEGRA_186_NVDEC_FIRMWARE "nvidia/tegra186/nvdec.bin"
+
+static const struct nvdec_config nvdec_t186_config = {
+	.firmware = NVIDIA_TEGRA_186_NVDEC_FIRMWARE,
+	.version = 0x18,
+	.supports_sid = true,
+	.num_instances = 1,
+};
+
+#define NVIDIA_TEGRA_194_NVDEC_FIRMWARE "nvidia/tegra194/nvdec.bin"
+
+static const struct nvdec_config nvdec_t194_config = {
+	.firmware = NVIDIA_TEGRA_194_NVDEC_FIRMWARE,
+	.version = 0x19,
+	.supports_sid = true,
+	.num_instances = 2,
+};
+
+static const struct of_device_id tegra_nvdec_of_match[] = {
+	{ .compatible = "nvidia,tegra210-nvdec", .data = &nvdec_t210_config },
+	{ .compatible = "nvidia,tegra186-nvdec", .data = &nvdec_t186_config },
+	{ .compatible = "nvidia,tegra194-nvdec", .data = &nvdec_t194_config },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, tegra_nvdec_of_match);
+
+static int nvdec_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct host1x_syncpt **syncpts;
+	struct nvdec *nvdec;
+	u32 instance;
+	int err;
+
+	/* inherit DMA mask from host1x parent */
+	err = dma_coerce_mask_and_coherent(dev, *dev->parent->dma_mask);
+	if (err < 0) {
+		dev_err(&pdev->dev, "failed to set DMA mask: %d\n", err);
+		return err;
+	}
+
+	nvdec = devm_kzalloc(dev, sizeof(*nvdec), GFP_KERNEL);
+	if (!nvdec)
+		return -ENOMEM;
+
+	nvdec->config = of_device_get_match_data(dev);
+
+	syncpts = devm_kzalloc(dev, sizeof(*syncpts), GFP_KERNEL);
+	if (!syncpts)
+		return -ENOMEM;
+
+	nvdec->regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
+	if (IS_ERR(nvdec->regs))
+		return PTR_ERR(nvdec->regs);
+
+	nvdec->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(nvdec->clk)) {
+		dev_err(&pdev->dev, "failed to get clock\n");
+		return PTR_ERR(nvdec->clk);
+	}
+
+	err = of_property_read_u32(dev->of_node, "nvidia,instance", &instance);
+	if (err < 0)
+		instance = 0;
+
+	if (instance >= nvdec->config->num_instances)
+		return -EINVAL;
+
+	nvdec->falcon.dev = dev;
+	nvdec->falcon.regs = nvdec->regs;
+
+	err = falcon_init(&nvdec->falcon);
+	if (err < 0)
+		return err;
+
+	platform_set_drvdata(pdev, nvdec);
+
+	INIT_LIST_HEAD(&nvdec->client.base.list);
+	nvdec->client.base.ops = &nvdec_client_ops;
+	nvdec->client.base.dev = dev;
+	if (instance == 0)
+		nvdec->client.base.class = HOST1X_CLASS_NVDEC;
+	else
+		nvdec->client.base.class = HOST1X_CLASS_NVDEC1;
+	nvdec->client.base.syncpts = syncpts;
+	nvdec->client.base.num_syncpts = 1;
+	nvdec->dev = dev;
+
+	INIT_LIST_HEAD(&nvdec->client.list);
+	nvdec->client.version = nvdec->config->version;
+	nvdec->client.ops = &nvdec_ops;
+
+	err = host1x_client_register(&nvdec->client.base);
+	if (err < 0) {
+		dev_err(dev, "failed to register host1x client: %d\n", err);
+		goto exit_falcon;
+	}
+
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 500);
+	pm_runtime_use_autosuspend(&pdev->dev);
+
+	return 0;
+
+exit_falcon:
+	falcon_exit(&nvdec->falcon);
+
+	return err;
+}
+
+static int nvdec_remove(struct platform_device *pdev)
+{
+	struct nvdec *nvdec = platform_get_drvdata(pdev);
+	int err;
+
+	err = host1x_client_unregister(&nvdec->client.base);
+	if (err < 0) {
+		dev_err(&pdev->dev, "failed to unregister host1x client: %d\n",
+			err);
+		return err;
+	}
+
+	if (pm_runtime_enabled(&pdev->dev))
+		pm_runtime_disable(&pdev->dev);
+	else
+		nvdec_runtime_suspend(&pdev->dev);
+
+	falcon_exit(&nvdec->falcon);
+
+	return 0;
+}
+
+static const struct dev_pm_ops nvdec_pm_ops = {
+	SET_RUNTIME_PM_OPS(nvdec_runtime_suspend, nvdec_runtime_resume, NULL)
+};
+
+struct platform_driver tegra_nvdec_driver = {
+	.driver = {
+		.name = "tegra-nvdec",
+		.of_match_table = tegra_nvdec_of_match,
+		.pm = &nvdec_pm_ops
+	},
+	.probe = nvdec_probe,
+	.remove = nvdec_remove,
+};
+
+#if IS_ENABLED(CONFIG_ARCH_TEGRA_210_SOC)
+MODULE_FIRMWARE(NVIDIA_TEGRA_210_NVDEC_FIRMWARE);
+#endif
+#if IS_ENABLED(CONFIG_ARCH_TEGRA_186_SOC)
+MODULE_FIRMWARE(NVIDIA_TEGRA_186_NVDEC_FIRMWARE);
+#endif
+#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
+MODULE_FIRMWARE(NVIDIA_TEGRA_194_NVDEC_FIRMWARE);
+#endif
diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index fbb6447b8659..e2ddf3fcaa9a 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -132,6 +132,12 @@ static const struct host1x_sid_entry tegra186_sid_table[] = {
 		.offset = 0x30,
 		.limit = 0x34
 	},
+	{
+		/* NVDEC */
+		.base = 0x1b00,
+		.offset = 0x30,
+		.limit = 0x34
+	},
 };
 
 static const struct host1x_info host1x06_info = {
@@ -156,6 +162,18 @@ static const struct host1x_sid_entry tegra194_sid_table[] = {
 		.offset = 0x30,
 		.limit = 0x34
 	},
+	{
+		/* NVDEC */
+		.base = 0x1b00,
+		.offset = 0x30,
+		.limit = 0x34
+	},
+	{
+		/* NVDEC1 */
+		.base = 0x1bc0,
+		.offset = 0x30,
+		.limit = 0x34
+	},
 };
 
 static const struct host1x_info host1x07_info = {
diff --git a/include/linux/host1x.h b/include/linux/host1x.h
index 9b6784708f2e..d7d415bcf78b 100644
--- a/include/linux/host1x.h
+++ b/include/linux/host1x.h
@@ -15,6 +15,8 @@ enum host1x_class {
 	HOST1X_CLASS_GR2D_SB = 0x52,
 	HOST1X_CLASS_VIC = 0x5D,
 	HOST1X_CLASS_GR3D = 0x60,
+	HOST1X_CLASS_NVDEC = 0xF0,
+	HOST1X_CLASS_NVDEC1 = 0xF5,
 };
 
 struct host1x;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: Add YAML bindings for NVDEC
  2021-08-11 10:50 ` [PATCH v3 1/3] dt-bindings: Add YAML bindings for NVDEC Mikko Perttunen
@ 2021-08-17 21:20   ` Rob Herring
  2021-08-18  8:24     ` Mikko Perttunen
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2021-08-17 21:20 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: thierry.reding, jonathanh, airlied, daniel, dri-devel,
	linux-tegra, devicetree

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@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@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: 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.

> +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.

> +
> +additionalProperties: true

This should be false or 'unevaluatedProperties: false'

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: Add YAML bindings for NVDEC
  2021-08-17 21:20   ` Rob Herring
@ 2021-08-18  8:24     ` Mikko Perttunen
  2021-08-18 13:18       ` Thierry Reding
  0 siblings, 1 reply; 9+ messages in thread
From: Mikko Perttunen @ 2021-08-18  8:24 UTC (permalink / raw)
  To: Rob Herring, Mikko Perttunen
  Cc: thierry.reding, jonathanh, airlied, daniel, dri-devel,
	linux-tegra, devicetree

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@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@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: 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.

Thanks,
Mikko

> 
> Rob
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: Add YAML bindings for NVDEC
  2021-08-18  8:24     ` Mikko Perttunen
@ 2021-08-18 13:18       ` Thierry Reding
  2021-08-20 18:00           ` Rob Herring
  0 siblings, 1 reply; 9+ messages in thread
From: Thierry Reding @ 2021-08-18 13:18 UTC (permalink / raw)
  To: Mikko Perttunen, Rob Herring
  Cc: Mikko Perttunen, jonathanh, airlied, daniel, dri-devel,
	linux-tegra, devicetree

[-- Attachment #1: Type: text/plain, Size: 8312 bytes --]

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@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@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: 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:

--- >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@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.

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.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: Add YAML bindings for NVDEC
  2021-08-18 13:18       ` Thierry Reding
@ 2021-08-20 18:00           ` Rob Herring
  0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2021-08-20 18:00 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Mikko Perttunen, Mikko Perttunen, Jon Hunter, David Airlie,
	Daniel Vetter, dri-devel, linux-tegra, devicetree

On Wed, Aug 18, 2021 at 8:18 AM Thierry Reding <thierry.reding@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@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@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: 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@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

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: Add YAML bindings for NVDEC
@ 2021-08-20 18:00           ` Rob Herring
  0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2021-08-20 18:00 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Mikko Perttunen, Mikko Perttunen, Jon Hunter, David Airlie,
	Daniel Vetter, dri-devel, linux-tegra, devicetree

On Wed, Aug 18, 2021 at 8:18 AM Thierry Reding <thierry.reding@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@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@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: 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@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

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-08-20 18:00 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-11 10:50 [PATCH v3 0/3] NVIDIA Tegra NVDEC support Mikko Perttunen
2021-08-11 10:50 ` [PATCH v3 1/3] dt-bindings: Add YAML bindings for NVDEC Mikko Perttunen
2021-08-17 21:20   ` Rob Herring
2021-08-18  8:24     ` Mikko Perttunen
2021-08-18 13:18       ` Thierry Reding
2021-08-20 18:00         ` Rob Herring
2021-08-20 18:00           ` Rob Herring
2021-08-11 10:50 ` [PATCH v3 2/3] arm64: tegra: Add NVDEC to Tegra186/194 device trees Mikko Perttunen
2021-08-11 10:50 ` [PATCH v3 3/3] drm/tegra: Add NVDEC driver Mikko Perttunen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.