Message-ID: <ZAb2G7kqsEvrBhpG@kekkonen.localdomain>
Date:   Tue, 7 Mar 2023 10:30:19 +0200
From:   Sakari Ailus <sakari.ailus@...ux.intel.com>
To:     "Wu, Wentong" <wentong.wu@...el.com>
Cc:     Hans de Goede <hdegoede@...hat.com>,
        "mchehab@...nel.org" <mchehab@...nel.org>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "Pandruvada, Srinivas" <srinivas.pandruvada@...el.com>,
        "pierre-louis.bossart@...ux.intel.com" 
        <pierre-louis.bossart@...ux.intel.com>,
        "Wang, Zhifeng" <zhifeng.wang@...el.com>,
        "Ye, Xiang" <xiang.ye@...el.com>,
        "Qiu, Tian Shu" <tian.shu.qiu@...el.com>,
        "Cao, Bingbu" <bingbu.cao@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 0/3] media: pci: intel: ivsc: Add driver of Intel
 Visual Sensing Controller(IVSC)

Hi Wentong,

On Tue, Mar 07, 2023 at 08:17:04AM +0000, Wu, Wentong wrote:
> 
> 
> > -----Original Message-----
> > From: Hans de Goede <hdegoede@...hat.com>
> > 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.

-- 
Regards,

Sakari Ailus

Powered by blists - more mailing lists