| Message ID | 20230213022347.2480307-1-wentong.wu@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: pci: intel: ivsc: Add driver of Intel Visual Sensing Controller(IVSC) | expand |
On Mon, 2023-02-13 at 10:23 +0800, Wentong Wu wrote: > v2 without change log, why? > 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. > > Implementation: > There are two different drivers to handle ACE and CSI hardware > modules > inside IVSC. > - mei_csi: MEI client driver to send commands and receive > notifications > from CSI module. > - mei_ace: MEI client driver to send commands and get status from > ACE > module. > Interface is exposed via ivsc.h to acquire and release camera sensor > and > CSI-2 link. > > Below diagram shows connections of IVSC/ISH/IPU/Camera sensor. > --------------------------------------------------------------------- > -------- > > Host > > Processor > > | > > > > | > > ----------------- ----------------- ------------- > > -- | > > | | | | | > > | I2C | > > | IPU | | ISH | |camera > > driver|--| | > > | | | | | > > | | | > > ----------------- ----------------- ------------- > > -- | | > > | | > > | | | > > | | ------------- > > -- | | > > | | | > > | | | > > | | | IVSC driver > > | | | > > | | | > > | | | > > | | ------------- > > -- | | > > | | > > | | | > ----------------|-----------------------|----------------------|----- > ----|--- > | CSI | I2C > |SPI | > | | > | | > ----------------|-----------------------|---------------- > | | > > IVSC | | > > | | > > | | > > | | > > ----------------- ----------------- | > > | | > > | | | | | > > | | > > | CSI | | ACE | |------ > > | | > > | | | | > > | | > > ----------------- ----------------- > > | | > > | | I2C > > | | > ----------------|-----------------------|---------------- > | > | CSI > | | > | > | | > -------------------------------- > | > | | > I2C | > | camera sensor |------------------------- > ----| > | | > -------------------------------- > > Wentong Wu (3): > media: pci: intel: ivsc: Add CSI submodule > media: pci: intel: ivsc: Add ACE submodule > media: pci: intel: ivsc: Add acquire/release API for ivsc > > drivers/media/pci/Kconfig | 1 + > drivers/media/pci/intel/Makefile | 2 + > drivers/media/pci/intel/ivsc/Kconfig | 12 + > drivers/media/pci/intel/ivsc/Makefile | 7 + > drivers/media/pci/intel/ivsc/ivsc.c | 84 +++++ > drivers/media/pci/intel/ivsc/mei_ace.c | 472 > +++++++++++++++++++++++++ > drivers/media/pci/intel/ivsc/mei_ace.h | 36 ++ > drivers/media/pci/intel/ivsc/mei_csi.c | 342 ++++++++++++++++++ > drivers/media/pci/intel/ivsc/mei_csi.h | 60 ++++ > include/linux/ivsc.h | 74 ++++ > 10 files changed, 1090 insertions(+) > create mode 100644 drivers/media/pci/intel/ivsc/Kconfig > create mode 100644 drivers/media/pci/intel/ivsc/Makefile > create mode 100644 drivers/media/pci/intel/ivsc/ivsc.c > create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.c > create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.h > create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.c > create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.h > create mode 100644 include/linux/ivsc.h >
Hi Hillf Danton, Thanks for your review > -----Original Message----- > From: Hillf Danton <hdanton@sina.com> > Sent: Monday, February 13, 2023 11:42 AM > > On Mon, 13 Feb 2023 10:23:45 +0800 Wentong Wu <wentong.wu@intel.com> > > + > > +/* send a command to firmware and mutex must be held by caller */ > > +static int mei_csi_send(u8 *buf, size_t len) { > > + struct csi_cmd *cmd = (struct csi_cmd *)buf; > > + int ret; > > + > > + reinit_completion(&csi->cmd_completion); > > Could you specify why reinit is needed here? This allows new command(e.g. 'get status' command to check current firmware status) to be downloaded to firmware for debugging purpose in case no response for current command though I have never saw this happen. > What is hurt without it? No hurt for current implementation. I can remove it. > > Same question for the reinit in 2/3. same ack for 2/3 > > > + > > + ret = mei_cldev_send(csi->cldev, buf, len); > > + if (ret < 0) > > + goto out; > > + > > + ret = wait_for_completion_killable_timeout(&csi->cmd_completion, > > + CSI_CMD_TIMEOUT); > > + if (ret < 0) { > > + goto out; > > + } else if (!ret) { > > + ret = -ETIMEDOUT; > > + goto out; > > + }
Hi Srinivas, Thanks for your review > -----Original Message----- > From: Pandruvada, Srinivas <srinivas.pandruvada@intel.com> > Sent: Monday, February 13, 2023 11:21 AM > > On Mon, 2023-02-13 at 10:23 +0800, Wentong Wu wrote: > > > > v2 without change log, why? This addressed 'mei_csi.c:(.text+0x1a6): undefined reference to __udivdi3' issue on i386 arch. I will add change log on following version. > > > 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. > > > > Implementation: > > There are two different drivers to handle ACE and CSI hardware modules > > inside IVSC. > > - mei_csi: MEI client driver to send commands and receive > > notifications from CSI module. > > - mei_ace: MEI client driver to send commands and get status from ACE > > module. > > Interface is exposed via ivsc.h to acquire and release camera sensor > > and > > CSI-2 link. > > > > Below diagram shows connections of IVSC/ISH/IPU/Camera sensor. > > --------------------------------------------------------------------- > > -------- > > > Host > > > Processor > > > | > > > > > > | > > > ----------------- ----------------- ------------- > > > -- | > > > | | | | | > > > | I2C | > > > | IPU | | ISH | |camera > > > driver|--| | > > > | | | | | > > > | | | > > > ----------------- ----------------- ------------- > > > -- | | > > > | | > > > | | | > > > | | ------------- > > > -- | | > > > | | | > > > | | | > > > | | | IVSC driver > > > | | | > > > | | | > > > | | | > > > | | ------------- > > > -- | | > > > | | > > > | | | > > ----------------|-----------------------|----------------------|----- > > ----|--- > > | CSI | I2C > > |SPI | > > | | > > | | > > ----------------|-----------------------|---------------- > > | | > > > IVSC | | > > > | | > > > | | > > > | | > > > ----------------- ----------------- | > > > | | > > > | | | | | > > > | | > > > | CSI | | ACE | |------ > > > | | > > > | | | | > > > | | > > > ----------------- ----------------- > > > | | > > > | | I2C > > > | | > > ----------------|-----------------------|---------------- > > | > > | CSI > > | | > > | > > | | > > -------------------------------- > > | > > | | I2C > > | > > | camera sensor |------------------------- > > ----| > > | | > > -------------------------------- > > > > Wentong Wu (3): > > media: pci: intel: ivsc: Add CSI submodule > > media: pci: intel: ivsc: Add ACE submodule > > media: pci: intel: ivsc: Add acquire/release API for ivsc > > > > drivers/media/pci/Kconfig | 1 + > > drivers/media/pci/intel/Makefile | 2 + > > drivers/media/pci/intel/ivsc/Kconfig | 12 + > > drivers/media/pci/intel/ivsc/Makefile | 7 + > > drivers/media/pci/intel/ivsc/ivsc.c | 84 +++++ > > drivers/media/pci/intel/ivsc/mei_ace.c | 472 > > +++++++++++++++++++++++++ > > drivers/media/pci/intel/ivsc/mei_ace.h | 36 ++ > > drivers/media/pci/intel/ivsc/mei_csi.c | 342 ++++++++++++++++++ > > drivers/media/pci/intel/ivsc/mei_csi.h | 60 ++++ > > include/linux/ivsc.h | 74 ++++ > > 10 files changed, 1090 insertions(+) > > create mode 100644 drivers/media/pci/intel/ivsc/Kconfig > > create mode 100644 drivers/media/pci/intel/ivsc/Makefile > > create mode 100644 drivers/media/pci/intel/ivsc/ivsc.c > > create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.c > > create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.h > > create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.c > > create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.h > > create mode 100644 include/linux/ivsc.h > >
> -----Original Message----- > From: Hillf Danton <hdanton@sina.com> > Sent: Tuesday, February 14, 2023 4:36 PM > > On Tue, 14 Feb 2023 06:25:35 +0000 Wentong Wu <wentong.wu@intel.com> > > > > This allows new command(e.g. 'get status' command to check current > > firmware= > > status) to be downloaded to firmware for debugging purpose in case no > > resp= onse for current command though I have never saw this happen. > > =20 > > Thanks for your specification. > > Another question, is it likely for the wakeup to come after reinit because of for > instance timeout on the waiter side? Thanks, first I want to clarify why we have timeout here, in case firmware stuck somehow or IVSC device is removed suddenly, calling thread can't be wakeup if wait forever here. And firmware gets high priority to handle command and response, also there is timeout mechanism inside firmware, I think there will be some problem with firmware if driver gets command response after 5s which will be captured by driver code(if (csi->cmd_response.cmd_id != cmd->cmd_id)) but I have never saw this happen. BR, Wentong
Hello 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 Do you have plan to support these features in the ivsc driver in the future ? > 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. How does the privacy mode work, and how can the user trust that the closed-source IVSC and IME firmwares will honour the privacy settings ? > 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. > > Implementation: > There are two different drivers to handle ACE and CSI hardware modules > inside IVSC. > - mei_csi: MEI client driver to send commands and receive notifications > from CSI module. > - mei_ace: MEI client driver to send commands and get status from ACE > module. > Interface is exposed via ivsc.h to acquire and release camera sensor and > CSI-2 link. Do I understand correctly, from your diagram below, that the communication between the IME and IVSC goes through SPI ? > Below diagram shows connections of IVSC/ISH/IPU/Camera sensor. > ----------------------------------------------------------------------------- > | Host Processor | > | | > | ----------------- ----------------- --------------- | > | | | | | | | I2C | > | | IPU | | ISH | |camera driver|--| | > | | | | | | | | | > | ----------------- ----------------- --------------- | | > | | | | | | > | | | --------------- | | > | | | | | | | > | | | | IVSC driver | | | > | | | | | | | > | | | --------------- | | > | | | | | | > ----------------|-----------------------|----------------------|---------|--- > | CSI | I2C |SPI | > | | | | > ----------------|-----------------------|---------------- | | > | IVSC | | | | > | | | | | > | ----------------- ----------------- | | | > | | | | | | | | > | | CSI | | ACE | |------| | > | | | | | | | > | ----------------- ----------------- | | > | | | I2C | | > ----------------|-----------------------|---------------- | > | CSI | | > | | | > -------------------------------- | > | | I2C | > | camera sensor |-----------------------------| > | | > -------------------------------- > > Wentong Wu (3): > media: pci: intel: ivsc: Add CSI submodule > media: pci: intel: ivsc: Add ACE submodule > media: pci: intel: ivsc: Add acquire/release API for ivsc > > drivers/media/pci/Kconfig | 1 + > drivers/media/pci/intel/Makefile | 2 + > drivers/media/pci/intel/ivsc/Kconfig | 12 + > drivers/media/pci/intel/ivsc/Makefile | 7 + > drivers/media/pci/intel/ivsc/ivsc.c | 84 +++++ > drivers/media/pci/intel/ivsc/mei_ace.c | 472 +++++++++++++++++++++++++ > drivers/media/pci/intel/ivsc/mei_ace.h | 36 ++ > drivers/media/pci/intel/ivsc/mei_csi.c | 342 ++++++++++++++++++ > drivers/media/pci/intel/ivsc/mei_csi.h | 60 ++++ > include/linux/ivsc.h | 74 ++++ > 10 files changed, 1090 insertions(+) > create mode 100644 drivers/media/pci/intel/ivsc/Kconfig > create mode 100644 drivers/media/pci/intel/ivsc/Makefile > create mode 100644 drivers/media/pci/intel/ivsc/ivsc.c > create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.c > create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.h > create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.c > create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.h > create mode 100644 include/linux/ivsc.h
Hi, Wentong, On 2/15/23 5:43 PM, Laurent Pinchart wrote: > Hello 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 > > Do you have plan to support these features in the ivsc driver in the > future ? > >> 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. > > How does the privacy mode work, and how can the user trust that the > closed-source IVSC and IME firmwares will honour the privacy settings ? > Continue with question from Laurent, How IVSC handle the privacy request from user? Is there some notification mechanism to user-space? I'd have concern if IVSC driver need private callback to request back-end(e.g. ISP driver) to handle stream cutting. >> 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. >> >> Implementation: >> There are two different drivers to handle ACE and CSI hardware modules >> inside IVSC. >> - mei_csi: MEI client driver to send commands and receive notifications >> from CSI module. >> - mei_ace: MEI client driver to send commands and get status from ACE >> module. >> Interface is exposed via ivsc.h to acquire and release camera sensor and >> CSI-2 link. > > Do I understand correctly, from your diagram below, that the > communication between the IME and IVSC goes through SPI ? > >> Below diagram shows connections of IVSC/ISH/IPU/Camera sensor. >> ----------------------------------------------------------------------------- >> | Host Processor | >> | | >> | ----------------- ----------------- --------------- | >> | | | | | | | I2C | >> | | IPU | | ISH | |camera driver|--| | >> | | | | | | | | | >> | ----------------- ----------------- --------------- | | >> | | | | | | >> | | | --------------- | | >> | | | | | | | >> | | | | IVSC driver | | | >> | | | | | | | >> | | | --------------- | | >> | | | | | | >> ----------------|-----------------------|----------------------|---------|--- >> | CSI | I2C |SPI | >> | | | | >> ----------------|-----------------------|---------------- | | >> | IVSC | | | | >> | | | | | >> | ----------------- ----------------- | | | >> | | | | | | | | >> | | CSI | | ACE | |------| | >> | | | | | | | >> | ----------------- ----------------- | | >> | | | I2C | | >> ----------------|-----------------------|---------------- | >> | CSI | | >> | | | >> -------------------------------- | >> | | I2C | >> | camera sensor |-----------------------------| >> | | >> -------------------------------- >> >> Wentong Wu (3): >> media: pci: intel: ivsc: Add CSI submodule >> media: pci: intel: ivsc: Add ACE submodule >> media: pci: intel: ivsc: Add acquire/release API for ivsc >> >> drivers/media/pci/Kconfig | 1 + >> drivers/media/pci/intel/Makefile | 2 + >> drivers/media/pci/intel/ivsc/Kconfig | 12 + >> drivers/media/pci/intel/ivsc/Makefile | 7 + >> drivers/media/pci/intel/ivsc/ivsc.c | 84 +++++ >> drivers/media/pci/intel/ivsc/mei_ace.c | 472 +++++++++++++++++++++++++ >> drivers/media/pci/intel/ivsc/mei_ace.h | 36 ++ >> drivers/media/pci/intel/ivsc/mei_csi.c | 342 ++++++++++++++++++ >> drivers/media/pci/intel/ivsc/mei_csi.h | 60 ++++ >> include/linux/ivsc.h | 74 ++++ >> 10 files changed, 1090 insertions(+) >> create mode 100644 drivers/media/pci/intel/ivsc/Kconfig >> create mode 100644 drivers/media/pci/intel/ivsc/Makefile >> create mode 100644 drivers/media/pci/intel/ivsc/ivsc.c >> create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.c >> create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.h >> create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.c >> create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.h >> create mode 100644 include/linux/ivsc.h >
Hi Bingbu, Wentong, On Wed, Feb 15, 2023 at 08:09:50PM +0800, Bingbu Cao wrote: > > Hi, Wentong, > > On 2/15/23 5:43 PM, Laurent Pinchart wrote: > > Hello 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 > > > > Do you have plan to support these features in the ivsc driver in the > > future ? > > > >> 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. > > > > How does the privacy mode work, and how can the user trust that the > > closed-source IVSC and IME firmwares will honour the privacy settings ? > > > > Continue with question from Laurent, > > How IVSC handle the privacy request from user? Is there some notification > mechanism to user-space? I'd have concern if IVSC driver need private callback > to request back-end(e.g. ISP driver) to handle stream cutting. How does the privacy mode work, does it just pass zeroes (or other dummy data) towards the host or nothing? A V4L2 control can be used for the purpose of passing the information to the user space at least.
Hi, Sakari, On 2/16/23 9:12 PM, Sakari Ailus wrote: > Hi Bingbu, Wentong, > > On Wed, Feb 15, 2023 at 08:09:50PM +0800, Bingbu Cao wrote: >> >> Hi, Wentong, >> >> On 2/15/23 5:43 PM, Laurent Pinchart wrote: >>> Hello 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 >>> >>> Do you have plan to support these features in the ivsc driver in the >>> future ? >>> >>>> 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. >>> >>> How does the privacy mode work, and how can the user trust that the >>> closed-source IVSC and IME firmwares will honour the privacy settings ? As I know, without IVSC, once user enable the privacy mode, the Intel Converged Security Engine will configure the IPU camera mask (security register), which will mask the specific CSI2 port and produce dummy imaging data. For the case with IVSC, there is no final solution on Linux so far I think. Wentong, is IVSC trying to cut off the stream and then notify user and IPU? >>> >> >> Continue with question from Laurent, >> >> How IVSC handle the privacy request from user? Is there some notification >> mechanism to user-space? I'd have concern if IVSC driver need private callback >> to request back-end(e.g. ISP driver) to handle stream cutting. > > How does the privacy mode work, does it just pass zeroes (or other dummy > data) towards the host or nothing? > > A V4L2 control can be used for the purpose of passing the information to > the user space at least. >
> -----Original Message----- > From: Hillf Danton <hdanton@sina.com> > Sent: Wednesday, February 15, 2023 7:54 AM > > On Tue, 14 Feb 2023 12:40:12 +0000 Wentong Wu <wentong.wu@intel.com> > > > -----Original Message----- > > > From: Hillf Danton <hdanton@sina.com> > > > Sent: Tuesday, February 14, 2023 4:36 PM > > >=20 > > > On Tue, 14 Feb 2023 06:25:35 +0000 Wentong Wu <wentong.wu@intel.com> > > > > > > > > This allows new command(e.g. 'get status' command to check current > > > > firmware=3D > > > > status) to be downloaded to firmware for debugging purpose in > > > > case no resp=3D onse for current command though I have never saw this > happen. > > > > =3D20 > > >=20 > > > Thanks for your specification. > > >=20 > > > Another question, is it likely for the wakeup to come after reinit > > >becaus= > > e of for > > > instance timeout on the waiter side? > > > > Thanks, first I want to clarify why we have timeout here, in case > > firmware = stuck somehow or IVSC device is removed suddenly, calling > > thread can't be w= akeup if wait forever here. > > Fair enough on the waiter side. > > > > And firmware gets high priority to handle command and response, also > > there = is timeout mechanism inside firmware, I think there will be > > some problem wi= th firmware if driver gets command response after 5s > > which will be captured= by driver code(if (csi->cmd_response.cmd_id > > !=3D cmd->cmd_id)) but I have = never saw this happen. > > The problem with firmware could be emulated by directly skipping the wait for > completion and returning ETIMEDOUT, in order to see what will happen if > wakeup comes later than timeout. Thanks, but what do you mean "emulated"?
> -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Sent: Wednesday, February 15, 2023 5:43 PM > > Hello 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 > > Do you have plan to support these features in the ivsc driver in the future ? Not sure, but the first step is to upstream this driver. > > > 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. > > How does the privacy mode work, and how can the user trust that the closed- > source IVSC and IME firmwares will honour the privacy settings ? No camera data will be allowed to go through IVSC, and then there will be no data on IVSC CSI transmitter side. > > > 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. > > > > Implementation: > > There are two different drivers to handle ACE and CSI hardware modules > > inside IVSC. > > - mei_csi: MEI client driver to send commands and receive > > notifications from CSI module. > > - mei_ace: MEI client driver to send commands and get status from ACE > > module. > > Interface is exposed via ivsc.h to acquire and release camera sensor > > and > > CSI-2 link. > > Do I understand correctly, from your diagram below, that the communication > between the IME and IVSC goes through SPI ? > > > Below diagram shows connections of IVSC/ISH/IPU/Camera sensor. > > ---------------------------------------------------------------------- > > ------- > > | Host Processor | > > | | > > | ----------------- ----------------- --------------- | > > | | | | | | | I2C | > > | | IPU | | ISH | |camera driver|--| | > > | | | | | | | | | > > | ----------------- ----------------- --------------- | | > > | | | | | | > > | | | --------------- | | > > | | | | | | | > > | | | | IVSC driver | | | > > | | | | | | | > > | | | --------------- | | > > | | | | | | > > ----------------|-----------------------|----------------------|---------|--- > > | CSI | I2C |SPI | > > | | | | > > ----------------|-----------------------|---------------- | | > > | IVSC | | | | > > | | | | | > > | ----------------- ----------------- | | | > > | | | | | | | | > > | | CSI | | ACE | |------| | > > | | | | | | | > > | ----------------- ----------------- | | > > | | | I2C | | > > ----------------|-----------------------|---------------- | > > | CSI | | > > | | | > > -------------------------------- | > > | | I2C | > > | camera sensor |-----------------------------| > > | | > > -------------------------------- > > > > Wentong Wu (3): > > media: pci: intel: ivsc: Add CSI submodule > > media: pci: intel: ivsc: Add ACE submodule > > media: pci: intel: ivsc: Add acquire/release API for ivsc > > > > drivers/media/pci/Kconfig | 1 + > > drivers/media/pci/intel/Makefile | 2 + > > drivers/media/pci/intel/ivsc/Kconfig | 12 + > > drivers/media/pci/intel/ivsc/Makefile | 7 + > > drivers/media/pci/intel/ivsc/ivsc.c | 84 +++++ > > drivers/media/pci/intel/ivsc/mei_ace.c | 472 > > +++++++++++++++++++++++++ drivers/media/pci/intel/ivsc/mei_ace.h | > > 36 ++ drivers/media/pci/intel/ivsc/mei_csi.c | 342 ++++++++++++++++++ > > drivers/media/pci/intel/ivsc/mei_csi.h | 60 ++++ > > include/linux/ivsc.h | 74 ++++ > > 10 files changed, 1090 insertions(+) > > create mode 100644 drivers/media/pci/intel/ivsc/Kconfig > > create mode 100644 drivers/media/pci/intel/ivsc/Makefile > > create mode 100644 drivers/media/pci/intel/ivsc/ivsc.c > > create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.c > > create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.h > > create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.c > > create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.h > > create mode 100644 include/linux/ivsc.h > > -- > Regards, > > Laurent Pinchart
> -----Original Message----- > From: Bingbu Cao <bingbu.cao@linux.intel.com> > Sent: Friday, February 17, 2023 9:44 AM > > Hi, Sakari, > > On 2/16/23 9:12 PM, Sakari Ailus wrote: > > Hi Bingbu, Wentong, > > > > On Wed, Feb 15, 2023 at 08:09:50PM +0800, Bingbu Cao wrote: > >> > >> Hi, Wentong, > >> > >> On 2/15/23 5:43 PM, Laurent Pinchart wrote: > >>> Hello 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 > >>> > >>> Do you have plan to support these features in the ivsc driver in the > >>> future ? > >>> > >>>> 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. > >>> > >>> How does the privacy mode work, and how can the user trust that the > >>> closed-source IVSC and IME firmwares will honour the privacy settings ? > > As I know, without IVSC, once user enable the privacy mode, the Intel > Converged Security Engine will configure the IPU camera mask (security register), > which will mask the specific CSI2 port and produce dummy imaging data. For the > case with IVSC, there is no final solution on Linux so far I think. > > Wentong, is IVSC trying to cut off the stream and then notify user and IPU? yes > > >>> > >> > >> Continue with question from Laurent, > >> > >> How IVSC handle the privacy request from user? Is there some > >> notification mechanism to user-space? IVSC has already defined privacy callback for host IPU/camera driver. > > I'd have concern if IVSC driver > >> need private callback to request back-end(e.g. ISP driver) to handle stream > cutting. > > > > How does the privacy mode work, does it just pass zeroes (or other > > dummy > > data) towards the host or nothing? No data on CSI transmitter side > > > > A V4L2 control can be used for the purpose of passing the information > > to the user space at least. I will take some time to review V4L2 sub-device and control mechanism, and then update the driver. BR, Wentong > > > > -- > Best regards, > Bingbu Cao
> -----Original Message----- > From: Hillf Danton <hdanton@sina.com> > Sent: Friday, February 17, 2023 2:28 PM > > On Fri, 17 Feb 2023 05:52:38 +0000 Wentong Wu <wentong.wu@intel.com> > > > > Thanks, but what do you mean "emulated"?=20 > > > Construct a scenario that ensures wakeup comes late because of no wait for > completion. And we can see what will happen with the current test cases. Thanks, I will remove reinit_completion > > > static int mei_csi_send(u8 *buf, size_t len) > > +{ > > + struct csi_cmd *cmd = (struct csi_cmd *)buf; > > + int ret; > > + > > + reinit_completion(&csi->cmd_completion); > > + > > + ret = mei_cldev_send(csi->cldev, buf, len); > > + if (ret < 0) > > + goto out; > > return -ETIMEDOUT; > > > + > > + ret = wait_for_completion_killable_timeout(&csi->cmd_completion, > > + CSI_CMD_TIMEOUT); > > + if (ret < 0) { > > + goto out; > > + } else if (!ret) { > > + ret = -ETIMEDOUT; > > + goto out; > > + } > > + > > + /* command response status */ > > + ret = csi->cmd_response.status; > > + if (ret) { > > + ret = -EINVAL; > > + goto out; > > + } > > + > > + if (csi->cmd_response.cmd_id != cmd->cmd_id) > > + ret = -EINVAL; > > + > > +out: > > + return ret; > > +}
Hi Wentong, On Fri, Feb 17, 2023 at 06:28:32AM +0000, Wu, Wentong wrote: > > > > -----Original Message----- > > From: Bingbu Cao <bingbu.cao@linux.intel.com> > > Sent: Friday, February 17, 2023 9:44 AM > > > > Hi, Sakari, > > > > On 2/16/23 9:12 PM, Sakari Ailus wrote: > > > Hi Bingbu, Wentong, > > > > > > On Wed, Feb 15, 2023 at 08:09:50PM +0800, Bingbu Cao wrote: > > >> > > >> Hi, Wentong, > > >> > > >> On 2/15/23 5:43 PM, Laurent Pinchart wrote: > > >>> Hello 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 > > >>> > > >>> Do you have plan to support these features in the ivsc driver in the > > >>> future ? > > >>> > > >>>> 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. > > >>> > > >>> How does the privacy mode work, and how can the user trust that the > > >>> closed-source IVSC and IME firmwares will honour the privacy settings ? > > > > As I know, without IVSC, once user enable the privacy mode, the Intel > > Converged Security Engine will configure the IPU camera mask (security register), > > which will mask the specific CSI2 port and produce dummy imaging data. For the > > case with IVSC, there is no final solution on Linux so far I think. > > > > Wentong, is IVSC trying to cut off the stream and then notify user and IPU? > > yes Does the CSI-2 transmitter on IVCS go to some LP mode during this time, or does the receiver need to initialise the bus again when the stream resuems? > > > > > >>> > > >> > > >> Continue with question from Laurent, > > >> > > >> How IVSC handle the privacy request from user? Is there some > > >> notification mechanism to user-space? > > IVSC has already defined privacy callback for host IPU/camera driver. > > > > I'd have concern if IVSC driver > > >> need private callback to request back-end(e.g. ISP driver) to handle stream > > cutting. > > > > > > How does the privacy mode work, does it just pass zeroes (or other > > > dummy > > > data) towards the host or nothing? > > No data on CSI transmitter side Can it stop in the middle of the frame? Or is it guaranteed to produce full frames (assuming the sensor does)? > > > > > > > A V4L2 control can be used for the purpose of passing the information > > > to the user space at least. > > I will take some time to review V4L2 sub-device and control mechanism, > and then update the driver.
Hello Wentong, On Fri, Feb 17, 2023 at 06:20:10AM +0000, Wu, Wentong wrote: > On Wednesday, February 15, 2023 5:43 PM, Laurent Pinchart wrote: > > 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 > > > > Do you have plan to support these features in the ivsc driver in the future ? > > Not sure, but the first step is to upstream this driver. Sure, no problem. > > > 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. > > > > How does the privacy mode work, and how can the user trust that the closed- > > source IVSC and IME firmwares will honour the privacy settings ? > > No camera data will be allowed to go through IVSC, and then there will > be no data on IVSC CSI transmitter side. But how can I be sure that the IVSC will not use the camera behind my back, if it's all controlled through a closed-source firmware ? > > > 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. > > > > > > Implementation: > > > There are two different drivers to handle ACE and CSI hardware modules > > > inside IVSC. > > > - mei_csi: MEI client driver to send commands and receive notifications from CSI module. > > > - mei_ace: MEI client driver to send commands and get status from ACE module. > > > Interface is exposed via ivsc.h to acquire and release camera sensor and > > > CSI-2 link. > > > > Do I understand correctly, from your diagram below, that the communication > > between the IME and IVSC goes through SPI ? > > > > > Below diagram shows connections of IVSC/ISH/IPU/Camera sensor. > > > ----------------------------------------------------------------------------- > > > | Host Processor | > > > | | > > > | ----------------- ----------------- --------------- | > > > | | | | | | | I2C | > > > | | IPU | | ISH | |camera driver|--| | > > > | | | | | | | | | > > > | ----------------- ----------------- --------------- | | > > > | | | | | | > > > | | | --------------- | | > > > | | | | | | | > > > | | | | IVSC driver | | | > > > | | | | | | | > > > | | | --------------- | | > > > | | | | | | > > > ----------------|-----------------------|----------------------|---------|--- > > > | CSI | I2C |SPI | > > > | | | | > > > ----------------|-----------------------|---------------- | | > > > | IVSC | | | | > > > | | | | | > > > | ----------------- ----------------- | | | > > > | | | | | | | | > > > | | CSI | | ACE | |------| | > > > | | | | | | | > > > | ----------------- ----------------- | | > > > | | | I2C | | > > > ----------------|-----------------------|---------------- | > > > | CSI | | > > > | | | > > > -------------------------------- | > > > | | I2C | > > > | camera sensor |-----------------------------| > > > | | > > > -------------------------------- > > > > > > Wentong Wu (3): > > > media: pci: intel: ivsc: Add CSI submodule > > > media: pci: intel: ivsc: Add ACE submodule > > > media: pci: intel: ivsc: Add acquire/release API for ivsc > > > > > > drivers/media/pci/Kconfig | 1 + > > > drivers/media/pci/intel/Makefile | 2 + > > > drivers/media/pci/intel/ivsc/Kconfig | 12 + > > > drivers/media/pci/intel/ivsc/Makefile | 7 + > > > drivers/media/pci/intel/ivsc/ivsc.c | 84 +++++ > > > drivers/media/pci/intel/ivsc/mei_ace.c | 472 +++++++++++++++++++++++++ > > > drivers/media/pci/intel/ivsc/mei_ace.h |> 36 ++ > > > drivers/media/pci/intel/ivsc/mei_csi.c | 342 ++++++++++++++++++ > > > drivers/media/pci/intel/ivsc/mei_csi.h | 60 ++++ > > > include/linux/ivsc.h | 74 ++++ > > > 10 files changed, 1090 insertions(+) > > > create mode 100644 drivers/media/pci/intel/ivsc/Kconfig > > > create mode 100644 drivers/media/pci/intel/ivsc/Makefile > > > create mode 100644 drivers/media/pci/intel/ivsc/ivsc.c > > > create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.c > > > create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.h > > > create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.c > > > create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.h > > > create mode 100644 include/linux/ivsc.h
Hi Sakari, > -----Original Message----- > From: Sakari Ailus <sakari.ailus@linux.intel.com> > Sent: Friday, February 17, 2023 6:50 PM > > Hi Wentong, > > On Fri, Feb 17, 2023 at 06:28:32AM +0000, Wu, Wentong wrote: > > > > > > > -----Original Message----- > > > From: Bingbu Cao <bingbu.cao@linux.intel.com> > > > Sent: Friday, February 17, 2023 9:44 AM > > > > > > Hi, Sakari, > > > > > > On 2/16/23 9:12 PM, Sakari Ailus wrote: > > > > Hi Bingbu, Wentong, > > > > > > > > On Wed, Feb 15, 2023 at 08:09:50PM +0800, Bingbu Cao wrote: > > > >> > > > >> Hi, Wentong, > > > >> > > > >> On 2/15/23 5:43 PM, Laurent Pinchart wrote: > > > >>> Hello 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 > > > >>> > > > >>> Do you have plan to support these features in the ivsc driver in > > > >>> the future ? > > > >>> > > > >>>> 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. > > > >>> > > > >>> How does the privacy mode work, and how can the user trust that > > > >>> the closed-source IVSC and IME firmwares will honour the privacy > settings ? > > > > > > As I know, without IVSC, once user enable the privacy mode, the > > > Intel Converged Security Engine will configure the IPU camera mask > > > (security register), which will mask the specific CSI2 port and > > > produce dummy imaging data. For the case with IVSC, there is no final > solution on Linux so far I think. > > > > > > Wentong, is IVSC trying to cut off the stream and then notify user and IPU? > > > > yes > > Does the CSI-2 transmitter on IVCS go to some LP mode during this time, Yes, The low power mode is following the D-Phy spec. > or does > the receiver need to initialise the bus again when the stream resuems? No, it's the same link. > > > > > > > > > >>> > > > >> > > > >> Continue with question from Laurent, > > > >> > > > >> How IVSC handle the privacy request from user? Is there some > > > >> notification mechanism to user-space? > > > > IVSC has already defined privacy callback for host IPU/camera driver. > > > > > > I'd have concern if IVSC driver > > > >> need private callback to request back-end(e.g. ISP driver) to > > > >> handle stream > > > cutting. > > > > > > > > How does the privacy mode work, does it just pass zeroes (or other > > > > dummy > > > > data) towards the host or nothing? > > > > No data on CSI transmitter side > > Can it stop in the middle of the frame? No, Or is it guaranteed to produce full frames > (assuming the sensor does)? Yes, full frame will be guaranteed as camera sensor does. > > > > > > > > > > > A V4L2 control can be used for the purpose of passing the > > > > information to the user space at least. > > > > I will take some time to review V4L2 sub-device and control mechanism, > > and then update the driver. > > -- > Kind regards, > > Sakari Ailus
> -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Sent: Friday, February 17, 2023 7:12 PM > > Hello Wentong, > > On Fri, Feb 17, 2023 at 06:20:10AM +0000, Wu, Wentong wrote: > > On Wednesday, February 15, 2023 5:43 PM, Laurent Pinchart wrote: > > > 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 > > > > > > Do you have plan to support these features in the ivsc driver in the future ? > > > > Not sure, but the first step is to upstream this driver. > > Sure, no problem. > > > > > 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. > > > > > > How does the privacy mode work, and how can the user trust that the > > > closed- source IVSC and IME firmwares will honour the privacy settings ? > > > > No camera data will be allowed to go through IVSC, and then there will > > be no data on IVSC CSI transmitter side. > > But how can I be sure that the IVSC will not use the camera behind my back, if > it's all controlled through a closed-source firmware ? Actually I don't know how to answer your question, but this is guaranteed though we have no plan to open-source the firmware. > > > > > 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. > > > > > > > > Implementation: > > > > There are two different drivers to handle ACE and CSI hardware > > > > modules inside IVSC. > > > > - mei_csi: MEI client driver to send commands and receive notifications > from CSI module. > > > > - mei_ace: MEI client driver to send commands and get status from ACE > module. > > > > Interface is exposed via ivsc.h to acquire and release camera > > > > sensor and > > > > CSI-2 link. > > > > > > Do I understand correctly, from your diagram below, that the > > > communication between the IME and IVSC goes through SPI ? > > > > > > > Below diagram shows connections of IVSC/ISH/IPU/Camera sensor. > > > > ------------------------------------------------------------------ > > > > ----------- > > > > | Host Processor | > > > > | | > > > > | ----------------- ----------------- --------------- | > > > > | | | | | | | I2C | > > > > | | IPU | | ISH | |camera driver|--| | > > > > | | | | | | | | | > > > > | ----------------- ----------------- --------------- | | > > > > | | | | | | > > > > | | | --------------- | | > > > > | | | | | | | > > > > | | | | IVSC driver | | | > > > > | | | | | | | > > > > | | | --------------- | | > > > > | | | | | | > > > > ----------------|-----------------------|----------------------|---------|--- > > > > | CSI | I2C |SPI | > > > > | | | | > > > > ----------------|-----------------------|---------------- | | > > > > | IVSC | | | | > > > > | | | | | > > > > | ----------------- ----------------- | | | > > > > | | | | | | | | > > > > | | CSI | | ACE | |------| | > > > > | | | | | | | > > > > | ----------------- ----------------- | | > > > > | | | I2C | | > > > > ----------------|-----------------------|---------------- | > > > > | CSI | | > > > > | | | > > > > -------------------------------- | > > > > | | I2C | > > > > | camera sensor |-----------------------------| > > > > | | > > > > -------------------------------- > > > > > > > > Wentong Wu (3): > > > > media: pci: intel: ivsc: Add CSI submodule > > > > media: pci: intel: ivsc: Add ACE submodule > > > > media: pci: intel: ivsc: Add acquire/release API for ivsc > > > > > > > > drivers/media/pci/Kconfig | 1 + > > > > drivers/media/pci/intel/Makefile | 2 + > > > > drivers/media/pci/intel/ivsc/Kconfig | 12 + > > > > drivers/media/pci/intel/ivsc/Makefile | 7 + > > > > drivers/media/pci/intel/ivsc/ivsc.c | 84 +++++ > > > > drivers/media/pci/intel/ivsc/mei_ace.c | 472 > > > > +++++++++++++++++++++++++ drivers/media/pci/intel/ivsc/mei_ace.h > > > > |> 36 ++ drivers/media/pci/intel/ivsc/mei_csi.c | 342 > > > > ++++++++++++++++++ drivers/media/pci/intel/ivsc/mei_csi.h | 60 ++++ > > > > include/linux/ivsc.h | 74 ++++ > > > > 10 files changed, 1090 insertions(+) create mode 100644 > > > > drivers/media/pci/intel/ivsc/Kconfig > > > > create mode 100644 drivers/media/pci/intel/ivsc/Makefile > > > > create mode 100644 drivers/media/pci/intel/ivsc/ivsc.c > > > > create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.c > > > > create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.h > > > > create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.c > > > > create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.h > > > > create mode 100644 include/linux/ivsc.h > > -- > Regards, > > Laurent Pinchart
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. - The CSI-2 configuration should take place when streaming starts on the sensor (as well as IVSC). - Device links need to be set up via IPU bridge which now is called CIO2 bridge (cio2-bridge.c). Any questions, comments?
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. Regards, Hans > > - The CSI-2 configuration should take place when streaming starts on the > sensor (as well as IVSC). > > - Device links need to be set up via IPU bridge which now is called CIO2 > bridge (cio2-bridge.c). > > Any questions, comments? >
> -----Original Message----- > From: Sakari Ailus <sakari.ailus@linux.intel.com> > 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
> -----Original Message----- > From: Hans de Goede <hdegoede@redhat.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. Thanks Wentong > > Regards, > > Hans > > > > > > > - The CSI-2 configuration should take place when streaming starts on the > > sensor (as well as IVSC). > > > > - Device links need to be set up via IPU bridge which now is called CIO2 > > bridge (cio2-bridge.c). > > > > Any questions, comments? > >
Hi Wentong, On Tue, Mar 07, 2023 at 08:17:04AM +0000, Wu, Wentong wrote: > > > > -----Original Message----- > > From: Hans de Goede <hdegoede@redhat.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.
> -----Original Message----- > From: Sakari Ailus <sakari.ailus@linux.intel.com> > 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@redhat.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. 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
Hi, On 3/7/23 09:40, Wu, Wentong wrote: > > >> -----Original Message----- >> From: Sakari Ailus <sakari.ailus@linux.intel.com> >> 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@redhat.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. > > 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
> -----Original Message----- > From: Hans de Goede <hdegoede@redhat.com> > Sent: Tuesday, March 7, 2023 5:10 PM > > Hi, > > On 3/7/23 09:40, Wu, Wentong wrote: > > > > > >> -----Original Message----- > >> From: Sakari Ailus <sakari.ailus@linux.intel.com> > >> 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@redhat.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. > > > > 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. sensor's parent is the LJCA I2C device whose driver is being upstream https://www.spinics.net/lists/kernel/msg4702552.htmland and sensor's power is controlled by IVSC instead of INT3472 if IVSC enabled. struct device_link *device_link_add(struct device *consumer, struct device *supplier, u32 flags) So probably we have to add above device_link_add in LJCA I2C's driver, and we can find the consumer(camera sensor) with ACPI API, but the supplier, mei_ace, is mei client device under mei framework and it's dynamically allocated device instead of ACPI device, probably I can find its parent with some ACPI lookup from this LJCA I2C device, but unfortunately mei framework doesn't export the API to find mei client device with its parent bus device(struct mei_device). I'm not sure if modeling this mei_ace as LJCA I2C's runtime power control is acceptable, if yes, probably this mei_ace driver have to go with LJCA I2C device driver. BR, Wentong > > 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
Hi, On 3/9/23 02:08, Wu, Wentong wrote: > > >> -----Original Message----- >> From: Hans de Goede <hdegoede@redhat.com> >> Sent: Tuesday, March 7, 2023 5:10 PM >> >> Hi, >> >> On 3/7/23 09:40, Wu, Wentong wrote: >>> >>> >>>> -----Original Message----- >>>> From: Sakari Ailus <sakari.ailus@linux.intel.com> >>>> 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@redhat.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. >>> >>> 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. > > sensor's parent is the LJCA I2C device whose driver is being upstream > https://www.spinics.net/lists/kernel/msg4702552.htmland and sensor's > power is controlled by IVSC instead of INT3472 if IVSC enabled. I believe that the INT3472 code is still involved at least on a Dell Latitude 9420 the INT3472 code still needs to set the clock-enable and the privacy-LED GPIOs otherwise the main camera won't work. So I'm not sure what you mean with "sensor's power is controlled by IVSC instead of INT3472" ? > struct device_link *device_link_add(struct device *consumer, > struct device *supplier, u32 flags) > > So probably we have to add above device_link_add in LJCA I2C's driver, > and we can find the consumer(camera sensor) with ACPI API, but the > supplier, mei_ace, is mei client device under mei framework and it's > dynamically allocated device instead of ACPI device, probably I can find > its parent with some ACPI lookup from this LJCA I2C device, but > unfortunately mei framework doesn't export the API to find mei client > device with its parent bus device(struct mei_device). > > I'm not sure if modeling this mei_ace as LJCA I2C's runtime power > control is acceptable, if yes, probably this mei_ace driver have to go with > LJCA I2C device driver. Looking at the ACPI table the sensor ACPI device has 2 _DEP-s listed the I2C controller and the INT3472 device. Since we are already doing similar setup in the INT3472 device that seems like a good place to add the device_link()-s (it can return -EPROBE_DEFER to wait for the mei_ace to show up). But when the INT3472 code runs, the consumer device does not exist yet and AFAICT the same is true when the LCJA i2c-controller driver is getting registered. The consumer only exists when the i2c_client is instantiated and at that point the sensor drivers probe() method can run immediately and we are too late to add the device_link. As a hobby project I have been working on atomisp2 support and I have a similar issue there. There is no INT3472 device there, but there is a _DSM method which needs to be used to figure out which ACPI GPIO resource is reset / powerdown and if the GPIOs are active-low or active high. I have written a little helper function to call the _DSM and to then turn this into lookups and call devm_acpi_dev_add_driver_gpios(). Since on atomisp2 we cannot use the INT3472 driver to delay the sensor-driver probe and have the INT3472 driver setup the GPIO lookup, at least for the sensor drivers used with atomisp2 there is going to be a need to add a single line to probe() like this: v4l2_get_acpi_sensor_info(&i2c_client->dev, NULL); To me it sounds like we need to do something similar here and extend the helper function which I have written (but not yet submitted upstream) : https://github.com/jwrdegoede/linux-sunxi/commit/e2287979db43d46fa7d354c1bde92eb6219b613d To also setup the device-links needed for the runtime-pm solution to getting the i2c passed through to the sensor. Ideally v4l2_get_acpi_sensor_info() should return void (easier to use in the sensor drivers) but I think it should return an int, so that it can e.g. return -EPROBE_DEFER to wait for the mei_ace. Regards, Hans >> 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 >
Hi > -----Original Message----- > From: Hans de Goede <hdegoede@redhat.com> > Sent: Thursday, March 9, 2023 5:28 PM > > Hi, > > On 3/9/23 02:08, Wu, Wentong wrote: > > > > > >> -----Original Message----- > >> From: Hans de Goede <hdegoede@redhat.com> > >> Sent: Tuesday, March 7, 2023 5:10 PM > >> > >> Hi, > >> > >> On 3/7/23 09:40, Wu, Wentong wrote: > >>> > >>> > >>>> -----Original Message----- > >>>> From: Sakari Ailus <sakari.ailus@linux.intel.com> > >>>> 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@redhat.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. > >>> > >>> 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. > > > > sensor's parent is the LJCA I2C device whose driver is being upstream > > https://www.spinics.net/lists/kernel/msg4702552.htmland and sensor's > > power is controlled by IVSC instead of INT3472 if IVSC enabled. > > I believe that the INT3472 code is still involved at least on a Dell Latitude 9420 > the INT3472 code still needs to set the clock-enable and the privacy-LED GPIOs > otherwise the main camera won't work. > > So I'm not sure what you mean with "sensor's power is controlled by IVSC > instead of INT3472" ? Could you please share your kernel log and DSDT? Thanks BR, Wentong > > > > struct device_link *device_link_add(struct device *consumer, > > struct device *supplier, u32 > > flags) > > > > So probably we have to add above device_link_add in LJCA I2C's driver, > > and we can find the consumer(camera sensor) with ACPI API, but the > > supplier, mei_ace, is mei client device under mei framework and it's > > dynamically allocated device instead of ACPI device, probably I can > > find its parent with some ACPI lookup from this LJCA I2C device, but > > unfortunately mei framework doesn't export the API to find mei client > > device with its parent bus device(struct mei_device). > > > > I'm not sure if modeling this mei_ace as LJCA I2C's runtime power > > control is acceptable, if yes, probably this mei_ace driver have to go > > with LJCA I2C device driver. > > Looking at the ACPI table the sensor ACPI device has 2 _DEP-s listed the I2C > controller and the INT3472 device. Since we are already doing similar setup in > the INT3472 device that seems like a good place to add the device_link()-s (it can > return -EPROBE_DEFER to wait for the mei_ace to show up). > > But when the INT3472 code runs, the consumer device does not exist yet and > AFAICT the same is true when the LCJA i2c-controller driver is getting registered. > The consumer only exists when the i2c_client is instantiated and at that point > the sensor drivers probe() method can run immediately and we are too late to > add the device_link. > > As a hobby project I have been working on atomisp2 support and I have a similar > issue there. There is no INT3472 device there, but there is a _DSM method which > needs to be used to figure out which ACPI GPIO resource is reset / powerdown > and if the GPIOs are active-low or active high. > > I have written a little helper function to call the _DSM and to then turn this into > lookups and call devm_acpi_dev_add_driver_gpios(). > > Since on atomisp2 we cannot use the INT3472 driver to delay the sensor-driver > probe and have the INT3472 driver setup the GPIO lookup, at least for the > sensor drivers used with > atomisp2 there is going to be a need to add a single line to probe() like this: > > v4l2_get_acpi_sensor_info(&i2c_client->dev, NULL); > > To me it sounds like we need to do something similar here and extend the helper > function which I have written (but not yet submitted upstream) : > > https://github.com/jwrdegoede/linux- > sunxi/commit/e2287979db43d46fa7d354c1bde92eb6219b613d > > To also setup the device-links needed for the runtime-pm solution to getting the > i2c passed through to the sensor. > > Ideally v4l2_get_acpi_sensor_info() should return void (easier to use in the > sensor drivers) but I think it should return an int, so that it can e.g. return - > EPROBE_DEFER to wait for the mei_ace. > > Regards, > > Hans > > > > > >> 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 > >
<re-added the previous Cc list, which I dropped because of the large attachments> Hi Wentong, On 3/9/23 15:29, Wu, Wentong wrote: > Hi Hans, > > Thanks > > And AFAICT, there is no IVSC device on your Dell Latitude 9420 where the platform is based on TGL instead of ADL, and I have never heard IVSC runs on TGL, if no IVSC, INT3472 will control sensor's power. > And I will double confirm with people who know dell product well tomorrow. Ah, I was under the impression that there was an IVSC there because: 1. The sensor driver for the used sensor (tries to) poke the IVSC 2. Things did not work without building the IVSC drivers, but that might be due to a dependency on the LCJA GPIO expander instead But you might very well be right, that would also explain the "intel vsc not ready" messages in dmesg. If with the IVSC case the IVSC controls the power to the sensor too, then another option might be to model the I2C-switch + the power-control as a powerdown GPIO for the sensor, which most sensor drivers already try to use. The advantage of doing this would be that GPIO lookups can reference the GPIO provider + consumer by device-name so then we don't need to have both devices instantiated at the time of adding the GPIO lookup. And in that case we could e.g. add the lookup before registering the I2C controller. Sakari, what do you think of instead of using runtime-pm + devlinks having the IVSC code export a powerdown GPIO to the sensor ? This also decouples the ivsc powerstate from the sensor power-state which might be useful if we ever want to use some of the more advanced ivsc features, where AFAICT the ivsc fully controls the sensor. Regards, Hans >> -----Original Message----- >> From: Hans de Goede <hdegoede@redhat.com> >> Sent: Thursday, March 9, 2023 9:30 PM >> To: Wu, Wentong <wentong.wu@intel.com> >> Subject: Re: [PATCH v2 0/3] media: pci: intel: ivsc: Add driver of Intel Visual >> Sensing Controller(IVSC) >> >> Hi Wentong, >> >> Attached are the requested dmesg + acpidump for the Dell Latitude 9420. >> >> Regards, >> >> Hans >> >> >> >> >> On 3/9/23 14:21, Wu, Wentong wrote: >>> Hi >>> >>>> -----Original Message----- >>>> From: Hans de Goede <hdegoede@redhat.com> >>>> Sent: Thursday, March 9, 2023 5:28 PM >>>> >>>> Hi, >>>> >>>> On 3/9/23 02:08, Wu, Wentong wrote: >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Hans de Goede <hdegoede@redhat.com> >>>>>> Sent: Tuesday, March 7, 2023 5:10 PM >>>>>> >>>>>> Hi, >>>>>> >>>>>> On 3/7/23 09:40, Wu, Wentong wrote: >>>>>>> >>>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: Sakari Ailus <sakari.ailus@linux.intel.com> >>>>>>>> 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@redhat.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. >>>>>>> >>>>>>> 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. >>>>> >>>>> sensor's parent is the LJCA I2C device whose driver is being >>>>> upstream https://www.spinics.net/lists/kernel/msg4702552.htmland and >>>>> sensor's power is controlled by IVSC instead of INT3472 if IVSC enabled. >>>> >>>> I believe that the INT3472 code is still involved at least on a Dell >>>> Latitude 9420 the INT3472 code still needs to set the clock-enable >>>> and the privacy-LED GPIOs otherwise the main camera won't work. >>>> >>>> So I'm not sure what you mean with "sensor's power is controlled by >>>> IVSC instead of INT3472" ? >>> >>> Could you please share your kernel log and DSDT? Thanks >>> >>> BR, >>> Wentong >>>> >>>> >>>>> struct device_link *device_link_add(struct device *consumer, >>>>> struct device *supplier, u32 >>>>> flags) >>>>> >>>>> So probably we have to add above device_link_add in LJCA I2C's >>>>> driver, and we can find the consumer(camera sensor) with ACPI API, >>>>> but the supplier, mei_ace, is mei client device under mei framework >>>>> and it's dynamically allocated device instead of ACPI device, >>>>> probably I can find its parent with some ACPI lookup from this LJCA >>>>> I2C device, but unfortunately mei framework doesn't export the API >>>>> to find mei client device with its parent bus device(struct mei_device). >>>>> >>>>> I'm not sure if modeling this mei_ace as LJCA I2C's runtime power >>>>> control is acceptable, if yes, probably this mei_ace driver have to >>>>> go with LJCA I2C device driver. >>>> >>>> Looking at the ACPI table the sensor ACPI device has 2 _DEP-s listed >>>> the I2C controller and the INT3472 device. Since we are already doing >>>> similar setup in the INT3472 device that seems like a good place to >>>> add the device_link()-s (it can return -EPROBE_DEFER to wait for the mei_ace >> to show up). >>>> >>>> But when the INT3472 code runs, the consumer device does not exist >>>> yet and AFAICT the same is true when the LCJA i2c-controller driver is getting >> registered. >>>> The consumer only exists when the i2c_client is instantiated and at >>>> that point the sensor drivers probe() method can run immediately and >>>> we are too late to add the device_link. >>>> >>>> As a hobby project I have been working on atomisp2 support and I have >>>> a similar issue there. There is no INT3472 device there, but there is >>>> a _DSM method which needs to be used to figure out which ACPI GPIO >>>> resource is reset / powerdown and if the GPIOs are active-low or active high. >>>> >>>> I have written a little helper function to call the _DSM and to then >>>> turn this into lookups and call devm_acpi_dev_add_driver_gpios(). >>>> >>>> Since on atomisp2 we cannot use the INT3472 driver to delay the >>>> sensor-driver probe and have the INT3472 driver setup the GPIO >>>> lookup, at least for the sensor drivers used with >>>> atomisp2 there is going to be a need to add a single line to probe() like this: >>>> >>>> v4l2_get_acpi_sensor_info(&i2c_client->dev, NULL); >>>> >>>> To me it sounds like we need to do something similar here and extend >>>> the helper function which I have written (but not yet submitted upstream) : >>>> >>>> https://github.com/jwrdegoede/linux- >>>> sunxi/commit/e2287979db43d46fa7d354c1bde92eb6219b613d >>>> >>>> To also setup the device-links needed for the runtime-pm solution to >>>> getting the i2c passed through to the sensor. >>>> >>>> Ideally v4l2_get_acpi_sensor_info() should return void (easier to use >>>> in the sensor drivers) but I think it should return an int, so that >>>> it can e.g. return - EPROBE_DEFER to wait for the mei_ace. >>>> >>>> Regards, >>>> >>>> Hans >>>> >>>> >>>> >>>> >>>>>> 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 >>>>> >>>
> -----Original Message----- > From: Hans de Goede <hdegoede@redhat.com> > Sent: Thursday, March 9, 2023 11:24 PM > > <re-added the previous Cc list, which I dropped because of the large > attachments> > > Hi Wentong, > > On 3/9/23 15:29, Wu, Wentong wrote: > > Hi Hans, > > > > Thanks > > > > And AFAICT, there is no IVSC device on your Dell Latitude 9420 where the > platform is based on TGL instead of ADL, and I have never heard IVSC runs on > TGL, if no IVSC, INT3472 will control sensor's power. > > And I will double confirm with people who know dell product well tomorrow. > > Ah, I was under the impression that there was an IVSC there because: > > 1. The sensor driver for the used sensor (tries to) poke the IVSC 2. Things did not > work without building the IVSC drivers, but that might > be due to a dependency on the LCJA GPIO expander instead Below is your dmesg log, the required SPI controller for IVSC isn't here. [ 35.538114] ljca 2-6:1.0: acked sem wait timed out ret:0 timeout:20 ack:0 [ 35.538129] ljca 2-6:1.0: MNG_ENUM_SPI failed ret:-110 len:7 num:0 [ 35.538621] ljca 2-6:1.0: LJCA USB device init success [ 35.538776] usbcore: registered new interface driver ljca Also I checked your SSDT, there is no IVSC device and the sensor device depends on INT3472 instead of IVSC device as on my setup. > > But you might very well be right, that would also explain the "intel vsc not ready" > messages in dmesg. > > If with the IVSC case the IVSC controls the power to the sensor too, then > another option might be to model the I2C-switch + the power-control as a > powerdown GPIO for the sensor, which most sensor drivers already try to use. > The advantage of doing this would be that GPIO lookups can reference the GPIO > provider + consumer by device-name so then we don't need to have both > devices instantiated at the time of > adding the GPIO lookup. And in that case we could e.g. add the lookup > before registering the I2C controller. Can we add IVSC device to acpi_honor_dep_ids, so that when everything is done during mei_ace probe, acpi_dev_clear_dependencies can make sensor start probe? BR, Wentong > > Sakari, what do you think of instead of using runtime-pm + devlinks having the > IVSC code export a powerdown GPIO to the sensor ? > > This also decouples the ivsc powerstate from the sensor power-state which > might be useful if we ever want to use some of the more advanced ivsc features, > where AFAICT the ivsc fully controls the sensor. > > Regards, > > Hans > > > > > >> -----Original Message----- > >> From: Hans de Goede <hdegoede@redhat.com> > >> Sent: Thursday, March 9, 2023 9:30 PM > >> To: Wu, Wentong <wentong.wu@intel.com> > >> Subject: Re: [PATCH v2 0/3] media: pci: intel: ivsc: Add driver of > >> Intel Visual Sensing Controller(IVSC) > >> > >> Hi Wentong, > >> > >> Attached are the requested dmesg + acpidump for the Dell Latitude 9420. > >> > >> Regards, > >> > >> Hans > >> > >> > >> > >> > >> On 3/9/23 14:21, Wu, Wentong wrote: > >>> Hi > >>> > >>>> -----Original Message----- > >>>> From: Hans de Goede <hdegoede@redhat.com> > >>>> Sent: Thursday, March 9, 2023 5:28 PM > >>>> > >>>> Hi, > >>>> > >>>> On 3/9/23 02:08, Wu, Wentong wrote: > >>>>> > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Hans de Goede <hdegoede@redhat.com> > >>>>>> Sent: Tuesday, March 7, 2023 5:10 PM > >>>>>> > >>>>>> Hi, > >>>>>> > >>>>>> On 3/7/23 09:40, Wu, Wentong wrote: > >>>>>>> > >>>>>>> > >>>>>>>> -----Original Message----- > >>>>>>>> From: Sakari Ailus <sakari.ailus@linux.intel.com> > >>>>>>>> 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@redhat.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. > >>>>>>> > >>>>>>> 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. > >>>>> > >>>>> sensor's parent is the LJCA I2C device whose driver is being > >>>>> upstream https://www.spinics.net/lists/kernel/msg4702552.htmland > >>>>> and sensor's power is controlled by IVSC instead of INT3472 if IVSC > enabled. > >>>> > >>>> I believe that the INT3472 code is still involved at least on a > >>>> Dell Latitude 9420 the INT3472 code still needs to set the > >>>> clock-enable and the privacy-LED GPIOs otherwise the main camera won't > work. > >>>> > >>>> So I'm not sure what you mean with "sensor's power is controlled by > >>>> IVSC instead of INT3472" ? > >>> > >>> Could you please share your kernel log and DSDT? Thanks > >>> > >>> BR, > >>> Wentong > >>>> > >>>> > >>>>> struct device_link *device_link_add(struct device *consumer, > >>>>> struct device *supplier, u32 > >>>>> flags) > >>>>> > >>>>> So probably we have to add above device_link_add in LJCA I2C's > >>>>> driver, and we can find the consumer(camera sensor) with ACPI API, > >>>>> but the supplier, mei_ace, is mei client device under mei > >>>>> framework and it's dynamically allocated device instead of ACPI > >>>>> device, probably I can find its parent with some ACPI lookup from > >>>>> this LJCA I2C device, but unfortunately mei framework doesn't > >>>>> export the API to find mei client device with its parent bus device(struct > mei_device). > >>>>> > >>>>> I'm not sure if modeling this mei_ace as LJCA I2C's runtime power > >>>>> control is acceptable, if yes, probably this mei_ace driver have > >>>>> to go with LJCA I2C device driver. > >>>> > >>>> Looking at the ACPI table the sensor ACPI device has 2 _DEP-s > >>>> listed the I2C controller and the INT3472 device. Since we are > >>>> already doing similar setup in the INT3472 device that seems like a > >>>> good place to add the device_link()-s (it can return -EPROBE_DEFER > >>>> to wait for the mei_ace > >> to show up). > >>>> > >>>> But when the INT3472 code runs, the consumer device does not exist > >>>> yet and AFAICT the same is true when the LCJA i2c-controller driver > >>>> is getting > >> registered. > >>>> The consumer only exists when the i2c_client is instantiated and at > >>>> that point the sensor drivers probe() method can run immediately > >>>> and we are too late to add the device_link. > >>>> > >>>> As a hobby project I have been working on atomisp2 support and I > >>>> have a similar issue there. There is no INT3472 device there, but > >>>> there is a _DSM method which needs to be used to figure out which > >>>> ACPI GPIO resource is reset / powerdown and if the GPIOs are active-low > or active high. > >>>> > >>>> I have written a little helper function to call the _DSM and to > >>>> then turn this into lookups and call devm_acpi_dev_add_driver_gpios(). > >>>> > >>>> Since on atomisp2 we cannot use the INT3472 driver to delay the > >>>> sensor-driver probe and have the INT3472 driver setup the GPIO > >>>> lookup, at least for the sensor drivers used with > >>>> atomisp2 there is going to be a need to add a single line to probe() like this: > >>>> > >>>> v4l2_get_acpi_sensor_info(&i2c_client->dev, NULL); > >>>> > >>>> To me it sounds like we need to do something similar here and > >>>> extend the helper function which I have written (but not yet submitted > upstream) : > >>>> > >>>> https://github.com/jwrdegoede/linux- > >>>> sunxi/commit/e2287979db43d46fa7d354c1bde92eb6219b613d > >>>> > >>>> To also setup the device-links needed for the runtime-pm solution > >>>> to getting the i2c passed through to the sensor. > >>>> > >>>> Ideally v4l2_get_acpi_sensor_info() should return void (easier to > >>>> use in the sensor drivers) but I think it should return an int, so > >>>> that it can e.g. return - EPROBE_DEFER to wait for the mei_ace. > >>>> > >>>> Regards, > >>>> > >>>> Hans > >>>> > >>>> > >>>> > >>>> > >>>>>> 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 > >>>>> > >>>
> -----Original Message----- > From: Hans de Goede <hdegoede@redhat.com> > Sent: Thursday, March 9, 2023 11:24 PM > > <re-added the previous Cc list, which I dropped because of the large > attachments> > > Hi Wentong, > > On 3/9/23 15:29, Wu, Wentong wrote: > > Hi Hans, > > > > Thanks > > > > And AFAICT, there is no IVSC device on your Dell Latitude 9420 where the > platform is based on TGL instead of ADL, and I have never heard IVSC runs on > TGL, if no IVSC, INT3472 will control sensor's power. > > And I will double confirm with people who know dell product well tomorrow. > > Ah, I was under the impression that there was an IVSC there because: > > 1. The sensor driver for the used sensor (tries to) poke the IVSC 2. Things did not > work without building the IVSC drivers, but that might > be due to a dependency on the LCJA GPIO expander instead > > But you might very well be right, that would also explain the "intel vsc not ready" > messages in dmesg. > > If with the IVSC case the IVSC controls the power to the sensor too, then > another option might be to model the I2C-switch + the power-control as a > powerdown GPIO for the sensor, which most sensor drivers already try to use. > The advantage of doing this would be that GPIO lookups can reference the GPIO > provider + consumer by device-name so then we don't need to have both > devices instantiated at the time of > adding the GPIO lookup. And in that case we could e.g. add the lookup > before registering the I2C controller. Thanks, So the drivers of sensors connected to IVSC have to add power up/down code. BR, Wentong > > Sakari, what do you think of instead of using runtime-pm + devlinks having the > IVSC code export a powerdown GPIO to the sensor ? > > This also decouples the ivsc powerstate from the sensor power-state which > might be useful if we ever want to use some of the more advanced ivsc features, > where AFAICT the ivsc fully controls the sensor. > > Regards, > > Hans > > > > > >> -----Original Message----- > >> From: Hans de Goede <hdegoede@redhat.com> > >> Sent: Thursday, March 9, 2023 9:30 PM > >> To: Wu, Wentong <wentong.wu@intel.com> > >> Subject: Re: [PATCH v2 0/3] media: pci: intel: ivsc: Add driver of > >> Intel Visual Sensing Controller(IVSC) > >> > >> Hi Wentong, > >> > >> Attached are the requested dmesg + acpidump for the Dell Latitude 9420. > >> > >> Regards, > >> > >> Hans > >> > >> > >> > >> > >> On 3/9/23 14:21, Wu, Wentong wrote: > >>> Hi > >>> > >>>> -----Original Message----- > >>>> From: Hans de Goede <hdegoede@redhat.com> > >>>> Sent: Thursday, March 9, 2023 5:28 PM > >>>> > >>>> Hi, > >>>> > >>>> On 3/9/23 02:08, Wu, Wentong wrote: > >>>>> > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Hans de Goede <hdegoede@redhat.com> > >>>>>> Sent: Tuesday, March 7, 2023 5:10 PM > >>>>>> > >>>>>> Hi, > >>>>>> > >>>>>> On 3/7/23 09:40, Wu, Wentong wrote: > >>>>>>> > >>>>>>> > >>>>>>>> -----Original Message----- > >>>>>>>> From: Sakari Ailus <sakari.ailus@linux.intel.com> > >>>>>>>> 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@redhat.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. > >>>>>>> > >>>>>>> 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. > >>>>> > >>>>> sensor's parent is the LJCA I2C device whose driver is being > >>>>> upstream https://www.spinics.net/lists/kernel/msg4702552.htmland > >>>>> and sensor's power is controlled by IVSC instead of INT3472 if IVSC > enabled. > >>>> > >>>> I believe that the INT3472 code is still involved at least on a > >>>> Dell Latitude 9420 the INT3472 code still needs to set the > >>>> clock-enable and the privacy-LED GPIOs otherwise the main camera won't > work. > >>>> > >>>> So I'm not sure what you mean with "sensor's power is controlled by > >>>> IVSC instead of INT3472" ? > >>> > >>> Could you please share your kernel log and DSDT? Thanks > >>> > >>> BR, > >>> Wentong > >>>> > >>>> > >>>>> struct device_link *device_link_add(struct device *consumer, > >>>>> struct device *supplier, u32 > >>>>> flags) > >>>>> > >>>>> So probably we have to add above device_link_add in LJCA I2C's > >>>>> driver, and we can find the consumer(camera sensor) with ACPI API, > >>>>> but the supplier, mei_ace, is mei client device under mei > >>>>> framework and it's dynamically allocated device instead of ACPI > >>>>> device, probably I can find its parent with some ACPI lookup from > >>>>> this LJCA I2C device, but unfortunately mei framework doesn't > >>>>> export the API to find mei client device with its parent bus device(struct > mei_device). > >>>>> > >>>>> I'm not sure if modeling this mei_ace as LJCA I2C's runtime power > >>>>> control is acceptable, if yes, probably this mei_ace driver have > >>>>> to go with LJCA I2C device driver. > >>>> > >>>> Looking at the ACPI table the sensor ACPI device has 2 _DEP-s > >>>> listed the I2C controller and the INT3472 device. Since we are > >>>> already doing similar setup in the INT3472 device that seems like a > >>>> good place to add the device_link()-s (it can return -EPROBE_DEFER > >>>> to wait for the mei_ace > >> to show up). > >>>> > >>>> But when the INT3472 code runs, the consumer device does not exist > >>>> yet and AFAICT the same is true when the LCJA i2c-controller driver > >>>> is getting > >> registered. > >>>> The consumer only exists when the i2c_client is instantiated and at > >>>> that point the sensor drivers probe() method can run immediately > >>>> and we are too late to add the device_link. > >>>> > >>>> As a hobby project I have been working on atomisp2 support and I > >>>> have a similar issue there. There is no INT3472 device there, but > >>>> there is a _DSM method which needs to be used to figure out which > >>>> ACPI GPIO resource is reset / powerdown and if the GPIOs are active-low > or active high. > >>>> > >>>> I have written a little helper function to call the _DSM and to > >>>> then turn this into lookups and call devm_acpi_dev_add_driver_gpios(). > >>>> > >>>> Since on atomisp2 we cannot use the INT3472 driver to delay the > >>>> sensor-driver probe and have the INT3472 driver setup the GPIO > >>>> lookup, at least for the sensor drivers used with > >>>> atomisp2 there is going to be a need to add a single line to probe() like this: > >>>> > >>>> v4l2_get_acpi_sensor_info(&i2c_client->dev, NULL); > >>>> > >>>> To me it sounds like we need to do something similar here and > >>>> extend the helper function which I have written (but not yet submitted > upstream) : > >>>> > >>>> https://github.com/jwrdegoede/linux- > >>>> sunxi/commit/e2287979db43d46fa7d354c1bde92eb6219b613d > >>>> > >>>> To also setup the device-links needed for the runtime-pm solution > >>>> to getting the i2c passed through to the sensor. > >>>> > >>>> Ideally v4l2_get_acpi_sensor_info() should return void (easier to > >>>> use in the sensor drivers) but I think it should return an int, so > >>>> that it can e.g. return - EPROBE_DEFER to wait for the mei_ace. > >>>> > >>>> Regards, > >>>> > >>>> Hans > >>>> > >>>> > >>>> > >>>> > >>>>>> 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 > >>>>> > >>>
Hi, On 3/16/23 09:11, Wu, Wentong wrote: > > >> -----Original Message----- >> From: Hans de Goede <hdegoede@redhat.com> >> Sent: Thursday, March 9, 2023 11:24 PM >> >> <re-added the previous Cc list, which I dropped because of the large >> attachments> >> >> Hi Wentong, >> >> On 3/9/23 15:29, Wu, Wentong wrote: >>> Hi Hans, >>> >>> Thanks >>> >>> And AFAICT, there is no IVSC device on your Dell Latitude 9420 where the >> platform is based on TGL instead of ADL, and I have never heard IVSC runs on >> TGL, if no IVSC, INT3472 will control sensor's power. >>> And I will double confirm with people who know dell product well tomorrow. >> >> Ah, I was under the impression that there was an IVSC there because: >> >> 1. The sensor driver for the used sensor (tries to) poke the IVSC 2. Things did not >> work without building the IVSC drivers, but that might >> be due to a dependency on the LCJA GPIO expander instead >> >> But you might very well be right, that would also explain the "intel vsc not ready" >> messages in dmesg. >> >> If with the IVSC case the IVSC controls the power to the sensor too, then >> another option might be to model the I2C-switch + the power-control as a >> powerdown GPIO for the sensor, which most sensor drivers already try to use. >> The advantage of doing this would be that GPIO lookups can reference the GPIO >> provider + consumer by device-name so then we don't need to have both >> devices instantiated at the time of >> adding the GPIO lookup. And in that case we could e.g. add the lookup >> before registering the I2C controller. > > Thanks, > > So the drivers of sensors connected to IVSC have to add power up/down code. If we go the model it as a powerdown GPIO route, yes. Note most sensor drivers already have support for this since this is used in the INT3472 case already. Regards, Hans > > BR, > Wentong >> >> Sakari, what do you think of instead of using runtime-pm + devlinks having the >> IVSC code export a powerdown GPIO to the sensor ? >> >> This also decouples the ivsc powerstate from the sensor power-state which >> might be useful if we ever want to use some of the more advanced ivsc features, >> where AFAICT the ivsc fully controls the sensor. >> >> Regards, >> >> Hans >> >> >> >> >>>> -----Original Message----- >>>> From: Hans de Goede <hdegoede@redhat.com> >>>> Sent: Thursday, March 9, 2023 9:30 PM >>>> To: Wu, Wentong <wentong.wu@intel.com> >>>> Subject: Re: [PATCH v2 0/3] media: pci: intel: ivsc: Add driver of >>>> Intel Visual Sensing Controller(IVSC) >>>> >>>> Hi Wentong, >>>> >>>> Attached are the requested dmesg + acpidump for the Dell Latitude 9420. >>>> >>>> Regards, >>>> >>>> Hans >>>> >>>> >>>> >>>> >>>> On 3/9/23 14:21, Wu, Wentong wrote: >>>>> Hi >>>>> >>>>>> -----Original Message----- >>>>>> From: Hans de Goede <hdegoede@redhat.com> >>>>>> Sent: Thursday, March 9, 2023 5:28 PM >>>>>> >>>>>> Hi, >>>>>> >>>>>> On 3/9/23 02:08, Wu, Wentong wrote: >>>>>>> >>>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: Hans de Goede <hdegoede@redhat.com> >>>>>>>> Sent: Tuesday, March 7, 2023 5:10 PM >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> On 3/7/23 09:40, Wu, Wentong wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>>> -----Original Message----- >>>>>>>>>> From: Sakari Ailus <sakari.ailus@linux.intel.com> >>>>>>>>>> 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@redhat.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. >>>>>>>>> >>>>>>>>> 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. >>>>>>> >>>>>>> sensor's parent is the LJCA I2C device whose driver is being >>>>>>> upstream https://www.spinics.net/lists/kernel/msg4702552.htmland >>>>>>> and sensor's power is controlled by IVSC instead of INT3472 if IVSC >> enabled. >>>>>> >>>>>> I believe that the INT3472 code is still involved at least on a >>>>>> Dell Latitude 9420 the INT3472 code still needs to set the >>>>>> clock-enable and the privacy-LED GPIOs otherwise the main camera won't >> work. >>>>>> >>>>>> So I'm not sure what you mean with "sensor's power is controlled by >>>>>> IVSC instead of INT3472" ? >>>>> >>>>> Could you please share your kernel log and DSDT? Thanks >>>>> >>>>> BR, >>>>> Wentong >>>>>> >>>>>> >>>>>>> struct device_link *device_link_add(struct device *consumer, >>>>>>> struct device *supplier, u32 >>>>>>> flags) >>>>>>> >>>>>>> So probably we have to add above device_link_add in LJCA I2C's >>>>>>> driver, and we can find the consumer(camera sensor) with ACPI API, >>>>>>> but the supplier, mei_ace, is mei client device under mei >>>>>>> framework and it's dynamically allocated device instead of ACPI >>>>>>> device, probably I can find its parent with some ACPI lookup from >>>>>>> this LJCA I2C device, but unfortunately mei framework doesn't >>>>>>> export the API to find mei client device with its parent bus device(struct >> mei_device). >>>>>>> >>>>>>> I'm not sure if modeling this mei_ace as LJCA I2C's runtime power >>>>>>> control is acceptable, if yes, probably this mei_ace driver have >>>>>>> to go with LJCA I2C device driver. >>>>>> >>>>>> Looking at the ACPI table the sensor ACPI device has 2 _DEP-s >>>>>> listed the I2C controller and the INT3472 device. Since we are >>>>>> already doing similar setup in the INT3472 device that seems like a >>>>>> good place to add the device_link()-s (it can return -EPROBE_DEFER >>>>>> to wait for the mei_ace >>>> to show up). >>>>>> >>>>>> But when the INT3472 code runs, the consumer device does not exist >>>>>> yet and AFAICT the same is true when the LCJA i2c-controller driver >>>>>> is getting >>>> registered. >>>>>> The consumer only exists when the i2c_client is instantiated and at >>>>>> that point the sensor drivers probe() method can run immediately >>>>>> and we are too late to add the device_link. >>>>>> >>>>>> As a hobby project I have been working on atomisp2 support and I >>>>>> have a similar issue there. There is no INT3472 device there, but >>>>>> there is a _DSM method which needs to be used to figure out which >>>>>> ACPI GPIO resource is reset / powerdown and if the GPIOs are active-low >> or active high. >>>>>> >>>>>> I have written a little helper function to call the _DSM and to >>>>>> then turn this into lookups and call devm_acpi_dev_add_driver_gpios(). >>>>>> >>>>>> Since on atomisp2 we cannot use the INT3472 driver to delay the >>>>>> sensor-driver probe and have the INT3472 driver setup the GPIO >>>>>> lookup, at least for the sensor drivers used with >>>>>> atomisp2 there is going to be a need to add a single line to probe() like this: >>>>>> >>>>>> v4l2_get_acpi_sensor_info(&i2c_client->dev, NULL); >>>>>> >>>>>> To me it sounds like we need to do something similar here and >>>>>> extend the helper function which I have written (but not yet submitted >> upstream) : >>>>>> >>>>>> https://github.com/jwrdegoede/linux- >>>>>> sunxi/commit/e2287979db43d46fa7d354c1bde92eb6219b613d >>>>>> >>>>>> To also setup the device-links needed for the runtime-pm solution >>>>>> to getting the i2c passed through to the sensor. >>>>>> >>>>>> Ideally v4l2_get_acpi_sensor_info() should return void (easier to >>>>>> use in the sensor drivers) but I think it should return an int, so >>>>>> that it can e.g. return - EPROBE_DEFER to wait for the mei_ace. >>>>>> >>>>>> Regards, >>>>>> >>>>>> Hans >>>>>> >>>>>> >>>>>> >>>>>> >>>>>>>> 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 >>>>>>> >>>>> >
Hi, On 3/16/23 03:58, Wu, Wentong wrote: > > >> -----Original Message----- >> From: Hans de Goede <hdegoede@redhat.com> >> Sent: Thursday, March 9, 2023 11:24 PM >> >> <re-added the previous Cc list, which I dropped because of the large >> attachments> >> >> Hi Wentong, >> >> On 3/9/23 15:29, Wu, Wentong wrote: >>> Hi Hans, >>> >>> Thanks >>> >>> And AFAICT, there is no IVSC device on your Dell Latitude 9420 where the >> platform is based on TGL instead of ADL, and I have never heard IVSC runs on >> TGL, if no IVSC, INT3472 will control sensor's power. >>> And I will double confirm with people who know dell product well tomorrow. >> >> Ah, I was under the impression that there was an IVSC there because: >> >> 1. The sensor driver for the used sensor (tries to) poke the IVSC 2. Things did not >> work without building the IVSC drivers, but that might >> be due to a dependency on the LCJA GPIO expander instead > > Below is your dmesg log, the required SPI controller for IVSC isn't here. > > [ 35.538114] ljca 2-6:1.0: acked sem wait timed out ret:0 timeout:20 ack:0 > [ 35.538129] ljca 2-6:1.0: MNG_ENUM_SPI failed ret:-110 len:7 num:0 > [ 35.538621] ljca 2-6:1.0: LJCA USB device init success > [ 35.538776] usbcore: registered new interface driver ljca > > Also I checked your SSDT, there is no IVSC device and the sensor device depends on > INT3472 instead of IVSC device as on my setup. Ack. >> But you might very well be right, that would also explain the "intel vsc not ready" >> messages in dmesg. >> >> If with the IVSC case the IVSC controls the power to the sensor too, then >> another option might be to model the I2C-switch + the power-control as a >> powerdown GPIO for the sensor, which most sensor drivers already try to use. >> The advantage of doing this would be that GPIO lookups can reference the GPIO >> provider + consumer by device-name so then we don't need to have both >> devices instantiated at the time of >> adding the GPIO lookup. And in that case we could e.g. add the lookup >> before registering the I2C controller. > > Can we add IVSC device to acpi_honor_dep_ids, so that when everything is done during > mei_ace probe, acpi_dev_clear_dependencies can make sensor start probe? Does the sensor ACPI device node have an ACPI _DEP on the IVSC device ? If yes, then yes we can add the IVSC device to acpi_honor_dep_id and make mei_ace probe call acpi_dev_clear_dependencies(). Regards, Hans >> Sakari, what do you think of instead of using runtime-pm + devlinks having the >> IVSC code export a powerdown GPIO to the sensor ? >> >> This also decouples the ivsc powerstate from the sensor power-state which >> might be useful if we ever want to use some of the more advanced ivsc features, >> where AFAICT the ivsc fully controls the sensor. >> >> Regards, >> >> Hans >> >> >> >> >>>> -----Original Message----- >>>> From: Hans de Goede <hdegoede@redhat.com> >>>> Sent: Thursday, March 9, 2023 9:30 PM >>>> To: Wu, Wentong <wentong.wu@intel.com> >>>> Subject: Re: [PATCH v2 0/3] media: pci: intel: ivsc: Add driver of >>>> Intel Visual Sensing Controller(IVSC) >>>> >>>> Hi Wentong, >>>> >>>> Attached are the requested dmesg + acpidump for the Dell Latitude 9420. >>>> >>>> Regards, >>>> >>>> Hans >>>> >>>> >>>> >>>> >>>> On 3/9/23 14:21, Wu, Wentong wrote: >>>>> Hi >>>>> >>>>>> -----Original Message----- >>>>>> From: Hans de Goede <hdegoede@redhat.com> >>>>>> Sent: Thursday, March 9, 2023 5:28 PM >>>>>> >>>>>> Hi, >>>>>> >>>>>> On 3/9/23 02:08, Wu, Wentong wrote: >>>>>>> >>>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: Hans de Goede <hdegoede@redhat.com> >>>>>>>> Sent: Tuesday, March 7, 2023 5:10 PM >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> On 3/7/23 09:40, Wu, Wentong wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>>> -----Original Message----- >>>>>>>>>> From: Sakari Ailus <sakari.ailus@linux.intel.com> >>>>>>>>>> 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@redhat.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. >>>>>>>>> >>>>>>>>> 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. >>>>>>> >>>>>>> sensor's parent is the LJCA I2C device whose driver is being >>>>>>> upstream https://www.spinics.net/lists/kernel/msg4702552.htmland >>>>>>> and sensor's power is controlled by IVSC instead of INT3472 if IVSC >> enabled. >>>>>> >>>>>> I believe that the INT3472 code is still involved at least on a >>>>>> Dell Latitude 9420 the INT3472 code still needs to set the >>>>>> clock-enable and the privacy-LED GPIOs otherwise the main camera won't >> work. >>>>>> >>>>>> So I'm not sure what you mean with "sensor's power is controlled by >>>>>> IVSC instead of INT3472" ? >>>>> >>>>> Could you please share your kernel log and DSDT? Thanks >>>>> >>>>> BR, >>>>> Wentong >>>>>> >>>>>> >>>>>>> struct device_link *device_link_add(struct device *consumer, >>>>>>> struct device *supplier, u32 >>>>>>> flags) >>>>>>> >>>>>>> So probably we have to add above device_link_add in LJCA I2C's >>>>>>> driver, and we can find the consumer(camera sensor) with ACPI API, >>>>>>> but the supplier, mei_ace, is mei client device under mei >>>>>>> framework and it's dynamically allocated device instead of ACPI >>>>>>> device, probably I can find its parent with some ACPI lookup from >>>>>>> this LJCA I2C device, but unfortunately mei framework doesn't >>>>>>> export the API to find mei client device with its parent bus device(struct >> mei_device). >>>>>>> >>>>>>> I'm not sure if modeling this mei_ace as LJCA I2C's runtime power >>>>>>> control is acceptable, if yes, probably this mei_ace driver have >>>>>>> to go with LJCA I2C device driver. >>>>>> >>>>>> Looking at the ACPI table the sensor ACPI device has 2 _DEP-s >>>>>> listed the I2C controller and the INT3472 device. Since we are >>>>>> already doing similar setup in the INT3472 device that seems like a >>>>>> good place to add the device_link()-s (it can return -EPROBE_DEFER >>>>>> to wait for the mei_ace >>>> to show up). >>>>>> >>>>>> But when the INT3472 code runs, the consumer device does not exist >>>>>> yet and AFAICT the same is true when the LCJA i2c-controller driver >>>>>> is getting >>>> registered. >>>>>> The consumer only exists when the i2c_client is instantiated and at >>>>>> that point the sensor drivers probe() method can run immediately >>>>>> and we are too late to add the device_link. >>>>>> >>>>>> As a hobby project I have been working on atomisp2 support and I >>>>>> have a similar issue there. There is no INT3472 device there, but >>>>>> there is a _DSM method which needs to be used to figure out which >>>>>> ACPI GPIO resource is reset / powerdown and if the GPIOs are active-low >> or active high. >>>>>> >>>>>> I have written a little helper function to call the _DSM and to >>>>>> then turn this into lookups and call devm_acpi_dev_add_driver_gpios(). >>>>>> >>>>>> Since on atomisp2 we cannot use the INT3472 driver to delay the >>>>>> sensor-driver probe and have the INT3472 driver setup the GPIO >>>>>> lookup, at least for the sensor drivers used with >>>>>> atomisp2 there is going to be a need to add a single line to probe() like this: >>>>>> >>>>>> v4l2_get_acpi_sensor_info(&i2c_client->dev, NULL); >>>>>> >>>>>> To me it sounds like we need to do something similar here and >>>>>> extend the helper function which I have written (but not yet submitted >> upstream) : >>>>>> >>>>>> https://github.com/jwrdegoede/linux- >>>>>> sunxi/commit/e2287979db43d46fa7d354c1bde92eb6219b613d >>>>>> >>>>>> To also setup the device-links needed for the runtime-pm solution >>>>>> to getting the i2c passed through to the sensor. >>>>>> >>>>>> Ideally v4l2_get_acpi_sensor_info() should return void (easier to >>>>>> use in the sensor drivers) but I think it should return an int, so >>>>>> that it can e.g. return - EPROBE_DEFER to wait for the mei_ace. >>>>>> >>>>>> Regards, >>>>>> >>>>>> Hans >>>>>> >>>>>> >>>>>> >>>>>> >>>>>>>> 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 >>>>>>> >>>>> >
> -----Original Message----- > From: Hans de Goede <hdegoede@redhat.com> > Sent: Thursday, March 16, 2023 5:04 PM > > Hi, > > On 3/16/23 03:58, Wu, Wentong wrote: > > > > > >> -----Original Message----- > >> From: Hans de Goede <hdegoede@redhat.com> > >> Sent: Thursday, March 9, 2023 11:24 PM > >> > >> <re-added the previous Cc list, which I dropped because of the large > >> attachments> > >> > >> Hi Wentong, > >> > >> On 3/9/23 15:29, Wu, Wentong wrote: > >>> Hi Hans, > >>> > >>> Thanks > >>> > >>> And AFAICT, there is no IVSC device on your Dell Latitude 9420 where > >>> the > >> platform is based on TGL instead of ADL, and I have never heard IVSC > >> runs on TGL, if no IVSC, INT3472 will control sensor's power. > >>> And I will double confirm with people who know dell product well tomorrow. > >> > >> Ah, I was under the impression that there was an IVSC there because: > >> > >> 1. The sensor driver for the used sensor (tries to) poke the IVSC 2. > >> Things did not work without building the IVSC drivers, but that might > >> be due to a dependency on the LCJA GPIO expander instead > > > > Below is your dmesg log, the required SPI controller for IVSC isn't here. > > > > [ 35.538114] ljca 2-6:1.0: acked sem wait timed out ret:0 timeout:20 ack:0 > > [ 35.538129] ljca 2-6:1.0: MNG_ENUM_SPI failed ret:-110 len:7 num:0 > > [ 35.538621] ljca 2-6:1.0: LJCA USB device init success > > [ 35.538776] usbcore: registered new interface driver ljca > > > > Also I checked your SSDT, there is no IVSC device and the sensor > > device depends on > > INT3472 instead of IVSC device as on my setup. > > Ack. > > >> But you might very well be right, that would also explain the "intel vsc not > ready" > >> messages in dmesg. > >> > >> If with the IVSC case the IVSC controls the power to the sensor too, > >> then another option might be to model the I2C-switch + the > >> power-control as a powerdown GPIO for the sensor, which most sensor > drivers already try to use. > >> The advantage of doing this would be that GPIO lookups can reference > >> the GPIO provider + consumer by device-name so then we don't need to > >> have both devices instantiated at the time of > >> adding the GPIO lookup. And in that case we could e.g. add the lookup > >> before registering the I2C controller. > > > > Can we add IVSC device to acpi_honor_dep_ids, so that when everything > > is done during mei_ace probe, acpi_dev_clear_dependencies can make sensor > start probe? > > Does the sensor ACPI device node have an ACPI _DEP on the IVSC device ? Yes, > > If yes, then yes we can add the IVSC device to acpi_honor_dep_id and make > mei_ace probe call acpi_dev_clear_dependencies(). But I prefer the powerdown gpio model, because we have to follow the commands sequences as below which is required by firmware, runtime pm is hard to achieve this. + /* switch camera sensor ownership to host */ + ret = ace_set_camera_owner(ACE_CAMERA_HOST); + if (ret) + goto error; + + /* switch CSI-2 link to host */ + ret = csi_set_link_owner(CSI_LINK_HOST, callback, context); + if (ret) + goto release_camera; + + /* configure CSI-2 link */ + ret = csi_set_link_cfg(nr_of_lanes, link_freq); + if (ret) + goto release_csi; BR, Wentong > > Regards, > > Hans > > > > >> Sakari, what do you think of instead of using runtime-pm + devlinks > >> having the IVSC code export a powerdown GPIO to the sensor ? > >> > >> This also decouples the ivsc powerstate from the sensor power-state > >> which might be useful if we ever want to use some of the more > >> advanced ivsc features, where AFAICT the ivsc fully controls the sensor. > >> > >> Regards, > >> > >> Hans > >> > >> > >> > >> > >>>> -----Original Message----- > >>>> From: Hans de Goede <hdegoede@redhat.com> > >>>> Sent: Thursday, March 9, 2023 9:30 PM > >>>> To: Wu, Wentong <wentong.wu@intel.com> > >>>> Subject: Re: [PATCH v2 0/3] media: pci: intel: ivsc: Add driver of > >>>> Intel Visual Sensing Controller(IVSC) > >>>> > >>>> Hi Wentong, > >>>> > >>>> Attached are the requested dmesg + acpidump for the Dell Latitude 9420. > >>>> > >>>> Regards, > >>>> > >>>> Hans > >>>> > >>>> > >>>> > >>>> > >>>> On 3/9/23 14:21, Wu, Wentong wrote: > >>>>> Hi > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Hans de Goede <hdegoede@redhat.com> > >>>>>> Sent: Thursday, March 9, 2023 5:28 PM > >>>>>> > >>>>>> Hi, > >>>>>> > >>>>>> On 3/9/23 02:08, Wu, Wentong wrote: > >>>>>>> > >>>>>>> > >>>>>>>> -----Original Message----- > >>>>>>>> From: Hans de Goede <hdegoede@redhat.com> > >>>>>>>> Sent: Tuesday, March 7, 2023 5:10 PM > >>>>>>>> > >>>>>>>> Hi, > >>>>>>>> > >>>>>>>> On 3/7/23 09:40, Wu, Wentong wrote: > >>>>>>>>> > >>>>>>>>> > >>>>>>>>>> -----Original Message----- > >>>>>>>>>> From: Sakari Ailus <sakari.ailus@linux.intel.com> > >>>>>>>>>> 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@redhat.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. > >>>>>>>>> > >>>>>>>>> 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. > >>>>>>> > >>>>>>> sensor's parent is the LJCA I2C device whose driver is being > >>>>>>> upstream https://www.spinics.net/lists/kernel/msg4702552.htmland > >>>>>>> and sensor's power is controlled by IVSC instead of INT3472 if > >>>>>>> IVSC > >> enabled. > >>>>>> > >>>>>> I believe that the INT3472 code is still involved at least on a > >>>>>> Dell Latitude 9420 the INT3472 code still needs to set the > >>>>>> clock-enable and the privacy-LED GPIOs otherwise the main camera > >>>>>> won't > >> work. > >>>>>> > >>>>>> So I'm not sure what you mean with "sensor's power is controlled > >>>>>> by IVSC instead of INT3472" ? > >>>>> > >>>>> Could you please share your kernel log and DSDT? Thanks > >>>>> > >>>>> BR, > >>>>> Wentong > >>>>>> > >>>>>> > >>>>>>> struct device_link *device_link_add(struct device *consumer, > >>>>>>> struct device *supplier, u32 > >>>>>>> flags) > >>>>>>> > >>>>>>> So probably we have to add above device_link_add in LJCA I2C's > >>>>>>> driver, and we can find the consumer(camera sensor) with ACPI > >>>>>>> API, but the supplier, mei_ace, is mei client device under mei > >>>>>>> framework and it's dynamically allocated device instead of ACPI > >>>>>>> device, probably I can find its parent with some ACPI lookup > >>>>>>> from this LJCA I2C device, but unfortunately mei framework > >>>>>>> doesn't export the API to find mei client device with its parent > >>>>>>> bus device(struct > >> mei_device). > >>>>>>> > >>>>>>> I'm not sure if modeling this mei_ace as LJCA I2C's runtime > >>>>>>> power control is acceptable, if yes, probably this mei_ace > >>>>>>> driver have to go with LJCA I2C device driver. > >>>>>> > >>>>>> Looking at the ACPI table the sensor ACPI device has 2 _DEP-s > >>>>>> listed the I2C controller and the INT3472 device. Since we are > >>>>>> already doing similar setup in the INT3472 device that seems like > >>>>>> a good place to add the device_link()-s (it can return > >>>>>> -EPROBE_DEFER to wait for the mei_ace > >>>> to show up). > >>>>>> > >>>>>> But when the INT3472 code runs, the consumer device does not > >>>>>> exist yet and AFAICT the same is true when the LCJA > >>>>>> i2c-controller driver is getting > >>>> registered. > >>>>>> The consumer only exists when the i2c_client is instantiated and > >>>>>> at that point the sensor drivers probe() method can run > >>>>>> immediately and we are too late to add the device_link. > >>>>>> > >>>>>> As a hobby project I have been working on atomisp2 support and I > >>>>>> have a similar issue there. There is no INT3472 device there, but > >>>>>> there is a _DSM method which needs to be used to figure out which > >>>>>> ACPI GPIO resource is reset / powerdown and if the GPIOs are > >>>>>> active-low > >> or active high. > >>>>>> > >>>>>> I have written a little helper function to call the _DSM and to > >>>>>> then turn this into lookups and call devm_acpi_dev_add_driver_gpios(). > >>>>>> > >>>>>> Since on atomisp2 we cannot use the INT3472 driver to delay the > >>>>>> sensor-driver probe and have the INT3472 driver setup the GPIO > >>>>>> lookup, at least for the sensor drivers used with > >>>>>> atomisp2 there is going to be a need to add a single line to probe() like > this: > >>>>>> > >>>>>> v4l2_get_acpi_sensor_info(&i2c_client->dev, NULL); > >>>>>> > >>>>>> To me it sounds like we need to do something similar here and > >>>>>> extend the helper function which I have written (but not yet > >>>>>> submitted > >> upstream) : > >>>>>> > >>>>>> https://github.com/jwrdegoede/linux- > >>>>>> sunxi/commit/e2287979db43d46fa7d354c1bde92eb6219b613d > >>>>>> > >>>>>> To also setup the device-links needed for the runtime-pm solution > >>>>>> to getting the i2c passed through to the sensor. > >>>>>> > >>>>>> Ideally v4l2_get_acpi_sensor_info() should return void (easier to > >>>>>> use in the sensor drivers) but I think it should return an int, > >>>>>> so that it can e.g. return - EPROBE_DEFER to wait for the mei_ace. > >>>>>> > >>>>>> Regards, > >>>>>> > >>>>>> Hans > >>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>>>>>> 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 > >>>>>>> > >>>>> > >
Hi Wentong, On Fri, Mar 17, 2023 at 07:30:19AM +0000, Wu, Wentong wrote: > > > > -----Original Message----- > > From: Hans de Goede <hdegoede@redhat.com> > > Sent: Thursday, March 16, 2023 5:04 PM > > > > Hi, > > > > On 3/16/23 03:58, Wu, Wentong wrote: > > > > > > > > >> -----Original Message----- > > >> From: Hans de Goede <hdegoede@redhat.com> > > >> Sent: Thursday, March 9, 2023 11:24 PM > > >> > > >> <re-added the previous Cc list, which I dropped because of the large > > >> attachments> > > >> > > >> Hi Wentong, > > >> > > >> On 3/9/23 15:29, Wu, Wentong wrote: > > >>> Hi Hans, > > >>> > > >>> Thanks > > >>> > > >>> And AFAICT, there is no IVSC device on your Dell Latitude 9420 where > > >>> the > > >> platform is based on TGL instead of ADL, and I have never heard IVSC > > >> runs on TGL, if no IVSC, INT3472 will control sensor's power. > > >>> And I will double confirm with people who know dell product well tomorrow. > > >> > > >> Ah, I was under the impression that there was an IVSC there because: > > >> > > >> 1. The sensor driver for the used sensor (tries to) poke the IVSC 2. > > >> Things did not work without building the IVSC drivers, but that might > > >> be due to a dependency on the LCJA GPIO expander instead > > > > > > Below is your dmesg log, the required SPI controller for IVSC isn't here. > > > > > > [ 35.538114] ljca 2-6:1.0: acked sem wait timed out ret:0 timeout:20 ack:0 > > > [ 35.538129] ljca 2-6:1.0: MNG_ENUM_SPI failed ret:-110 len:7 num:0 > > > [ 35.538621] ljca 2-6:1.0: LJCA USB device init success > > > [ 35.538776] usbcore: registered new interface driver ljca > > > > > > Also I checked your SSDT, there is no IVSC device and the sensor > > > device depends on > > > INT3472 instead of IVSC device as on my setup. > > > > Ack. > > > > >> But you might very well be right, that would also explain the "intel vsc not > > ready" > > >> messages in dmesg. > > >> > > >> If with the IVSC case the IVSC controls the power to the sensor too, > > >> then another option might be to model the I2C-switch + the > > >> power-control as a powerdown GPIO for the sensor, which most sensor > > drivers already try to use. > > >> The advantage of doing this would be that GPIO lookups can reference > > >> the GPIO provider + consumer by device-name so then we don't need to > > >> have both devices instantiated at the time of > > >> adding the GPIO lookup. And in that case we could e.g. add the lookup > > >> before registering the I2C controller. > > > > > > Can we add IVSC device to acpi_honor_dep_ids, so that when everything > > > is done during mei_ace probe, acpi_dev_clear_dependencies can make sensor > > start probe? > > > > Does the sensor ACPI device node have an ACPI _DEP on the IVSC device ? > > Yes, > > > > > If yes, then yes we can add the IVSC device to acpi_honor_dep_id and make > > mei_ace probe call acpi_dev_clear_dependencies(). > > But I prefer the powerdown gpio model, because we have to follow the commands > sequences as below which is required by firmware, runtime pm is hard to achieve this. How so? I don't insist on the runtime PM based solution but I'd rather not have changes to virtually all sensor drivers --- this is an external chip to them. > + /* switch camera sensor ownership to host */ > + ret = ace_set_camera_owner(ACE_CAMERA_HOST); > + if (ret) > + goto error; > + > + /* switch CSI-2 link to host */ > + ret = csi_set_link_owner(CSI_LINK_HOST, callback, context); > + if (ret) > + goto release_camera; > + > + /* configure CSI-2 link */ > + ret = csi_set_link_cfg(nr_of_lanes, link_freq); > + if (ret) > + goto release_csi;
> -----Original Message----- > From: Sakari Ailus <sakari.ailus@linux.intel.com> > Sent: Friday, March 17, 2023 4:59 PM > > Hi Wentong, > > On Fri, Mar 17, 2023 at 07:30:19AM +0000, Wu, Wentong wrote: > > > > > > > -----Original Message----- > > > From: Hans de Goede <hdegoede@redhat.com> > > > Sent: Thursday, March 16, 2023 5:04 PM > > > > > > Hi, > > > > > > On 3/16/23 03:58, Wu, Wentong wrote: > > > > > > > > > > > >> -----Original Message----- > > > >> From: Hans de Goede <hdegoede@redhat.com> > > > >> Sent: Thursday, March 9, 2023 11:24 PM > > > >> > > > >> <re-added the previous Cc list, which I dropped because of the > > > >> large > > > >> attachments> > > > >> > > > >> Hi Wentong, > > > >> > > > >> On 3/9/23 15:29, Wu, Wentong wrote: > > > >>> Hi Hans, > > > >>> > > > >>> Thanks > > > >>> > > > >>> And AFAICT, there is no IVSC device on your Dell Latitude 9420 > > > >>> where the > > > >> platform is based on TGL instead of ADL, and I have never heard > > > >> IVSC runs on TGL, if no IVSC, INT3472 will control sensor's power. > > > >>> And I will double confirm with people who know dell product well > tomorrow. > > > >> > > > >> Ah, I was under the impression that there was an IVSC there because: > > > >> > > > >> 1. The sensor driver for the used sensor (tries to) poke the IVSC 2. > > > >> Things did not work without building the IVSC drivers, but that might > > > >> be due to a dependency on the LCJA GPIO expander instead > > > > > > > > Below is your dmesg log, the required SPI controller for IVSC isn't here. > > > > > > > > [ 35.538114] ljca 2-6:1.0: acked sem wait timed out ret:0 timeout:20 > ack:0 > > > > [ 35.538129] ljca 2-6:1.0: MNG_ENUM_SPI failed ret:-110 len:7 num:0 > > > > [ 35.538621] ljca 2-6:1.0: LJCA USB device init success > > > > [ 35.538776] usbcore: registered new interface driver ljca > > > > > > > > Also I checked your SSDT, there is no IVSC device and the sensor > > > > device depends on > > > > INT3472 instead of IVSC device as on my setup. > > > > > > Ack. > > > > > > >> But you might very well be right, that would also explain the > > > >> "intel vsc not > > > ready" > > > >> messages in dmesg. > > > >> > > > >> If with the IVSC case the IVSC controls the power to the sensor > > > >> too, then another option might be to model the I2C-switch + the > > > >> power-control as a powerdown GPIO for the sensor, which most > > > >> sensor > > > drivers already try to use. > > > >> The advantage of doing this would be that GPIO lookups can > > > >> reference the GPIO provider + consumer by device-name so then we > > > >> don't need to have both devices instantiated at the time of > > > >> adding the GPIO lookup. And in that case we could e.g. add the lookup > > > >> before registering the I2C controller. > > > > > > > > Can we add IVSC device to acpi_honor_dep_ids, so that when > > > > everything is done during mei_ace probe, > > > > acpi_dev_clear_dependencies can make sensor > > > start probe? > > > > > > Does the sensor ACPI device node have an ACPI _DEP on the IVSC device ? > > > > Yes, > > > > > > > > If yes, then yes we can add the IVSC device to acpi_honor_dep_id and > > > make mei_ace probe call acpi_dev_clear_dependencies(). > > > > But I prefer the powerdown gpio model, because we have to follow the > > commands sequences as below which is required by firmware, runtime pm is > hard to achieve this. > > How so? But we have to find a way to download commands to firmware following below sequence before writing camera sensor's registers to start camera sensor's steam. BR, Wentong > > I don't insist on the runtime PM based solution but I'd rather not have changes > to virtually all sensor drivers --- this is an external chip to them. > > > + /* switch camera sensor ownership to host */ > > + ret = ace_set_camera_owner(ACE_CAMERA_HOST); > > + if (ret) > > + goto error; > > + > > + /* switch CSI-2 link to host */ > > + ret = csi_set_link_owner(CSI_LINK_HOST, callback, context); > > + if (ret) > > + goto release_camera; > > + > > + /* configure CSI-2 link */ > > + ret = csi_set_link_cfg(nr_of_lanes, link_freq); > > + if (ret) > > + goto release_csi; > > -- > Kind regards, > > Sakari Ailus
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. Implementation: There are two different drivers to handle ACE and CSI hardware modules inside IVSC. - mei_csi: MEI client driver to send commands and receive notifications from CSI module. - mei_ace: MEI client driver to send commands and get status from ACE module. Interface is exposed via ivsc.h to acquire and release camera sensor and CSI-2 link. Below diagram shows connections of IVSC/ISH/IPU/Camera sensor. ----------------------------------------------------------------------------- | Host Processor | | | | ----------------- ----------------- --------------- | | | | | | | | I2C | | | IPU | | ISH | |camera driver|--| | | | | | | | | | | | ----------------- ----------------- --------------- | | | | | | | | | | | --------------- | | | | | | | | | | | | | IVSC driver | | | | | | | | | | | | | --------------- | | | | | | | | ----------------|-----------------------|----------------------|---------|--- | CSI | I2C |SPI | | | | | ----------------|-----------------------|---------------- | | | IVSC | | | | | | | | | | ----------------- ----------------- | | | | | | | | | | | | | CSI | | ACE | |------| | | | | | | | | | ----------------- ----------------- | | | | | I2C | | ----------------|-----------------------|---------------- | | CSI | | | | | -------------------------------- | | | I2C | | camera sensor |-----------------------------| | | -------------------------------- Wentong Wu (3): media: pci: intel: ivsc: Add CSI submodule media: pci: intel: ivsc: Add ACE submodule media: pci: intel: ivsc: Add acquire/release API for ivsc drivers/media/pci/Kconfig | 1 + drivers/media/pci/intel/Makefile | 2 + drivers/media/pci/intel/ivsc/Kconfig | 12 + drivers/media/pci/intel/ivsc/Makefile | 7 + drivers/media/pci/intel/ivsc/ivsc.c | 84 +++++ drivers/media/pci/intel/ivsc/mei_ace.c | 472 +++++++++++++++++++++++++ drivers/media/pci/intel/ivsc/mei_ace.h | 36 ++ drivers/media/pci/intel/ivsc/mei_csi.c | 342 ++++++++++++++++++ drivers/media/pci/intel/ivsc/mei_csi.h | 60 ++++ include/linux/ivsc.h | 74 ++++ 10 files changed, 1090 insertions(+) create mode 100644 drivers/media/pci/intel/ivsc/Kconfig create mode 100644 drivers/media/pci/intel/ivsc/Makefile create mode 100644 drivers/media/pci/intel/ivsc/ivsc.c create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.c create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.h create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.c create mode 100644 drivers/media/pci/intel/ivsc/mei_csi.h create mode 100644 include/linux/ivsc.h