Re: [PATCH v1 05/19] staging: media: tegra-video: expand VI and VIP support to Tegra30

[Mikko Perttunen <mperttunen@nvidia.com>]

On Wednesday, August 27, 2025 1:47 PM Svyatoslav wrote:
> 27 серпня 2025 р. 07:29:40 GMT+03:00, Mikko Perttunen 
<mperttunen@nvidia.com> пише:
> >On Tuesday, August 19, 2025 9:16 PM Svyatoslav Ryhel wrote:
> >> Exisitng VI and VIP implementation for Tegra20 is fully compatible with
> >> Tegra30.
> >> 
> >> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> >> ---
> >> 
> >>  drivers/staging/media/tegra-video/Makefile | 1 +
> >>  drivers/staging/media/tegra-video/vi.c     | 3 +++
> >>  drivers/staging/media/tegra-video/vi.h     | 2 +-
> >>  drivers/staging/media/tegra-video/video.c  | 4 ++++
> >>  drivers/staging/media/tegra-video/vip.c    | 5 ++++-
> >>  5 files changed, 13 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/staging/media/tegra-video/Makefile
> >> b/drivers/staging/media/tegra-video/Makefile index
> >> 6c7552e05109..96380b5dbd8b 100644
> >> --- a/drivers/staging/media/tegra-video/Makefile
> >> +++ b/drivers/staging/media/tegra-video/Makefile
> >> @@ -6,5 +6,6 @@ tegra-video-objs := \
> >> 
> >>  		csi.o
> >>  
> >>  tegra-video-$(CONFIG_ARCH_TEGRA_2x_SOC)  += tegra20.o
> >> 
> >> +tegra-video-$(CONFIG_ARCH_TEGRA_3x_SOC)  += tegra20.o
> >> 
> >>  tegra-video-$(CONFIG_ARCH_TEGRA_210_SOC) += tegra210.o
> >>  obj-$(CONFIG_VIDEO_TEGRA) += tegra-video.o
> >> 
> >> diff --git a/drivers/staging/media/tegra-video/vi.c
> >> b/drivers/staging/media/tegra-video/vi.c index c9276ff76157..71be205cacb5
> >> 100644
> >> --- a/drivers/staging/media/tegra-video/vi.c
> >> +++ b/drivers/staging/media/tegra-video/vi.c
> >> @@ -1959,6 +1959,9 @@ static const struct of_device_id
> >> tegra_vi_of_id_table[] = { #if defined(CONFIG_ARCH_TEGRA_2x_SOC)
> >> 
> >>  	{ .compatible = "nvidia,tegra20-vi",  .data = &tegra20_vi_soc },
> >>  
> >>  #endif
> >> 
> >> +#if defined(CONFIG_ARCH_TEGRA_3x_SOC)
> >> +	{ .compatible = "nvidia,tegra30-vi",  .data = &tegra20_vi_soc },
> >> +#endif
> >> 
> >>  #if defined(CONFIG_ARCH_TEGRA_210_SOC)
> >>  
> >>  	{ .compatible = "nvidia,tegra210-vi", .data = &tegra210_vi_soc },
> >>  
> >>  #endif
> >> 
> >> diff --git a/drivers/staging/media/tegra-video/vi.h
> >> b/drivers/staging/media/tegra-video/vi.h index 1e6a5caa7082..cac0c0d0e225
> >> 100644
> >> --- a/drivers/staging/media/tegra-video/vi.h
> >> +++ b/drivers/staging/media/tegra-video/vi.h
> >> @@ -296,7 +296,7 @@ struct tegra_video_format {
> >> 
> >>  	u32 fourcc;
> >>  
> >>  };
> >> 
> >> -#if defined(CONFIG_ARCH_TEGRA_2x_SOC)
> >> +#if defined(CONFIG_ARCH_TEGRA_2x_SOC) ||
> >> defined(CONFIG_ARCH_TEGRA_3x_SOC)
> >> 
> >>  extern const struct tegra_vi_soc tegra20_vi_soc;
> >>  #endif
> >>  #if defined(CONFIG_ARCH_TEGRA_210_SOC)
> >> 
> >> diff --git a/drivers/staging/media/tegra-video/video.c
> >> b/drivers/staging/media/tegra-video/video.c index
> >> 074ad0dc56ca..a25885f93cd7 100644
> >> --- a/drivers/staging/media/tegra-video/video.c
> >> +++ b/drivers/staging/media/tegra-video/video.c
> >> @@ -127,6 +127,10 @@ static const struct of_device_id
> >> host1x_video_subdevs[] = { { .compatible = "nvidia,tegra20-vip", },
> >> 
> >>  	{ .compatible = "nvidia,tegra20-vi", },
> >>  
> >>  #endif
> >> 
> >> +#if defined(CONFIG_ARCH_TEGRA_3x_SOC)
> >> +	{ .compatible = "nvidia,tegra30-vip", },
> >> +	{ .compatible = "nvidia,tegra30-vi", },
> >> +#endif
> >> 
> >>  #if defined(CONFIG_ARCH_TEGRA_210_SOC)
> >>  
> >>  	{ .compatible = "nvidia,tegra210-csi", },
> >>  	{ .compatible = "nvidia,tegra210-vi", },
> >> 
> >> diff --git a/drivers/staging/media/tegra-video/vip.c
> >> b/drivers/staging/media/tegra-video/vip.c index
> >> 5ec717f3afd5..00e08a9971d5
> >> 100644
> >> --- a/drivers/staging/media/tegra-video/vip.c
> >> +++ b/drivers/staging/media/tegra-video/vip.c
> >> @@ -263,13 +263,16 @@ static void tegra_vip_remove(struct platform_device
> >> *pdev) pm_runtime_disable(&pdev->dev);
> >> 
> >>  }
> >> 
> >> -#if defined(CONFIG_ARCH_TEGRA_2x_SOC)
> >> +#if defined(CONFIG_ARCH_TEGRA_2x_SOC) ||
> >> defined(CONFIG_ARCH_TEGRA_3x_SOC)
> >> 
> >>  extern const struct tegra_vip_soc tegra20_vip_soc;
> >>  #endif
> >>  
> >>  static const struct of_device_id tegra_vip_of_id_table[] = {
> >>  #if defined(CONFIG_ARCH_TEGRA_2x_SOC)
> >>  
> >>  	{ .compatible = "nvidia,tegra20-vip", .data = &tegra20_vip_soc },
> >> 
> >> +#endif
> >> +#if defined(CONFIG_ARCH_TEGRA_3x_SOC)
> >> +	{ .compatible = "nvidia,tegra30-vip", .data = &tegra20_vip_soc },
> >> 
> >>  #endif
> >>  
> >>  	{ }
> >>  
> >>  };
> >
> >If tegra30-vip is compatible with tegra20-vip, we don't need to add the
> >compatible string into the driver. Just mark it as 'compatible =
> >"nvidia,tegra30-vip", "nvidia,tegra20-vip";' in the device tree (and as Rob
> >alluded, have this compat string pair as an option in the device tree
> >schema).
> While I am fine with using fallback but it may be a good idea to have a
> separate compatible so in case tegra30 would need a specific set of ops
> (tegra20 and tegra30 VIs are not exact match) no additional changes into
> schema would be required.

The standard practice is to use the compatible string fallback in device tree 
when we believe the hardware to be compatible. If that later turns out not to 
be the case (which should be unlikely), the schema can be adjusted along with 
the source code changes.

> >Cheers,
> >Mikko