Re: [PATCH v3 2/9] media: platform: Add c3 mipi csi2 driver

[Tomi Valkeinen]

Hi,

On 05/11/2024 13:39, Jacopo Mondi wrote:

> > > Could you clarify what other streams you plan to support ? As you
> > > support routing I presume you are preparing to capture
> > > multiple streams of data like image + embedded data, or to support
> > > sensors which sends data on different virtual channels ?
> > >
> > > How do you see this driver evolve ? Will it be augmented with an
> > > additional source pad directed to a video device where to capture
> > > embedded data from ?
> > >
> > > I'm wondering because if PAD_SINK become multiplexed, you won't be
> > > allowed to set a format there. It only works now because you have a
> > > single stream.
> > >
> > > Speaking of which, as you prepare to support multiple streams, I would
> > > specify the stream number when calling v4l2_subdev_state_get_format().
> > >
> > >           fmt = v4l2_subdev_state_get_format(state, format->pad, 0);
> > Thanks for your suggestion.
> >
> > But this MIPI CSI2 hardware module doesn't have the ability to separate data
> > , such as image + embedded data.
> >
> > So there are no plans to support other streams.
>
> I see. Now that I've reviewed the adapter subdevice path I realized
> that it's the adapter that can split data based on VC/DT to either the
> ISP direct path or to DDR.
>
> The CSI-2 RX then transports streams as received from the sensor
> directly to the adapter.
>
> In order to support capturing embedded data to DDR in the adapter the
> embedded data stream needs a representation in this subdevice as well,
> so that the source pad is multiplexed as well and the adapter receives
> two streams that it can eventually split.
>
>
>        Sensor         CSI-2Rx         Adapter
>     +-----------+     +------+       +--------+
>     0-- ED --\  |     |      |       |     /->0---> Embedded Data (DDR)
>     |         ->0====>0 ==== 0 ====> 0====|   |
>     0-- I ---/  |     |      |       |     \->0--->    Image (ISP)
>     +-----------+     +------+       +--------+
>
> When going to support embedded data capture this driver should create
> two routes and allow enabling/disabling the embedded data one.

This depends on the hardware. What's the bus between the CSI-2 rx and 
the adapter? If it's a custom bus that basically allows passing forward 
anything received from the CSI-2 bus, then the user should be free to 
configure any routes in the CSI-2 RX subdevice.

The adapter subdev, however, as it is limited by the HW, should only 
support two streams if that is what the hardware supports.

> Tomi in cc for inputs.
>
> For now, if you don't support capturing embedded data, I would remove
> routing support from here and from the adapter.

Right, if the drivers only support a single stream, just drop the 
routing support.

 Tomi