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

[PATCH v1 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.

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 | 143 +++++++++++++++++++++++---
 drivers/staging/media/tegra-vde/vde.h |  18 ++++
 2 files changed, 148 insertions(+), 13 deletions(-)

-- 
2.33.1

[PATCH v1 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>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/staging/media/tegra-vde/vde.c | 120 +++++++++++++++++++++++++-
 drivers/staging/media/tegra-vde/vde.h |  18 ++++
 2 files changed, 137 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/tegra-vde/vde.c b/drivers/staging/media/tegra-vde/vde.c
index 859f60a70904..42da57f191ef 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -85,6 +85,92 @@ static int tegra_vde_wait_mbe(struct tegra_vde *vde)
 					  (tmp >= 0x10), 1, 100);
 }
 
+static struct tegra_vde_bo *
+tegra_vde_alloc_bo(struct tegra_vde *vde, 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 NULL;
+
+	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);
+		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);
+	}
+
+	return bo;
+
+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 NULL;
+}
+
+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 +511,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 +1083,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 +1210,12 @@ static int tegra_vde_probe(struct platform_device *pdev)
 
 	pm_runtime_put(dev);
 
+	vde->secure_bo = tegra_vde_alloc_bo(vde, DMA_FROM_DEVICE, 4096);
+	if (!vde->secure_bo) {
+		dev_err(dev, "Failed to allocate secure BO\n");
+		goto err_pm_runtime;
+	}
+
 	return 0;
 
 err_pm_runtime:
