On 07/08/2024 23:22, Jacopo Mondi wrote:
> The rkisp1 driver stores ISP configuration parameters in the fixed
> rkisp1_params_cfg structure. As the members of the structure are part of
> the userspace API, the structure layout is immutable and cannot be
> extended further. Introducing new parameters or modifying the existing
> ones would change the buffer layout and cause breakages in existing
> applications.
>
> The allow for future extensions to the ISP parameters, introduce a new
> extensible parameters format, with a new format 4CC. Document usage of
> the new format in the rkisp1 admin guide.
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
> Reviewed-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
> Reviewed-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Tested-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
> Acked-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> Documentation/admin-guide/media/rkisp1.rst | 11 +++-
> .../media/v4l/metafmt-rkisp1.rst | 57 ++++++++++++++++---
> drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
> include/uapi/linux/videodev2.h | 1 +
> 4 files changed, 59 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/admin-guide/media/rkisp1.rst b/Documentation/admin-guide/media/rkisp1.rst
> index 6f14d9561fa5..6c878c71442f 100644
> --- a/Documentation/admin-guide/media/rkisp1.rst
> +++ b/Documentation/admin-guide/media/rkisp1.rst
> @@ -114,11 +114,18 @@ to be applied to the hardware during a video stream, allowing userspace
> to dynamically modify values such as black level, cross talk corrections
> and others.
>
> -The buffer format is defined by struct :c:type:`rkisp1_params_cfg`, and
> -userspace should set
> +The ISP driver supports two different parameters configuration methods, the
> +`fixed parameters format` or the `extensible parameters format`.
> +
> +When using the `fixed parameters` method the buffer format is defined by struct
> +:c:type:`rkisp1_params_cfg`, and userspace should set
> :ref:`V4L2_META_FMT_RK_ISP1_PARAMS <v4l2-meta-fmt-rk-isp1-params>` as the
> dataformat.
>
> +When using the `extensible parameters` method the buffer format is defined by
> +struct :c:type:`rkisp1_ext_params_cfg`, and userspace should set
> +:ref:`V4L2_META_FMT_RK_ISP1_EXT_PARAMS <v4l2-meta-fmt-rk-isp1-ext-params>` as
> +the dataformat.
>
> Capturing Video Frames Example
> ==============================
> diff --git a/Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst b/Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst
> index fa04f00bcd2e..4e3f4ea9e1c8 100644
> --- a/Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst
> +++ b/Documentation/userspace-api/media/v4l/metafmt-rkisp1.rst
> @@ -1,28 +1,67 @@
> .. SPDX-License-Identifier: GPL-2.0
>
> -.. _v4l2-meta-fmt-rk-isp1-params:
> -
> .. _v4l2-meta-fmt-rk-isp1-stat-3a:
>
> -*****************************************************************************
> -V4L2_META_FMT_RK_ISP1_PARAMS ('rk1p'), V4L2_META_FMT_RK_ISP1_STAT_3A ('rk1s')
> -*****************************************************************************
> +************************************************************************************************************************
> +V4L2_META_FMT_RK_ISP1_PARAMS ('rk1p'), V4L2_META_FMT_RK_ISP1_STAT_3A ('rk1s'), V4L2_META_FMT_RK_ISP1_EXT_PARAMS ('rk1e')
> +************************************************************************************************************************
>
> +========================
> Configuration parameters
> ========================
>
> -The configuration parameters are passed to the
> +The configuration of the RkISP1 ISP is performed by userspace by providing
> +parameters for the ISP to the driver using the :c:type:`v4l2_meta_format`
> +interface.
> +
> +There are two methods that allow to configure the ISP, the `fixed parameters`
> +configuration format and the `extensible parameters` configuration
> +format.
> +
> +.. _v4l2-meta-fmt-rk-isp1-params:
> +
> +Fixed parameters configuration format
> +=====================================
> +
> +When using the fixed configuration format, parameters are passed to the
> :ref:`rkisp1_params <rkisp1_params>` metadata output video node, using
> -the :c:type:`v4l2_meta_format` interface. The buffer contains
> -a single instance of the C structure :c:type:`rkisp1_params_cfg` defined in
> -``rkisp1-config.h``. So the structure can be obtained from the buffer by:
> +the `V4L2_META_FMT_RK_ISP1_PARAMS` meta format.
> +
> +The buffer contains a single instance of the C structure
> +:c:type:`rkisp1_params_cfg` defined in ``rkisp1-config.h``. So the structure can
> +be obtained from the buffer by:
>
> .. code-block:: c
>
> struct rkisp1_params_cfg *params = (struct rkisp1_params_cfg*) buffer;
>
> +This method supports a subset of the ISP features only, new applications should
> +use the extensible parameters method.
> +
> +.. _v4l2-meta-fmt-rk-isp1-ext-params:
> +
> +Extensible parameters configuration format
> +==========================================
> +
> +When using the extensible configuration format, parameters are passed to the
> +:ref:`rkisp1_params <rkisp1_params>` metadata output video node, using
> +the `V4L2_META_FMT_RK_ISP1_EXT_PARAMS` meta format.
> +
> +The buffer contains a single instance of the C structure
> +:c:type:`rkisp1_ext_params_cfg` defined in ``rkisp1-config.h``. The
> +:c:type:`rkisp1_ext_params_cfg` structure is designed to allow userspace to
> +populate the data buffer with only the configuration data for the ISP blocks it
> +intends to configure. The extensible parameters format design allows developers
> +to define new block types to support new configuration parameters, and defines a
> +versioning scheme so that it can be extended and versioned without breaking
> +compatibility with existing applications.
> +
> +For these reasons, this configuration method if preferred over the `fixed
> +parameters` format alternative.
if preferred -> is preferred
Regards,
Hans
> +
> .. rkisp1_stat_buffer
>
> +===========================
> 3A and histogram statistics
> ===========================
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 5eb4d797d259..257d69341953 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1458,6 +1458,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> case V4L2_META_FMT_VIVID: descr = "Vivid Metadata"; break;
> case V4L2_META_FMT_RK_ISP1_PARAMS: descr = "Rockchip ISP1 3A Parameters"; break;
> case V4L2_META_FMT_RK_ISP1_STAT_3A: descr = "Rockchip ISP1 3A Statistics"; break;
> + case V4L2_META_FMT_RK_ISP1_EXT_PARAMS: descr = "Rockchip ISP1 Ext 3A Params"; break;
> case V4L2_PIX_FMT_NV12_8L128: descr = "NV12 (8x128 Linear)"; break;
> case V4L2_PIX_FMT_NV12M_8L128: descr = "NV12M (8x128 Linear)"; break;
> case V4L2_PIX_FMT_NV12_10BE_8L128: descr = "10-bit NV12 (8x128 Linear, BE)"; break;
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 4e91362da6da..725e86c4bbbd 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -854,6 +854,7 @@ struct v4l2_pix_format {
> /* Vendor specific - used for RK_ISP1 camera sub-system */
> #define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */
> #define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */
> +#define V4L2_META_FMT_RK_ISP1_EXT_PARAMS v4l2_fourcc('R', 'K', '1', 'E') /* Rockchip ISP1 3a Extensible Parameters */
>
> /* Vendor specific - used for RaspberryPi PiSP */
> #define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */
