[PATCH 0/3] NVIDIA Tegra210 NVJPG support

[PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Diogo Ivo]

Hello,

This series adds support for the NVJPG hardware accelerator found in the
Tegra210 SoC.

The kernel driver is essentially a copy of the NVDEC driver as both
engines are Falcon-based.

For the userspace part I have written a Mesa Gallium backend [1] that,
while still very much experimental, works in decoding images with VA-API.

I have been using ffmpeg to call VA-API with the following command:

ffmpeg -v verbose -hwaccel vaapi -hwaccel_device /dev/dri/renderD129 -i <input.jpg> -pix_fmt bgra -f fbdev /dev/fb0

which decodes <input.jpg> and shows the result in the framebuffer.

The firmware for the engine can be obtained from a Linux for Tegra
distribution. Due to the way the Gallium implementation works for Tegra
the GPU also needs to be enabled.

Thanks!

Diogo

To: Thierry Reding <thierry.reding@gmail.com>
To: Mikko Perttunen <mperttunen@nvidia.com>
To: David Airlie <airlied@gmail.com>
To: Simona Vetter <simona@ffwll.ch>
To: Jonathan Hunter <jonathanh@nvidia.com>
To: Philipp Zabel <p.zabel@pengutronix.de>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-tegra@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>

[1]: https://gitlab.freedesktop.org/d.ivo/mesa/-/tree/diogo/vaapi_gl?ref_type=heads

---
Diogo Ivo (3):
      drm/tegra: Add NVJPG driver
      arm64: tegra: Add NVJPG power-domain node
      arm64: tegra: Add NVJPG node

 arch/arm64/boot/dts/nvidia/tegra210.dtsi |  14 +-
 drivers/gpu/drm/tegra/Makefile           |   1 +
 drivers/gpu/drm/tegra/drm.c              |   2 +
 drivers/gpu/drm/tegra/drm.h              |   1 +
 drivers/gpu/drm/tegra/nvjpg.c            | 379 +++++++++++++++++++++++++++++++
 5 files changed, 396 insertions(+), 1 deletion(-)
---
base-commit: 386b76a190ce68afc19d481f17cab1f5479c719b
change-id: 20250605-diogo-nvjpg-e0d4c57126c5

Best regards,
-- 
Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>

[PATCH 1/3] drm/tegra: Add NVJPG driver

[Diogo Ivo]

Add support for booting and using NVJPG on Tegra210 to the Host1x
and TegraDRM drivers. This driver only supports the new TegraDRM uAPI.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
 drivers/gpu/drm/tegra/Makefile |   1 +
 drivers/gpu/drm/tegra/drm.c    |   2 +
 drivers/gpu/drm/tegra/drm.h    |   1 +
 drivers/gpu/drm/tegra/nvjpg.c  | 379 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 383 insertions(+)

diff --git a/drivers/gpu/drm/tegra/Makefile b/drivers/gpu/drm/tegra/Makefile
index 6fc4b504e7861df8802f3f0f5d6faf9eb167b27a..e399b40d64a1d2070b27c2219a36693a8e3edc61 100644
--- a/drivers/gpu/drm/tegra/Makefile
+++ b/drivers/gpu/drm/tegra/Makefile
@@ -25,6 +25,7 @@ tegra-drm-y := \
 	falcon.o \
 	vic.o \
 	nvdec.o \
+	nvjpg.o \
 	riscv.o
 
 tegra-drm-y += trace.o
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 4596073fe28fa1beb6689da305775f4468714548..f44fb145920ae77dda82780a659a98c88ea25a5a 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1383,6 +1383,7 @@ static const struct of_device_id host1x_drm_subdevs[] = {
 	{ .compatible = "nvidia,tegra210-sor1", },
 	{ .compatible = "nvidia,tegra210-vic", },
 	{ .compatible = "nvidia,tegra210-nvdec", },
+	{ .compatible = "nvidia,tegra210-nvjpg", },
 	{ .compatible = "nvidia,tegra186-display", },
 	{ .compatible = "nvidia,tegra186-dc", },
 	{ .compatible = "nvidia,tegra186-sor", },
@@ -1421,6 +1422,7 @@ static struct platform_driver * const drivers[] = {
 	&tegra_gr3d_driver,
 	&tegra_vic_driver,
 	&tegra_nvdec_driver,
+	&tegra_nvjpg_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 0b65e69f3a8add906e48b471804ad45bb3241455..64c8577720245564c421b85057a02419e235a6b6 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -212,5 +212,6 @@ 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;
+extern struct platform_driver tegra_nvjpg_driver;
 
 #endif /* HOST1X_DRM_H */
diff --git a/drivers/gpu/drm/tegra/nvjpg.c b/drivers/gpu/drm/tegra/nvjpg.c
new file mode 100644
index 0000000000000000000000000000000000000000..57aa74d339d9cea08746aa91b5c8d846962988ee
--- /dev/null
+++ b/drivers/gpu/drm/tegra/nvjpg.c
@@ -0,0 +1,379 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/host1x.h>
+#include <linux/iommu.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+
+#include "drm.h"
+#include "falcon.h"
+
+struct nvjpg_config {
+	const char *firmware;
+	unsigned int version;
+};
+
+struct nvjpg {
+	struct falcon falcon;
+
+	void __iomem *regs;
+	struct tegra_drm_client client;
+	struct device *dev;
+	struct clk *clk;
+	struct reset_control *rst;
+
+	/* Platform configuration */
+	const struct nvjpg_config *config;
+};
+
+static inline struct nvjpg *to_nvjpg(struct tegra_drm_client *client)
+{
+	return container_of(client, struct nvjpg, client);
+}
+
+static int nvjpg_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 nvjpg *nvjpg = to_nvjpg(drm);
+	int err;
+
+	err = host1x_client_iommu_attach(client);
+	if (err < 0 && err != -ENODEV) {
+		dev_err(nvjpg->dev, "failed to attach to domain: %d\n", err);
+		return err;
+	}
+
+	err = tegra_drm_register_client(tegra, drm);
+	if (err < 0)
+		goto detach;
+
+	/*
+	 * Inherit the DMA parameters (such as maximum segment size) from the
+	 * parent host1x device.
+	 */
+	client->dev->dma_parms = client->host->dma_parms;
+
+	return 0;
+
+detach:
+	host1x_client_iommu_detach(client);
+
+	return err;
+}
+
+static int nvjpg_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 nvjpg *nvjpg = to_nvjpg(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;
+
+	pm_runtime_dont_use_autosuspend(client->dev);
+	pm_runtime_force_suspend(client->dev);
+
+	host1x_client_iommu_detach(client);
+
+	if (client->group) {
+		dma_unmap_single(nvjpg->dev, nvjpg->falcon.firmware.phys,
+				 nvjpg->falcon.firmware.size, DMA_TO_DEVICE);
+		tegra_drm_free(tegra, nvjpg->falcon.firmware.size,
+			       nvjpg->falcon.firmware.virt,
+			       nvjpg->falcon.firmware.iova);
+	} else {
+		dma_free_coherent(nvjpg->dev, nvjpg->falcon.firmware.size,
+				  nvjpg->falcon.firmware.virt,
+				  nvjpg->falcon.firmware.iova);
+	}
+
+	return 0;
+}
+
+static const struct host1x_client_ops nvjpg_client_ops = {
+	.init = nvjpg_init,
+	.exit = nvjpg_exit,
+};
+
+static int nvjpg_load_falcon_firmware(struct nvjpg *nvjpg)
+{
+	struct host1x_client *client = &nvjpg->client.base;
+	struct tegra_drm *tegra = nvjpg->client.drm;
+	dma_addr_t iova;
+	size_t size;
+	void *virt;
+	int err;
+
+	if (nvjpg->falcon.firmware.virt)
+		return 0;
+
+	err = falcon_read_firmware(&nvjpg->falcon, nvjpg->config->firmware);
+	if (err < 0)
+		return err;
+
+	size = nvjpg->falcon.firmware.size;
+
+	if (!client->group) {
+		virt = dma_alloc_coherent(nvjpg->dev, size, &iova, GFP_KERNEL);
+
+		err = dma_mapping_error(nvjpg->dev, iova);
+		if (err < 0)
+			return err;
+	} else {
+		virt = tegra_drm_alloc(tegra, size, &iova);
+		if (IS_ERR(virt))
+			return PTR_ERR(virt);
+	}
+
+	nvjpg->falcon.firmware.virt = virt;
+	nvjpg->falcon.firmware.iova = iova;
+
+	err = falcon_load_firmware(&nvjpg->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(nvjpg->dev, virt, size, DMA_TO_DEVICE);
+
+		err = dma_mapping_error(nvjpg->dev, phys);
+		if (err < 0)
+			goto cleanup;
+
+		nvjpg->falcon.firmware.phys = phys;
+	}
+
+	return 0;
+
+cleanup:
+	if (!client->group)
+		dma_free_coherent(nvjpg->dev, size, virt, iova);
+	else
+		tegra_drm_free(tegra, size, virt, iova);
+
+	return err;
+}
+
+static __maybe_unused int nvjpg_runtime_resume(struct device *dev)
+{
+	struct nvjpg *nvjpg = dev_get_drvdata(dev);
+	int err;
+
+	err = clk_prepare_enable(nvjpg->clk);
+	if (err < 0)
+		return err;
+
+	usleep_range(20, 30);
+
+	if (nvjpg->rst) {
+		err = reset_control_acquire(nvjpg->rst);
+		if (err < 0) {
+			dev_err(dev, "failed to acquire reset: %d\n", err);
+			goto disable_clk;
+		}
+
+		err = reset_control_deassert(nvjpg->rst);
+		if (err < 0) {
+			dev_err(dev, "failed to deassert reset: %d\n", err);
+			goto release_reset;
+		}
+
+		usleep_range(20, 30);
+	}
+
+	err = nvjpg_load_falcon_firmware(nvjpg);
+	if (err < 0)
+		goto assert_reset;
+
+	err = falcon_boot(&nvjpg->falcon);
+	if (err < 0)
+		goto assert_reset;
+
+	return 0;
+
+assert_reset:
+	reset_control_assert(nvjpg->rst);
+release_reset:
+	reset_control_release(nvjpg->rst);
+disable_clk:
+	clk_disable_unprepare(nvjpg->clk);
+	return err;
+}
+
+static __maybe_unused int nvjpg_runtime_suspend(struct device *dev)
+{
+	struct nvjpg *nvjpg = dev_get_drvdata(dev);
+	int err;
+
+	if (nvjpg->rst) {
+		err = reset_control_assert(nvjpg->rst);
+		if (err < 0) {
+			dev_err(dev, "failed to assert reset: %d\n", err);
+			return err;
+		}
+
+		usleep_range(20, 30);
+		reset_control_release(nvjpg->rst);
+	}
+
+	clk_disable_unprepare(nvjpg->clk);
+
+	return 0;
+}
+
+static int nvjpg_can_use_memory_ctx(struct tegra_drm_client *client, bool *supported)
+{
+	*supported = false;
+
+	return 0;
+}
+
+static const struct tegra_drm_client_ops nvjpg_ops = {
+	.get_streamid_offset = NULL,
+	.can_use_memory_ctx = nvjpg_can_use_memory_ctx,
+};
+#define NVIDIA_TEGRA_210_NVJPG_FIRMWARE "nvidia/tegra210/nvjpg.bin"
+
+static const struct nvjpg_config nvjpg_t210_config = {
+	.firmware = NVIDIA_TEGRA_210_NVJPG_FIRMWARE,
+	.version = 0x21,
+};
+
+static const struct of_device_id tegra_nvjpg_of_match[] = {
+	{ .compatible = "nvidia,tegra210-nvjpg", .data = &nvjpg_t210_config },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, tegra_nvjpg_of_match);
+
+static int nvjpg_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct nvjpg *nvjpg;
+	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;
+	}
+
+	nvjpg = devm_kzalloc(dev, sizeof(*nvjpg), GFP_KERNEL);
+	if (!nvjpg)
+		return -ENOMEM;
+
+	nvjpg->config = of_device_get_match_data(dev);
+
+	nvjpg->regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
+	if (IS_ERR(nvjpg->regs))
+		return PTR_ERR(nvjpg->regs);
+
+	nvjpg->rst = devm_reset_control_get_exclusive_released(&pdev->dev, "nvjpg");
+	if (IS_ERR(nvjpg->rst)) {
+		err = PTR_ERR(nvjpg->rst);
+
+		if (err != -EBUSY || WARN_ON(!pdev->dev.pm_domain)) {
+			dev_err(&pdev->dev, "failed to get reset control: %d\n",
+				err);
+			return err;
+		}
+
+		/*
+		 * At this point, the reset control is most likely being used
+		 * by the generic power domain implementation. With any luck
+		 * the power domain will have taken care of resetting the SOR
+		 * and we don't have to do anything.
+		 */
+		nvjpg->rst = NULL;
+	}
+
+	nvjpg->clk = devm_clk_get(dev, "nvjpg");
+	if (IS_ERR(nvjpg->clk)) {
+		dev_err(&pdev->dev, "failed to get clock\n");
+		return PTR_ERR(nvjpg->clk);
+	}
+
+	nvjpg->falcon.dev = dev;
+	nvjpg->falcon.regs = nvjpg->regs;
+
+	err = falcon_init(&nvjpg->falcon);
+	if (err < 0)
+		return err;
+
+	platform_set_drvdata(pdev, nvjpg);
+
+	INIT_LIST_HEAD(&nvjpg->client.base.list);
+	nvjpg->client.base.ops = &nvjpg_client_ops;
+	nvjpg->client.base.dev = dev;
+	nvjpg->client.base.class = HOST1X_CLASS_NVJPG;
+	nvjpg->dev = dev;
+
+	INIT_LIST_HEAD(&nvjpg->client.list);
+	nvjpg->client.version = nvjpg->config->version;
+	nvjpg->client.ops = &nvjpg_ops;
+
+	err = host1x_client_register(&nvjpg->client.base);
+	if (err < 0) {
+		dev_err(dev, "failed to register host1x client: %d\n", err);
+		goto exit_falcon;
+	}
+
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_set_autosuspend_delay(dev, 500);
+	devm_pm_runtime_enable(dev);
+
+	return 0;
+
+exit_falcon:
+	falcon_exit(&nvjpg->falcon);
+
+	return err;
+}
+
+static void nvjpg_remove(struct platform_device *pdev)
+{
+	struct nvjpg *nvjpg = platform_get_drvdata(pdev);
+
+	host1x_client_unregister(&nvjpg->client.base);
+	falcon_exit(&nvjpg->falcon);
+}
+
+static const struct dev_pm_ops nvjpg_pm_ops = {
+	SET_RUNTIME_PM_OPS(nvjpg_runtime_suspend, nvjpg_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+};
+
+struct platform_driver tegra_nvjpg_driver = {
+	.driver = {
+		.name = "tegra-nvjpg",
+		.of_match_table = tegra_nvjpg_of_match,
+		.pm = &nvjpg_pm_ops
+	},
+	.probe = nvjpg_probe,
+	.remove = nvjpg_remove,
+};
+
+#if IS_ENABLED(CONFIG_ARCH_TEGRA_210_SOC)
+MODULE_FIRMWARE(NVIDIA_TEGRA_210_NVJPG_FIRMWARE);
+#endif

-- 
2.49.0

[PATCH 2/3] arm64: tegra: Add NVJPG power-domain node

[Diogo Ivo]

Add the NVJPG power-domain node in order to support the NVJPG
accelerator.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 402b0ede1472af625d9d9e811f5af306d436cc98..6f8cdf012f0f12a16716e9d479c46b330bbb7dda 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -947,6 +947,12 @@ pd_xusbhost: xusbc {
 				resets = <&tegra_car TEGRA210_CLK_XUSB_HOST>;
 				#power-domain-cells = <0>;
 			};
+
+			pd_nvjpg: nvjpg {
+				clocks = <&tegra_car TEGRA210_CLK_NVJPG>;
+				resets = <&tegra_car 195>;
+				#power-domain-cells = <0>;
+			};
 		};
 	};
 

