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

Sowjanya Komatineni
2020-04-26T04:47:54Z

On 4/25/20 9:42 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 26.04.2020 05:10, Dmitry Osipenko пишет:
>> 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?
>>
>> ...
>>>> The driver will need to have a bit better separation if it's supposed to
>>>> have a common core for all SoCs. Each incompatible VI/CSI hardware
>>>> version should have its own kernel module.
>>> currently other Tegra host1x driver (drm) also does similar. Single
>>> module for all Tegra SoCs.
>> DRM driver has a proper separation of the sub-drivers where sub-driver
>> won't load on unsupported hardware. The tegra-video driver should do the
>> same, i.e. VI and CSI should be individual drivers (and not OPS). There
>> could be a some common core, but for now it's not obvious to me what
>> that core should be, maybe just the video.c.
> Although, you're right that tegra_drm is compiled as a single module.
> That's not good, I'm actually not sure now whether it is possible to
> modularize host1x drivers properly without changing the whole host1x bus.

We already went thru driver structure changes during earlier versions
and internal feedbacks.

Not sure of restructuring again now. Also reg wastage of space, tegra
specific implementation is not huge also for T186 and later we have
RTCPU firmware that driver communicates with and it light weight. Based
on internal discussion, no upstream support for prior Tegra210.

So, probably what we have regarding structure is ok except video formats
I will move to Tegra210 and change defines to use Tegra210 prefix.