[PATCH v2 0/3] Add NVIDIA Tegra114 support to video decoder driver

[PATCH v2 0/3] Add NVIDIA Tegra114 support to video decoder driver

[Dmitry Osipenko]

Video decoder of Tegra114/124 SoCs uses additional memory buffer required
for decoding of protected content. We won't support that content, but it
is impossible to disable access to the buffer, hence a stub buffer needs
to be provided. This series enables decoder driver only for Tegra114
because Tegra124 support requires more non-trivial changes on both kernel
and userspace sides.

Changelog:

v2: - Changed tegra_vde_alloc_bo() to return errno and fix unassigned
      error code in tegra_vde_probe().

Dmitry Osipenko (1):
  media: staging: tegra-vde: Reorder misc device registration

Thierry Reding (2):
  media: staging: tegra-vde: Support reference picture marking
  media: staging: tegra-vde: Properly mark invalid entries

 drivers/staging/media/tegra-vde/vde.c | 147 +++++++++++++++++++++++---
 drivers/staging/media/tegra-vde/vde.h |  18 ++++
 2 files changed, 152 insertions(+), 13 deletions(-)

-- 
2.33.1

[PATCH v2 1/3] media: staging: tegra-vde: Support reference picture marking

[Dmitry Osipenko]

From: Thierry Reding <treding@nvidia.com>

Tegra114 and Tegra124 support reference picture marking, which will
cause BSEV to write picture marking data to SDRAM. Make sure there is
a valid destination address for that data to avoid error messages from
the memory controller.

[digetx@gmail.com: added BO support and moved secure BO allocation to kernel]
Tested-by: Anton Bambura <jenneron@protonmail.com> # T114 ASUS TF701T
Signed-off-by: Thierry Reding <treding@nvidia.com>
Co-developed-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/staging/media/tegra-vde/vde.c | 124 +++++++++++++++++++++++++-
 drivers/staging/media/tegra-vde/vde.h |  18 ++++
 2 files changed, 141 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/tegra-vde/vde.c b/drivers/staging/media/tegra-vde/vde.c
index 859f60a70904..81f3e9e7e4d6 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -85,6 +85,96 @@ static int tegra_vde_wait_mbe(struct tegra_vde *vde)
 					  (tmp >= 0x10), 1, 100);
 }
 
+static int tegra_vde_alloc_bo(struct tegra_vde *vde,
+			      struct tegra_vde_bo **ret_bo,
+			      enum dma_data_direction dma_dir,
+			      size_t size)
+{
+	struct device *dev = vde->miscdev.parent;
+	struct tegra_vde_bo *bo;
+	int err;
+
+	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
+	if (!bo)
+		return -ENOMEM;
+
+	bo->vde = vde;
+	bo->size = size;
+	bo->dma_dir = dma_dir;
+	bo->dma_attrs = DMA_ATTR_WRITE_COMBINE |
+			DMA_ATTR_NO_KERNEL_MAPPING;
+
+	if (!vde->domain)
+		bo->dma_attrs |= DMA_ATTR_FORCE_CONTIGUOUS;
+
+	bo->dma_cookie = dma_alloc_attrs(dev, bo->size, &bo->dma_handle,
+					 GFP_KERNEL, bo->dma_attrs);
+	if (!bo->dma_cookie) {
+		dev_err(dev, "Failed to allocate DMA buffer of size: %zu\n",
+			bo->size);
+		err = -ENOMEM;
+		goto free_bo;
+	}
+
+	err = dma_get_sgtable_attrs(dev, &bo->sgt, bo->dma_cookie,
+				    bo->dma_handle, bo->size, bo->dma_attrs);
+	if (err) {
+		dev_err(dev, "Failed to get DMA buffer SG table: %d\n", err);
+		goto free_attrs;
+	}
+
+	err = dma_map_sgtable(dev, &bo->sgt, bo->dma_dir, bo->dma_attrs);
+	if (err) {
+		dev_err(dev, "Failed to map DMA buffer SG table: %d\n", err);
+		goto free_table;
+	}
+
+	if (vde->domain) {
+		err = tegra_vde_iommu_map(vde, &bo->sgt, &bo->iova, bo->size);
+		if (err) {
+			dev_err(dev, "Failed to map DMA buffer IOVA: %d\n", err);
+			goto unmap_sgtable;
+		}
+
+		bo->dma_addr = iova_dma_addr(&vde->iova, bo->iova);
+	} else {
+		bo->dma_addr = sg_dma_address(bo->sgt.sgl);
+	}
+
+	*ret_bo = bo;
+
+	return 0;
+
+unmap_sgtable:
+	dma_unmap_sgtable(dev, &bo->sgt, bo->dma_dir, bo->dma_attrs);
+free_table:
+	sg_free_table(&bo->sgt);
+free_attrs:
+	dma_free_attrs(dev, bo->size, bo->dma_cookie, bo->dma_handle,
+		       bo->dma_attrs);
+free_bo:
+	kfree(bo);
+
+	return err;
+}
+
+static void tegra_vde_free_bo(struct tegra_vde_bo *bo)
+{
+	struct tegra_vde *vde = bo->vde;
+	struct device *dev = vde->miscdev.parent;
+
+	if (vde->domain)
+		tegra_vde_iommu_unmap(vde, bo->iova);
+
+	dma_unmap_sgtable(dev, &bo->sgt, bo->dma_dir, bo->dma_attrs);
+
+	sg_free_table(&bo->sgt);
+
+	dma_free_attrs(dev, bo->size, bo->dma_cookie, bo->dma_handle,
+		       bo->dma_attrs);
+	kfree(bo);
+}
+
 static int tegra_vde_setup_mbe_frame_idx(struct tegra_vde *vde,
 					 unsigned int refs_nb,
 					 bool setup_refs)
@@ -425,6 +515,9 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde,
 
 	tegra_vde_writel(vde, bitstream_data_addr, vde->sxe, 0x6C);
 
+	if (vde->soc->supports_ref_pic_marking)
+		tegra_vde_writel(vde, vde->secure_bo->dma_addr, vde->sxe, 0x7c);
+
 	value = 0x10000005;
 	value |= ctx->pic_width_in_mbs << 11;
 	value |= ctx->pic_height_in_mbs << 3;
@@ -994,6 +1087,8 @@ static int tegra_vde_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, vde);
 
+	vde->soc = of_device_get_match_data(&pdev->dev);
+
 	vde->sxe = devm_platform_ioremap_resource_byname(pdev, "sxe");
 	if (IS_ERR(vde->sxe))
 		return PTR_ERR(vde->sxe);
@@ -1119,6 +1214,12 @@ static int tegra_vde_probe(struct platform_device *pdev)
 
 	pm_runtime_put(dev);
 
+	err = tegra_vde_alloc_bo(vde, &vde->secure_bo, DMA_FROM_DEVICE, 4096);
+	if (err) {
+		dev_err(dev, "Failed to allocate secure BO: %d\n", err);
+		goto err_pm_runtime;
+	}
+
 	return 0;
 
 err_pm_runtime:
@@ -1142,6 +1243,8 @@ static int tegra_vde_remove(struct platform_device *pdev)
 	struct tegra_vde *vde = platform_get_drvdata(pdev);
 	struct device *dev = &pdev->dev;
 