-- 
2.49.0

[PATCH 3/3] arm64: tegra: Add NVJPG node

[Diogo Ivo]

The Tegra X1 chip contains a NVJPG accelerator capable of
encoding/decoding JPEG files in hardware, so add its DT node.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 6f8cdf012f0f12a16716e9d479c46b330bbb7dda..087f38256fd40f57c4685e907f9682eb49ee31db 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -253,7 +253,13 @@ vic@54340000 {
 		nvjpg@54380000 {
 			compatible = "nvidia,tegra210-nvjpg";
 			reg = <0x0 0x54380000 0x0 0x00040000>;
-			status = "disabled";
+			clocks = <&tegra_car TEGRA210_CLK_NVJPG>;
+			clock-names = "nvjpg";
+			resets = <&tegra_car 195>;
+			reset-names = "nvjpg";
+
+			iommus = <&mc TEGRA_SWGROUP_NVJPG>;
+			power-domains = <&pd_nvjpg>;
 		};
 
 		dsib: dsi@54400000 {

-- 
2.49.0

Re: [PATCH 1/3] drm/tegra: Add NVJPG driver

[Mikko Perttunen]

On 6/6/25 7:45 PM, Diogo Ivo wrote:
> Add support for booting and using NVJPG on Tegra210 to the Host1x
> and TegraDRM drivers. This driver only supports the new TegraDRM uAPI.

Hello Diogo -- I'm happy to see this driver!

> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
>   drivers/gpu/drm/tegra/Makefile |   1 +
>   drivers/gpu/drm/tegra/drm.c    |   2 +
>   drivers/gpu/drm/tegra/drm.h    |   1 +
>   drivers/gpu/drm/tegra/nvjpg.c  | 379 +++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 383 insertions(+)
> ...
> +
> +static __maybe_unused int nvjpg_runtime_resume(struct device *dev)
> +{
> +	struct nvjpg *nvjpg = dev_get_drvdata(dev);
> +	int err;
> +
> +	err = clk_prepare_enable(nvjpg->clk);
> +	if (err < 0)
> +		return err;
> +
> +	usleep_range(20, 30);
> +
> +	if (nvjpg->rst) {
> +		err = reset_control_acquire(nvjpg->rst);
> +		if (err < 0) {
> +			dev_err(dev, "failed to acquire reset: %d\n", err);
> +			goto disable_clk;
> +		}
> +
> +		err = reset_control_deassert(nvjpg->rst);
> +		if (err < 0) {
> +			dev_err(dev, "failed to deassert reset: %d\n", err);
> +			goto release_reset;
> +		}
> +
> +		usleep_range(20, 30);
> +	}

Do we need this manual reset handling? NVJPG is only on T210+ where the 
power domain code handles the reset as well. Did you run into any issues 
with it?

(As a note, the reset_control_* functions are no-ops on a NULL reset. So 
the 'if' here is unnecessary.)

> +
> +	err = nvjpg_load_falcon_firmware(nvjpg);
> +	if (err < 0)
> +		goto assert_reset;
> +
> +	err = falcon_boot(&nvjpg->falcon);
> +	if (err < 0)
> +		goto assert_reset;
> +
> +	return 0;
> +
> +assert_reset:
> +	reset_control_assert(nvjpg->rst);
> +release_reset:
> +	reset_control_release(nvjpg->rst);
> +disable_clk:
> +	clk_disable_unprepare(nvjpg->clk);
> +	return err;
> +}
> ...
> +
> +static int nvjpg_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct nvjpg *nvjpg;
> +	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;
> +	}
> +
> +	nvjpg = devm_kzalloc(dev, sizeof(*nvjpg), GFP_KERNEL);
> +	if (!nvjpg)
> +		return -ENOMEM;
> +
> +	nvjpg->config = of_device_get_match_data(dev);
> +
> +	nvjpg->regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);

This can be devm_platform_ioremap_resource -- slightly simpler.

> +	if (IS_ERR(nvjpg->regs))
> +		return PTR_ERR(nvjpg->regs);
> +
> +	nvjpg->rst = devm_reset_control_get_exclusive_released(&pdev->dev, "nvjpg");
> +	if (IS_ERR(nvjpg->rst)) {
> +		err = PTR_ERR(nvjpg->rst);
> +
> +		if (err != -EBUSY || WARN_ON(!pdev->dev.pm_domain)) {
> +			dev_err(&pdev->dev, "failed to get reset control: %d\n",
> +				err);
> +			return err;
> +		}
> +
> +		/*
> +		 * At this point, the reset control is most likely being used
> +		 * by the generic power domain implementation. With any luck
> +		 * the power domain will have taken care of resetting the SOR
> +		 * and we don't have to do anything.
> +		 */
> +		nvjpg->rst = NULL;
> +	}

I see you've taken this from sor.c, but I think it should be 
unnecessary. I imagine the code in sor.c is overcomplicated as well, 
maybe because we used not to have the power domain implementation.

> +
> +	nvjpg->clk = devm_clk_get(dev, "nvjpg");
> +	if (IS_ERR(nvjpg->clk)) {
> +		dev_err(&pdev->dev, "failed to get clock\n");
> +		return PTR_ERR(nvjpg->clk);
> +	}

Probably a good idea to set the clock rate to max (see vic.c).

> +
> +	nvjpg->falcon.dev = dev;
> +	nvjpg->falcon.regs = nvjpg->regs;
> +
> +	err = falcon_init(&nvjpg->falcon);
> +	if (err < 0)
> +		return err;
> +
> +	platform_set_drvdata(pdev, nvjpg);
> +
> +	INIT_LIST_HEAD(&nvjpg->client.base.list);
> +	nvjpg->client.base.ops = &nvjpg_client_ops;
> +	nvjpg->client.base.dev = dev;
> +	nvjpg->client.base.class = HOST1X_CLASS_NVJPG;
> +	nvjpg->dev = dev;
> +
> +	INIT_LIST_HEAD(&nvjpg->client.list);
> +	nvjpg->client.version = nvjpg->config->version;
> +	nvjpg->client.ops = &nvjpg_ops;
> +
> +	err = host1x_client_register(&nvjpg->client.base);
> +	if (err < 0) {
> +		dev_err(dev, "failed to register host1x client: %d\n", err);
> +		goto exit_falcon;
> +	}
> +
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_set_autosuspend_delay(dev, 500);
> +	devm_pm_runtime_enable(dev);
> +
> +	return 0;
> +
> +exit_falcon:
> +	falcon_exit(&nvjpg->falcon);
> +
> +	return err;
> +}
> +
> +static void nvjpg_remove(struct platform_device *pdev)
> +{
> +	struct nvjpg *nvjpg = platform_get_drvdata(pdev);
> +
> +	host1x_client_unregister(&nvjpg->client.base);
> +	falcon_exit(&nvjpg->falcon);
> +}
> +
> +static const struct dev_pm_ops nvjpg_pm_ops = {
> +	SET_RUNTIME_PM_OPS(nvjpg_runtime_suspend, nvjpg_runtime_resume, NULL)
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				pm_runtime_force_resume)
> +};

There are modern, improved variants with no SET_ prefix.

Thanks,
Mikko

> +
> +struct platform_driver tegra_nvjpg_driver = {
> +	.driver = {
> +		.name = "tegra-nvjpg",
> +		.of_match_table = tegra_nvjpg_of_match,
> +		.pm = &nvjpg_pm_ops
> +	},
> +	.probe = nvjpg_probe,
> +	.remove = nvjpg_remove,
> +};
> +
> +#if IS_ENABLED(CONFIG_ARCH_TEGRA_210_SOC)
> +MODULE_FIRMWARE(NVIDIA_TEGRA_210_NVJPG_FIRMWARE);
> +#endif
> 

Re: [PATCH 2/3] arm64: tegra: Add NVJPG power-domain node

[Mikko Perttunen]

On 6/6/25 7:45 PM, Diogo Ivo wrote:
> Add the NVJPG power-domain node in order to support the NVJPG
> accelerator.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
>   arch/arm64/boot/dts/nvidia/tegra210.dtsi | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> index 402b0ede1472af625d9d9e811f5af306d436cc98..6f8cdf012f0f12a16716e9d479c46b330bbb7dda 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> @@ -947,6 +947,12 @@ pd_xusbhost: xusbc {
>   				resets = <&tegra_car TEGRA210_CLK_XUSB_HOST>;
>   				#power-domain-cells = <0>;
>   			};
> +
> +			pd_nvjpg: nvjpg {
> +				clocks = <&tegra_car TEGRA210_CLK_NVJPG>;
> +				resets = <&tegra_car 195>;
> +				#power-domain-cells = <0>;
> +			};
>   		};
>   	};
>   
> 

Please mention Tegra210 in the commit subject. Otherwise,

Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>

Re: [PATCH 3/3] arm64: tegra: Add NVJPG node

[Mikko Perttunen]

On 6/6/25 7:45 PM, Diogo Ivo wrote:
> The Tegra X1 chip contains a NVJPG accelerator capable of
> encoding/decoding JPEG files in hardware, so add its DT node.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
>   arch/arm64/boot/dts/nvidia/tegra210.dtsi | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> index 6f8cdf012f0f12a16716e9d479c46b330bbb7dda..087f38256fd40f57c4685e907f9682eb49ee31db 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> @@ -253,7 +253,13 @@ vic@54340000 {
>   		nvjpg@54380000 {
>   			compatible = "nvidia,tegra210-nvjpg";
>   			reg = <0x0 0x54380000 0x0 0x00040000>;
> -			status = "disabled";
> +			clocks = <&tegra_car TEGRA210_CLK_NVJPG>;
> +			clock-names = "nvjpg";
> +			resets = <&tegra_car 195>;
> +			reset-names = "nvjpg";
> +
> +			iommus = <&mc TEGRA_SWGROUP_NVJPG>;
> +			power-domains = <&pd_nvjpg>;
>   		};
>   
>   		dsib: dsi@54400000 {
> 

Please mention Tegra210 in the commit subject, and perhaps adjust the 
commit message to say that you're updating and enabling the device tree 
node (rather than adding, since it's already there).

With that,

Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>

Re: [PATCH 1/3] drm/tegra: Add NVJPG driver

[Thierry Reding]

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

On Tue, Jun 10, 2025 at 12:26:07PM +0900, Mikko Perttunen wrote:
> On 6/6/25 7:45 PM, Diogo Ivo wrote:
> > Add support for booting and using NVJPG on Tegra210 to the Host1x
> > and TegraDRM drivers. This driver only supports the new TegraDRM uAPI.
> 
> Hello Diogo -- I'm happy to see this driver!

So am I, nice work!

[...]
> > +	if (IS_ERR(nvjpg->regs))
> > +		return PTR_ERR(nvjpg->regs);
> > +
> > +	nvjpg->rst = devm_reset_control_get_exclusive_released(&pdev->dev, "nvjpg");
> > +	if (IS_ERR(nvjpg->rst)) {
> > +		err = PTR_ERR(nvjpg->rst);
> > +
> > +		if (err != -EBUSY || WARN_ON(!pdev->dev.pm_domain)) {
> > +			dev_err(&pdev->dev, "failed to get reset control: %d\n",
> > +				err);
> > +			return err;
> > +		}
> > +
> > +		/*
> > +		 * At this point, the reset control is most likely being used
> > +		 * by the generic power domain implementation. With any luck
> > +		 * the power domain will have taken care of resetting the SOR
> > +		 * and we don't have to do anything.
> > +		 */
> > +		nvjpg->rst = NULL;
> > +	}
> 
> I see you've taken this from sor.c, but I think it should be unnecessary. I
> imagine the code in sor.c is overcomplicated as well, maybe because we used
> not to have the power domain implementation.

Agreed. SOR is also slightly older than NVJPG and used on Tegra124 where
we don't use power domains, so most of these quirks are for backwards-
compatibility. If we can avoid them for NVJPG, that'd be great.

Thierry

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

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Thierry Reding]

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

On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
> Hello,
> 
> This series adds support for the NVJPG hardware accelerator found in the
> Tegra210 SoC.
> 
> The kernel driver is essentially a copy of the NVDEC driver as both
> engines are Falcon-based.
> 
> For the userspace part I have written a Mesa Gallium backend [1] that,
> while still very much experimental, works in decoding images with VA-API.

Nice. It's good to see that there's some use in this after all. I
haven't taken an in-depth look yet, but from a high-level point of view
this looks like what I had imagined back when I started out with this
driver.

This made my day, thank you!

Thierry

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

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Thierry Reding]

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

On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
> Hello,
> 
> This series adds support for the NVJPG hardware accelerator found in the
> Tegra210 SoC.
> 
> The kernel driver is essentially a copy of the NVDEC driver as both
> engines are Falcon-based.
> 
> For the userspace part I have written a Mesa Gallium backend [1] that,
> while still very much experimental, works in decoding images with VA-API.
> 
> I have been using ffmpeg to call VA-API with the following command:
> 
> ffmpeg -v verbose -hwaccel vaapi -hwaccel_device /dev/dri/renderD129 -i <input.jpg> -pix_fmt bgra -f fbdev /dev/fb0
> 
> which decodes <input.jpg> and shows the result in the framebuffer.
> 
> The firmware for the engine can be obtained from a Linux for Tegra
> distribution.

By the way, have you tried running this on anything newer than Tegra210?
Given your progress on this, we can probably start thinking about
submitting the binaries to linux-firmware.

> Due to the way the Gallium implementation works for Tegra
> the GPU also needs to be enabled.

I wonder if maybe we can get rid of this requirement. While it's
certainly nice to have the GPU enabled, there may be cases where using
only the other engines may be advantageous. Originally when I had worked
on VIC, I was looking at how it could be used for compositing without
getting the GPU involved. That's something that Android devices tend(ed)
to do because of the power savings that come with it.

Anyway, not a big deal, depending on the GPU for now is fine.

Thierry

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

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Mikko Perttunen]

On 6/10/25 6:05 PM, Thierry Reding wrote:
> On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
>> Hello,
>>
>> This series adds support for the NVJPG hardware accelerator found in the
>> Tegra210 SoC.
>>
>> The kernel driver is essentially a copy of the NVDEC driver as both
>> engines are Falcon-based.
>>
>> For the userspace part I have written a Mesa Gallium backend [1] that,
>> while still very much experimental, works in decoding images with VA-API.
>>
>> I have been using ffmpeg to call VA-API with the following command:
>>
>> ffmpeg -v verbose -hwaccel vaapi -hwaccel_device /dev/dri/renderD129 -i <input.jpg> -pix_fmt bgra -f fbdev /dev/fb0
>>
>> which decodes <input.jpg> and shows the result in the framebuffer.
>>
>> The firmware for the engine can be obtained from a Linux for Tegra
>> distribution.
> 
> By the way, have you tried running this on anything newer than Tegra210?
> Given your progress on this, we can probably start thinking about
> submitting the binaries to linux-firmware.

FWIW, the impression I have is that NVJPG is basically unchanged all the 
way to Tegra234. So if we add stream ID support and the firmwares, it'll 
probably just work. Tegra234 has the quirk that it has two instances of 
NVJPG -- these have to be distinguished by their different class IDs. 
But we should go ahead with the T210 support first.

> 
>> Due to the way the Gallium implementation works for Tegra
>> the GPU also needs to be enabled.
> 
> I wonder if maybe we can get rid of this requirement. While it's
> certainly nice to have the GPU enabled, there may be cases where using
> only the other engines may be advantageous. Originally when I had worked
> on VIC, I was looking at how it could be used for compositing without
> getting the GPU involved. That's something that Android devices tend(ed)
> to do because of the power savings that come with it.
> 
> Anyway, not a big deal, depending on the GPU for now is fine.
> 
> Thierry

Re: [PATCH 1/3] drm/tegra: Add NVJPG driver

[Diogo Ivo]

Hi Mikko, thanks for the quick review!

