Hi, On 3/7/23 09:40, Wu, Wentong wrote: > > >> -----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. Then maybe we need to do the device_link_add somewhere else. The mainline kernel delays probing of camera sensors on Intel platforms until the INT3472 driver has probed the INT3472 device on which the sensors have an ACPI _DEP. This is already used to make sure that clock lookups and regulator info is in place before the sensor's probe() function runs. So that when the driver does clk_get() it succeeds and so that regulator_get() does not end up returning a dummy regulator. So I think the code adding the device_link-s for the IVSC should be added to: drivers/platform/x86/intel/int3472/discrete.c and then the runtime-resume will happen before the sensor's probe() function runs. Likewise drivers/platform/x86/intel/int3472/discrete.c should also ensure that the ivsc driver's probe() has run before it calls acpi_dev_clear_dependencies(). The acpi_dev_clear_dependencies() call in discrete.c tells the ACPI subsystem to go ahead and create the i2c-clients for the sensors and allow the sensor drivers to get loaded and probe the sensor. Regards, Hans