+	tegra_vde_free_bo(vde->secure_bo);
+
 	/*
 	 * As it increments RPM usage_count even on errors, we don't need to
 	 * check the returned code here.
@@ -1214,8 +1317,27 @@ static const struct dev_pm_ops tegra_vde_pm_ops = {
 				tegra_vde_pm_resume)
 };
 
+static const struct tegra_vde_soc tegra124_vde_soc = {
+	.supports_ref_pic_marking = true,
+};
+
+static const struct tegra_vde_soc tegra114_vde_soc = {
+	.supports_ref_pic_marking = true,
+};
+
+static const struct tegra_vde_soc tegra30_vde_soc = {
+	.supports_ref_pic_marking = false,
+};
+
+static const struct tegra_vde_soc tegra20_vde_soc = {
+	.supports_ref_pic_marking = false,
+};
+
 static const struct of_device_id tegra_vde_of_match[] = {
-	{ .compatible = "nvidia,tegra20-vde", },
+	{ .compatible = "nvidia,tegra124-vde", .data = &tegra124_vde_soc },
+	{ .compatible = "nvidia,tegra114-vde", .data = &tegra114_vde_soc },
+	{ .compatible = "nvidia,tegra30-vde", .data = &tegra30_vde_soc },
+	{ .compatible = "nvidia,tegra20-vde", .data = &tegra20_vde_soc },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, tegra_vde_of_match);
diff --git a/drivers/staging/media/tegra-vde/vde.h b/drivers/staging/media/tegra-vde/vde.h
index 5561291b0c88..bbd42b8d9991 100644
--- a/drivers/staging/media/tegra-vde/vde.h
+++ b/drivers/staging/media/tegra-vde/vde.h
@@ -24,6 +24,22 @@ struct iommu_domain;
 struct reset_control;
 struct dma_buf_attachment;
 
+struct tegra_vde_soc {
+	bool supports_ref_pic_marking;
+};
+
+struct tegra_vde_bo {
+	struct iova *iova;
+	struct sg_table sgt;
+	struct tegra_vde *vde;
+	enum dma_data_direction dma_dir;
+	unsigned long dma_attrs;
+	dma_addr_t dma_handle;
+	dma_addr_t dma_addr;
+	void *dma_cookie;
+	size_t size;
+};
+
 struct tegra_vde {
 	void __iomem *sxe;
 	void __iomem *bsev;
@@ -48,6 +64,8 @@ struct tegra_vde {
 	struct iova_domain iova;
 	struct iova *iova_resv_static_addresses;
 	struct iova *iova_resv_last_page;
+	const struct tegra_vde_soc *soc;
+	struct tegra_vde_bo *secure_bo;
 	dma_addr_t iram_lists_addr;
 	u32 *iram;
 };
-- 
2.33.1

[PATCH v2 2/3] media: staging: tegra-vde: Properly mark invalid entries

[Dmitry Osipenko]

From: Thierry Reding <treding@nvidia.com>

Entries in the reference picture list are marked as invalid by setting
the frame ID to 0x3f.

Signed-off-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/staging/media/tegra-vde/vde.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/tegra-vde/vde.c b/drivers/staging/media/tegra-vde/vde.c
index 81f3e9e7e4d6..f4f65eb78d44 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -340,7 +340,7 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde *vde,
 			value |= frame->frame_num;
 		} else {
 			aux_addr = 0x6ADEAD00;
-			value = 0;
+			value = 0x3f;
 		}
 
 		tegra_vde_setup_iram_entry(vde, 0, i, value, aux_addr);
-- 
2.33.1

[PATCH v2 3/3] media: staging: tegra-vde: Reorder misc device registration

[Dmitry Osipenko]

Register misc device in the end of driver's probing since device should
become visible to userspace once driver is fully prepared. Do the opposite
in case of driver removal. This is a minor improvement that doesn't solve
any problem, but makes code more consistent.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/staging/media/tegra-vde/vde.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/vde.c b/drivers/staging/media/tegra-vde/vde.c
index f4f65eb78d44..a8f1a024c343 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -1193,12 +1193,6 @@ static int tegra_vde_probe(struct platform_device *pdev)
 		goto err_gen_free;
 	}
 
-	err = misc_register(&vde->miscdev);
-	if (err) {
-		dev_err(dev, "Failed to register misc device: %d\n", err);
-		goto err_deinit_iommu;
-	}
-
 	pm_runtime_enable(dev);
 	pm_runtime_use_autosuspend(dev);
 	pm_runtime_set_autosuspend_delay(dev, 300);
@@ -1220,15 +1214,20 @@ static int tegra_vde_probe(struct platform_device *pdev)
 		goto err_pm_runtime;
 	}
 
+	err = misc_register(&vde->miscdev);
+	if (err) {
+		dev_err(dev, "Failed to register misc device: %d\n", err);
+		goto err_free_secure_bo;
+	}
+
 	return 0;
 
+err_free_secure_bo:
+	tegra_vde_free_bo(vde->secure_bo);
 err_pm_runtime:
-	misc_deregister(&vde->miscdev);
-
 	pm_runtime_dont_use_autosuspend(dev);
 	pm_runtime_disable(dev);
 
-err_deinit_iommu:
 	tegra_vde_iommu_deinit(vde);
 
 err_gen_free:
@@ -1243,6 +1242,8 @@ static int tegra_vde_remove(struct platform_device *pdev)
 	struct tegra_vde *vde = platform_get_drvdata(pdev);
 	struct device *dev = &pdev->dev;
 
+	misc_deregister(&vde->miscdev);
+
 	tegra_vde_free_bo(vde->secure_bo);
 
 	/*
@@ -1261,8 +1262,6 @@ static int tegra_vde_remove(struct platform_device *pdev)
 	pm_runtime_put_noidle(dev);
 	clk_disable_unprepare(vde->clk);
 
-	misc_deregister(&vde->miscdev);
-
 	tegra_vde_dmabuf_cache_unmap_all(vde);
 	tegra_vde_iommu_deinit(vde);
 
-- 
2.33.1

Re: [PATCH v2 0/3] Add NVIDIA Tegra114 support to video decoder driver

[Nicolas Dufresne]

Hi Dmitry,

Le lundi 15 novembre 2021 à 01:47 +0300, Dmitry Osipenko a écrit :
> Video decoder of Tegra114/124 SoCs uses additional memory buffer required
> for decoding of protected content. We won't support that content, but it
> is impossible to disable access to the buffer, hence a stub buffer needs
> to be provided. This series enables decoder driver only for Tegra114
> because Tegra124 support requires more non-trivial changes on both kernel
> and userspace sides.

I believe the stateless API is quite in place now, but I only see maintenance on
this staging driver. I don't believe it really make sense to keep maintaining a
staging driver without any step forward de-staging it. I believe it gives the
wrong message on the Kernel staging purpose.

I'm not criticizing your effort, I believe you are doing nice work for you
community, but would prefer to see this driver be ported to the official kernel
APIs rather then being maintain as staging till the end of time.

regards,
Nicolas

> 
> Changelog:
> 
> v2: - Changed tegra_vde_alloc_bo() to return errno and fix unassigned
>       error code in tegra_vde_probe().
> 
> Dmitry Osipenko (1):
>   media: staging: tegra-vde: Reorder misc device registration
> 
> Thierry Reding (2):
>   media: staging: tegra-vde: Support reference picture marking
>   media: staging: tegra-vde: Properly mark invalid entries
> 
>  drivers/staging/media/tegra-vde/vde.c | 147 +++++++++++++++++++++++---
>  drivers/staging/media/tegra-vde/vde.h |  18 ++++
>  2 files changed, 152 insertions(+), 13 deletions(-)
> 

Re: [PATCH v2 0/3] Add NVIDIA Tegra114 support to video decoder driver

[Hans Verkuil]

On 15/11/2021 15:43, Nicolas Dufresne wrote:
> Hi Dmitry,
> 
> Le lundi 15 novembre 2021 à 01:47 +0300, Dmitry Osipenko a écrit :
>> Video decoder of Tegra114/124 SoCs uses additional memory buffer required
>> for decoding of protected content. We won't support that content, but it
>> is impossible to disable access to the buffer, hence a stub buffer needs
>> to be provided. This series enables decoder driver only for Tegra114
>> because Tegra124 support requires more non-trivial changes on both kernel
>> and userspace sides.
> 
> I believe the stateless API is quite in place now, but I only see maintenance on
> this staging driver. I don't believe it really make sense to keep maintaining a
> staging driver without any step forward de-staging it. I believe it gives the
> wrong message on the Kernel staging purpose.
> 
> I'm not criticizing your effort, I believe you are doing nice work for you
> community, but would prefer to see this driver be ported to the official kernel
> APIs rather then being maintain as staging till the end of time.

I agree with Nicolas here. This driver only support H264 and the stateless API
for that is now in mainline. So there is no reason not to convert to the
stateless codec API and move this driver to mainline.

It would be really nice to see that happen.

Without any progress on that I am inclined to remove this driver some time
next year.

Regards,

	Hans

> 
> regards,
> Nicolas
> 
>>
>> Changelog:
>>
>> v2: - Changed tegra_vde_alloc_bo() to return errno and fix unassigned
>>       error code in tegra_vde_probe().
>>
>> Dmitry Osipenko (1):
>>   media: staging: tegra-vde: Reorder misc device registration
>>
>> Thierry Reding (2):
>>   media: staging: tegra-vde: Support reference picture marking
>>   media: staging: tegra-vde: Properly mark invalid entries
>>
>>  drivers/staging/media/tegra-vde/vde.c | 147 +++++++++++++++++++++++---
>>  drivers/staging/media/tegra-vde/vde.h |  18 ++++
>>  2 files changed, 152 insertions(+), 13 deletions(-)
>>
> 

Re: [PATCH v2 0/3] Add NVIDIA Tegra114 support to video decoder driver

[Dmitry Osipenko]

15.11.2021 17:49, Hans Verkuil пишет:
> On 15/11/2021 15:43, Nicolas Dufresne wrote:
>> Hi Dmitry,
>>
>> Le lundi 15 novembre 2021 à 01:47 +0300, Dmitry Osipenko a écrit :
>>> Video decoder of Tegra114/124 SoCs uses additional memory buffer required
>>> for decoding of protected content. We won't support that content, but it
>>> is impossible to disable access to the buffer, hence a stub buffer needs
>>> to be provided. This series enables decoder driver only for Tegra114
>>> because Tegra124 support requires more non-trivial changes on both kernel
>>> and userspace sides.
>>
>> I believe the stateless API is quite in place now, but I only see maintenance on
>> this staging driver. I don't believe it really make sense to keep maintaining a
>> staging driver without any step forward de-staging it. I believe it gives the
>> wrong message on the Kernel staging purpose.
>>
>> I'm not criticizing your effort, I believe you are doing nice work for you
>> community, but would prefer to see this driver be ported to the official kernel
>> APIs rather then being maintain as staging till the end of time.
> 
> I agree with Nicolas here. This driver only support H264 and the stateless API
> for that is now in mainline. So there is no reason not to convert to the
> stateless codec API and move this driver to mainline.
> 
> It would be really nice to see that happen.
> 
> Without any progress on that I am inclined to remove this driver some time
> next year.

I'll prioritize the v4l patches of this driver. The reason it wasn't
done yet is because the current custom UAPI works perfectly fine and I
was busy with a bit more important patches so far.