Hi Laurent, > -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Sent: Friday, February 17, 2023 7:19 PM > > Hi Wentong, > > On Fri, Feb 17, 2023 at 06:10:22AM +0000, Wu, Wentong wrote: > > On Sent: Wednesday, February 15, 2023 5:46 PM, Laurent Pinchart wrote: > > > On Wed, Feb 15, 2023 at 10:03:29AM +0100, Hans de Goede wrote: > > > > On 2/14/23 17:06, Sakari Ailus wrote: > > > > > On Mon, Feb 13, 2023 at 10:23:47AM +0800, Wentong Wu wrote: > > > > >> IVSC directly connects to camera sensor on source side, and on > > > > >> output side it not only connects ISH via I2C, but also exposes > > > > >> MIPI > > > > >> CSI-2 interface to output camera sensor data. IVSC can use the > > > > >> camera sensor data to do AI algorithm, and send the results to ISH. > > > > >> On the other end, IVSC can share camera sensor to host by > > > > >> routing the raw camera sensor data to the exposed MIPI CSI-2 > > > > >> interface. But they can not work at the same time, so software > > > > >> APIs are defined to sync the ownership. > > > > >> > > > > >> This commit defines the interfaces between IVSC and camera > > > > >> sensor driver in include/linux/ivsc.h. The camera driver > > > > >> controls ownership of the CSI-2 link and sensor with the > > > > >> acquire/release APIs. When acquiring camera, lane number and > > > > >> link freq are also required by IVSC frame router. > > > > > > > > > > The more I learn about this system, the more I'm inclined to > > > > > think this functionality should be exposed as a V4L2 sub-device. > > > > > IVSC doesn't really do anything to the data (as long as it > > > > > directs it towards the CSI-2 receiver in the SoC), but it is > > > > > definitely part of the image pipeline. > > > > > > > > Yes I happened to discuss this exact same thing with Laurent at > > > > FOSDEM and we also came to the conclusion that the IVSC chip > > > > should be modeled as a V4L2 sub-device. > > > > > > Agreed. > > > > Thanks for your quick review and conclusion, I'm fresh to media > > sub-system, is there any convention that I should follow to upstream > > this kind of v4l2 sub-device driver so that not too much back and > > forth? > > This is a tentative proposal, as I'm not very familiar with the hardware > architecture: > > - The subdev should have two pads, a sink pad connected to the camera > sensor, and a source pad connected to the CSI-2 receiver in the IPU. > > - As for any new driver, the subdev driver should use the active state > managed by the v4l2-subdev core. This requires calling > v4l2_subdev_init_finalize() at probe time. See > https://git.linuxtv.org/media_tree.git/commit/?id=a2514b9a634a for an > example of a subdev driver converted to this mechanism. > > - As we're talking about CSI-2, the subdev driver should use the streams > API that was recently merged, and in particular support the > .get_frame_desc(), .set_routing(), .enable_streams() and > .disable_streams() operations. > > - I don't see a need to support V4L2 controls in the subdev driver, but > I may be missing something. > > - The driver should be validated with v4l2-compliance, part of > v4l-utils. Thanks, one more question, because ivsc just routes the received data from sensor and no format convert, how we handle the get_fmt and set_fmt callback for ivsc? Thanks Wentong > > > > > > I suppose the intended use cases assume a single instance of > > > > > IVSC (as well as MEI) but there can, and often are, be multiple > > > > > camera sensors in the system. The decision whether to request > > > > > pass-through from IVCS can't be done in the camera sensor > > > > > driver, and should not be visible to the camera sensor driver. > > > > > Exposing IVSC as a V4L2 sub-device makes this trivial to > > > > > address, as the IVSC driver's V4L2 sub-device video s_stream() operation > gets called before streaming is started. > > > > > > > > > > The information whether IVSC is found between the camera sensor > > > > > and the host's CSI-2 receiver (IPU in this case) should come > > > > > from system firmware and accessed most probably by what is > > > > > called cio2-bridge at the moment. > > > > > > > > > > The privacy status can be a V4L2 control. > > > > > > > > > > Also cc Hans. > > > > > > > > > >> Signed-off-by: Wentong Wu <wentong.wu@xxxxxxxxx> > > > > >> --- > > > > >> drivers/media/pci/intel/ivsc/Makefile | 1 + > > > > >> drivers/media/pci/intel/ivsc/ivsc.c | 84 > +++++++++++++++++++++++++++ > > > > >> include/linux/ivsc.h | 55 ++++++++++++++++++ > > > > >> 3 files changed, 140 insertions(+) create mode 100644 > > > > >> drivers/media/pci/intel/ivsc/ivsc.c > > [snip] > > -- > Regards, > > Laurent Pinchart