On 6/10/25 4:26 AM, Mikko Perttunen wrote:
> On 6/6/25 7:45 PM, Diogo Ivo wrote:
>> Add support for booting and using NVJPG on Tegra210 to the Host1x
>> and TegraDRM drivers. This driver only supports the new TegraDRM uAPI.
> 
> Hello Diogo -- I'm happy to see this driver!
> 
>>
>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>> ---
>>   drivers/gpu/drm/tegra/Makefile |   1 +
>>   drivers/gpu/drm/tegra/drm.c    |   2 +
>>   drivers/gpu/drm/tegra/drm.h    |   1 +
>>   drivers/gpu/drm/tegra/nvjpg.c  | 379 +++++++++++++++++++++++++++++++ 
>> ++++++++++
>>   4 files changed, 383 insertions(+)
>> ...
>> +
>> +static __maybe_unused int nvjpg_runtime_resume(struct device *dev)
>> +{
>> +    struct nvjpg *nvjpg = dev_get_drvdata(dev);
>> +    int err;
>> +
>> +    err = clk_prepare_enable(nvjpg->clk);
>> +    if (err < 0)
>> +        return err;
>> +
>> +    usleep_range(20, 30);
>> +
>> +    if (nvjpg->rst) {
>> +        err = reset_control_acquire(nvjpg->rst);
>> +        if (err < 0) {
>> +            dev_err(dev, "failed to acquire reset: %d\n", err);
>> +            goto disable_clk;
>> +        }
>> +
>> +        err = reset_control_deassert(nvjpg->rst);
>> +        if (err < 0) {
>> +            dev_err(dev, "failed to deassert reset: %d\n", err);
>> +            goto release_reset;
>> +        }
>> +
>> +        usleep_range(20, 30);
>> +    }
> 
> Do we need this manual reset handling? NVJPG is only on T210+ where the 
> power domain code handles the reset as well. Did you run into any issues 
> with it?

Everything works just fine without the manual reset handling. I was
unsure if I should add it to the driver and in the end I decided to do
it because I thought that it would be better if the driver had explicit
control over it. However if delegating this to the power domain code is
enough I'll delete it for v2.

>> +
>> +    err = nvjpg_load_falcon_firmware(nvjpg);
>> +    if (err < 0)
>> +        goto assert_reset;
>> +
>> +    err = falcon_boot(&nvjpg->falcon);
>> +    if (err < 0)
>> +        goto assert_reset;
>> +
>> +    return 0;
>> +
>> +assert_reset:
>> +    reset_control_assert(nvjpg->rst);
>> +release_reset:
>> +    reset_control_release(nvjpg->rst);
>> +disable_clk:
>> +    clk_disable_unprepare(nvjpg->clk);
>> +    return err;
>> +}
>> ...
>> +
>> +static int nvjpg_probe(struct platform_device *pdev)
>> +{
>> +    struct device *dev = &pdev->dev;
>> +    struct nvjpg *nvjpg;
>> +    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;
>> +    }
>> +
>> +    nvjpg = devm_kzalloc(dev, sizeof(*nvjpg), GFP_KERNEL);
>> +    if (!nvjpg)
>> +        return -ENOMEM;
>> +
>> +    nvjpg->config = of_device_get_match_data(dev);
>> +
>> +    nvjpg->regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> 
> This can be devm_platform_ioremap_resource -- slightly simpler.

Thanks, I'll update that for v2.

>> +    if (IS_ERR(nvjpg->regs))
>> +        return PTR_ERR(nvjpg->regs);
>> +
>> +    nvjpg->rst = devm_reset_control_get_exclusive_released(&pdev- 
>> >dev, "nvjpg");
>> +    if (IS_ERR(nvjpg->rst)) {
>> +        err = PTR_ERR(nvjpg->rst);
>> +
>> +        if (err != -EBUSY || WARN_ON(!pdev->dev.pm_domain)) {
>> +            dev_err(&pdev->dev, "failed to get reset control: %d\n",
>> +                err);
>> +            return err;
>> +        }
>> +
>> +        /*
>> +         * At this point, the reset control is most likely being used
>> +         * by the generic power domain implementation. With any luck
>> +         * the power domain will have taken care of resetting the SOR
>> +         * and we don't have to do anything.
>> +         */
>> +        nvjpg->rst = NULL;
>> +    }
> 
> I see you've taken this from sor.c, but I think it should be 
> unnecessary. I imagine the code in sor.c is overcomplicated as well, 
> maybe because we used not to have the power domain implementation.

Following my comment above I'll delete this block for v2.

>> +
>> +    nvjpg->clk = devm_clk_get(dev, "nvjpg");
>> +    if (IS_ERR(nvjpg->clk)) {
>> +        dev_err(&pdev->dev, "failed to get clock\n");
>> +        return PTR_ERR(nvjpg->clk);
>> +    }
> 
> Probably a good idea to set the clock rate to max (see vic.c).

Sounds good, I'll update it for v2.

>> +
>> +    nvjpg->falcon.dev = dev;
>> +    nvjpg->falcon.regs = nvjpg->regs;
>> +
>> +    err = falcon_init(&nvjpg->falcon);
>> +    if (err < 0)
>> +        return err;
>> +
>> +    platform_set_drvdata(pdev, nvjpg);
>> +
>> +    INIT_LIST_HEAD(&nvjpg->client.base.list);
>> +    nvjpg->client.base.ops = &nvjpg_client_ops;
>> +    nvjpg->client.base.dev = dev;
>> +    nvjpg->client.base.class = HOST1X_CLASS_NVJPG;
>> +    nvjpg->dev = dev;
>> +
>> +    INIT_LIST_HEAD(&nvjpg->client.list);
>> +    nvjpg->client.version = nvjpg->config->version;
>> +    nvjpg->client.ops = &nvjpg_ops;
>> +
>> +    err = host1x_client_register(&nvjpg->client.base);
>> +    if (err < 0) {
>> +        dev_err(dev, "failed to register host1x client: %d\n", err);
>> +        goto exit_falcon;
>> +    }
>> +
>> +    pm_runtime_use_autosuspend(dev);
>> +    pm_runtime_set_autosuspend_delay(dev, 500);
>> +    devm_pm_runtime_enable(dev);
>> +
>> +    return 0;
>> +
>> +exit_falcon:
>> +    falcon_exit(&nvjpg->falcon);
>> +
>> +    return err;
>> +}
>> +
>> +static void nvjpg_remove(struct platform_device *pdev)
>> +{
>> +    struct nvjpg *nvjpg = platform_get_drvdata(pdev);
>> +
>> +    host1x_client_unregister(&nvjpg->client.base);
>> +    falcon_exit(&nvjpg->falcon);
>> +}
>> +
>> +static const struct dev_pm_ops nvjpg_pm_ops = {
>> +    SET_RUNTIME_PM_OPS(nvjpg_runtime_suspend, nvjpg_runtime_resume, 
>> NULL)
>> +    SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>> +                pm_runtime_force_resume)
>> +};
> 
> There are modern, improved variants with no SET_ prefix.

I'll update it for v2 as well.

Thank you for your time!
Diogo

Re: [PATCH 1/3] drm/tegra: Add NVJPG driver

[Diogo Ivo]

Hi Thierry,

On 6/10/25 9:44 AM, Thierry Reding wrote:
> On Tue, Jun 10, 2025 at 12:26:07PM +0900, Mikko Perttunen wrote:
>> On 6/6/25 7:45 PM, Diogo Ivo wrote:
>>> Add support for booting and using NVJPG on Tegra210 to the Host1x
>>> and TegraDRM drivers. This driver only supports the new TegraDRM uAPI.
>>
>> Hello Diogo -- I'm happy to see this driver!
> 
> So am I, nice work!

Thank you Mikko and Thierry for the kind words :)

> [...]
>>> +	if (IS_ERR(nvjpg->regs))
>>> +		return PTR_ERR(nvjpg->regs);
>>> +
>>> +	nvjpg->rst = devm_reset_control_get_exclusive_released(&pdev->dev, "nvjpg");
>>> +	if (IS_ERR(nvjpg->rst)) {
>>> +		err = PTR_ERR(nvjpg->rst);
>>> +
>>> +		if (err != -EBUSY || WARN_ON(!pdev->dev.pm_domain)) {
>>> +			dev_err(&pdev->dev, "failed to get reset control: %d\n",
>>> +				err);
>>> +			return err;
>>> +		}
>>> +
>>> +		/*
>>> +		 * At this point, the reset control is most likely being used
>>> +		 * by the generic power domain implementation. With any luck
>>> +		 * the power domain will have taken care of resetting the SOR
>>> +		 * and we don't have to do anything.
>>> +		 */
>>> +		nvjpg->rst = NULL;
>>> +	}
>>
>> I see you've taken this from sor.c, but I think it should be unnecessary. I
>> imagine the code in sor.c is overcomplicated as well, maybe because we used
>> not to have the power domain implementation.
> 
> Agreed. SOR is also slightly older than NVJPG and used on Tegra124 where
> we don't use power domains, so most of these quirks are for backwards-
> compatibility. If we can avoid them for NVJPG, that'd be great.

Sounds good, I'll get rid of this explicit handling in v2.

Thanks,
Diogo

Re: [PATCH 2/3] arm64: tegra: Add NVJPG power-domain node

[Diogo Ivo]

On 6/10/25 5:57 AM, Mikko Perttunen wrote:
> On 6/6/25 7:45 PM, Diogo Ivo wrote:
>> Add the NVJPG power-domain node in order to support the NVJPG
>> accelerator.
>>
>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>> ---
>>   arch/arm64/boot/dts/nvidia/tegra210.dtsi | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/ 
>> boot/dts/nvidia/tegra210.dtsi
>> index 
>> 402b0ede1472af625d9d9e811f5af306d436cc98..6f8cdf012f0f12a16716e9d479c46b330bbb7dda 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>> @@ -947,6 +947,12 @@ pd_xusbhost: xusbc {
>>                   resets = <&tegra_car TEGRA210_CLK_XUSB_HOST>;
>>                   #power-domain-cells = <0>;
>>               };
>> +
>> +            pd_nvjpg: nvjpg {
>> +                clocks = <&tegra_car TEGRA210_CLK_NVJPG>;
>> +                resets = <&tegra_car 195>;
>> +                #power-domain-cells = <0>;
>> +            };
>>           };
>>       };
>>
> 
> Please mention Tegra210 in the commit subject. Otherwise,

Will do, for v2 I'll collect the tag and mention Tegra210.

> Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>

Re: [PATCH 3/3] arm64: tegra: Add NVJPG node

[Diogo Ivo]

