RE: [PATCH v2 0/3] media: pci: intel: ivsc: Add driver of Intel Visual Sensing Controller(IVSC)

[Wu, Wentong]

> -----Original Message-----
> From: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> Sent: Wednesday, March 1, 2023 6:34 PM
> 
> Hi Wentong,
> 
> On Mon, Feb 13, 2023 at 10:23:44AM +0800, Wentong Wu wrote:
> > Intel Visual Sensing Controller (IVSC), codenamed "Clover Falls", is a
> > companion chip designed to provide secure and low power vision
> > capability to IA platforms. IVSC is available in existing commercial
> > platforms from multiple OEMs.
> >
> > The primary use case of IVSC is to bring in context awareness. IVSC
> > interfaces directly with the platform main camera sensor via a CSI-2
> > link and processes the image data with the embedded AI engine. The
> > detected events are sent over I2C to ISH (Intel Sensor Hub) for
> > additional data fusion from multiple sensors. The fusion results are
> > used to implement advanced use cases like:
> >  - Face detection to unlock screen
> >  - Detect user presence to manage backlight setting or waking up
> > system
> >
> > Since the Image Processing Unit(IPU) used on the host processor needs
> > to configure the CSI-2 link in normal camera usages, the CSI-2 link
> > and camera sensor can only be used in mutually-exclusive ways by host
> > IPU and IVSC. By default the IVSC owns the CSI-2 link and camera
> > sensor. The IPU driver can take ownership of the CSI-2 link and camera
> > sensor using interfaces provided by this IVSC driver.
> >
> > Switching ownership requires an interface with two different hardware
> > modules inside IVSC. The software interface to these modules is via
> > Intel MEI (The Intel Management Engine) commands. These two hardware
> > modules have two different MEI UUIDs to enumerate. These hardware
> modules are:
> >  - ACE (Algorithm Context Engine): This module is for algorithm
> > computing when IVSC owns camera sensor. Also ACE module controls
> > camera sensor's ownership. This hardware module is used to set ownership of
> camera sensor.
> >  - CSI (Camera Serial Interface): This module is used to route camera
> > sensor data either to IVSC or to host for IPU driver and application.
> >
> > IVSC also provides a privacy mode. When privacy mode is turned on,
> > camera sensor can't be used. This means that both ACE and host IPU
> > can't get image data. And when this mode is turned on, host IPU driver
> > is informed via a registered callback, so that user can be notified.
> >
> > In summary, to acquire ownership of camera by IPU driver, first ACE
> > module needs to be informed of ownership and then to setup MIPI CSI-2
> > link for the camera sensor and IPU.
> 
> I thought this for a while and did some research, and I can suggest the
> following:

Thanks a lot

> 
> - The IVSC sub-device implements a control for privacy (V4L2_CID_PRIVACY
>   is a good fit).

Agree, thanks

> 
> - Camera sensor access needs to be requested from IVSC before accessing the
>   sensor via I²C. The IVSC ownership control needs to be in the right
>   setting for this to work, and device links can be used for that purpose
>   (see device_link_add()). With DL_FLAG_PM_RUNTIME and
> DL_FLAG_RPM_ACTIVE,
>   the supplier devices will be PM runtime resumed before the consumer
>   (camera sensor). As these devices are purely virtual on host side and has
>   no power state as such, you can use runtime PM callbacks to transfer the
>   ownership.
> 

That's pretty cool, and the device link will be setup during mei_ace's probe with
device_link_add, but we should guarantee there is no i2c transfer during sensor
driver's probe. And most of upstream sensor drivers don't initial i2c transfer
during probe, but I have no idea what will be like for our upstreaming sensor drivers.

> - The CSI-2 configuration should take place when streaming starts on the
>   sensor (as well as IVSC).

IPU driver should make sure start sensor streaming first and then IVSC sub-dev,
which will match the command downloading sequence required by firmware. 

> 
> - Device links need to be set up via IPU bridge which now is called  CIO2
>   bridge (cio2-bridge.c).

Reviewing the code

> 
> Any questions, comments?
> 
> --
> Kind regards,
> 
> Sakari Ailus