Hi Sakari, On 2/14/23 17:06, Sakari Ailus wrote: > Hi Wentong, > > Thanks for the patchset. > > On Mon, Feb 13, 2023 at 10:23:47AM +0800, Wentong Wu wrote: >> IVSC directly connects to camera sensor on source side, and on >> output side it not only connects ISH via I2C, but also exposes >> MIPI CSI-2 interface to output camera sensor data. IVSC can use >> the camera sensor data to do AI algorithm, and send the results >> to ISH. On the other end, IVSC can share camera sensor to host >> by routing the raw camera sensor data to the exposed MIPI CSI-2 >> interface. But they can not work at the same time, so software >> APIs are defined to sync the ownership. >> >> This commit defines the interfaces between IVSC and camera sensor >> driver in include/linux/ivsc.h. The camera driver controls >> ownership of the CSI-2 link and sensor with the acquire/release >> APIs. When acquiring camera, lane number and link freq are also >> required by IVSC frame router. > > The more I learn about this system, the more I'm inclined to think this > functionality should be exposed as a V4L2 sub-device. IVSC doesn't really > do anything to the data (as long as it directs it towards the CSI-2 > receiver in the SoC), but it is definitely part of the image pipeline. Yes I happened to discuss this exact same thing with Laurent at FOSDEM and we also came to the conclusion that the IVSC chip should be modeled as a V4L2 sub-device. Regards, Hans > > I suppose the intended use cases assume a single instance of IVSC (as well > as MEI) but there can, and often are, be multiple camera sensors in the > system. The decision whether to request pass-through from IVCS can't be > done in the camera sensor driver, and should not be visible to the camera > sensor driver. Exposing IVSC as a V4L2 sub-device makes this trivial to > address, as the IVSC driver's V4L2 sub-device video s_stream() operation > gets called before streaming is started. > > The information whether IVSC is found between the camera sensor and the > host's CSI-2 receiver (IPU in this case) should come from system firmware > and accessed most probably by what is called cio2-bridge at the moment. > > The privacy status can be a V4L2 control. > > Also cc Hans. > >> >> Signed-off-by: Wentong Wu <wentong.wu@xxxxxxxxx> >> --- >> drivers/media/pci/intel/ivsc/Makefile | 1 + >> drivers/media/pci/intel/ivsc/ivsc.c | 84 +++++++++++++++++++++++++++ >> include/linux/ivsc.h | 55 ++++++++++++++++++ >> 3 files changed, 140 insertions(+) >> create mode 100644 drivers/media/pci/intel/ivsc/ivsc.c >> >> diff --git a/drivers/media/pci/intel/ivsc/Makefile b/drivers/media/pci/intel/ivsc/Makefile >> index de0a425c22c2..b8b6fc1083be 100644 >> --- a/drivers/media/pci/intel/ivsc/Makefile >> +++ b/drivers/media/pci/intel/ivsc/Makefile >> @@ -4,3 +4,4 @@ >> >> obj-$(CONFIG_INTEL_VSC) += mei_csi.o >> obj-$(CONFIG_INTEL_VSC) += mei_ace.o >> +obj-$(CONFIG_INTEL_VSC) += ivsc.o >> diff --git a/drivers/media/pci/intel/ivsc/ivsc.c b/drivers/media/pci/intel/ivsc/ivsc.c >> new file mode 100644 >> index 000000000000..12996b587639 >> --- /dev/null >> +++ b/drivers/media/pci/intel/ivsc/ivsc.c >> @@ -0,0 +1,84 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (C) 2023 Intel Corporation. All rights reserved. >> + * Intel Visual Sensing Controller interface >> + */ >> + >> +#include <linux/delay.h> >> +#include <linux/ivsc.h> >> +#include <linux/module.h> >> +#include <linux/mutex.h> >> + >> +#include "mei_ace.h" >> +#include "mei_csi.h" >> + >> +/* lock for ivsc APIs */ >> +static DEFINE_MUTEX(ivsc_mutex); >> + >> +int ivsc_acquire_camera(u32 nr_of_lanes, u64 link_freq, >> + void (*callback)(void *, enum ivsc_privacy_status), >> + void *context) >> +{ >> + int ret; >> + >> + mutex_lock(&ivsc_mutex); >> + >> + /* 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; >> + >> + mutex_unlock(&ivsc_mutex); >> + >> + return 0; >> + >> +release_csi: >> + csi_set_link_owner(CSI_LINK_IVSC, NULL, NULL); >> + >> +release_camera: >> + ace_set_camera_owner(ACE_CAMERA_IVSC); >> + >> +error: >> + mutex_unlock(&ivsc_mutex); >> + >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(ivsc_acquire_camera); >> + >> +int ivsc_release_camera(void) >> +{ >> + int ret; >> + >> + mutex_lock(&ivsc_mutex); >> + >> + /* switch CSI-2 link to IVSC */ >> + ret = csi_set_link_owner(CSI_LINK_IVSC, NULL, NULL); >> + if (ret) >> + goto error; >> + >> + /* switch camera sensor ownership to IVSC */ >> + ret = ace_set_camera_owner(ACE_CAMERA_IVSC); >> + >> +error: >> + mutex_unlock(&ivsc_mutex); >> + >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(ivsc_release_camera); >> + >> +MODULE_AUTHOR("Wentong Wu <wentong.wu@xxxxxxxxx>"); >> +MODULE_AUTHOR("Zhifeng Wang <zhifeng.wang@xxxxxxxxx>"); >> +MODULE_SOFTDEP("pre: mei_csi mei_ace"); >> +MODULE_DESCRIPTION("IVSC interface"); >> +MODULE_LICENSE("GPL"); >> +MODULE_IMPORT_NS(IVSC); >> diff --git a/include/linux/ivsc.h b/include/linux/ivsc.h >> index 6572ca4f340c..bc9006cd6efc 100644 >> --- a/include/linux/ivsc.h >> +++ b/include/linux/ivsc.h >> @@ -16,4 +16,59 @@ enum ivsc_privacy_status { >> IVSC_PRIVACY_MAX, >> }; >> >> +#if IS_ENABLED(CONFIG_INTEL_VSC) >> +/* >> + * @brief Acquire camera sensor ownership to host and setup >> + * the CSI-2 link between host and IVSC >> + * >> + * IVSC provides a privacy mode. When the privacy mode is turned >> + * on, camera sensor can't be used. This means that both IVSC and >> + * host Image Processing Unit(IPU) can't get image data. And when >> + * this mode is turned on, host Image Processing Unit(IPU) driver >> + * is informed via the registered callback, so that user can be >> + * notified. >> + * >> + * @param nr_of_lanes Number of data lanes used on the CSI-2 link >> + * @param link_freq Frequency of the CSI-2 link >> + * @param callback The pointer of privacy callback function >> + * @param context Privacy callback function runtime context >> + * >> + * @retval 0 If success >> + * @retval -EIO IO error >> + * @retval -EINVAL Invalid argument >> + * @retval -EAGAIN IVSC device not ready >> + * @retval negative values for other errors >> + */ >> +int ivsc_acquire_camera(u32 nr_of_lanes, u64 link_freq, >> + void (*callback)(void *, enum ivsc_privacy_status), >> + void *context); >> + >> +/* >> + * @brief Release camera sensor ownership and stop the CSI-2 >> + * link between host and IVSC >> + * >> + * @retval 0 If success >> + * @retval -EIO IO error >> + * @retval -EINVAL Invalid argument >> + * @retval -EAGAIN IVSC device not ready >> + * @retval negative values for other errors >> + */ >> +int ivsc_release_camera(void); >> + >> +#else >> +static inline >> +int ivsc_acquire_camera(u32 nr_of_lanes, u64 link_freq, >> + void (*callback)(void *, enum ivsc_privacy_status), >> + void *context) >> +{ >> + return 0; >> +} >> + >> +static inline int ivsc_release_camera(void) >> +{ >> + return 0; >> +} >> + >> +#endif >> + >> #endif >