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: Tuesday, March 7, 2023 4:30 PM
> 
> Hi Wentong,
> 
> On Tue, Mar 07, 2023 at 08:17:04AM +0000, Wu, Wentong wrote:
> >
> >
> > > -----Original Message-----
> > > From: Hans de Goede <hdegoede@xxxxxxxxxx>
> > > Sent: Wednesday, March 1, 2023 6:42 PM
> > >
> > > Hi,
> > >
> > > On 3/1/23 11:34, Sakari Ailus wrote:
> > > > 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:
> > > >
> > > > - The IVSC sub-device implements a control for privacy (V4L2_CID_PRIVACY
> > > >   is a good fit).
> > > >
> > > > - 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.
> > >
> > > Interesting proposal to use device-links + runtime-pm for this
> > > instead of modelling this as an i2c-mux. FWIW I'm fine with going
> > > this route instead of using an i2c-mux approach.
> > >
> > > I have been thinking about the i2c-mux approach a bit and the
> > > problem is that we are not really muxing but want to turn on/off
> > > control and AFAIK the i2c-mux framework simply leaves the mux muxed
> > > to the last used i2c-chain, so control will never be released when the i2c
> transfers are done.
> > >
> > > And if were to somehow modify things (or maybe there already is some
> > > release
> > > callback) then the downside becomes that the i2c-mux core code
> > > operates at the i2c transfer level. So each i2c read/write would then enable +
> disavle control.
> > >
> > > Modelling this using something like runtime pm as such is a much
> > > better fit because then we request control once on probe / stream-on
> > > and release it once we are fully done, rather then requesting +
> > > releasing control once per i2c- transfer.
> >
> > Seems runtime pm can't fix the problem of initial i2c transfer during
> > sensor driver probe, probably we have to switch to i2c-mux modeling way.
> 
> What do you mean? The supplier devices are resumed before the driver's probe
> is called.

But we setup the link with device_link_add during IVSC driver's probe, we can't
guarantee driver probe's sequence.
> 
> --
> Regards,
> 
> Sakari Ailus