Re: [PATCH v7 4/9] media: uapi: Add CAMSS ISP configuration definition

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

On 15/09/2026 09:11, Loic Poulain wrote:
> Add the uapi header qcom-camss-config.h defining the ISP parameter
> structures used by the CAMSS Offline Processing Engine (OPE) driver.
> This includes structures for white balance, chroma enhancement and
> color correction configuration.
> 
> The block type enum is shared across CAMSS ISPs so that userspace only
> has to deal with a single set of block identifiers.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>   include/uapi/linux/qcom-camss-config.h | 212 +++++++++++++++++++++++++++++++++
>   1 file changed, 212 insertions(+)
> 
> diff --git a/include/uapi/linux/qcom-camss-config.h b/include/uapi/linux/qcom-camss-config.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..e3d6cc4df7de00345e8142c81392dd42338c6ead
> --- /dev/null
> +++ b/include/uapi/linux/qcom-camss-config.h
> @@ -0,0 +1,212 @@
> +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
> +/*
> + * Qualcomm CAMSS ISP Driver - Userspace API
> + *
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#ifndef _UAPI_LINUX_QCOM_CAMSS_CONFIG_H
> +#define _UAPI_LINUX_QCOM_CAMSS_CONFIG_H
> +
> +#include <linux/types.h>
> +#include <linux/media/v4l2-isp.h>
> +
> +/**
> + * enum camss_params_block_type - CAMSS ISP parameter block identifiers
> + *
> + * Each value identifies one ISP processing block.  The value is placed in
> + * the @type field of &struct v4l2_isp_params_block_header.
> + *
> + * @CAMSS_PARAMS_OPE_WB_GAIN: white balance gains and offsets (CLC_WB),
> + *	&struct camss_params_ope_wb_gain
> + * @CAMSS_PARAMS_OPE_CHROMA_ENHAN: RGB to YUV colour transfer matrix
> + *	(CLC_CHROMA_ENHAN), &struct camss_params_ope_chroma_enhan
> + * @CAMSS_PARAMS_OPE_COLOR_CORRECT: colour correction matrix (CLC_CC),
> + *	&struct camss_params_ope_color_correct
> + * @CAMSS_PARAMS_OPE_GAMMA: per-channel gamma correction curves (CLC_GLUT),
> + *	&struct camss_params_ope_gamma
> + */
> +enum camss_params_block_type {
> +	CAMSS_PARAMS_OPE_WB_GAIN = 1,
> +	CAMSS_PARAMS_OPE_CHROMA_ENHAN = 2,
> +	CAMSS_PARAMS_OPE_COLOR_CORRECT = 3,
> +	CAMSS_PARAMS_OPE_GAMMA = 4,
> +};

Most of the OPE specific naming can be dropped, I shared the ICP 
data-structure analysis already:

https://patchwork.linuxtv.org/project/linux-media/patch/20260426000346.1158633-1-bryan.odonoghue@linaro.org/

i.e.
> +struct camss_params_ope_color_correct {
> +	struct v4l2_isp_params_block_header header;
> +	__u16 a[3];
> +	__u16 b[3];
> +	__u16 c[3];
> +	__u16 k[3];
> +	__u16 qfactor;
> +	__u16 _pad[3];
> +} __attribute__((aligned(8)));

For ICP I have:

+struct camss_params_color_correct {
+	struct v4l2_isp_params_block_header header;
+	__u16 a[3];
+	__u16 b[3];
+	__u16 c[3];
+	__u16 k[3];
+	__u16 qfactor;
+	__u16 _pad[3];
+} __attribute__((aligned(8)));

So this data-structure is shared camss_params_ope_color_correct -> 
camss_params_color_correct

> +/**
> + * struct camss_params_ope_gamma - per-channel gamma correction curves
> + *
> + * Implements the CLC_GLUT pipeline module, applied in the RGB domain.  It
> + * holds one independent lookup table per colour channel. Each table is a
> + * direct (not segmented) map of input level to output level.
> + *
> + * Each table has @CAMSS_OPE_GAMMA_LUT_SIZE (256) entries of 16-bit unsigned
> + * output. Each entry holds an X-bit output value depending on internal bus,
> + * the upper bits are ignored by the hardware.
> + *
> + * On Agatti OPE, The module maps a 12-bit input to an 8-bit output, the top
> + * 8 input bits index the table and the low 4 bits are used to linearly
> + * interpolate between adjacent entries. Entry i therefore represents the
> + * output for input level i/255 of full scale.
> + * Output range: 0x00 = black, 0xFF = white.

That's not specific to Agatti, its hwo the Qcom gamma silicon works right ?

Same RTL synthesised in OPE, IPE and IFE. So you don't need to make this 
OPE specific commentary.

> + *
> + * How to fill a curve (same on all three channels for pure luminance
> + * gamma; different curves per channel additionally shift colour balance):
> + *
> + *   Identity (pass-through, gamma 1.0):
> + *     lut[i] = i;                                // i = 0..255
> + *
> + *   Encode with gamma g (e.g. sRGB-like, g = 2.2):
> + *     lut[i] = round(pow(i / 255.0, 1.0 / g) * 255.0);
> + *
> + * @header:  block header; @header.type = CAMSS_PARAMS_OPE_GAMMA
> + * @glut:    green channel gamma curve
> + * @blut:    blue  channel gamma curve
> + * @rlut:    red   channel gamma curve
> + */
> +struct camss_params_ope_gamma {
> +	struct v4l2_isp_params_block_header header;
> +	__u16 glut[CAMSS_OPE_GAMMA_LUT_SIZE];
> +	__u16 blut[CAMSS_OPE_GAMMA_LUT_SIZE];
> +	__u16 rlut[CAMSS_OPE_GAMMA_LUT_SIZE];
> +} __attribute__((aligned(8)));

For the ICP I have

+struct camss_params_glut {
+	struct v4l2_isp_params_block_header header;
+	__u8  instance;
+	__u8  _pad[7];
+	__u16 r[CAMSS_GLUT_LUT_SIZE];
+	__u16 g[CAMSS_GLUT_LUT_SIZE];
+	__u16 b[CAMSS_GLUT_LUT_SIZE];
+} __attribute__((aligned(8)));

So it makes more sense to differentiate the firmware type to the 
register-mapping type. i.e. the above becomes camss_params_icp_gamma {} 
whereas your subumitted change would be just camss_params_gamma {}

> +#define CAMSS_PARAMS_OPE_MAX_PAYLOAD		\
> +	(sizeof(struct camss_params_ope_wb_gain)	+\
> +	 sizeof(struct camss_params_ope_chroma_enhan)	+\
> +	 sizeof(struct camss_params_ope_color_correct)	+\
> +	 sizeof(struct camss_params_ope_gamma))
> +
> +#endif /* _UAPI_LINUX_QCOM_CAMSS_CONFIG_H */
>