| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Y+uxbQi7seGf+adP@kekkonen.localdomain>
Date: Tue, 14 Feb 2023 18:06:05 +0200
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Wentong Wu <wentong.wu@...el.com>
Cc: mchehab@...nel.org, linux-media@...r.kernel.org,
srinivas.pandruvada@...el.com,
pierre-louis.bossart@...ux.intel.com, zhifeng.wang@...el.com,
xiang.ye@...el.com, tian.shu.qiu@...el.com, bingbu.cao@...el.com,
linux-kernel@...r.kernel.org, Hans de Goede <hdegoede@...hat.com>
Subject: Re: [PATCH v2 3/3] media: pci: intel: ivsc: Add acquire/release API
for ivsc
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.
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@...el.com>
> ---
> 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@...el.com>");
> +MODULE_AUTHOR("Zhifeng Wang <zhifeng.wang@...el.com>");
> +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
--
Kind regards,
Sakari Ailus
Powered by blists - more mailing lists