@@ -1142,6 +1239,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 +1313,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 v1 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 42da57f191ef..ba7ef280423b 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -336,7 +336,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 v1 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 ba7ef280423b..404d963690c5 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -1189,12 +1189,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);
@@ -1216,15 +1210,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:
@@ -1239,6 +1238,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);
 
 	/*
@@ -1257,8 +1258,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 v1 1/3] media: staging: tegra-vde: Support reference picture marking

[Dmitry Osipenko]

15.11.2021 01:23, Dmitry Osipenko пишет:
> +	vde->secure_bo = tegra_vde_alloc_bo(vde, DMA_FROM_DEVICE, 4096);
> +	if (!vde->secure_bo) {
> +		dev_err(dev, "Failed to allocate secure BO\n");
> +		goto err_pm_runtime;
> +	}

My eye just caught that by accident err variable isn't assigned to
-ENOMEM here. I'll make v2 shortly.

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

[Dan Carpenter]

On Mon, Nov 15, 2021 at 01:34:18AM +0300, Dmitry Osipenko wrote:
> 15.11.2021 01:23, Dmitry Osipenko пишет:
> > +	vde->secure_bo = tegra_vde_alloc_bo(vde, DMA_FROM_DEVICE, 4096);
> > +	if (!vde->secure_bo) {
> > +		dev_err(dev, "Failed to allocate secure BO\n");
> > +		goto err_pm_runtime;
> > +	}
> 
> My eye just caught that by accident err variable isn't assigned to
> -ENOMEM here. I'll make v2 shortly.

Smatch has a check for this so we would have caught it.  :)

regards,
dan carpenter

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

[Dmitry Osipenko]

15.11.2021 15:44, Dan Carpenter пишет:
> On Mon, Nov 15, 2021 at 01:34:18AM +0300, Dmitry Osipenko wrote:
>> 15.11.2021 01:23, Dmitry Osipenko пишет:
>>> +	vde->secure_bo = tegra_vde_alloc_bo(vde, DMA_FROM_DEVICE, 4096);
>>> +	if (!vde->secure_bo) {
>>> +		dev_err(dev, "Failed to allocate secure BO\n");
>>> +		goto err_pm_runtime;
>>> +	}
>>
>> My eye just caught that by accident err variable isn't assigned to
>> -ENOMEM here. I'll make v2 shortly.
> 
> Smatch has a check for this so we would have caught it.  :)

Whish smatch was built-in into kernel and I could simply run "make
smatch". On the other hand, I know that you're periodically checking
upstream with smatch and patching the found bugs, so maybe it's fine
as-is. Thank you for yours work on smatch, it's a very valuable tool.

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

[Dan Carpenter]

On Mon, Nov 15, 2021 at 05:34:47PM +0300, Dmitry Osipenko wrote:
> 15.11.2021 15:44, Dan Carpenter пишет:
> > On Mon, Nov 15, 2021 at 01:34:18AM +0300, Dmitry Osipenko wrote:
> >> 15.11.2021 01:23, Dmitry Osipenko пишет:
> >>> +	vde->secure_bo = tegra_vde_alloc_bo(vde, DMA_FROM_DEVICE, 4096);
> >>> +	if (!vde->secure_bo) {
> >>> +		dev_err(dev, "Failed to allocate secure BO\n");
> >>> +		goto err_pm_runtime;
> >>> +	}
> >>
> >> My eye just caught that by accident err variable isn't assigned to
> >> -ENOMEM here. I'll make v2 shortly.
> > 
> > Smatch has a check for this so we would have caught it.  :)
> 
> Whish smatch was built-in into kernel and I could simply run "make
> smatch". On the other hand, I know that you're periodically checking
> upstream with smatch and patching the found bugs, so maybe it's fine
> as-is.

I run it every day, and it's integrated into the zero day bot and Mauro
runs it.  So someone would have caught it.

regards,
dan carpenter

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

[Dmitry Osipenko]

15.11.2021 18:48, Dan Carpenter пишет:
> On Mon, Nov 15, 2021 at 05:34:47PM +0300, Dmitry Osipenko wrote:
>> 15.11.2021 15:44, Dan Carpenter пишет:
>>> On Mon, Nov 15, 2021 at 01:34:18AM +0300, Dmitry Osipenko wrote:
>>>> 15.11.2021 01:23, Dmitry Osipenko пишет:
>>>>> +	vde->secure_bo = tegra_vde_alloc_bo(vde, DMA_FROM_DEVICE, 4096);
>>>>> +	if (!vde->secure_bo) {
>>>>> +		dev_err(dev, "Failed to allocate secure BO\n");
>>>>> +		goto err_pm_runtime;
>>>>> +	}
>>>>
>>>> My eye just caught that by accident err variable isn't assigned to
>>>> -ENOMEM here. I'll make v2 shortly.
>>>
>>> Smatch has a check for this so we would have caught it.  :)
>>
>> Whish smatch was built-in into kernel and I could simply run "make
>> smatch". On the other hand, I know that you're periodically checking
>> upstream with smatch and patching the found bugs, so maybe it's fine
>> as-is.
> 
> I run it every day, and it's integrated into the zero day bot and Mauro
> runs it.  So someone would have caught it.

Very nice, I haven't noticed that 0-day runs smatch. I assume the smatch
reports from the bots are private to you and Mauro since I never saw
them in my inbox and on LKML, correct?

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

[Dan Carpenter]

On Wed, Nov 17, 2021 at 07:19:30PM +0300, Dmitry Osipenko wrote:
> 15.11.2021 18:48, Dan Carpenter пишет:
> > On Mon, Nov 15, 2021 at 05:34:47PM +0300, Dmitry Osipenko wrote:
> >> 15.11.2021 15:44, Dan Carpenter пишет:
> >>> On Mon, Nov 15, 2021 at 01:34:18AM +0300, Dmitry Osipenko wrote:
> >>>> 15.11.2021 01:23, Dmitry Osipenko пишет:
> >>>>> +	vde->secure_bo = tegra_vde_alloc_bo(vde, DMA_FROM_DEVICE, 4096);
> >>>>> +	if (!vde->secure_bo) {
> >>>>> +		dev_err(dev, "Failed to allocate secure BO\n");
> >>>>> +		goto err_pm_runtime;
> >>>>> +	}
> >>>>
> >>>> My eye just caught that by accident err variable isn't assigned to
> >>>> -ENOMEM here. I'll make v2 shortly.
> >>>
> >>> Smatch has a check for this so we would have caught it.  :)
> >>
> >> Whish smatch was built-in into kernel and I could simply run "make
> >> smatch". On the other hand, I know that you're periodically checking
> >> upstream with smatch and patching the found bugs, so maybe it's fine
> >> as-is.
> > 
> > I run it every day, and it's integrated into the zero day bot and Mauro
> > runs it.  So someone would have caught it.
> 
> Very nice, I haven't noticed that 0-day runs smatch. I assume the smatch
> reports from the bots are private to you and Mauro since I never saw
> them in my inbox and on LKML, correct?

The 0-day bot is not great about running Smatch honestly...  It never
ran Smatch on this.  It would have replied to the thread.

regards,
dan carpenter

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

[Dan Carpenter]

It's not hard to run Smatch yourself...

Depending on if you're on a apt distro or yum distro then fetch the
dependencies with one of the follow commands:
apt-get install gcc make sqlite3 libsqlite3-dev libdbd-sqlite3-perl libssl-dev libtry-tiny-perl
yum install gcc make sqlite3 sqlite-devel sqlite perl-DBD-SQLite openssl-devel perl-Try-Tiny

git clone https://github.com/error27/smatch
cd smatch
make
cd ~/kernel_source/
~/smatch/smatch_scripts/kchecker drivers/subsystem/

regards,
dan carpenter

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

[Joe Perches]

On Thu, 2021-11-18 at 09:14 +0300, Dan Carpenter wrote:
> It's not hard to run Smatch yourself...
> 
> Depending on if you're on a apt distro or yum distro then fetch the
> dependencies with one of the follow commands:
> apt-get install gcc make sqlite3 libsqlite3-dev libdbd-sqlite3-perl libssl-dev libtry-tiny-perl
> yum install gcc make sqlite3 sqlite-devel sqlite perl-DBD-SQLite openssl-devel perl-Try-Tiny
> 
> git clone https://github.com/error27/smatch
> cd smatch
> make
> cd ~/kernel_source/
> ~/smatch/smatch_scripts/kchecker drivers/subsystem/

Might want to stick something in Documentation about that.

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

[Dan Carpenter]

On Wed, Nov 17, 2021 at 10:21:00PM -0800, Joe Perches wrote:
> On Thu, 2021-11-18 at 09:14 +0300, Dan Carpenter wrote:
> > It's not hard to run Smatch yourself...
> > 
> > Depending on if you're on a apt distro or yum distro then fetch the
> > dependencies with one of the follow commands:
> > apt-get install gcc make sqlite3 libsqlite3-dev libdbd-sqlite3-perl libssl-dev libtry-tiny-perl
> > yum install gcc make sqlite3 sqlite-devel sqlite perl-DBD-SQLite openssl-devel perl-Try-Tiny
> > 
> > git clone https://github.com/error27/smatch
> > cd smatch
> > make
> > cd ~/kernel_source/
> > ~/smatch/smatch_scripts/kchecker drivers/subsystem/
> 
> Might want to stick something in Documentation about that.
> 

That's a good idea.  I was trying to figure out where it should go and
I looked at Documentation/process/submit-checklist.rst and point 4 had
me a bit puzzled:

4) ppc64 is a good architecture for cross-compilation checking because it
   tends to use ``unsigned long`` for 64-bit quantities.

It took me a while to realize that this must have been written when 64
bit systems were new and rare.  :P  This documentation is old old old...

regards,
dan carpenter

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

[Dan Carpenter]

Thanks, Joe.

I'll create a Documentation/dev-tools/smatch.rst file.

regards,
dan carpenter

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

[Dmitry Osipenko]

18.11.2021 09:14, Dan Carpenter пишет:
> It's not hard to run Smatch yourself...
> 
> Depending on if you're on a apt distro or yum distro then fetch the
> dependencies with one of the follow commands:
> apt-get install gcc make sqlite3 libsqlite3-dev libdbd-sqlite3-perl libssl-dev libtry-tiny-perl
> yum install gcc make sqlite3 sqlite-devel sqlite perl-DBD-SQLite openssl-devel perl-Try-Tiny
> 
> git clone https://github.com/error27/smatch
> cd smatch
> make
> cd ~/kernel_source/
> ~/smatch/smatch_scripts/kchecker drivers/subsystem/

Thanks, I was running Smatch couple times in the past. Finding how to
run Smatch isn't the problem, the thing is that Smatch either isn't
packaged by distros or packaged version is outdated, hence there is a
need to maintain it by yourself.

Also, is it guaranteed that Smatch will always work properly with
linux-next?

I imagine more developers could start to engage in using Smatch if
kernel supported 'make smatch' command which would automate the process
of fetching, building and running Smatch.

Couldn't the "kernel" version of Smatch reside in the kernel's tools/?
Or maybe just the parts of Smatch that are necessary for kernel
checking, like kernel's DB/scripts and etc. Doesn't it make sense?

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

[Dan Carpenter]

On Thu, Nov 18, 2021 at 04:56:38PM +0300, Dmitry Osipenko wrote:
> 18.11.2021 09:14, Dan Carpenter пишет:
> > It's not hard to run Smatch yourself...
> > 
> > Depending on if you're on a apt distro or yum distro then fetch the
> > dependencies with one of the follow commands:
> > apt-get install gcc make sqlite3 libsqlite3-dev libdbd-sqlite3-perl libssl-dev libtry-tiny-perl
> > yum install gcc make sqlite3 sqlite-devel sqlite perl-DBD-SQLite openssl-devel perl-Try-Tiny
> > 
> > git clone https://github.com/error27/smatch 
> > cd smatch
> > make
> > cd ~/kernel_source/
> > ~/smatch/smatch_scripts/kchecker drivers/subsystem/
> 
> Thanks, I was running Smatch couple times in the past. Finding how to
> run Smatch isn't the problem, the thing is that Smatch either isn't
> packaged by distros or packaged version is outdated, hence there is a
> need to maintain it by yourself.
> 
> Also, is it guaranteed that Smatch will always work properly with
> linux-next?

I work against linux-next every day so generally, yes.  But that reminds
me that linux-next broke while I was on vacation and I haven't yet
pushed the fixes.

> 
> I imagine more developers could start to engage in using Smatch if
> kernel supported 'make smatch' command which would automate the process
> of fetching, building and running Smatch.
> 
> Couldn't the "kernel" version of Smatch reside in the kernel's tools/?
> Or maybe just the parts of Smatch that are necessary for kernel
> checking, like kernel's DB/scripts and etc. Doesn't it make sense?

I'm not sure that makes sense really...  I'll expand on that in a bit
but the shorter answer is also that I don't have the bandwidth to make
it work.  I just suck at releases and testing.  So this would bitrot and
be horrible.

Smatch does need a better way to manage data for other projects.  Right
now linux-next is the first class citizen.  It's the only thing where
I'm positive that it gets tested regularly.  All the data in
smatch_data/ is from linux-next.

And also there should be a better way to check specific version of the
kernel because people quite often use the same directory and just check
out v4.12 to test that and switch back.  I do that and I've got scripts
on my system ./switch_to_tree4v1.sh which set up the symlinks for me.

But for linux-next it's fine.  Also by the time kernels have been
released the remaining Smatch warnings are almost all false positives.

To me the data in smatch_data/ is not so important as the cross function
database.  And the cross function database can't be distributed.  It's
too huge and it's specific to a given .config.

regards,
dan carpenter

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

[Dmitry Osipenko]

19.11.2021 15:30, Dan Carpenter пишет:
> On Thu, Nov 18, 2021 at 04:56:38PM +0300, Dmitry Osipenko wrote:
>> 18.11.2021 09:14, Dan Carpenter пишет:
>>> It's not hard to run Smatch yourself...
>>>
>>> Depending on if you're on a apt distro or yum distro then fetch the
>>> dependencies with one of the follow commands:
>>> apt-get install gcc make sqlite3 libsqlite3-dev libdbd-sqlite3-perl libssl-dev libtry-tiny-perl
>>> yum install gcc make sqlite3 sqlite-devel sqlite perl-DBD-SQLite openssl-devel perl-Try-Tiny
>>>
>>> git clone https://github.com/error27/smatch 
>>> cd smatch
>>> make
>>> cd ~/kernel_source/
>>> ~/smatch/smatch_scripts/kchecker drivers/subsystem/
>>
>> Thanks, I was running Smatch couple times in the past. Finding how to
>> run Smatch isn't the problem, the thing is that Smatch either isn't
>> packaged by distros or packaged version is outdated, hence there is a
>> need to maintain it by yourself.
>>
>> Also, is it guaranteed that Smatch will always work properly with
>> linux-next?
> 
> I work against linux-next every day so generally, yes.  But that reminds
> me that linux-next broke while I was on vacation and I haven't yet
> pushed the fixes.
> 
>>
>> I imagine more developers could start to engage in using Smatch if
>> kernel supported 'make smatch' command which would automate the process
>> of fetching, building and running Smatch.
>>
>> Couldn't the "kernel" version of Smatch reside in the kernel's tools/?
>> Or maybe just the parts of Smatch that are necessary for kernel
>> checking, like kernel's DB/scripts and etc. Doesn't it make sense?
> 
> I'm not sure that makes sense really...  I'll expand on that in a bit
> but the shorter answer is also that I don't have the bandwidth to make
> it work.  I just suck at releases and testing.  So this would bitrot and
> be horrible.
> 
> Smatch does need a better way to manage data for other projects.  Right
> now linux-next is the first class citizen.  It's the only thing where
> I'm positive that it gets tested regularly.  All the data in
> smatch_data/ is from linux-next.
> 
> And also there should be a better way to check specific version of the
> kernel because people quite often use the same directory and just check
> out v4.12 to test that and switch back.  I do that and I've got scripts
> on my system ./switch_to_tree4v1.sh which set up the symlinks for me.
> 
> But for linux-next it's fine.  Also by the time kernels have been
> released the remaining Smatch warnings are almost all false positives.
> 
> To me the data in smatch_data/ is not so important as the cross function
> database.  And the cross function database can't be distributed.  It's
> too huge and it's specific to a given .config.

Thank you for the clarification.