On 6/10/25 6:01 AM, Mikko Perttunen wrote:
> On 6/6/25 7:45 PM, Diogo Ivo wrote:
>> The Tegra X1 chip contains a NVJPG accelerator capable of
>> encoding/decoding JPEG files in hardware, so add its DT node.
>>
>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>> ---
>>   arch/arm64/boot/dts/nvidia/tegra210.dtsi | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/ 
>> boot/dts/nvidia/tegra210.dtsi
>> index 
>> 6f8cdf012f0f12a16716e9d479c46b330bbb7dda..087f38256fd40f57c4685e907f9682eb49ee31db 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>> @@ -253,7 +253,13 @@ vic@54340000 {
>>           nvjpg@54380000 {
>>               compatible = "nvidia,tegra210-nvjpg";
>>               reg = <0x0 0x54380000 0x0 0x00040000>;
>> -            status = "disabled";
>> +            clocks = <&tegra_car TEGRA210_CLK_NVJPG>;
>> +            clock-names = "nvjpg";
>> +            resets = <&tegra_car 195>;
>> +            reset-names = "nvjpg";
>> +
>> +            iommus = <&mc TEGRA_SWGROUP_NVJPG>;
>> +            power-domains = <&pd_nvjpg>;
>>           };
>>           dsib: dsi@54400000 {
>>
> 
> Please mention Tegra210 in the commit subject, and perhaps adjust the 
> commit message to say that you're updating and enabling the device tree 
> node (rather than adding, since it's already there).

Will do, for v2 I'll collect the tag, mention Tegra210 and change the
commit message.

> With that,
> 
> Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Diogo Ivo]

On 6/10/25 9:58 AM, Thierry Reding wrote:
> On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
>> Hello,
>>
>> This series adds support for the NVJPG hardware accelerator found in the
>> Tegra210 SoC.
>>
>> The kernel driver is essentially a copy of the NVDEC driver as both
>> engines are Falcon-based.
>>
>> For the userspace part I have written a Mesa Gallium backend [1] that,
>> while still very much experimental, works in decoding images with VA-API.
> 
> Nice. It's good to see that there's some use in this after all. I
> haven't taken an in-depth look yet, but from a high-level point of view
> this looks like what I had imagined back when I started out with this
> driver.

Good to know that the structure makes sense to you :)

I still have some kinks to iron out before submitting a merge request in
Mesa but if you have the time to take a look before that and you already
have some comments before that that would be great.

> This made my day, thank you!

Happy to know that!

Diogo

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Diogo Ivo]

On 6/10/25 10:05 AM, Thierry Reding wrote:
> On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
>> Hello,
>>
>> This series adds support for the NVJPG hardware accelerator found in the
>> Tegra210 SoC.
>>
>> The kernel driver is essentially a copy of the NVDEC driver as both
>> engines are Falcon-based.
>>
>> For the userspace part I have written a Mesa Gallium backend [1] that,
>> while still very much experimental, works in decoding images with VA-API.
>>
>> I have been using ffmpeg to call VA-API with the following command:
>>
>> ffmpeg -v verbose -hwaccel vaapi -hwaccel_device /dev/dri/renderD129 -i <input.jpg> -pix_fmt bgra -f fbdev /dev/fb0
>>
>> which decodes <input.jpg> and shows the result in the framebuffer.
>>
>> The firmware for the engine can be obtained from a Linux for Tegra
>> distribution.
> 
> By the way, have you tried running this on anything newer than Tegra210?
> Given your progress on this, we can probably start thinking about
> submitting the binaries to linux-firmware.

Since I don't have access to other Tegra platforms I haven't been able
to test this driver there. For this I need help from someone with access
to more hardware, I can send a version that just adds the extra compatibles
and we could see if it works.

As for the firmwares that would be great!

>> Due to the way the Gallium implementation works for Tegra
>> the GPU also needs to be enabled.
> 
> I wonder if maybe we can get rid of this requirement. While it's
> certainly nice to have the GPU enabled, there may be cases where using
> only the other engines may be advantageous. Originally when I had worked
> on VIC, I was looking at how it could be used for compositing without
> getting the GPU involved. That's something that Android devices tend(ed)
> to do because of the power savings that come with it.

Yes I think this is possible to do, it mainly has involves properly
handling the Gallium driver initialization. I'll take a look at it
before submitting the MR for the Gallium driver.

Diogo

> Anyway, not a big deal, depending on the GPU for now is fine.

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Diogo Ivo]

On 6/10/25 10:52 AM, Mikko Perttunen wrote:
> On 6/10/25 6:05 PM, Thierry Reding wrote:
>> On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
>>> Hello,
>>>
>>> This series adds support for the NVJPG hardware accelerator found in the
>>> Tegra210 SoC.
>>>
>>> The kernel driver is essentially a copy of the NVDEC driver as both
>>> engines are Falcon-based.
>>>
>>> For the userspace part I have written a Mesa Gallium backend [1] that,
>>> while still very much experimental, works in decoding images with VA- 
>>> API.
>>>
>>> I have been using ffmpeg to call VA-API with the following command:
>>>
>>> ffmpeg -v verbose -hwaccel vaapi -hwaccel_device /dev/dri/renderD129 
>>> -i <input.jpg> -pix_fmt bgra -f fbdev /dev/fb0
>>>
>>> which decodes <input.jpg> and shows the result in the framebuffer.
>>>
>>> The firmware for the engine can be obtained from a Linux for Tegra
>>> distribution.
>>
>> By the way, have you tried running this on anything newer than Tegra210?
>> Given your progress on this, we can probably start thinking about
>> submitting the binaries to linux-firmware.
> 
> FWIW, the impression I have is that NVJPG is basically unchanged all the 
> way to Tegra234. So if we add stream ID support and the firmwares, it'll 
> probably just work. Tegra234 has the quirk that it has two instances of 
> NVJPG -- these have to be distinguished by their different class IDs. 
> But we should go ahead with the T210 support first.

I have a question here, what exactly are the stream IDs? While working
on the driver this came up and I didn't manage to figure it out.

Diogo

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Thierry Reding]

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

On Wed, Jun 11, 2025 at 01:05:40PM +0100, Diogo Ivo wrote:
> 
> 
> On 6/10/25 10:52 AM, Mikko Perttunen wrote:
> > On 6/10/25 6:05 PM, Thierry Reding wrote:
> > > On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
> > > > Hello,
> > > > 
> > > > This series adds support for the NVJPG hardware accelerator found in the
> > > > Tegra210 SoC.
> > > > 
> > > > The kernel driver is essentially a copy of the NVDEC driver as both
> > > > engines are Falcon-based.
> > > > 
> > > > For the userspace part I have written a Mesa Gallium backend [1] that,
> > > > while still very much experimental, works in decoding images
> > > > with VA- API.
> > > > 
> > > > I have been using ffmpeg to call VA-API with the following command:
> > > > 
> > > > ffmpeg -v verbose -hwaccel vaapi -hwaccel_device
> > > > /dev/dri/renderD129 -i <input.jpg> -pix_fmt bgra -f fbdev
> > > > /dev/fb0
> > > > 
> > > > which decodes <input.jpg> and shows the result in the framebuffer.
> > > > 
> > > > The firmware for the engine can be obtained from a Linux for Tegra
> > > > distribution.
> > > 
> > > By the way, have you tried running this on anything newer than Tegra210?
> > > Given your progress on this, we can probably start thinking about
> > > submitting the binaries to linux-firmware.
> > 
> > FWIW, the impression I have is that NVJPG is basically unchanged all the
> > way to Tegra234. So if we add stream ID support and the firmwares, it'll
> > probably just work. Tegra234 has the quirk that it has two instances of
> > NVJPG -- these have to be distinguished by their different class IDs.
> > But we should go ahead with the T210 support first.
> 
> I have a question here, what exactly are the stream IDs? While working
> on the driver this came up and I didn't manage to figure it out.

Stream IDs are a way to identify memory transactions as belonging to a
certain device. This comes into play when working with the IOMMU (which
is a Tegra SMMU on Tegra210 and earlier, and an ARM SMMU on Tegra) and
is used to isolate DMA capable devices. Basically for every stream ID
you get a separate I/O address space. NVJPG will have its own address
space, and so will VIC. Each device can only access whatever has been
mapped to it's I/O address space. That means NVJPG can't interfere with
VIC and vice-versa. And neither can any of these engines read from or
write to random system memory if badly programmed.

For Tegra SMMU there's no such thing as programmable stream IDs, so the
stream ID is fixed for the given device.

On newer chips (Tegra186 and later, or maybe it wasn't until Tegra194),
certain IP blocks have special registers that can be used to override
the stream ID. There's also a way to set the stream ID via command
streams, which means that you can have different I/O address spaces (I
think we call them memory context) per engine, which means that you can
isolate different processes using the same engine from each other.

Again, for Tegra210 that's nothing we need to worry about. For newer
chips it's probably just a matter of adding .get_streamid_offset() and
.can_use_memory_ctx() implementations.

Thierry

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

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Thierry Reding]

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

On Wed, Jun 11, 2025 at 01:04:14PM +0100, Diogo Ivo wrote:
> 
> 
> On 6/10/25 10:05 AM, Thierry Reding wrote:
> > On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
> > > Hello,
> > > 
> > > This series adds support for the NVJPG hardware accelerator found in the
> > > Tegra210 SoC.
> > > 
> > > The kernel driver is essentially a copy of the NVDEC driver as both
> > > engines are Falcon-based.
> > > 
> > > For the userspace part I have written a Mesa Gallium backend [1] that,
> > > while still very much experimental, works in decoding images with VA-API.
> > > 
> > > I have been using ffmpeg to call VA-API with the following command:
> > > 
> > > ffmpeg -v verbose -hwaccel vaapi -hwaccel_device /dev/dri/renderD129 -i <input.jpg> -pix_fmt bgra -f fbdev /dev/fb0
> > > 
> > > which decodes <input.jpg> and shows the result in the framebuffer.
> > > 
> > > The firmware for the engine can be obtained from a Linux for Tegra
> > > distribution.
> > 
> > By the way, have you tried running this on anything newer than Tegra210?
> > Given your progress on this, we can probably start thinking about
> > submitting the binaries to linux-firmware.
> 
> Since I don't have access to other Tegra platforms I haven't been able
> to test this driver there. For this I need help from someone with access
> to more hardware, I can send a version that just adds the extra compatibles
> and we could see if it works.
> 
> As for the firmwares that would be great!

I think both of these are things that Mikko and I can help with.

> > > Due to the way the Gallium implementation works for Tegra
> > > the GPU also needs to be enabled.
> > 
> > I wonder if maybe we can get rid of this requirement. While it's
> > certainly nice to have the GPU enabled, there may be cases where using
> > only the other engines may be advantageous. Originally when I had worked
> > on VIC, I was looking at how it could be used for compositing without
> > getting the GPU involved. That's something that Android devices tend(ed)
> > to do because of the power savings that come with it.
> 
> Yes I think this is possible to do, it mainly has involves properly
> handling the Gallium driver initialization. I'll take a look at it
> before submitting the MR for the Gallium driver.

