Re: [PATCH v10 06/17] media: uapi: Add controls for Mali-C55 ISP

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Daniel,

Thanks for the update.

On 6/24/25 13:21, Daniel Scally wrote:
> Add definitions and documentation for the custom control that will
> be needed by the Mali-C55 ISP driver. This will be a read only
> bitmask of the driver's capabilities, informing userspace of which
> blocks are fitted and which are absent.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Changes in v10:
> 
> 	- None
> 
> Changes in v9:
> 
> 	- New patch
> ---
>   .../userspace-api/media/drivers/index.rst          |  1 +
>   .../userspace-api/media/drivers/mali-c55.rst       | 55 ++++++++++++++++++++++
>   include/uapi/linux/media/arm/mali-c55-config.h     | 26 ++++++++++
>   include/uapi/linux/v4l2-controls.h                 |  6 +++
>   4 files changed, 88 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/drivers/index.rst b/Documentation/userspace-api/media/drivers/index.rst
> index d706cb47b1122b6e145a02ab826eb3ecc7997c2b..02967c9b18d6e90f414ccc1329c09bffee895e68 100644
> --- a/Documentation/userspace-api/media/drivers/index.rst
> +++ b/Documentation/userspace-api/media/drivers/index.rst
> @@ -32,6 +32,7 @@ For more details see the file COPYING in the source distribution of Linux.
>   	cx2341x-uapi
>   	dw100
>   	imx-uapi
> +	mali-c55
>   	max2175
>   	npcm-video
>   	omap3isp-uapi
> diff --git a/Documentation/userspace-api/media/drivers/mali-c55.rst b/Documentation/userspace-api/media/drivers/mali-c55.rst
> new file mode 100644
> index 0000000000000000000000000000000000000000..e8519da77d737b91a931bbe47920af707eebf110
> --- /dev/null
> +++ b/Documentation/userspace-api/media/drivers/mali-c55.rst
> @@ -0,0 +1,55 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +Arm Mali-C55 ISP driver
> +=======================
> +
> +The Arm Mali-C55 ISP driver implements a single driver-specific control:
> +
> +``V4L2_CID_MALI_C55_CAPABILITIES``

Add: (integer)

?

> +    Detail the capabilities of the ISP by giving detail about the fitted blocks.
> +
> +    .. flat-table:: Bitmask meaning definitions
> +	:header-rows: 1
> +	:widths: 2 4 8
> +
> +	* - Bit
> +	  - Macro
> +	  - Meaning
> +        * - 0
> +          - MALI_C55_GPS_PONG_FITTED

How about a V4L2 prefix for these? Generally the controls have been 
V4L2_CID_something and the macros for the possible values have been 
named V4L2_something.

> +          - Pong configuration space is fitted in the ISP
> +        * - 1
> +          - MALI_C55_GPS_WDR_FITTED
> +          - WDR Framestitch, offset and gain is fitted in the ISP
> +        * - 2
> +          - MALI_C55_GPS_COMPRESSION_FITTED
> +          - Temper compression is fitted in the ISP
> +        * - 3
> +          - MALI_C55_GPS_TEMPER_FITTED
> +          - Temper is fitted in the ISP
> +        * - 4
> +          - MALI_C55_GPS_SINTER_LITE_FITTED
> +          - Sinter Lite is fitted in the ISP instead of the full Sinter version
> +        * - 5
> +          - MALI_C55_GPS_SINTER_FITTED
> +          - Sinter is fitted in the ISP
> +        * - 6
> +          - MALI_C55_GPS_IRIDIX_LTM_FITTED
> +          - Iridix local tone mappine is fitted in the ISP
> +        * - 7
> +          - MALI_C55_GPS_IRIDIX_GTM_FITTED
> +          - Iridix global tone mapping is fitted in the ISP
> +        * - 8
> +          - MALI_C55_GPS_CNR_FITTED
> +          - Colour noise reduction is fitted in the ISP
> +        * - 9
> +          - MALI_C55_GPS_FRSCALER_FITTED
> +          - The full resolution pipe scaler is fitted in the ISP
> +        * - 10
> +          - MALI_C55_GPS_DS_PIPE_FITTED
> +          - The downscale pipe is fitted in the ISP
> +
> +    The Mali-C55 ISP can be configured in a number of ways to include or exclude
> +    blocks which may not be necessary. This control provides a way for the
> +    driver to communicate to userspace which of the blocks are fitted in the
> +    design.
> \ No newline at end of file

Oops.

> diff --git a/include/uapi/linux/media/arm/mali-c55-config.h b/include/uapi/linux/media/arm/mali-c55-config.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..2bd60a0d78786be368c2e51b1a0a63fd2a5f690b
> --- /dev/null
> +++ b/include/uapi/linux/media/arm/mali-c55-config.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * ARM Mali-C55 ISP Driver - Userspace API
> + *
> + * Copyright (C) 2023 Ideas on Board Oy
> + */
> +
> +#ifndef __UAPI_MALI_C55_CONFIG_H
> +#define __UAPI_MALI_C55_CONFIG_H
> +
> +#include <linux/v4l2-controls.h>
> +
> +#define V4L2_CID_MALI_C55_CAPABILITIES		(V4L2_CID_USER_MALI_C55_BASE + 0x0)
> +#define MALI_C55_GPS_PONG_FITTED		BIT(0)

Since when has been BIT() defined for UAPI? (1U << x)?

> +#define MALI_C55_GPS_WDR_FITTED			BIT(1)
> +#define MALI_C55_GPS_COMPRESSION_FITTED		BIT(2)
> +#define MALI_C55_GPS_TEMPER_FITTED		BIT(3)
> +#define MALI_C55_GPS_SINTER_LITE_FITTED		BIT(4)
> +#define MALI_C55_GPS_SINTER_FITTED		BIT(5)
> +#define MALI_C55_GPS_IRIDIX_LTM_FITTED		BIT(6)
> +#define MALI_C55_GPS_IRIDIX_GTM_FITTED		BIT(7)
> +#define MALI_C55_GPS_CNR_FITTED			BIT(8)
> +#define MALI_C55_GPS_FRSCALER_FITTED		BIT(9)
> +#define MALI_C55_GPS_DS_PIPE_FITTED		BIT(10)
> +
> +#endif /* __UAPI_MALI_C55_CONFIG_H */
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 72e32814ea83dee5f1202c1249eac7cf3b85a22a..a31105115410bd041a3ac492ce86becd93a62d87 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 the Arm Mali-C55 ISP driver controls.
> + * We reserve 8 controls for this driver
> + */
> +#define V4L2_CID_USER_MALI_C55_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 */
> 


-- 
Sakari Ailus