Re: [PATCH v3 2/3] media: pci: intel: ivsc: Add ACE submodule

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Wentong,

On Thu, Mar 30, 2023 at 07:32:27AM +0000, Wu, Wentong wrote:
> Hi Sakari,
> 
> Thanks
> 
> > -----Original Message-----
> > From: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Sent: Wednesday, March 29, 2023 4:04 PM
> > 
> > Hi Wentong,
> > 
> > On Mon, Mar 27, 2023 at 02:23:07PM +0800, Wentong Wu wrote:
> > > ACE is a submodule of IVSC which controls camera sensor's ownership,
> > > belonging to host or IVSC. When IVSC owns camera sensor, it is for
> > > algorithm computing. When host wants to control camera sensor, ACE
> > > module needs to be informed of ownership with defined interface.
> > >
> > > The interface is via MEI. There is a separate MEI UUID, which this
> > > driver uses to enumerate.
> > >
> > > To switch ownership of camera sensor between IVSC and host, the caller
> > > specifies the defined ownership information which will be sent to
> > > firmware by sending MEI command.
> > >
> > > Device link(device_link_add) is used to set the right camera sensor
> > > ownership before accessing the sensor via I²C. With DL_FLAG_PM_RUNTIME
> > > and DL_FLAG_RPM_ACTIVE, the supplier device will be PM runtime resumed
> > > before the consumer(camera sensor).
> > > So use runtime PM callbacks to transfer the ownership between host and
> > > IVSC.
> > >
> > > Signed-off-by: Wentong Wu <wentong.wu@intel.com>
> > > ---
> > >  drivers/media/pci/intel/ivsc/Makefile  |   3 +
> > >  drivers/media/pci/intel/ivsc/mei_ace.c | 534
> > > +++++++++++++++++++++++++++++++++
> > >  2 files changed, 537 insertions(+)
> > >  create mode 100644 drivers/media/pci/intel/ivsc/mei_ace.c
> > >
> > > diff --git a/drivers/media/pci/intel/ivsc/Makefile
> > > b/drivers/media/pci/intel/ivsc/Makefile
> > > index 7e4c5f0..a641f14 100644
> > > --- a/drivers/media/pci/intel/ivsc/Makefile
> > > +++ b/drivers/media/pci/intel/ivsc/Makefile
> > > @@ -5,3 +5,6 @@
> > >  obj-$(CONFIG_INTEL_VSC) += ivsc-csi.o  ivsc-csi-y += mei_csi.o
> > > ivsc-csi-y += csi_bridge.o
> > > +
> > > +obj-$(CONFIG_INTEL_VSC) += ivsc-ace.o ivsc-ace-y += mei_ace.o
> > > diff --git a/drivers/media/pci/intel/ivsc/mei_ace.c
> > > b/drivers/media/pci/intel/ivsc/mei_ace.c
> > > new file mode 100644
> > > index 0000000..434b072
> > > --- /dev/null
> > > +++ b/drivers/media/pci/intel/ivsc/mei_ace.c
> > > @@ -0,0 +1,534 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Copyright (C) 2023 Intel Corporation. All rights reserved.
> > > + * Intel Visual Sensing Controller ACE Linux driver  */
> > > +
> > > +/*
> > > + * To set ownership of camera sensor, there is specific command,
> > > +which
> > > + * is sent via MEI protocol. That's a two-step scheme where the
> > > +firmware
> > > + * first acks receipt of the command and later responses the command
> > > +was
> > > + * executed. The command sending function uses "completion" as the
> > > + * synchronization mechanism. The notification for command is
> > > +received
> > > + * via a mei callback which wakes up the caller. There can be only
> > > +one
> > > + * outstanding command at a time.
> > 
> > Could you document the dependencies in the sensor initialisation (ace +
> > csi) 
> 
> This is about ownership instead of dependency, if host sensor driver configure
> sensor with ownership on IVSC, probably it will be changed by firmware somehow.
> 
> > and how the sensor is powered (i.e. no direct control of the PMIC from the
> > host?)? 
> 
> The power line is directly connected to IVSC instead of host, when ownership
> switched to host, sensor is already powered up by firmware. 

Good. Please also document this in the comment.

-- 
Kind regards,

Sakari Ailus