Okay, great. But I think it's definitely something that we can defer if
it's non-trivial.

Thierry

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

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Mikko Perttunen]

On 6/12/25 12:06 AM, Thierry Reding wrote:
> On Wed, Jun 11, 2025 at 01:05:40PM +0100, Diogo Ivo wrote:
>>
>>
>> On 6/10/25 10:52 AM, Mikko Perttunen wrote:
>>> On 6/10/25 6:05 PM, Thierry Reding wrote:
>>>> On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
>>>>> Hello,
>>>>>
>>>>> This series adds support for the NVJPG hardware accelerator found in the
>>>>> Tegra210 SoC.
>>>>>
>>>>> The kernel driver is essentially a copy of the NVDEC driver as both
>>>>> engines are Falcon-based.
>>>>>
>>>>> For the userspace part I have written a Mesa Gallium backend [1] that,
>>>>> while still very much experimental, works in decoding images
>>>>> with VA- API.
>>>>>
>>>>> I have been using ffmpeg to call VA-API with the following command:
>>>>>
>>>>> ffmpeg -v verbose -hwaccel vaapi -hwaccel_device
>>>>> /dev/dri/renderD129 -i <input.jpg> -pix_fmt bgra -f fbdev
>>>>> /dev/fb0
>>>>>
>>>>> which decodes <input.jpg> and shows the result in the framebuffer.
>>>>>
>>>>> The firmware for the engine can be obtained from a Linux for Tegra
>>>>> distribution.
>>>>
>>>> By the way, have you tried running this on anything newer than Tegra210?
>>>> Given your progress on this, we can probably start thinking about
>>>> submitting the binaries to linux-firmware.
>>>
>>> FWIW, the impression I have is that NVJPG is basically unchanged all the
>>> way to Tegra234. So if we add stream ID support and the firmwares, it'll
>>> probably just work. Tegra234 has the quirk that it has two instances of
>>> NVJPG -- these have to be distinguished by their different class IDs.
>>> But we should go ahead with the T210 support first.
>>
>> I have a question here, what exactly are the stream IDs? While working
>> on the driver this came up and I didn't manage to figure it out.
> 
> Stream IDs are a way to identify memory transactions as belonging to a
> certain device. This comes into play when working with the IOMMU (which
> is a Tegra SMMU on Tegra210 and earlier, and an ARM SMMU on Tegra) and
> is used to isolate DMA capable devices. Basically for every stream ID
> you get a separate I/O address space. NVJPG will have its own address
> space, and so will VIC. Each device can only access whatever has been
> mapped to it's I/O address space. That means NVJPG can't interfere with
> VIC and vice-versa. And neither can any of these engines read from or
> write to random system memory if badly programmed.
> 
> For Tegra SMMU there's no such thing as programmable stream IDs, so the
> stream ID is fixed for the given device.
> 
> On newer chips (Tegra186 and later, or maybe it wasn't until Tegra194),

Tegra186 and newer -- all chips with the ARM SMMU. To add a little bit, 
each engine can address two stream IDs, one for firmware and one for 
data. All user specified buffers are mapped into the data IOMMU domain, 
and these are switched between jobs / applications.

As an aside, currently each engine has its own firmware stream ID, but 
that's a bit wasteful, since the kernel allocates a separate IOMMU 
domain for each. The firmwares are all read-only so they could be in a 
shared one. We've had to consolidate these on some platforms that ran 
out of IOMMU domains otherwise. Not really a concern with upstream 
platforms, though.

> certain IP blocks have special registers that can be used to override
> the stream ID. There's also a way to set the stream ID via command
> streams, which means that you can have different I/O address spaces (I
> think we call them memory context) per engine, which means that you can
> isolate different processes using the same engine from each other.
> 
> Again, for Tegra210 that's nothing we need to worry about. For newer
> chips it's probably just a matter of adding .get_streamid_offset() and
> .can_use_memory_ctx() implementations.

Also need to program the THI_STREAMID / TRANSCFG registers during boot.

Cheers,
Mikko

> 
> Thierry

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Diogo Ivo]

On 6/11/25 4:06 PM, Thierry Reding wrote:
> On Wed, Jun 11, 2025 at 01:05:40PM +0100, Diogo Ivo wrote:
>>
>>
>> On 6/10/25 10:52 AM, Mikko Perttunen wrote:
>>> On 6/10/25 6:05 PM, Thierry Reding wrote:
>>>> On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
>>>>> Hello,
>>>>>
>>>>> This series adds support for the NVJPG hardware accelerator found in the
>>>>> Tegra210 SoC.
>>>>>
>>>>> The kernel driver is essentially a copy of the NVDEC driver as both
>>>>> engines are Falcon-based.
>>>>>
>>>>> For the userspace part I have written a Mesa Gallium backend [1] that,
>>>>> while still very much experimental, works in decoding images
>>>>> with VA- API.
>>>>>
>>>>> I have been using ffmpeg to call VA-API with the following command:
>>>>>
>>>>> ffmpeg -v verbose -hwaccel vaapi -hwaccel_device
>>>>> /dev/dri/renderD129 -i <input.jpg> -pix_fmt bgra -f fbdev
>>>>> /dev/fb0
>>>>>
>>>>> which decodes <input.jpg> and shows the result in the framebuffer.
>>>>>
>>>>> The firmware for the engine can be obtained from a Linux for Tegra
>>>>> distribution.
>>>>
>>>> By the way, have you tried running this on anything newer than Tegra210?
>>>> Given your progress on this, we can probably start thinking about
>>>> submitting the binaries to linux-firmware.
>>>
>>> FWIW, the impression I have is that NVJPG is basically unchanged all the
>>> way to Tegra234. So if we add stream ID support and the firmwares, it'll
>>> probably just work. Tegra234 has the quirk that it has two instances of
>>> NVJPG -- these have to be distinguished by their different class IDs.
>>> But we should go ahead with the T210 support first.
>>
>> I have a question here, what exactly are the stream IDs? While working
>> on the driver this came up and I didn't manage to figure it out.
> 
> Stream IDs are a way to identify memory transactions as belonging to a
> certain device. This comes into play when working with the IOMMU (which
> is a Tegra SMMU on Tegra210 and earlier, and an ARM SMMU on Tegra) and
> is used to isolate DMA capable devices. Basically for every stream ID
> you get a separate I/O address space. NVJPG will have its own address
> space, and so will VIC. Each device can only access whatever has been
> mapped to it's I/O address space. That means NVJPG can't interfere with
> VIC and vice-versa. And neither can any of these engines read from or
> write to random system memory if badly programmed.

So if I understand this correctly a Stream ID corresponds to an IOMMU
domain right?

> For Tegra SMMU there's no such thing as programmable stream IDs, so the
> stream ID is fixed for the given device.
> 
> On newer chips (Tegra186 and later, or maybe it wasn't until Tegra194),
> certain IP blocks have special registers that can be used to override
> the stream ID. There's also a way to set the stream ID via command
> streams, which means that you can have different I/O address spaces (I
> think we call them memory context) per engine, which means that you can
> isolate different processes using the same engine from each other.
> 
> Again, for Tegra210 that's nothing we need to worry about. For newer
> chips it's probably just a matter of adding .get_streamid_offset() and
> .can_use_memory_ctx() implementations.

Ok, then in that case I'll keep the driver in its current state without
these implementations if that's ok. Connected with this I wanted to know
your thoughts on the best way to upstream this, is it better to wait for
testing on different platforms first and then if things work merge a
driver that works for all of them or go with Tegra210 first and then add
more platforms later on?

Thanks,
Diogo

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Diogo Ivo]

On 6/12/25 2:55 AM, Mikko Perttunen wrote:
> On 6/12/25 12:06 AM, Thierry Reding wrote:
>> On Wed, Jun 11, 2025 at 01:05:40PM +0100, Diogo Ivo wrote:
>>> I have a question here, what exactly are the stream IDs? While working
>>> on the driver this came up and I didn't manage to figure it out.
>>
>> Stream IDs are a way to identify memory transactions as belonging to a
>> certain device. This comes into play when working with the IOMMU (which
>> is a Tegra SMMU on Tegra210 and earlier, and an ARM SMMU on Tegra) and
>> is used to isolate DMA capable devices. Basically for every stream ID
>> you get a separate I/O address space. NVJPG will have its own address
>> space, and so will VIC. Each device can only access whatever has been
>> mapped to it's I/O address space. That means NVJPG can't interfere with
>> VIC and vice-versa. And neither can any of these engines read from or
>> write to random system memory if badly programmed.
>>
>> For Tegra SMMU there's no such thing as programmable stream IDs, so the
>> stream ID is fixed for the given device.
>>
>> On newer chips (Tegra186 and later, or maybe it wasn't until Tegra194),
> 
> Tegra186 and newer -- all chips with the ARM SMMU. To add a little bit, 
> each engine can address two stream IDs, one for firmware and one for 
> data. All user specified buffers are mapped into the data IOMMU domain, 
> and these are switched between jobs / applications.
> 
> As an aside, currently each engine has its own firmware stream ID, but 
> that's a bit wasteful, since the kernel allocates a separate IOMMU 
> domain for each. The firmwares are all read-only so they could be in a 
> shared one. We've had to consolidate these on some platforms that ran 
> out of IOMMU domains otherwise. Not really a concern with upstream 
> platforms, though.

Does this dual Stream ID also apply to Tegra210?

> Also need to program the THI_STREAMID / TRANSCFG registers during boot.

Thanks,
Diogo

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Diogo Ivo]

On 6/11/25 4:10 PM, Thierry Reding wrote:
> On Wed, Jun 11, 2025 at 01:04:14PM +0100, Diogo Ivo wrote:
>>
>>
>> On 6/10/25 10:05 AM, Thierry Reding wrote:
>>> On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
>>>> Hello,
>>>>
>>>> This series adds support for the NVJPG hardware accelerator found in the
>>>> Tegra210 SoC.
>>>>
>>>> The kernel driver is essentially a copy of the NVDEC driver as both
>>>> engines are Falcon-based.
>>>>
>>>> For the userspace part I have written a Mesa Gallium backend [1] that,
>>>> while still very much experimental, works in decoding images with VA-API.
>>>>
>>>> I have been using ffmpeg to call VA-API with the following command:
>>>>
>>>> ffmpeg -v verbose -hwaccel vaapi -hwaccel_device /dev/dri/renderD129 -i <input.jpg> -pix_fmt bgra -f fbdev /dev/fb0
>>>>
>>>> which decodes <input.jpg> and shows the result in the framebuffer.
>>>>
>>>> The firmware for the engine can be obtained from a Linux for Tegra
>>>> distribution.
>>>
>>> By the way, have you tried running this on anything newer than Tegra210?
>>> Given your progress on this, we can probably start thinking about
>>> submitting the binaries to linux-firmware.
>>
>> Since I don't have access to other Tegra platforms I haven't been able
>> to test this driver there. For this I need help from someone with access
>> to more hardware, I can send a version that just adds the extra compatibles
>> and we could see if it works.
>>
>> As for the firmwares that would be great!
> 
> I think both of these are things that Mikko and I can help with.

That would be great (as I imagine you have access to the hardware :) ).
Let me know if you have any questions!

>>>> Due to the way the Gallium implementation works for Tegra
>>>> the GPU also needs to be enabled.
>>>
>>> I wonder if maybe we can get rid of this requirement. While it's
>>> certainly nice to have the GPU enabled, there may be cases where using
>>> only the other engines may be advantageous. Originally when I had worked
>>> on VIC, I was looking at how it could be used for compositing without
>>> getting the GPU involved. That's something that Android devices tend(ed)
>>> to do because of the power savings that come with it.
>>
>> Yes I think this is possible to do, it mainly has involves properly
>> handling the Gallium driver initialization. I'll take a look at it
>> before submitting the MR for the Gallium driver.
> 
> Okay, great. But I think it's definitely something that we can defer if
> it's non-trivial.

Perfect then, in that case I'll skip it for now and open a Mesa MR after
taking care of some final details.

Thanks,
Diogo

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Mikko Perttunen]

On 6/16/25 7:23 PM, Diogo Ivo wrote:
> 
> 
> On 6/12/25 2:55 AM, Mikko Perttunen wrote:
>> On 6/12/25 12:06 AM, Thierry Reding wrote:
>>> On Wed, Jun 11, 2025 at 01:05:40PM +0100, Diogo Ivo wrote:
>>>> I have a question here, what exactly are the stream IDs? While working
>>>> on the driver this came up and I didn't manage to figure it out.
>>>
>>> Stream IDs are a way to identify memory transactions as belonging to a
>>> certain device. This comes into play when working with the IOMMU (which
>>> is a Tegra SMMU on Tegra210 and earlier, and an ARM SMMU on Tegra) and
>>> is used to isolate DMA capable devices. Basically for every stream ID
>>> you get a separate I/O address space. NVJPG will have its own address
>>> space, and so will VIC. Each device can only access whatever has been
>>> mapped to it's I/O address space. That means NVJPG can't interfere with
>>> VIC and vice-versa. And neither can any of these engines read from or
>>> write to random system memory if badly programmed.
>>>
>>> For Tegra SMMU there's no such thing as programmable stream IDs, so the
>>> stream ID is fixed for the given device.
>>>
>>> On newer chips (Tegra186 and later, or maybe it wasn't until Tegra194),
>>
>> Tegra186 and newer -- all chips with the ARM SMMU. To add a little 
>> bit, each engine can address two stream IDs, one for firmware and one 
>> for data. All user specified buffers are mapped into the data IOMMU 
>> domain, and these are switched between jobs / applications.
>>
>> As an aside, currently each engine has its own firmware stream ID, but 
>> that's a bit wasteful, since the kernel allocates a separate IOMMU 
>> domain for each. The firmwares are all read-only so they could be in a 
>> shared one. We've had to consolidate these on some platforms that ran 
>> out of IOMMU domains otherwise. Not really a concern with upstream 
>> platforms, though.
> 
> Does this dual Stream ID also apply to Tegra210?

No, only Tegra186 and later (chips with ARM SMMU).

> 
>> Also need to program the THI_STREAMID / TRANSCFG registers during boot.
> 
> Thanks,
> Diogo
> 

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Mikko Perttunen]

On 6/16/25 7:21 PM, Diogo Ivo wrote:
> 
> 
> On 6/11/25 4:06 PM, Thierry Reding wrote:
>> On Wed, Jun 11, 2025 at 01:05:40PM +0100, Diogo Ivo wrote:
>>>
>>>
>>> On 6/10/25 10:52 AM, Mikko Perttunen wrote:
>>>> On 6/10/25 6:05 PM, Thierry Reding wrote:
>>>>> On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
>>>>>> Hello,
>>>>>>
>>>>>> This series adds support for the NVJPG hardware accelerator found 
>>>>>> in the
>>>>>> Tegra210 SoC.
>>>>>>
>>>>>> The kernel driver is essentially a copy of the NVDEC driver as both
>>>>>> engines are Falcon-based.
>>>>>>
>>>>>> For the userspace part I have written a Mesa Gallium backend [1] 
>>>>>> that,
>>>>>> while still very much experimental, works in decoding images
>>>>>> with VA- API.
>>>>>>
>>>>>> I have been using ffmpeg to call VA-API with the following command:
>>>>>>
>>>>>> ffmpeg -v verbose -hwaccel vaapi -hwaccel_device
>>>>>> /dev/dri/renderD129 -i <input.jpg> -pix_fmt bgra -f fbdev
>>>>>> /dev/fb0
>>>>>>
>>>>>> which decodes <input.jpg> and shows the result in the framebuffer.
>>>>>>
>>>>>> The firmware for the engine can be obtained from a Linux for Tegra
>>>>>> distribution.
>>>>>
>>>>> By the way, have you tried running this on anything newer than 
>>>>> Tegra210?
>>>>> Given your progress on this, we can probably start thinking about
>>>>> submitting the binaries to linux-firmware.
>>>>
>>>> FWIW, the impression I have is that NVJPG is basically unchanged all 
>>>> the
>>>> way to Tegra234. So if we add stream ID support and the firmwares, 
>>>> it'll
>>>> probably just work. Tegra234 has the quirk that it has two instances of
>>>> NVJPG -- these have to be distinguished by their different class IDs.
>>>> But we should go ahead with the T210 support first.
>>>
>>> I have a question here, what exactly are the stream IDs? While working
>>> on the driver this came up and I didn't manage to figure it out.
>>
>> Stream IDs are a way to identify memory transactions as belonging to a
>> certain device. This comes into play when working with the IOMMU (which
>> is a Tegra SMMU on Tegra210 and earlier, and an ARM SMMU on Tegra) and
>> is used to isolate DMA capable devices. Basically for every stream ID
>> you get a separate I/O address space. NVJPG will have its own address
>> space, and so will VIC. Each device can only access whatever has been
>> mapped to it's I/O address space. That means NVJPG can't interfere with
>> VIC and vice-versa. And neither can any of these engines read from or
>> write to random system memory if badly programmed.
> 
> So if I understand this correctly a Stream ID corresponds to an IOMMU
> domain right?

Technically not necessarily, but in practice that's the case, as the 
IOMMU driver creates IOMMU domains for each stream ID in the device 
tree. They are similar to the SWGROUPs on Tegra210.

> 
>> For Tegra SMMU there's no such thing as programmable stream IDs, so the
>> stream ID is fixed for the given device.
>>
>> On newer chips (Tegra186 and later, or maybe it wasn't until Tegra194),
>> certain IP blocks have special registers that can be used to override
>> the stream ID. There's also a way to set the stream ID via command
>> streams, which means that you can have different I/O address spaces (I
>> think we call them memory context) per engine, which means that you can
>> isolate different processes using the same engine from each other.
>>
>> Again, for Tegra210 that's nothing we need to worry about. For newer
>> chips it's probably just a matter of adding .get_streamid_offset() and
>> .can_use_memory_ctx() implementations.
> 
> Ok, then in that case I'll keep the driver in its current state without
> these implementations if that's ok. Connected with this I wanted to know
> your thoughts on the best way to upstream this, is it better to wait for
> testing on different platforms first and then if things work merge a
> driver that works for all of them or go with Tegra210 first and then add
> more platforms later on?

Personally, I'd say to go for Tegra210 first.

> 
> Thanks,
> Diogo
> 

Cheers
Mikko

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Diogo Ivo]

On 6/17/25 5:40 AM, Mikko Perttunen wrote:
> 
> 
> On 6/16/25 7:21 PM, Diogo Ivo wrote:
>>
>>
>> On 6/11/25 4:06 PM, Thierry Reding wrote:
>>> On Wed, Jun 11, 2025 at 01:05:40PM +0100, Diogo Ivo wrote:
>>>>
>>>>
>>>> On 6/10/25 10:52 AM, Mikko Perttunen wrote:
>>>>> On 6/10/25 6:05 PM, Thierry Reding wrote:
>>>>>> On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> This series adds support for the NVJPG hardware accelerator found 
>>>>>>> in the
>>>>>>> Tegra210 SoC.
>>>>>>>
>>>>>>> The kernel driver is essentially a copy of the NVDEC driver as both
>>>>>>> engines are Falcon-based.
>>>>>>>
>>>>>>> For the userspace part I have written a Mesa Gallium backend [1] 
>>>>>>> that,
>>>>>>> while still very much experimental, works in decoding images
>>>>>>> with VA- API.
>>>>>>>
>>>>>>> I have been using ffmpeg to call VA-API with the following command:
>>>>>>>
>>>>>>> ffmpeg -v verbose -hwaccel vaapi -hwaccel_device
>>>>>>> /dev/dri/renderD129 -i <input.jpg> -pix_fmt bgra -f fbdev
>>>>>>> /dev/fb0
>>>>>>>
>>>>>>> which decodes <input.jpg> and shows the result in the framebuffer.
>>>>>>>
>>>>>>> The firmware for the engine can be obtained from a Linux for Tegra
>>>>>>> distribution.
>>>>>>
>>>>>> By the way, have you tried running this on anything newer than 
>>>>>> Tegra210?
>>>>>> Given your progress on this, we can probably start thinking about
>>>>>> submitting the binaries to linux-firmware.
>>>>>
>>>>> FWIW, the impression I have is that NVJPG is basically unchanged 
>>>>> all the
>>>>> way to Tegra234. So if we add stream ID support and the firmwares, 
>>>>> it'll
>>>>> probably just work. Tegra234 has the quirk that it has two 
>>>>> instances of
>>>>> NVJPG -- these have to be distinguished by their different class IDs.
>>>>> But we should go ahead with the T210 support first.
>>>>
>>>> I have a question here, what exactly are the stream IDs? While working
>>>> on the driver this came up and I didn't manage to figure it out.
>>>
>>> Stream IDs are a way to identify memory transactions as belonging to a
>>> certain device. This comes into play when working with the IOMMU (which
>>> is a Tegra SMMU on Tegra210 and earlier, and an ARM SMMU on Tegra) and
>>> is used to isolate DMA capable devices. Basically for every stream ID
>>> you get a separate I/O address space. NVJPG will have its own address
>>> space, and so will VIC. Each device can only access whatever has been
>>> mapped to it's I/O address space. That means NVJPG can't interfere with
>>> VIC and vice-versa. And neither can any of these engines read from or
>>> write to random system memory if badly programmed.
>>
>> So if I understand this correctly a Stream ID corresponds to an IOMMU
>> domain right?
> 
> Technically not necessarily, but in practice that's the case, as the 
> IOMMU driver creates IOMMU domains for each stream ID in the device 
> tree. They are similar to the SWGROUPs on Tegra210.

