On Mon, Feb 20, 2023 at 10:03:08AM +0100, Hans de Goede wrote: > 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. >From a framework point of view I would also recommend i2c-mux, but when it comes to the ACPI integration... I can feel your pain and I'm sorry about that :-( -- Regards, Laurent Pinchart
