Re: [PATCH v2 3/3] media: pci: intel: ivsc: Add acquire/release API for ivsc

[Sakari Ailus]

Hi Wentong,

On Wed, Mar 01, 2023 at 07:26:40AM +0000, Wu, Wentong wrote:
> 
> 
> > -----Original Message-----
> > From: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > Sent: Tuesday, February 28, 2023 4:24 PM
> > 
> > Hi Wentong,
> > 
> > On Tue, Feb 28, 2023 at 06:35:41AM +0000, Wu, Wentong wrote:
> > > Hi Sakari,
> > >
> > > few questions as switching to v4l2 sub-dev framework.
> > >
> > > > -----Original Message-----
> > > > From: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > > > Sent: Wednesday, February 15, 2023 12:06 AM
> > > >
> > > > Hi Wentong,
> > > >
> > > > Thanks for the patchset.
> > > >
> > > > 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.
> > > >
> > > > 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.
> > >
> > > This should be a control or event? If control, how user-space handle
> > > privacy stuff?
> > 
> > Changing control events generates events for the user space.
> > 
> > <URL:https://hverkuil.home.xs4all.nl/spec/userspace-api/v4l/dev-event.html>
> 
> Ok, V4L2_EVENT_CTRL event which reports 'struct v4l2_event_ctrl' data to user space
> 
> > 
> > >
> > > For the required link freq and lane number, is v4l2 control the
> > > correct way to configure them? If yes, seems there is no CID value for
> > > them so that we should custom some CID value(link freqm, lane number,
> > > and
> > > privacy) for ivsc in linux/v4l2-controls.h, is this acceptable?
> > 
> > You should obtain these using the V4L2 fwnode interface. Please see e.g.
> > drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c how that driver registers
> > a V4L2 async sub-device and a V4L2 async notifier.
> 
> Ok, something like v4l2_ctrl_find(source->ctrl_handler,
> V4L2_CID_LINK_FREQ); is to get sensor's link frequency, and the code like
> 'v4l2_subdev_call(source, pad, get_mbus_config, source, &mbus_config);
> num_of_lanes = mbus_config.bus.mipi_csi2.num_data_lanes;' is to get
> sensor's lane number.

Please use the V4L2 fwnode interface instead.

The privacy control should only be needed in the user space.

-- 
Regards,

Sakari Ailus