| 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: <20250522155641.GU12514@pendragon.ideasonboard.com>
Date: Thu, 22 May 2025 17:56:41 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Stefan Klug <stefan.klug@...asonboard.com>
Cc: linux-media@...r.kernel.org, Dafna Hirschfeld <dafna@...tmail.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Heiko Stuebner <heiko@...ech.de>,
linux-rockchip@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Hans Verkuil <hans@...erkuil.nl>
Subject: Re: [PATCH 2/3] media: rkisp1: Add
RKISP1_CID_SUPPORTED_PARAMS_BLOCKS control
Hi Stefan,
Thank you for the patch.
On Thu, May 22, 2025 at 05:08:39PM +0200, Stefan Klug wrote:
> Add a RKISP1_CID_SUPPORTED_PARAMS_BLOCKS V4L2 control to be able to
> query the parameters blocks supported by the current kernel on the
> current hardware from user space.
>
> Signed-off-by: Stefan Klug <stefan.klug@...asonboard.com>
> ---
> .../platform/rockchip/rkisp1/rkisp1-common.h | 2 +
> .../platform/rockchip/rkisp1/rkisp1-params.c | 50 ++++++++++++++++++-
> include/uapi/linux/rkisp1-config.h | 10 ++++
> include/uapi/linux/v4l2-controls.h | 6 +++
> 4 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> index ca952fd0829b..5f187f9efc7b 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> @@ -415,6 +415,8 @@ struct rkisp1_params {
> spinlock_t config_lock; /* locks the buffers list 'params' */
> struct list_head params;
>
> + struct v4l2_ctrl_handler ctrls;
> +
> const struct v4l2_meta_format *metafmt;
>
> enum v4l2_quantization quantization;
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> index 918eb06c7465..60c9b3c46593 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> @@ -2736,6 +2736,45 @@ static int rkisp1_params_init_vb2_queue(struct vb2_queue *q,
> return vb2_queue_init(q);
> }
>
> +static int rkisp1_ctrl_init(struct rkisp1_params *params)
> +{
> + int ret;
> +
> + v4l2_ctrl_handler_init(¶ms->ctrls, 1);
> +
> + struct v4l2_ctrl_config ctrl_config = {
> + .id = RKISP1_CID_SUPPORTED_PARAMS_BLOCKS,
> + .name = "Supported Params Blocks",
> + .type = V4L2_CTRL_TYPE_BITMASK,
> + .flags = V4L2_CTRL_FLAG_READ_ONLY,
> + };
struct v4l2_ctrl_config ctrl_config = {
.id = RKISP1_CID_SUPPORTED_PARAMS_BLOCKS,
.name = "Supported Params Blocks",
.type = V4L2_CTRL_TYPE_BITMASK,
.flags = V4L2_CTRL_FLAG_READ_ONLY,
};
int ret;
v4l2_ctrl_handler_init(¶ms->ctrls, 1);
Mixing code and variable declarations is still usually frown upon in the
kernel.
> +
> + for (unsigned int i = 0; i < ARRAY_SIZE(rkisp1_ext_params_handlers); i++) {
> + const struct rkisp1_ext_params_handler *block_handler;
> +
> + block_handler = &rkisp1_ext_params_handlers[i];
> + ctrl_config.max |= BIT(i);
> +
> + if ((params->rkisp1->info->features & block_handler->features) !=
> + block_handler->features)
> + continue;
> +
> + ctrl_config.def |= BIT(i);
> + }
> +
> + v4l2_ctrl_new_custom(¶ms->ctrls, &ctrl_config, NULL);
> +
> + params->vnode.vdev.ctrl_handler = ¶ms->ctrls;
> +
> + if (params->ctrls.error) {
> + ret = params->ctrls.error;
> + v4l2_ctrl_handler_free(¶ms->ctrls);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> int rkisp1_params_register(struct rkisp1_device *rkisp1)
> {
> struct rkisp1_params *params = &rkisp1->params;
> @@ -2776,10 +2815,16 @@ int rkisp1_params_register(struct rkisp1_device *rkisp1)
>
> video_set_drvdata(vdev, params);
>
> + ret = rkisp1_ctrl_init(params);
> + if (ret) {
> + dev_err(rkisp1->dev, "Control initialization error %d\n", ret);
> + goto err_mutex;
> + }
> +
> node->pad.flags = MEDIA_PAD_FL_SOURCE;
> ret = media_entity_pads_init(&vdev->entity, 1, &node->pad);
> if (ret)
> - goto err_mutex;
> + goto err_ctrl;
>
> ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
> if (ret) {
> @@ -2792,6 +2837,8 @@ int rkisp1_params_register(struct rkisp1_device *rkisp1)
>
> err_media:
> media_entity_cleanup(&vdev->entity);
> +err_ctrl:
> + v4l2_ctrl_handler_free(¶ms->ctrls);
> err_mutex:
> mutex_destroy(&node->vlock);
> return ret;
> @@ -2808,5 +2855,6 @@ void rkisp1_params_unregister(struct rkisp1_device *rkisp1)
>
> vb2_video_unregister_device(vdev);
> media_entity_cleanup(&vdev->entity);
> + v4l2_ctrl_handler_free(¶ms->ctrls);
> mutex_destroy(&node->vlock);
> }
> diff --git a/include/uapi/linux/rkisp1-config.h b/include/uapi/linux/rkisp1-config.h
> index 2d995f3c1ca3..4fc8f221d0c4 100644
> --- a/include/uapi/linux/rkisp1-config.h
> +++ b/include/uapi/linux/rkisp1-config.h
> @@ -1086,6 +1086,9 @@ enum rkisp1_ext_params_block_type {
> #define RKISP1_EXT_PARAMS_FL_BLOCK_DISABLE (1U << 0)
> #define RKISP1_EXT_PARAMS_FL_BLOCK_ENABLE (1U << 1)
>
> +/* A bitmask of parameters blocks supported on the current hardware. */
> +#define RKISP1_CID_SUPPORTED_PARAMS_BLOCKS (V4L2_CID_USER_RKISP1_BASE + 0x01)
> +
> /**
> * struct rkisp1_ext_params_block_header - RkISP1 extensible parameters block
> * header
> @@ -1520,6 +1523,13 @@ enum rksip1_ext_param_buffer_version {
> * V4L2 control. If such control is not available, userspace should assume only
> * RKISP1_EXT_PARAM_BUFFER_V1 is supported by the driver.
> *
> + * The read-only V4L2 control ``RKISP1_CID_SUPPORTED_PARAMS_BLOCKS`` can be used
> + * to query the blocks supported by the current hardware. It contains a bitmask
s/current hardware/device/
> + * where each bit represents the availability of the corresponding entry
> + * from the :c:type:`rkisp1_ext_params_block_type` enum. The max value of the
> + * control represents the blocks supported by the current kernel (independent of
> + * the current hardware).
* from the :c:type:`rkisp1_ext_params_block_type` enum. The current and default
* values of the control represents the blocks supported by the device instance,
* while the maximum value represents the blocks supported by the kernel driver,
* independently of the device instance.
I was going to say that the control should be documented in
Documentation/userspace-api/drivers/rkisp1.rst, but rkisp1-config.h is
pulled in the documentation tree by
Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst, so I'm OK with
this. Hans, Mauro, are you fine as well with documenting the control
here ?
Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> + *
> * For each ISP block that userspace wants to configure, a block-specific
> * structure is appended to the @data buffer, one after the other without gaps
> * in between nor overlaps. Userspace shall populate the @data_size field with
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 72e32814ea83..f836512e9deb 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -222,6 +222,12 @@ enum v4l2_colorfx {
> */
> #define V4L2_CID_USER_UVC_BASE (V4L2_CID_USER_BASE + 0x11e0)
>
> +/*
> + * The base for Rockchip ISP1 driver controls.
> + * We reserve 16 controls for this driver.
> + */
> +#define V4L2_CID_USER_RKISP1_BASE (V4L2_CID_USER_BASE + 0x1220)
> +
> /* MPEG-class control IDs */
> /* The MPEG controls are applicable to all codec controls
> * and the 'MPEG' part of the define is historical */
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists