Hi, On 2/20/23 09:57, Wu, Wentong wrote: > > >> -----Original Message----- >> From: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> >> Sent: Monday, February 20, 2023 4:37 PM >> >> Hi Wentong, >> >> On Mon, Feb 20, 2023 at 03:50:55AM +0000, Wu, Wentong wrote: >>> >>> >>>> -----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 for the detail, but I have one more question, during probe of >>> sensor(v4l2-sudev) driver, it will configure camera sensor connected >>> to IVSC via I2C, but before that it should acquire camera sensor's >>> ownership from IVSC, how v4l2 framework guarantee this? >> >> Please wrap the lines at 74 characters or so when replying. > > Thanks, I will. > >> >> Do you mean accessing the sensor via I²C also requires acquiring the sensor >> from IVSC? > > Yes Hmm, that is going to be a problem since we really want to have independent driver for the 2 which are not aware of each other. I think that maybe we can model this part of the ivsc functionality as an i2c-mux. But then we will need to somehow change the parent of the i2c device for the sensor to the output of this mux ... I guess this means adding some code (some hack likely) to drivers/platform/x86/intel/int3472/discrete.c which' proe function is already guaranteed to run before the sensor's i2c-client gets instantiated, because of the ACPI _DEP on the INT3472 ACPI device in the DSDT. This is going to be a tricky problem to tackle. Regards, Hans
