Re: [PATCH 1/6] media: uapi: v4l2-isp: Add extensible statistics

["Niklas Söderlund" <niklas.soderlund@ragnatech.se>]

Hi Antoine, Jacopo,

Thanks for your work.

On 2026-05-05 16:12:12 +0200, Jacopo Mondi wrote:
> From: Antoine Bouyer <antoine.bouyer@nxp.com>
> 
> Extend the v4l2-isp extensible format introduced for isp parameters buffers
> to the support buffers of ISP statistic.
> 
> Like for ISP configuration purpose, that will help supporting various ISP
> hardware versions reporting different statistics data with less impact on
> userspace.
> 
> Rename all 'v4l2_isp_params' types to generic 'v4l2_isp' types to
> prepare to use them for statistics as well and maintain the existing
> types for compatibility with existing userspace only.
> 
> Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
> [Rework to remove 'v4l2_isp_stats' and unify types]
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  include/uapi/linux/media/v4l2-isp.h | 125 +++++++++++++++++++++++-------------
>  1 file changed, 79 insertions(+), 46 deletions(-)
> 
> diff --git a/include/uapi/linux/media/v4l2-isp.h b/include/uapi/linux/media/v4l2-isp.h
> index 779168f9058e..e4607e1217e1 100644
> --- a/include/uapi/linux/media/v4l2-isp.h
> +++ b/include/uapi/linux/media/v4l2-isp.h
> @@ -13,25 +13,33 @@
>  #include <linux/types.h>
>  
>  /**
> - * enum v4l2_isp_params_version - V4L2 ISP parameters versioning
> + * enum v4l2_isp_version - V4L2 ISP serialization format versioning
>   *
> - * @V4L2_ISP_PARAMS_VERSION_V0: First version of the V4L2 ISP parameters format
> - *				(for compatibility)
> - * @V4L2_ISP_PARAMS_VERSION_V1: First version of the V4L2 ISP parameters format
> + * @V4L2_ISP_VERSION_V0: First version of the V4L2 ISP serialization format
> + *                       (for compatibility)
> + * @V4L2_ISP_VERSION_V1: First version of the V4L2 ISP serialization format
>   *
>   * V0 and V1 are identical in order to support drivers compatible with the V4L2
> - * ISP parameters format already upstreamed which use either 0 or 1 as their
> - * versioning identifier. Both V0 and V1 refers to the first version of the
> - * V4L2 ISP parameters format.
> + * ISP format already upstreamed which use either 0 or 1 as their versioning
> + * identifier. Both V0 and V1 refers to the first version of the V4L2 ISP
> + * serialization format.
>   *
> - * Future revisions of the V4L2 ISP parameters format should start from the
> + * Future revisions of the V4L2 ISP serialization format should start from the
>   * value of 2.
>   */
> -enum v4l2_isp_params_version {
> -	V4L2_ISP_PARAMS_VERSION_V0 = 0,
> -	V4L2_ISP_PARAMS_VERSION_V1
> +enum v4l2_isp_version {
> +	V4L2_ISP_VERSION_V0 = 0,
> +	V4L2_ISP_VERSION_V1
>  };
>  
> +/*
> + * Compatibility with existing users of v4l2_isp_params which pre-date the
> + * introduction of v4l2_isp_stats.
> + */
> +#define v4l2_isp_params_version			v4l2_isp_version
> +#define V4L2_ISP_PARAMS_VERSION_V0		V4L2_ISP_VERSION_V0
> +#define V4L2_ISP_PARAMS_VERSION_V1		V4L2_ISP_VERSION_V1
> +
>  #define V4L2_ISP_PARAMS_FL_BLOCK_DISABLE	(1U << 0)
>  #define V4L2_ISP_PARAMS_FL_BLOCK_ENABLE		(1U << 1)
>  
> @@ -39,64 +47,89 @@ enum v4l2_isp_params_version {
>   * Reserve the first 8 bits for V4L2_ISP_PARAMS_FL_* flag.
>   *
>   * Driver-specific flags should be defined as:
> - * #define DRIVER_SPECIFIC_FLAG0     ((1U << V4L2_ISP_PARAMS_FL_DRIVER_FLAGS(0))
> - * #define DRIVER_SPECIFIC_FLAG1     ((1U << V4L2_ISP_PARAMS_FL_DRIVER_FLAGS(1))
> + * #define DRIVER_SPECIFIC_FLAG0     ((1U << V4L2_ISP_FL_DRIVER_FLAGS(0))
> + * #define DRIVER_SPECIFIC_FLAG1     ((1U << V4L2_ISP_FL_DRIVER_FLAGS(1))
>   */
> -#define V4L2_ISP_PARAMS_FL_DRIVER_FLAGS(n)       ((n) + 8)
> +#define V4L2_ISP_FL_DRIVER_FLAGS(n)		((n) + 8)
>  
>  /**
> - * struct v4l2_isp_params_block_header - V4L2 extensible parameters block header
> - * @type: The parameters block type (driver-specific)
> + * struct v4l2_isp_block_header - V4L2 extensible block header
> + * @type: The parameters or statistics block type (driver-specific)
>   * @flags: A bitmask of block flags (driver-specific)
> - * @size: Size (in bytes) of the parameters block, including this header
> + * @size: Size (in bytes) of the block, including this header
>   *
> - * This structure represents the common part of all the ISP configuration
> - * blocks. Each parameters block shall embed an instance of this structure type
> - * as its first member, followed by the block-specific configuration data.
> + * This structure represents the common part of all the ISP configuration or
> + * statistic blocks. Each block shall embed an instance of this structure type
> + * as its first member, followed by the block-specific configuration or
> + * statistic data.
>   *
>   * The @type field is an ISP driver-specific value that identifies the block
> - * type. The @size field specifies the size of the parameters block.
> + * type. The @size field specifies the size of the block, including this
> + * header.
>   *
> - * The @flags field is a bitmask of per-block flags V4L2_PARAMS_ISP_FL_* and
> - * driver-specific flags specified by the driver header.
> + * The @flags field is a bitmask of per-block flags. If a block is used for
> + * configuration parameters this field can be a combination of
> + * V4L2_ISP_PARAMS_FL_* and driver-specific flags. If a block is used
> + * for statistics this fields is used to report optional
> + * driver-specific flags, if any.
>   */
> -struct v4l2_isp_params_block_header {
> +struct v4l2_isp_block_header {
>  	__u16 type;
>  	__u16 flags;
>  	__u32 size;
>  } __attribute__((aligned(8)));
>  
>  /**
> - * struct v4l2_isp_params_buffer - V4L2 extensible parameters configuration
> - * @version: The parameters buffer version (driver-specific)
> - * @data_size: The configuration data effective size, excluding this header
> - * @data: The configuration data
> + * v4l2_isp_params_block_header - V4L2 extensible parameters block header
> + *
> + * Compatibility with existing users of v4l2_isp_params_block_header
> + * which pre-date the introduction of v4l2_isp_block_header.
> + */
> +#define v4l2_isp_params_block_header v4l2_isp_block_header
> +
> +/**
> + * struct v4l2_isp_buffer - V4L2 extensible buffer
> + * @version: The extensible buffer version (driver-specific)
> + * @data_size: The data effective size, excluding this header
> + * @data: The configuration or statistics data
>   *
> - * This structure contains the configuration parameters of the ISP algorithms,
> - * serialized by userspace into a data buffer. Each configuration parameter
> - * block is represented by a block-specific structure which contains a
> - * :c:type:`v4l2_isp_params_block_header` entry as first member. Userspace
> - * populates the @data buffer with configuration parameters for the blocks that
> - * it intends to configure. As a consequence, the data buffer effective size
> - * changes according to the number of ISP blocks that userspace intends to
> - * configure and is set by userspace in the @data_size field.
> + * This structure contains ISP configuration parameters or ISP hardware
> + * statistics serialized into a data buffer. Each block is represented by a
> + * block-specific structure which contains a :c:type:`v4l2_isp_block_header`
> + * entry as first member.
>   *
> - * The parameters buffer is versioned by the @version field to allow modifying
> - * and extending its definition. Userspace shall populate the @version field to
> - * inform the driver about the version it intends to use. The driver will parse
> - * and handle the @data buffer according to the data layout specific to the
> - * indicated version and return an error if the desired version is not
> + * When used for ISP parameters, userspace populates the @data buffer with
> + * configuration parameters for the blocks that it intends to configure. As a
> + * consequence, the data buffer effective size changes according to the number
> + * of ISP blocks that userspace intends to configure.
> + *
> + * When used to report ISP statistics, the driver populates the @data buffer
> + * with statistics for each supported measurement block.
> + *
> + * The buffer is versioned by the @version field to allow modifying
> + * and extending its definition. The writer shall populate the @version field
> + * to inform the reader about the version it intends to use. The reader will
> + * parse and handle the @data buffer according to the data layout specific to
> + * the indicated version and return an error if the desired version is not
>   * supported.
>   *
> - * 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. Userspace shall populate the @data_size field with the effective
> - * size, in bytes, of the @data buffer.
> + * For each ISP block, a block-specific structure is appended to the @data
> + * buffer, one after the other without gaps in between. The writer shall
> + * populate the @data_size field with the effective size, in bytes, of the
> + * @data buffer.
>   */
> -struct v4l2_isp_params_buffer {
> +struct v4l2_isp_buffer {
>  	__u32 version;
>  	__u32 data_size;
>  	__u8 data[] __counted_by(data_size);
>  };
>  
> +/**
> + * v4l2_isp_params_buffer - V4L2 extensible parameters compatibility
> + *
> + * Compatibility with existing users of v4l2_isp_params_buffer which
> + * pre-date the introduction of v4l2_isp_buffer.
> + */
> +#define v4l2_isp_params_buffer v4l2_isp_buffer
> +
>  #endif /* _UAPI_V4L2_ISP_H_ */
> 
> -- 
> 2.53.0
> 

-- 
Kind Regards,
Niklas Söderlund