Ok that makes sense, thank you for the clarification :)

>> Ok, then in that case I'll keep the driver in its current state without
>> these implementations if that's ok. Connected with this I wanted to know
>> your thoughts on the best way to upstream this, is it better to wait for
>> testing on different platforms first and then if things work merge a
>> driver that works for all of them or go with Tegra210 first and then add
>> more platforms later on?
> 
> Personally, I'd say to go for Tegra210 first.

In that case I believe that in the v2 I sent out of the driver I addressed
both yours and Thierry's reviews and the driver should be in good condition
for Tegra210. What are the next steps in order to merge it?

Thanks,
Diogo

> Cheers
> Mikko

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Mikko Perttunen]

On 6/17/25 6:26 PM, Diogo Ivo wrote:
> 
> 
> On 6/17/25 5:40 AM, Mikko Perttunen wrote:
>>
>>
>> On 6/16/25 7:21 PM, Diogo Ivo wrote:
>>>
>>>
>>> On 6/11/25 4:06 PM, Thierry Reding wrote:
>>>> On Wed, Jun 11, 2025 at 01:05:40PM +0100, Diogo Ivo wrote:
>>>>>
>>>>>
>>>>> On 6/10/25 10:52 AM, Mikko Perttunen wrote:
>>>>>> On 6/10/25 6:05 PM, Thierry Reding wrote:
>>>>>>> On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> This series adds support for the NVJPG hardware accelerator 
>>>>>>>> found in the
>>>>>>>> Tegra210 SoC.
>>>>>>>>
>>>>>>>> The kernel driver is essentially a copy of the NVDEC driver as both
>>>>>>>> engines are Falcon-based.
>>>>>>>>
>>>>>>>> For the userspace part I have written a Mesa Gallium backend [1] 
>>>>>>>> that,
>>>>>>>> while still very much experimental, works in decoding images
>>>>>>>> with VA- API.
>>>>>>>>
>>>>>>>> I have been using ffmpeg to call VA-API with the following command:
>>>>>>>>
>>>>>>>> ffmpeg -v verbose -hwaccel vaapi -hwaccel_device
>>>>>>>> /dev/dri/renderD129 -i <input.jpg> -pix_fmt bgra -f fbdev
>>>>>>>> /dev/fb0
>>>>>>>>
>>>>>>>> which decodes <input.jpg> and shows the result in the framebuffer.
>>>>>>>>
>>>>>>>> The firmware for the engine can be obtained from a Linux for Tegra
>>>>>>>> distribution.
>>>>>>>
>>>>>>> By the way, have you tried running this on anything newer than 
>>>>>>> Tegra210?
>>>>>>> Given your progress on this, we can probably start thinking about
>>>>>>> submitting the binaries to linux-firmware.
>>>>>>
>>>>>> FWIW, the impression I have is that NVJPG is basically unchanged 
>>>>>> all the
>>>>>> way to Tegra234. So if we add stream ID support and the firmwares, 
>>>>>> it'll
>>>>>> probably just work. Tegra234 has the quirk that it has two 
>>>>>> instances of
>>>>>> NVJPG -- these have to be distinguished by their different class IDs.
>>>>>> But we should go ahead with the T210 support first.
>>>>>
>>>>> I have a question here, what exactly are the stream IDs? While working
>>>>> on the driver this came up and I didn't manage to figure it out.
>>>>
>>>> Stream IDs are a way to identify memory transactions as belonging to a
>>>> certain device. This comes into play when working with the IOMMU (which
>>>> is a Tegra SMMU on Tegra210 and earlier, and an ARM SMMU on Tegra) and
>>>> is used to isolate DMA capable devices. Basically for every stream ID
>>>> you get a separate I/O address space. NVJPG will have its own address
>>>> space, and so will VIC. Each device can only access whatever has been
>>>> mapped to it's I/O address space. That means NVJPG can't interfere with
>>>> VIC and vice-versa. And neither can any of these engines read from or
>>>> write to random system memory if badly programmed.
>>>
>>> So if I understand this correctly a Stream ID corresponds to an IOMMU
>>> domain right?
>>
>> Technically not necessarily, but in practice that's the case, as the 
>> IOMMU driver creates IOMMU domains for each stream ID in the device 
>> tree. They are similar to the SWGROUPs on Tegra210.
> 
> Ok that makes sense, thank you for the clarification :)
> 
>>> Ok, then in that case I'll keep the driver in its current state without
>>> these implementations if that's ok. Connected with this I wanted to know
>>> your thoughts on the best way to upstream this, is it better to wait for
>>> testing on different platforms first and then if things work merge a
>>> driver that works for all of them or go with Tegra210 first and then add
>>> more platforms later on?
>>
>> Personally, I'd say to go for Tegra210 first.
> 
> In that case I believe that in the v2 I sent out of the driver I addressed
> both yours and Thierry's reviews and the driver should be in good condition
> for Tegra210. What are the next steps in order to merge it?

I left one more comment on the v2 patch. With that fixed, if Thierry has 
no further objections, hopefully he can merge.

Cheers,
Mikko

> 
> Thanks,
> Diogo
> 
>> Cheers
>> Mikko
> 

Re: [PATCH 0/3] NVIDIA Tegra210 NVJPG support

[Diogo Ivo]

Hi Mikko,

On 6/30/25 7:14 AM, Mikko Perttunen wrote:
> On 6/17/25 6:26 PM, Diogo Ivo wrote:
>>
>>
>> On 6/17/25 5:40 AM, Mikko Perttunen wrote:
>>>
>>>
>>> On 6/16/25 7:21 PM, Diogo Ivo wrote:
>>>>
>>>>
>>>> On 6/11/25 4:06 PM, Thierry Reding wrote:
>>>>> On Wed, Jun 11, 2025 at 01:05:40PM +0100, Diogo Ivo wrote:
>>>>>>
>>>>>>
>>>>>> On 6/10/25 10:52 AM, Mikko Perttunen wrote:
>>>>>>> On 6/10/25 6:05 PM, Thierry Reding wrote:
>>>>>>>> On Fri, Jun 06, 2025 at 11:45:33AM +0100, Diogo Ivo wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> This series adds support for the NVJPG hardware accelerator 
>>>>>>>>> found in the
>>>>>>>>> Tegra210 SoC.
>>>>>>>>>
>>>>>>>>> The kernel driver is essentially a copy of the NVDEC driver as 
>>>>>>>>> both
>>>>>>>>> engines are Falcon-based.
>>>>>>>>>
>>>>>>>>> For the userspace part I have written a Mesa Gallium backend 
>>>>>>>>> [1] that,
>>>>>>>>> while still very much experimental, works in decoding images
>>>>>>>>> with VA- API.
>>>>>>>>>
>>>>>>>>> I have been using ffmpeg to call VA-API with the following 
>>>>>>>>> command:
>>>>>>>>>
>>>>>>>>> ffmpeg -v verbose -hwaccel vaapi -hwaccel_device
>>>>>>>>> /dev/dri/renderD129 -i <input.jpg> -pix_fmt bgra -f fbdev
>>>>>>>>> /dev/fb0
>>>>>>>>>
>>>>>>>>> which decodes <input.jpg> and shows the result in the framebuffer.
>>>>>>>>>
>>>>>>>>> The firmware for the engine can be obtained from a Linux for Tegra
>>>>>>>>> distribution.
>>>>>>>>
>>>>>>>> By the way, have you tried running this on anything newer than 
>>>>>>>> Tegra210?
>>>>>>>> Given your progress on this, we can probably start thinking about
>>>>>>>> submitting the binaries to linux-firmware.
>>>>>>>
>>>>>>> FWIW, the impression I have is that NVJPG is basically unchanged 
>>>>>>> all the
>>>>>>> way to Tegra234. So if we add stream ID support and the 
>>>>>>> firmwares, it'll
>>>>>>> probably just work. Tegra234 has the quirk that it has two 
>>>>>>> instances of
>>>>>>> NVJPG -- these have to be distinguished by their different class 
>>>>>>> IDs.
>>>>>>> But we should go ahead with the T210 support first.
>>>>>>
>>>>>> I have a question here, what exactly are the stream IDs? While 
>>>>>> working
>>>>>> on the driver this came up and I didn't manage to figure it out.
>>>>>
>>>>> Stream IDs are a way to identify memory transactions as belonging to a
>>>>> certain device. This comes into play when working with the IOMMU 
>>>>> (which
>>>>> is a Tegra SMMU on Tegra210 and earlier, and an ARM SMMU on Tegra) and
>>>>> is used to isolate DMA capable devices. Basically for every stream ID
>>>>> you get a separate I/O address space. NVJPG will have its own address
>>>>> space, and so will VIC. Each device can only access whatever has been
>>>>> mapped to it's I/O address space. That means NVJPG can't interfere 
>>>>> with
>>>>> VIC and vice-versa. And neither can any of these engines read from or
>>>>> write to random system memory if badly programmed.
>>>>
>>>> So if I understand this correctly a Stream ID corresponds to an IOMMU
>>>> domain right?
>>>
>>> Technically not necessarily, but in practice that's the case, as the 
>>> IOMMU driver creates IOMMU domains for each stream ID in the device 
>>> tree. They are similar to the SWGROUPs on Tegra210.
>>
>> Ok that makes sense, thank you for the clarification :)
>>
>>>> Ok, then in that case I'll keep the driver in its current state without
>>>> these implementations if that's ok. Connected with this I wanted to 
>>>> know
>>>> your thoughts on the best way to upstream this, is it better to wait 
>>>> for
>>>> testing on different platforms first and then if things work merge a
>>>> driver that works for all of them or go with Tegra210 first and then 
>>>> add
>>>> more platforms later on?
>>>
>>> Personally, I'd say to go for Tegra210 first.
>>
>> In that case I believe that in the v2 I sent out of the driver I 
>> addressed
>> both yours and Thierry's reviews and the driver should be in good 
>> condition
>> for Tegra210. What are the next steps in order to merge it?
> 
> I left one more comment on the v2 patch. With that fixed, if Thierry has 
> no further objections, hopefully he can merge.

I sent out v3 addressing this comment and picked up your Acked-by.

Thanks for the review!
Diogo

> Cheers,
> Mikko
> 
>>
>> Thanks,
>> Diogo
>>
>>> Cheers
>>> Mikko
>>
>