Re: Suggestion regarding x8 gang mode device tree changes

* Re: Suggestion regarding x8 gang mode device tree changes
       [not found] <e253fee3-5358-aaf1-d317-162dc8e98afc@nvidia.com>
@ 2020-10-29  9:33 ` Hans Verkuil
  2020-10-29 15:34   ` Sowjanya Komatineni
  2020-10-29 14:50 ` Sakari Ailus
  1 sibling, 1 reply; 14+ messages in thread
From: Hans Verkuil @ 2020-10-29  9:33 UTC (permalink / raw)
  To: Sowjanya Komatineni, Sakari Ailus, Thierry Reding, linux-media

On 29/10/2020 02:48, Sowjanya Komatineni wrote:
> Hi Sakari,
> 
> Missed to add you to below patch series for HDMI2CSI bridge support
> 
> https://patchwork.kernel.org/project/linux-media/cover/1603768763-25590-1-git-send-email-skomatineni@nvidia.com/
> 
> Patch-10 of this series is for x8 capture from HDMI2CSI bridge.
> 
> Would like to get your suggestion on x8 gang/combined ports capture implementation.
> 
> 
> Tegra VI/CSI don't have native x8. HDMI2CSI bridges uses dual ports with split image on to each port.
> So on Tegra side at SW level we can use dual ports as gang where both ports are simultaneously programmed for combined image capture.
> 
> Currently v2 patches use bus-width with V4L2 parallel bus type for this x8 gang mode implementation.
> So, driver parses endpoint and if bus type is parallel (for x8), it uses v4l2_ep.bus.parallel.bus_width other wise (x4 and below) it uses
> v4l2_ep.bus.mipi_csi2.num_data_lanes for getting lanes from DT.
> 
> x8 is not from native HW and current v2 version does not explicitly add 2nd port in device tree and driver uses consecutive ports as
> combined capture for same video device node.
> 
> From offline discussion with Hans, looks like its better to explicitly specify both ports used as gang/combined capture in device tree on
> Tegra side and bridge side also will expose both TX ports.
> 
> Current CSI driver implementation uses max 2 media pads port@0 as Sink and port@1 as Source. So can add 2nd endpoint in sink node and source
> nodes.
> 
> Below is tc358840 and csi device node change to explicitly add second source.
> 
> With this, VI/CSI driver can be updated to use second port only for identifying total combined ports for capture.
> 
> My understanding is there is no need for creating media links for 2nd port as both ports together are used as combined ports by the Tegra
> driver during streaming for the same csi subdev and video device nodes.

In my opinion both links should be shown in the media controller topology.
Basically these are just independent CSI ports (two independent TX ports on
the tc358840 and two independent RX ports on the Tegra SoC side), that are
combined afterwards into a single video stream.

While this is an HDMI receiver, it is conceivable to have two sensors instead
that combine to a single 3D side-by-side image. In that case each CSI port on
the Tegra goes to a separate sensor. You want to model this.

Regards,

	Hans

> 
> Please provide your suggestions/comments on this so I can take care of exposing both combined ports in v3.
> 
> tc358840@1f {
>     ...
>     ...
>     ports {
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         port@0 {
>             reg = <0>;
>             tc358840_out0: endpoint {
>                 link-frequencies = /bits/ 64 <297000000>;
>                 clock-lanes = <0>;
>                 data-lanes = <1 2 3 4>;
>                 remote-endpoint = <&tc358840_csi_in0>;
>             };
>         };
> 
>         port@1 {
>             reg = <1>;
>             tc358840_out1: endpoint {
>                 link-frequencies = /bits/ 64 <297000000>;
>                 clock-lanes = <0>;
>                 data-lanes = <1 2 3 4>;
>                 remote-endpoint = <&tc358840_csi_in1>;
>             };
>         };
>     };
> };
> 
> csi_chan: channel@2 {
>     reg = <2>     /* CSI Port No */
>     ..
>     ..
>     ports {
>         /* port@0 always for Sink pads */
>         port@0 {
>             reg = <0>;  /* Media Pad-0 (Sink) */
>             tc358840_csi_in0: endpoint@0 {
>                 reg = <0>;
>                 data-lanes = <1 2 3 4>;
>                 remote-endpoint = <&tc358840_tx0>;
>             };
> 
>             tc358840_csi_in1: endpoint@1 {
>                 reg = <1>;
>                 data-lanes = <1 2 3 4>;
>                 remote-endpoint = <&tc358840_tx1>;
>             };
>         }
> 
>         /* port@1 always Source pads */
>         port@1 {
>             reg = <1>;  /* Media Pad-1 (Source) */
>             tc358840_csi_in0: endpoint@0 {
>                 reg = <0>;
>                 remote-endpoint = <&tc358840_vi_in0>;
>             };
> 
>             tc358840_csi_in0: endpoint@1 {
>                 reg = <1>;
>                 remote-endpoint = <&tc358840_vi_in1>;
>             };
>         }
>     }
> }
> 
> 
> Thanks
> 
> Sowjanya
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread