Re: [RFC PATCH v10 6/9] media: tegra: Add Tegra210 Video input driver

[Sowjanya Komatineni]

On 4/25/20 7:48 PM, Sowjanya Komatineni wrote:
> On 4/25/20 7:38 PM, Dmitry Osipenko wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > 26.04.2020 05:19, Sowjanya Komatineni ÐÐÑÐÑ:
> > > On 4/25/20 7:10 PM, Dmitry Osipenko wrote:
> > > > External email: Use caution opening links or attachments
> > > >
> > > >
> > > > 26.04.2020 04:43, Sowjanya Komatineni ÐÐÑÐÑ:
> > > > ...
> > > > > > It looks to me that at least all those hardcoded HW format IDs do 
> > > > > > not
> > > > > > match the older SoCs.
> > > > > TPG hard coded formats are supported on prior Tegra.
> > > > >
> > > > > Other supported formats are SoC dependent and part of soc data in 
> > > > > the
> > > > > driver already.
> > > > But I don't see where that SoC-dependent definition is made in
> > > > terga210.c. That tegra_image_format enum looks T210-specific, isn't 
> > > > it?
> > > >
> > > > ...
> > > Video formats which are SoC variants are made soc specific in driver
> > > already tegra_vi_soc structure member video_formats
> > >
> > > tegra_image_format enum is same for T210 and T186
> > >
> > > For T194, enums will be diff and will have diff TEGRA194_VIDEO_FORMAT
> > > using corresponding Tegra194 video format enums
> > But it is also not the same for older SoCs, correct? All the
> > T210-specific things should be separated better, unique parts shouldn't
> > be kept in the common code.
> >
> > Hence the tegra_image_format should be renamed to tegra210_image_format
> > and moved out to t210.h, since it's not common. But then you'll probably
> > need to rename all TEGRA_ defines to TEGRA210_ to make t210.h reusable
> > by T186.
>
> We can't make t210.h reusable for t186 as all register defines are diff.
>
> only video format enums are same b/w them so to reuse that for t186 I 
> had that in common.
>
> Regarding defines, will change prefix as Tegra210
>
> > Also, in the end it may not worth the effort to share anything at all,
> > it could be cleaner to have a bit of duplication. Although, I have no
> > idea how T186 code will look like and what other parts of T210 could be
> > reused by T186. All this needs to be taken into account in order to
> > avoid struggling with the code's reshuffling in the future.
>
> Currently as image formats are same for t210 and t186 I had them in 
> common.h and for t194 where they are diff new enums will be added.
>
> Other tegra210 soc specific only are all part of tegra210.c/h
Will move video formats to tegra specific file (tegra210.c)...