[PATCH v2 0/2] media: mali-c55: Add support for CCM and Gamma

[PATCH v2 0/2] media: mali-c55: Add support for CCM and Gamma

[Jacopo Mondi]

Add support for Ccm and Gamma to the Mali-C55 ISP by defining the
corresponding blocks in the uAPI and implementing their handling in
the driver.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
Changes in v2:
- EDITME: describe what is new in this series revision.
- EDITME: use bulletpoints and terse descriptions.
- Link to v1: https://lore.kernel.org/r/20260616-mali-c55-ccm-gamma-v1-0-174fe4fedea3@ideasonboard.com

---
Jacopo Mondi (2):
      media: arm: mali-c55: Add support for CCM
      media: arm: mali-c55: Add support for RGB Gamma

 .../media/platform/arm/mali-c55/mali-c55-params.c  | 127 +++++++++++++++++++++
 .../platform/arm/mali-c55/mali-c55-registers.h     |   5 +
 include/uapi/linux/media/arm/mali-c55-config.h     |  84 +++++++++++++-
 3 files changed, 215 insertions(+), 1 deletion(-)
---
base-commit: 06cb687a5132fcffe624c0070576ab852ac6b568
change-id: 20260616-mali-c55-ccm-gamma-c02a0df59f98

Best regards,
-- 
Jacopo Mondi <jacopo.mondi@ideasonboard.com>

[PATCH v2 1/2] media: arm: mali-c55: Add support for CCM

[Jacopo Mondi]

From: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

Add support for the CCM (Color Correction Matrix) for the Mali C55 ISP.

Define a new block in the uAPI using the extensible v4l2-isp format and
implement support for configuring the CCM parameters in the mali-c55
ISP driver.

Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
 .../media/platform/arm/mali-c55/mali-c55-params.c  | 52 ++++++++++++++++++++++
 include/uapi/linux/media/arm/mali-c55-config.h     | 41 ++++++++++++++++-
 2 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
index de0e9d898db7..96f1b28a6d77 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
@@ -46,6 +46,7 @@
  * @awb_config:		For header->type == MALI_C55_PARAM_BLOCK_AWB_CONFIG
  * @shading_config:	For header->type == MALI_C55_PARAM_MESH_SHADING_CONFIG
  * @shading_selection:	For header->type == MALI_C55_PARAM_MESH_SHADING_SELECTION
+ * @ccm:		For header->type == MALI_C55_PARAM_BLOCK_CCM
  * @data:		Allows easy initialisation of a union variable with a
  *			pointer into a __u8 array.
  */
@@ -59,6 +60,7 @@ union mali_c55_params_block {
 	const struct mali_c55_params_awb_config *awb_config;
 	const struct mali_c55_params_mesh_shading_config *shading_config;
 	const struct mali_c55_params_mesh_shading_selection *shading_selection;
+	const struct mali_c55_params_ccm *ccm;
 	const __u8 *data;
 };
 
@@ -414,6 +416,52 @@ static void mali_c55_params_lsc_selection(struct mali_c55 *mali_c55,
 				 params->mesh_strength);
 }
 
+static void mali_c55_params_ccm(struct mali_c55 *mali_c55,
+				union mali_c55_params_block block)
+{
+	const struct mali_c55_params_ccm *params = block.ccm;
+
+	if (block.header->flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE) {
+		mali_c55_ctx_write(mali_c55, MALI_C55_REG_CCM_ENABLE, 0);
+		return;
+	}
+
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_R_R,
+				 MALI_C55_CCM_COEF_MASK, params->coeffs[0][0]);
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_R_G,
+				 MALI_C55_CCM_COEF_MASK, params->coeffs[0][1]);
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_R_B,
+				 MALI_C55_CCM_COEF_MASK, params->coeffs[0][2]);
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_G_R,
+				 MALI_C55_CCM_COEF_MASK, params->coeffs[1][0]);
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_G_G,
+				 MALI_C55_CCM_COEF_MASK, params->coeffs[1][1]);
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_G_B,
+				 MALI_C55_CCM_COEF_MASK, params->coeffs[1][2]);
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_B_R,
+				 MALI_C55_CCM_COEF_MASK, params->coeffs[2][0]);
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_B_G,
+				 MALI_C55_CCM_COEF_MASK, params->coeffs[2][1]);
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_B_B,
+				 MALI_C55_CCM_COEF_MASK, params->coeffs[2][2]);
+
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_GAIN_R,
+				 MALI_C55_CCM_ANTIFOG_GAIN_MASK, params->gains[0]);
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_GAIN_G,
+				 MALI_C55_CCM_ANTIFOG_GAIN_MASK, params->gains[1]);
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_GAIN_B,
+				 MALI_C55_CCM_ANTIFOG_GAIN_MASK, params->gains[2]);
+
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_OFFSET_R,
+				 MALI_C55_CCM_ANTIFOG_OFFSET_MASK, params->offs[0]);
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_OFFSET_G,
+				 MALI_C55_CCM_ANTIFOG_OFFSET_MASK, params->offs[1]);
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_OFFSET_B,
+				 MALI_C55_CCM_ANTIFOG_OFFSET_MASK, params->offs[2]);
+
+	mali_c55_ctx_write(mali_c55, MALI_C55_REG_CCM_ENABLE, 1);
+}
+
 static const mali_c55_params_handler mali_c55_params_handlers[] = {
 	[MALI_C55_PARAM_BLOCK_SENSOR_OFFS] = &mali_c55_params_sensor_offs,
 	[MALI_C55_PARAM_BLOCK_AEXP_HIST] = &mali_c55_params_aexp_hist,
@@ -426,6 +474,7 @@ static const mali_c55_params_handler mali_c55_params_handlers[] = {
 	[MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP] = &mali_c55_params_awb_gains,
 	[MALI_C55_PARAM_MESH_SHADING_CONFIG] = &mali_c55_params_lsc_config,
 	[MALI_C55_PARAM_MESH_SHADING_SELECTION] = &mali_c55_params_lsc_selection,
+	[MALI_C55_PARAM_BLOCK_CCM] = &mali_c55_params_ccm,
 };
 
 static const struct v4l2_isp_params_block_type_info
@@ -463,6 +512,9 @@ mali_c55_params_block_types_info[] = {
 	[MALI_C55_PARAM_MESH_SHADING_SELECTION] = {
 		.size = sizeof(struct mali_c55_params_mesh_shading_selection),
 	},
+	[MALI_C55_PARAM_BLOCK_CCM] = {
+		.size = sizeof(struct mali_c55_params_ccm),
+	},
 };
 
 static_assert(ARRAY_SIZE(mali_c55_params_handlers) ==
diff --git a/include/uapi/linux/media/arm/mali-c55-config.h b/include/uapi/linux/media/arm/mali-c55-config.h
index 3d335f950eeb..0b2085eed81b 100644
--- a/include/uapi/linux/media/arm/mali-c55-config.h
+++ b/include/uapi/linux/media/arm/mali-c55-config.h
@@ -219,6 +219,7 @@ struct mali_c55_stats_buffer {
  * @MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP: Auto-white balance gains for AEXP-0 tap
  * @MALI_C55_PARAM_MESH_SHADING_CONFIG : Mesh shading tables configuration
  * @MALI_C55_PARAM_MESH_SHADING_SELECTION: Mesh shading table selection
+ * @MALI_C55_PARAM_BLOCK_CCM: Colour correction matrix
  */
 enum mali_c55_param_block_type {
 	MALI_C55_PARAM_BLOCK_SENSOR_OFFS,
@@ -232,6 +233,7 @@ enum mali_c55_param_block_type {
 	MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP,
 	MALI_C55_PARAM_MESH_SHADING_CONFIG,
 	MALI_C55_PARAM_MESH_SHADING_SELECTION,
+	MALI_C55_PARAM_BLOCK_CCM,
 };
 
 /**
@@ -757,6 +759,42 @@ struct mali_c55_params_mesh_shading_selection {
 	__u16 mesh_strength;
 };
 
+/**
+ * struct mali_c55_params_ccm - Coefficients, offsets and gains for the colour
+ *				correction matrix
+ *
+ * The colour correction module converts images data from a sensor-specific
+ * colour space to known one.
+ *
+ * Colour correction is applied after demosaicing and each pixel is represented
+ * as a column vector of the three RGB colour channels on which the following
+ * operations take place:
+ * 1) An offset is subtracted from each colour channel
+ * 2) Each colour channel is multiplied by a gain
+ * 3) The pixel column vector is multiplied by the colour correction matrix
+ *
+ * This struct allows users to configure the coefficients for CCM and the
+ * per-channel offsets and gains. The nine matrix coefficients are expressed as
+ * signed Q4.8 Sign/Magnitude fixed-point numbers, the three gain multipliers
+ * are expressed as unsigned Q4.8 fixed-point numbers and the three offsets are
+ * expressed as a 12-bit unsigned integers.
+ *
+ * header.type should be set to MALI_C55_PARAM_BLOCK_CCM from
+ * :c:type:`mali_c55_param_block_type`.
+ *
+ * @header:	The Mali-C55 parameters block header
+ * @coeffs:	3x3 color conversion matrix coefficients in sign/magnitude
+ *		Q4.8 format
+ * @gains:	Gains for red, green and blue channels in unsigned Q4.8 format
+ * @offs:	Offsets for red, green and blue channels
+ */
+struct mali_c55_params_ccm {
+	struct v4l2_isp_params_block_header header;
+	__u16 coeffs[3][3];
+	__u16 gains[3];
+	__u16 offs[3];
+};
+
 /**
  * define MALI_C55_PARAMS_MAX_SIZE - Maximum size of all Mali C55 Parameters
  *
@@ -780,6 +818,7 @@ struct mali_c55_params_mesh_shading_selection {
 	sizeof(struct mali_c55_params_awb_config) +		\
 	sizeof(struct mali_c55_params_awb_gains) +		\
 	sizeof(struct mali_c55_params_mesh_shading_config) +	\
-	sizeof(struct mali_c55_params_mesh_shading_selection))
+	sizeof(struct mali_c55_params_mesh_shading_selection) +	\
+	sizeof(struct mali_c55_params_ccm))
 
 #endif /* __UAPI_MALI_C55_CONFIG_H */

-- 
2.54.0

[PATCH v2 2/2] media: arm: mali-c55: Add support for RGB Gamma

[Jacopo Mondi]

From: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

Add support for Gamma curve correction for the Mali C55 ISP.

Define a new block in the uAPI using the extensible v4l2-isp format and
implement support for configuring the RGB Gamma parameters in the
mali-c55 parameters handler.

Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

---
v2:
- Remove unused 'rgb_enable' member
- Address checkpatch issues
---
 .../media/platform/arm/mali-c55/mali-c55-params.c  | 75 ++++++++++++++++++++++
 .../platform/arm/mali-c55/mali-c55-registers.h     |  5 ++
 include/uapi/linux/media/arm/mali-c55-config.h     | 45 ++++++++++++-
 3 files changed, 124 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
index 96f1b28a6d77..21f031aaf595 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
@@ -47,6 +47,8 @@
  * @shading_config:	For header->type == MALI_C55_PARAM_MESH_SHADING_CONFIG
  * @shading_selection:	For header->type == MALI_C55_PARAM_MESH_SHADING_SELECTION
  * @ccm:		For header->type == MALI_C55_PARAM_BLOCK_CCM
+ * @gamma:		For header->type == MALI_C55_PARAM_BLOCK_GAMMA_FR and
+ *			header->type = MALI_C55_PARAM_BLOCK_GAMMA_DS
  * @data:		Allows easy initialisation of a union variable with a
  *			pointer into a __u8 array.
  */
@@ -61,6 +63,7 @@ union mali_c55_params_block {
 	const struct mali_c55_params_mesh_shading_config *shading_config;
 	const struct mali_c55_params_mesh_shading_selection *shading_selection;
 	const struct mali_c55_params_ccm *ccm;
+	const struct mali_c55_params_gamma *gamma;
 	const __u8 *data;
 };
 
@@ -462,6 +465,70 @@ static void mali_c55_params_ccm(struct mali_c55 *mali_c55,
 	mali_c55_ctx_write(mali_c55, MALI_C55_REG_CCM_ENABLE, 1);
 }
 
+static void mali_c55_params_gamma(struct mali_c55 *mali_c55,
+				  union mali_c55_params_block block,
+				  __u32 offset, __u32 lut_base)
+{
+	const struct mali_c55_params_gamma *params = block.gamma;
+
+	if (block.header->flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE) {
+		mali_c55_ctx_update_bits(mali_c55,
+					 MALI_C55_REG_GAMMA_RGB_ENABLE + offset,
+					 MALI_C55_GAMMA_ENABLE_MASK, 0x00);
+		return;
+	}
+
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_GAMMA_GAINS_1 + offset,
+				 MALI_C55_GAMMA_GAIN_R_MASK, params->gains[0]);
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_GAMMA_GAINS_1 + offset,
+				 MALI_C55_GAMMA_GAIN_G_MASK,
+				 MALI_C55_GAMMA_GAIN_G(params->gains[1]));
+	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_GAMMA_GAINS_2 + offset,
+				 MALI_C55_GAMMA_GAIN_B_MASK, params->gains[2]);
+	mali_c55_ctx_update_bits(mali_c55,
+				 MALI_C55_REG_GAMMA_OFFSETS_1 + offset,
+				 MALI_C55_GAMMA_OFFSET_R_MASK,
+				 params->offs[0]);
+	mali_c55_ctx_update_bits(mali_c55,
+				 MALI_C55_REG_GAMMA_OFFSETS_1 + offset,
+				 MALI_C55_GAMMA_OFFSET_G_MASK,
+				 MALI_C55_GAMMA_OFFSET_G(params->offs[1]));
+	mali_c55_ctx_update_bits(mali_c55,
+				 MALI_C55_REG_GAMMA_OFFSETS_2 + offset,
+				 MALI_C55_GAMMA_OFFSET_B_MASK,
+				 params->offs[2]);
+
+	for (unsigned int i = 0; i < MALI_C55_NUM_GAMMA_LUT_ELEMENTS; i++) {
+		__u32 addr = lut_base + (i * 4);
+
+		mali_c55_ctx_write(mali_c55, addr, params->lut[i]);
+	}
+
+	mali_c55_ctx_update_bits(mali_c55,
+				 MALI_C55_REG_GAMMA_RGB_ENABLE + offset,
+				 MALI_C55_GAMMA_ENABLE_MASK, 0x1);
+}
+
+static void mali_c55_params_gamma_fr(struct mali_c55 *mali_c55,
+				     union mali_c55_params_block block)
+{
+	return mali_c55_params_gamma(mali_c55, block,
+				     MALI_C55_CAP_DEV_FR_REG_OFFSET,
+				     MALI_C55_REG_FR_GAMMA_RGB_MEM);
+}
+
+static void mali_c55_params_gamma_ds(struct mali_c55 *mali_c55,
+				     union mali_c55_params_block block)
+{
+	/* We cannot apply parameters to DS if it is not fitted. */
+	if (!(mali_c55->capabilities & MALI_C55_GPS_DS_PIPE_FITTED))
+		return;
+
+	return mali_c55_params_gamma(mali_c55, block,
+				     MALI_C55_CAP_DEV_DS_REG_OFFSET,
+				     MALI_C55_REG_DS_GAMMA_RGB_MEM);
+}
+
 static const mali_c55_params_handler mali_c55_params_handlers[] = {
 	[MALI_C55_PARAM_BLOCK_SENSOR_OFFS] = &mali_c55_params_sensor_offs,
 	[MALI_C55_PARAM_BLOCK_AEXP_HIST] = &mali_c55_params_aexp_hist,
@@ -475,6 +542,8 @@ static const mali_c55_params_handler mali_c55_params_handlers[] = {
 	[MALI_C55_PARAM_MESH_SHADING_CONFIG] = &mali_c55_params_lsc_config,
 	[MALI_C55_PARAM_MESH_SHADING_SELECTION] = &mali_c55_params_lsc_selection,
 	[MALI_C55_PARAM_BLOCK_CCM] = &mali_c55_params_ccm,
+	[MALI_C55_PARAM_BLOCK_GAMMA_FR] = &mali_c55_params_gamma_fr,
+	[MALI_C55_PARAM_BLOCK_GAMMA_DS] = &mali_c55_params_gamma_ds,
 };
 
 static const struct v4l2_isp_params_block_type_info
@@ -515,6 +584,12 @@ mali_c55_params_block_types_info[] = {
 	[MALI_C55_PARAM_BLOCK_CCM] = {
 		.size = sizeof(struct mali_c55_params_ccm),
 	},
+	[MALI_C55_PARAM_BLOCK_GAMMA_FR] = {
+		.size = sizeof(struct mali_c55_params_gamma),
+	},
+	[MALI_C55_PARAM_BLOCK_GAMMA_DS] = {
+		.size = sizeof(struct mali_c55_params_gamma),
+	},
 };
 
 static_assert(ARRAY_SIZE(mali_c55_params_handlers) ==
diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-registers.h b/drivers/media/platform/arm/mali-c55/mali-c55-registers.h
index f098effde7b4..7a606bd2e843 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-registers.h
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-registers.h
@@ -425,11 +425,13 @@ enum mali_c55_interrupts {
 #define MALI_C55_REG_GAMMA_GAINS_1			0x1c068
 #define MALI_C55_GAMMA_GAIN_R_MASK			GENMASK(11, 0)
 #define MALI_C55_GAMMA_GAIN_G_MASK			GENMASK(27, 16)
+#define MALI_C55_GAMMA_GAIN_G(x)			((x) << 16)
 #define MALI_C55_REG_GAMMA_GAINS_2			0x1c06c
 #define MALI_C55_GAMMA_GAIN_B_MASK			GENMASK(11, 0)
 #define MALI_C55_REG_GAMMA_OFFSETS_1			0x1c070
 #define MALI_C55_GAMMA_OFFSET_R_MASK			GENMASK(11, 0)
 #define MALI_C55_GAMMA_OFFSET_G_MASK			GENMASK(27, 16)
+#define MALI_C55_GAMMA_OFFSET_G(x)				((x) << 16)
 #define MALI_C55_REG_GAMMA_OFFSETS_2			0x1c074
 #define MALI_C55_GAMMA_OFFSET_B_MASK			GENMASK(11, 0)
 
@@ -441,6 +443,9 @@ enum mali_c55_interrupts {
 #define MALI_C55_REG_FR_GAMMA_RGB_ENABLE		0x1c064
 #define MALI_C55_REG_DS_GAMMA_RGB_ENABLE		0x1c1d8
 
+#define MALI_C55_REG_FR_GAMMA_RGB_MEM			0x18280
+#define MALI_C55_REG_DS_GAMMA_RGB_MEM			0x18484
+
 #define MALI_C55_REG_FR_SCALER_HFILT			0x34a8
 #define MALI_C55_REG_FR_SCALER_VFILT			0x44a8
 #define MALI_C55_REG_DS_SCALER_HFILT			0x14a8
diff --git a/include/uapi/linux/media/arm/mali-c55-config.h b/include/uapi/linux/media/arm/mali-c55-config.h
index 0b2085eed81b..7e96564cb89c 100644
--- a/include/uapi/linux/media/arm/mali-c55-config.h
+++ b/include/uapi/linux/media/arm/mali-c55-config.h
@@ -36,6 +36,9 @@
  */
 #define MALI_C55_MAX_ZONES	(15 * 15)
 
+/* Number of RGB gamma LUT entries. */
+#define MALI_C55_NUM_GAMMA_LUT_ELEMENTS 129
+
 /**
  * struct mali_c55_ae_1024bin_hist - Auto Exposure 1024-bin histogram statistics
  *
@@ -220,6 +223,8 @@ struct mali_c55_stats_buffer {
  * @MALI_C55_PARAM_MESH_SHADING_CONFIG : Mesh shading tables configuration
  * @MALI_C55_PARAM_MESH_SHADING_SELECTION: Mesh shading table selection
  * @MALI_C55_PARAM_BLOCK_CCM: Colour correction matrix
+ * @MALI_C55_PARAM_BLOCK_GAMMA_FR: Gamma gain and offset for FR pipe
+ * @MALI_C55_PARAM_BLOCK_GAMMA_DS: Gamma gain and offset for DS pipe
  */
 enum mali_c55_param_block_type {
 	MALI_C55_PARAM_BLOCK_SENSOR_OFFS,
@@ -234,6 +239,8 @@ enum mali_c55_param_block_type {
 	MALI_C55_PARAM_MESH_SHADING_CONFIG,
 	MALI_C55_PARAM_MESH_SHADING_SELECTION,
 	MALI_C55_PARAM_BLOCK_CCM,
+	MALI_C55_PARAM_BLOCK_GAMMA_FR,
+	MALI_C55_PARAM_BLOCK_GAMMA_DS,
 };
 
 /**
@@ -795,6 +802,40 @@ struct mali_c55_params_ccm {
 	__u16 offs[3];
 };
 
+/**
+ * struct mali_c55_params_gamma - RGB Gamma correction
+ *
+ * Gamma correction is used to program a standard gamma curve such as the sRGB
+ * one. It provides gains and offsets to implement contrast adjustments.
+ *
+ * Gamma correction is applied on both the FR and DS pipes separately in the RGB
+ * colour domain where the following operations take place:
+ * 1) An offset is subtracted from each colour channel
+ * 2) Each colour channel is multiplied by a gain
+ * 3) The Gamma LUT is applied to each colour channel
+ *
+ * The Gamma LUT has 129 entries where each node is an unsigned 12 bit number.
+ * It is expected that LUT[0]=0 and LUT[128]=0xffff, with the other 127 values
+ * defining the Gamma correction curve.
+ *
+ * As one Gamma correction block is available on both the FR and DS pipes, the
+ * header.type field should be set to one of either
+ * MALI_C55_PARAM_BLOCK_GAMMA_FR or MALI_C55_PARAM_BLOCK_GAMMA_DS from
+ * :c:type:`mali_c55_param_block_type`.
+ *
+ * @header:	The Mali-C55 parameters block header
+ * @gains:	Gains for the red, green and blue channel in unsigned Q4.8 format
+ * @offs:	Offsets subtracted from the red, green and blue channels
+ *		in unsigned 12-bit format
+ * @lut:	129-node Gamma LUT in u0.12 format
+ */
+struct mali_c55_params_gamma {
+	struct v4l2_isp_params_block_header header;
+	__u16 gains[3];
+	__u16 offs[3];
+	__u32 lut[MALI_C55_NUM_GAMMA_LUT_ELEMENTS];
+};
+
 /**
  * define MALI_C55_PARAMS_MAX_SIZE - Maximum size of all Mali C55 Parameters
  *
@@ -819,6 +860,8 @@ struct mali_c55_params_ccm {
 	sizeof(struct mali_c55_params_awb_gains) +		\
 	sizeof(struct mali_c55_params_mesh_shading_config) +	\
 	sizeof(struct mali_c55_params_mesh_shading_selection) +	\
-	sizeof(struct mali_c55_params_ccm))
+	sizeof(struct mali_c55_params_ccm) +			\
+	sizeof(struct mali_c55_params_gamma) +			\
+	sizeof(struct mali_c55_params_gamma))
 
 #endif /* __UAPI_MALI_C55_CONFIG_H */

-- 
2.54.0

Re: [PATCH v2 0/2] media: mali-c55: Add support for CCM and Gamma

[Linus Walleij]

On Tue, Jun 16, 2026 at 4:36 PM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:

> Changes in v2:
> - EDITME: describe what is new in this series revision.
> - EDITME: use bulletpoints and terse descriptions.
> - Link to v1: https://lore.kernel.org/r/20260616-mali-c55-ccm-gamma-v1-0-174fe4fedea3@ideasonboard.com

Odd changes :D

Honestly, I think this is not your fault, b4 should not allow this.

Konstantin (Ryabitsev): could we make b4 just refuse to send patch series if
this changelog contains EDITME entries?

Yours,
Linus Walleij

Re: [PATCH v2 0/2] media: mali-c55: Add support for CCM and Gamma

[Jacopo Mondi]

Hi Linus

On Thu, Jun 18, 2026 at 03:28:12PM +0200, Linus Walleij wrote:
> On Tue, Jun 16, 2026 at 4:36 PM Jacopo Mondi
> <jacopo.mondi@ideasonboard.com> wrote:
>
> > Changes in v2:
> > - EDITME: describe what is new in this series revision.
> > - EDITME: use bulletpoints and terse descriptions.
> > - Link to v1: https://lore.kernel.org/r/20260616-mali-c55-ccm-gamma-v1-0-174fe4fedea3@ideasonboard.com
>
> Odd changes :D
>
> Honestly, I think this is not your fault, b4 should not allow this.

Ahah, indeed it is my fault instead as I forgot to update the entries

If helpful:

- Address checkpatch warning in mali_c55_params_gamma() (I didn't run
  b4 prep --check, sorry :)
- Remove unused member 'rgb_enable' from uAPI

>
> Konstantin (Ryabitsev): could we make b4 just refuse to send patch series if
> this changelog contains EDITME entries?

That would be nice!

>
> Yours,
> Linus Walleij

Re: [PATCH v2 0/2] media: mali-c55: Add support for CCM and Gamma

[Konstantin Ryabitsev]

On Thu, Jun 18, 2026 at 03:28:12PM +0200, Linus Walleij wrote:
> > Changes in v2:
> > - EDITME: describe what is new in this series revision.
> > - EDITME: use bulletpoints and terse descriptions.
> > - Link to v1: https://lore.kernel.org/r/20260616-mali-c55-ccm-gamma-v1-0-174fe4fedea3@ideasonboard.com
> 
> Odd changes :D
> 
> Honestly, I think this is not your fault, b4 should not allow this.
> 
> Konstantin (Ryabitsev): could we make b4 just refuse to send patch series if
> this changelog contains EDITME entries?

We already do this in pre-flight check, we just don't stop you from actually
ignoring the warning, because there can be legitimate situations in which this
should be ignored (e.g. you're sending a 'quick feedback' email to your
co-developers instead of making an actual submission).

We should have printed this warning before send:

    CRITICAL: Edit the cover: b4 prep --edit-cover

    Press Enter to ignore and send anyway or Ctrl-C to abort and fix

I don't think we should do a hard refusal here. We've already identified the
problem and invited the user to fix it.

-K

Re: [PATCH v2 0/2] media: mali-c55: Add support for CCM and Gamma

[Jacopo Mondi]

Hi Konstantin

On Thu, Jun 18, 2026 at 10:48:39AM -0400, Konstantin Ryabitsev wrote:
> On Thu, Jun 18, 2026 at 03:28:12PM +0200, Linus Walleij wrote:
> > > Changes in v2:
> > > - EDITME: describe what is new in this series revision.
> > > - EDITME: use bulletpoints and terse descriptions.
> > > - Link to v1: https://lore.kernel.org/r/20260616-mali-c55-ccm-gamma-v1-0-174fe4fedea3@ideasonboard.com
> >
> > Odd changes :D
> >
> > Honestly, I think this is not your fault, b4 should not allow this.
> >
> > Konstantin (Ryabitsev): could we make b4 just refuse to send patch series if
> > this changelog contains EDITME entries?
>
> We already do this in pre-flight check, we just don't stop you from actually
> ignoring the warning, because there can be legitimate situations in which this
> should be ignored (e.g. you're sending a 'quick feedback' email to your
> co-developers instead of making an actual submission).
>
> We should have printed this warning before send:
>
>     CRITICAL: Edit the cover: b4 prep --edit-cover
>
>     Press Enter to ignore and send anyway or Ctrl-C to abort and fix
>
> I don't think we should do a hard refusal here. We've already identified the
> problem and invited the user to fix it.


---
Some pre-flight checks are failing:
  - Edit the cover   : b4 prep --edit-cover
  - Run auto-to-cc   : b4 prep --auto-to-cc
---
Press Enter to ignore and send anyway or Ctrl-C to abort and fix
---

So my bad I ignored it. (the --auto-to-cc is however a false positive,
as I've run in on v1 and manually edited the recipients list).

>
> -K

Re: [PATCH v2 1/2] media: arm: mali-c55: Add support for CCM

[Linus Walleij]

Hi Jacopo,

thanks for your patch, and sorry for taking so long to review it. I
was reading up
on data sheets.

On Tue, Jun 16, 2026 at 4:36 PM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:

> From: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
>
> Add support for the CCM (Color Correction Matrix) for the Mali C55 ISP.
>
> Define a new block in the uAPI using the extensible v4l2-isp format and
> implement support for configuring the CCM parameters in the mali-c55
> ISP driver.
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

I compared the code to the C55 TRM and it checks out.
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

Re: [PATCH v2 2/2] media: arm: mali-c55: Add support for RGB Gamma

[Linus Walleij]

Hi Jacopo,

thanks for your patch!

On Tue, Jun 16, 2026 at 4:36 PM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:

> From: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
>
> Add support for Gamma curve correction for the Mali C55 ISP.
>
> Define a new block in the uAPI using the extensible v4l2-isp format and
> implement support for configuring the RGB Gamma parameters in the
> mali-c55 parameters handler.
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
(...)

> +static void mali_c55_params_gamma(struct mali_c55 *mali_c55,
> +                                 union mali_c55_params_block block,
> +                                 __u32 offset, __u32 lut_base)
> +{
> +       const struct mali_c55_params_gamma *params = block.gamma;
> +
> +       if (block.header->flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE) {
> +               mali_c55_ctx_update_bits(mali_c55,
> +                                        MALI_C55_REG_GAMMA_RGB_ENABLE + offset,
> +                                        MALI_C55_GAMMA_ENABLE_MASK, 0x00);
> +               return;
> +       }
> +
> +       mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_GAMMA_GAINS_1 + offset,
> +                                MALI_C55_GAMMA_GAIN_R_MASK, params->gains[0]);
> +       mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_GAMMA_GAINS_1 + offset,
> +                                MALI_C55_GAMMA_GAIN_G_MASK,
> +                                MALI_C55_GAMMA_GAIN_G(params->gains[1]));

It is a bit of confusion for the head when GAINS_1 is indexed to
gains[0] and gains[1] because the register i split.

> +       /* We cannot apply parameters to DS if it is not fitted. */
> +       if (!(mali_c55->capabilities & MALI_C55_GPS_DS_PIPE_FITTED))
> +               return;

I suppose this is a HW synthesis thin? Whether DS is fitted or not?
(Just curious.)

> @@ -425,11 +425,13 @@ enum mali_c55_interrupts {
>  #define MALI_C55_REG_GAMMA_GAINS_1                     0x1c068
>  #define MALI_C55_GAMMA_GAIN_R_MASK                     GENMASK(11, 0)
>  #define MALI_C55_GAMMA_GAIN_G_MASK                     GENMASK(27, 16)

Because of my confusion I would rename *GAINS_1
to *GAINS_RG..

> +#define MALI_C55_GAMMA_GAIN_G(x)                       ((x) << 16)
>  #define MALI_C55_REG_GAMMA_GAINS_2                     0x1c06c
>  #define MALI_C55_GAMMA_GAIN_B_MASK                     GENMASK(11, 0)

.. and *GAINS_2 to GAINS_B.

This would make it clear what the registers are for.

>  #define MALI_C55_REG_GAMMA_OFFSETS_1                   0x1c070
>  #define MALI_C55_GAMMA_OFFSET_R_MASK                   GENMASK(11, 0)
>  #define MALI_C55_GAMMA_OFFSET_G_MASK                   GENMASK(27, 16)

Same here *GAMMA_OFFSETS_RG

> +#define MALI_C55_GAMMA_OFFSET_G(x)                             ((x) << 16)
>  #define MALI_C55_REG_GAMMA_OFFSETS_2                   0x1c074
>  #define MALI_C55_GAMMA_OFFSET_B_MASK                   GENMASK(11, 0)

Etc.

You might have good reasons for this naming that I don't understand
(like they are named like that in some documentation, I checked the
register map document but it doesn't seem to name the individual registers
but call them as a group "fr gamma rgb".
Maybe I'm looking in the wrong place.
so either way:

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

Re: [PATCH v2 1/2] media: arm: mali-c55: Add support for CCM

[Vincenzo Frascino]

Hi Jacopo,

thank you for your patch!

And sorry for the delay in my review.

On 16/06/2026 15:36, Jacopo Mondi wrote:
> From: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> 
> Add support for the CCM (Color Correction Matrix) for the Mali C55 ISP.
> 
> Define a new block in the uAPI using the extensible v4l2-isp format and
> implement support for configuring the CCM parameters in the mali-c55
> ISP driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

I have a couple of questions. With this:

Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

> ---
>  .../media/platform/arm/mali-c55/mali-c55-params.c  | 52 ++++++++++++++++++++++
>  include/uapi/linux/media/arm/mali-c55-config.h     | 41 ++++++++++++++++-
>  2 files changed, 92 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> index de0e9d898db7..96f1b28a6d77 100644
> --- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> @@ -46,6 +46,7 @@
>   * @awb_config:		For header->type == MALI_C55_PARAM_BLOCK_AWB_CONFIG
>   * @shading_config:	For header->type == MALI_C55_PARAM_MESH_SHADING_CONFIG
>   * @shading_selection:	For header->type == MALI_C55_PARAM_MESH_SHADING_SELECTION
> + * @ccm:		For header->type == MALI_C55_PARAM_BLOCK_CCM
>   * @data:		Allows easy initialisation of a union variable with a
>   *			pointer into a __u8 array.
>   */
> @@ -59,6 +60,7 @@ union mali_c55_params_block {
>  	const struct mali_c55_params_awb_config *awb_config;
>  	const struct mali_c55_params_mesh_shading_config *shading_config;
>  	const struct mali_c55_params_mesh_shading_selection *shading_selection;
> +	const struct mali_c55_params_ccm *ccm;
>  	const __u8 *data;
>  };
>  
> @@ -414,6 +416,52 @@ static void mali_c55_params_lsc_selection(struct mali_c55 *mali_c55,
>  				 params->mesh_strength);
>  }
>  
> +static void mali_c55_params_ccm(struct mali_c55 *mali_c55,
> +				union mali_c55_params_block block)
> +{
> +	const struct mali_c55_params_ccm *params = block.ccm;
> +
> +	if (block.header->flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE) {
> +		mali_c55_ctx_write(mali_c55, MALI_C55_REG_CCM_ENABLE, 0);
> +		return;
> +	}
> +
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_R_R,
> +				 MALI_C55_CCM_COEF_MASK, params->coeffs[0][0]);

Should values be validated or masked before programming? Since this is
uAPI-controlled input, it may be worth rejecting values with bits outside
MALI_C55_CCM_COEF_MASK rather than silently truncating them in update_bits().

> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_R_G,
> +				 MALI_C55_CCM_COEF_MASK, params->coeffs[0][1]);
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_R_B,
> +				 MALI_C55_CCM_COEF_MASK, params->coeffs[0][2]);
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_G_R,
> +				 MALI_C55_CCM_COEF_MASK, params->coeffs[1][0]);
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_G_G,
> +				 MALI_C55_CCM_COEF_MASK, params->coeffs[1][1]);
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_G_B,
> +				 MALI_C55_CCM_COEF_MASK, params->coeffs[1][2]);
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_B_R,
> +				 MALI_C55_CCM_COEF_MASK, params->coeffs[2][0]);
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_B_G,
> +				 MALI_C55_CCM_COEF_MASK, params->coeffs[2][1]);
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_B_B,
> +				 MALI_C55_CCM_COEF_MASK, params->coeffs[2][2]);
> +
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_GAIN_R,
> +				 MALI_C55_CCM_ANTIFOG_GAIN_MASK, params->gains[0]);
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_GAIN_G,
> +				 MALI_C55_CCM_ANTIFOG_GAIN_MASK, params->gains[1]);
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_GAIN_B,
> +				 MALI_C55_CCM_ANTIFOG_GAIN_MASK, params->gains[2]);
> +
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_OFFSET_R,
> +				 MALI_C55_CCM_ANTIFOG_OFFSET_MASK, params->offs[0]);
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_OFFSET_G,
> +				 MALI_C55_CCM_ANTIFOG_OFFSET_MASK, params->offs[1]);
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_OFFSET_B,
> +				 MALI_C55_CCM_ANTIFOG_OFFSET_MASK, params->offs[2]);
> +
> +	mali_c55_ctx_write(mali_c55, MALI_C55_REG_CCM_ENABLE, 1);
> +}
> +
>  static const mali_c55_params_handler mali_c55_params_handlers[] = {
>  	[MALI_C55_PARAM_BLOCK_SENSOR_OFFS] = &mali_c55_params_sensor_offs,
>  	[MALI_C55_PARAM_BLOCK_AEXP_HIST] = &mali_c55_params_aexp_hist,
> @@ -426,6 +474,7 @@ static const mali_c55_params_handler mali_c55_params_handlers[] = {
>  	[MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP] = &mali_c55_params_awb_gains,
>  	[MALI_C55_PARAM_MESH_SHADING_CONFIG] = &mali_c55_params_lsc_config,
>  	[MALI_C55_PARAM_MESH_SHADING_SELECTION] = &mali_c55_params_lsc_selection,
> +	[MALI_C55_PARAM_BLOCK_CCM] = &mali_c55_params_ccm,
>  };
>  
>  static const struct v4l2_isp_params_block_type_info
> @@ -463,6 +512,9 @@ mali_c55_params_block_types_info[] = {
>  	[MALI_C55_PARAM_MESH_SHADING_SELECTION] = {
>  		.size = sizeof(struct mali_c55_params_mesh_shading_selection),
>  	},
> +	[MALI_C55_PARAM_BLOCK_CCM] = {
> +		.size = sizeof(struct mali_c55_params_ccm),
> +	},
>  };
>  
>  static_assert(ARRAY_SIZE(mali_c55_params_handlers) ==
> diff --git a/include/uapi/linux/media/arm/mali-c55-config.h b/include/uapi/linux/media/arm/mali-c55-config.h
> index 3d335f950eeb..0b2085eed81b 100644
> --- a/include/uapi/linux/media/arm/mali-c55-config.h
> +++ b/include/uapi/linux/media/arm/mali-c55-config.h
> @@ -219,6 +219,7 @@ struct mali_c55_stats_buffer {
>   * @MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP: Auto-white balance gains for AEXP-0 tap
>   * @MALI_C55_PARAM_MESH_SHADING_CONFIG : Mesh shading tables configuration
>   * @MALI_C55_PARAM_MESH_SHADING_SELECTION: Mesh shading table selection
> + * @MALI_C55_PARAM_BLOCK_CCM: Colour correction matrix
>   */
>  enum mali_c55_param_block_type {
>  	MALI_C55_PARAM_BLOCK_SENSOR_OFFS,
> @@ -232,6 +233,7 @@ enum mali_c55_param_block_type {
>  	MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP,
>  	MALI_C55_PARAM_MESH_SHADING_CONFIG,
>  	MALI_C55_PARAM_MESH_SHADING_SELECTION,
> +	MALI_C55_PARAM_BLOCK_CCM,
>  };
>  
>  /**
> @@ -757,6 +759,42 @@ struct mali_c55_params_mesh_shading_selection {
>  	__u16 mesh_strength;
>  };
>  
> +/**
> + * struct mali_c55_params_ccm - Coefficients, offsets and gains for the colour
> + *				correction matrix
> + *
> + * The colour correction module converts images data from a sensor-specific
> + * colour space to known one.
> + *
> + * Colour correction is applied after demosaicing and each pixel is represented
> + * as a column vector of the three RGB colour channels on which the following
> + * operations take place:
> + * 1) An offset is subtracted from each colour channel
> + * 2) Each colour channel is multiplied by a gain
> + * 3) The pixel column vector is multiplied by the colour correction matrix
> + *
> + * This struct allows users to configure the coefficients for CCM and the
> + * per-channel offsets and gains. The nine matrix coefficients are expressed as
> + * signed Q4.8 Sign/Magnitude fixed-point numbers, the three gain multipliers
> + * are expressed as unsigned Q4.8 fixed-point numbers and the three offsets are
> + * expressed as a 12-bit unsigned integers.
> + *
> + * header.type should be set to MALI_C55_PARAM_BLOCK_CCM from
> + * :c:type:`mali_c55_param_block_type`.
> + *
> + * @header:	The Mali-C55 parameters block header
> + * @coeffs:	3x3 color conversion matrix coefficients in sign/magnitude
> + *		Q4.8 format
> + * @gains:	Gains for red, green and blue channels in unsigned Q4.8 format
> + * @offs:	Offsets for red, green and blue channels
> + */
> +struct mali_c55_params_ccm {
> +	struct v4l2_isp_params_block_header header;
> +	__u16 coeffs[3][3];
> +	__u16 gains[3];
> +	__u16 offs[3];

The comment says offsets are 12-bit unsigned integers. Is there a reason why
instead of validation in the params parser so userspace gets an error for values
above 4095, we rely on register masking?

> +};
> +
>  /**
>   * define MALI_C55_PARAMS_MAX_SIZE - Maximum size of all Mali C55 Parameters
>   *
> @@ -780,6 +818,7 @@ struct mali_c55_params_mesh_shading_selection {
>  	sizeof(struct mali_c55_params_awb_config) +		\
>  	sizeof(struct mali_c55_params_awb_gains) +		\
>  	sizeof(struct mali_c55_params_mesh_shading_config) +	\
> -	sizeof(struct mali_c55_params_mesh_shading_selection))
> +	sizeof(struct mali_c55_params_mesh_shading_selection) +	\
> +	sizeof(struct mali_c55_params_ccm))
>  
>  #endif /* __UAPI_MALI_C55_CONFIG_H */
> 

-- 
Regards,
Vincenzo

Re: [PATCH v2 2/2] media: arm: mali-c55: Add support for RGB Gamma

[Vincenzo Frascino]

Hi Jacopo,

thank you for your patch!

On 16/06/2026 15:36, Jacopo Mondi wrote:
> From: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> 
> Add support for Gamma curve correction for the Mali C55 ISP.
> 
> Define a new block in the uAPI using the extensible v4l2-isp format and
> implement support for configuring the RGB Gamma parameters in the
> mali-c55 parameters handler.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

I have a couple of comments in addition to what Linus mentioned in his review.
With this:

Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

> 
> ---
> v2:
> - Remove unused 'rgb_enable' member
> - Address checkpatch issues
> ---
>  .../media/platform/arm/mali-c55/mali-c55-params.c  | 75 ++++++++++++++++++++++
>  .../platform/arm/mali-c55/mali-c55-registers.h     |  5 ++
>  include/uapi/linux/media/arm/mali-c55-config.h     | 45 ++++++++++++-
>  3 files changed, 124 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> index 96f1b28a6d77..21f031aaf595 100644
> --- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> @@ -47,6 +47,8 @@
>   * @shading_config:	For header->type == MALI_C55_PARAM_MESH_SHADING_CONFIG
>   * @shading_selection:	For header->type == MALI_C55_PARAM_MESH_SHADING_SELECTION
>   * @ccm:		For header->type == MALI_C55_PARAM_BLOCK_CCM
> + * @gamma:		For header->type == MALI_C55_PARAM_BLOCK_GAMMA_FR and
> + *			header->type = MALI_C55_PARAM_BLOCK_GAMMA_DS
>   * @data:		Allows easy initialisation of a union variable with a
>   *			pointer into a __u8 array.
>   */
> @@ -61,6 +63,7 @@ union mali_c55_params_block {
>  	const struct mali_c55_params_mesh_shading_config *shading_config;
>  	const struct mali_c55_params_mesh_shading_selection *shading_selection;
>  	const struct mali_c55_params_ccm *ccm;
> +	const struct mali_c55_params_gamma *gamma;
>  	const __u8 *data;
>  };
>  
> @@ -462,6 +465,70 @@ static void mali_c55_params_ccm(struct mali_c55 *mali_c55,
>  	mali_c55_ctx_write(mali_c55, MALI_C55_REG_CCM_ENABLE, 1);
>  }
>  
> +static void mali_c55_params_gamma(struct mali_c55 *mali_c55,
> +				  union mali_c55_params_block block,
> +				  __u32 offset, __u32 lut_base)
> +{
> +	const struct mali_c55_params_gamma *params = block.gamma;
> +
> +	if (block.header->flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE) {
> +		mali_c55_ctx_update_bits(mali_c55,
> +					 MALI_C55_REG_GAMMA_RGB_ENABLE + offset,
> +					 MALI_C55_GAMMA_ENABLE_MASK, 0x00);
> +		return;
> +	}
> +
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_GAMMA_GAINS_1 + offset,
> +				 MALI_C55_GAMMA_GAIN_R_MASK, params->gains[0]);
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_GAMMA_GAINS_1 + offset,
> +				 MALI_C55_GAMMA_GAIN_G_MASK,
> +				 MALI_C55_GAMMA_GAIN_G(params->gains[1]));
> +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_GAMMA_GAINS_2 + offset,
> +				 MALI_C55_GAMMA_GAIN_B_MASK, params->gains[2]);
> +	mali_c55_ctx_update_bits(mali_c55,
> +				 MALI_C55_REG_GAMMA_OFFSETS_1 + offset,
> +				 MALI_C55_GAMMA_OFFSET_R_MASK,
> +				 params->offs[0]);
> +	mali_c55_ctx_update_bits(mali_c55,
> +				 MALI_C55_REG_GAMMA_OFFSETS_1 + offset,
> +				 MALI_C55_GAMMA_OFFSET_G_MASK,
> +				 MALI_C55_GAMMA_OFFSET_G(params->offs[1]));
> +	mali_c55_ctx_update_bits(mali_c55,
> +				 MALI_C55_REG_GAMMA_OFFSETS_2 + offset,
> +				 MALI_C55_GAMMA_OFFSET_B_MASK,
> +				 params->offs[2]);
> +
> +	for (unsigned int i = 0; i < MALI_C55_NUM_GAMMA_LUT_ELEMENTS; i++) {
> +		__u32 addr = lut_base + (i * 4);
> +
> +		mali_c55_ctx_write(mali_c55, addr, params->lut[i]);
> +	}
> +
> +	mali_c55_ctx_update_bits(mali_c55,
> +				 MALI_C55_REG_GAMMA_RGB_ENABLE + offset,
> +				 MALI_C55_GAMMA_ENABLE_MASK, 0x1);
> +}
> +
> +static void mali_c55_params_gamma_fr(struct mali_c55 *mali_c55,
> +				     union mali_c55_params_block block)
> +{
> +	return mali_c55_params_gamma(mali_c55, block,
> +				     MALI_C55_CAP_DEV_FR_REG_OFFSET,
> +				     MALI_C55_REG_FR_GAMMA_RGB_MEM);
> +}
> +
> +static void mali_c55_params_gamma_ds(struct mali_c55 *mali_c55,
> +				     union mali_c55_params_block block)
> +{
> +	/* We cannot apply parameters to DS if it is not fitted. */
> +	if (!(mali_c55->capabilities & MALI_C55_GPS_DS_PIPE_FITTED))
> +		return;
> +
> +	return mali_c55_params_gamma(mali_c55, block,
> +				     MALI_C55_CAP_DEV_DS_REG_OFFSET,
> +				     MALI_C55_REG_DS_GAMMA_RGB_MEM);
> +}
> +
>  static const mali_c55_params_handler mali_c55_params_handlers[] = {
>  	[MALI_C55_PARAM_BLOCK_SENSOR_OFFS] = &mali_c55_params_sensor_offs,
>  	[MALI_C55_PARAM_BLOCK_AEXP_HIST] = &mali_c55_params_aexp_hist,
> @@ -475,6 +542,8 @@ static const mali_c55_params_handler mali_c55_params_handlers[] = {
>  	[MALI_C55_PARAM_MESH_SHADING_CONFIG] = &mali_c55_params_lsc_config,
>  	[MALI_C55_PARAM_MESH_SHADING_SELECTION] = &mali_c55_params_lsc_selection,
>  	[MALI_C55_PARAM_BLOCK_CCM] = &mali_c55_params_ccm,
> +	[MALI_C55_PARAM_BLOCK_GAMMA_FR] = &mali_c55_params_gamma_fr,
> +	[MALI_C55_PARAM_BLOCK_GAMMA_DS] = &mali_c55_params_gamma_ds,
>  };
>  
>  static const struct v4l2_isp_params_block_type_info
> @@ -515,6 +584,12 @@ mali_c55_params_block_types_info[] = {
>  	[MALI_C55_PARAM_BLOCK_CCM] = {
>  		.size = sizeof(struct mali_c55_params_ccm),
>  	},
> +	[MALI_C55_PARAM_BLOCK_GAMMA_FR] = {
> +		.size = sizeof(struct mali_c55_params_gamma),
> +	},
> +	[MALI_C55_PARAM_BLOCK_GAMMA_DS] = {
> +		.size = sizeof(struct mali_c55_params_gamma),
> +	},
>  };
>  
>  static_assert(ARRAY_SIZE(mali_c55_params_handlers) ==
> diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-registers.h b/drivers/media/platform/arm/mali-c55/mali-c55-registers.h
> index f098effde7b4..7a606bd2e843 100644
> --- a/drivers/media/platform/arm/mali-c55/mali-c55-registers.h
> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-registers.h
> @@ -425,11 +425,13 @@ enum mali_c55_interrupts {
>  #define MALI_C55_REG_GAMMA_GAINS_1			0x1c068
>  #define MALI_C55_GAMMA_GAIN_R_MASK			GENMASK(11, 0)
>  #define MALI_C55_GAMMA_GAIN_G_MASK			GENMASK(27, 16)
> +#define MALI_C55_GAMMA_GAIN_G(x)			((x) << 16)
>  #define MALI_C55_REG_GAMMA_GAINS_2			0x1c06c
>  #define MALI_C55_GAMMA_GAIN_B_MASK			GENMASK(11, 0)
>  #define MALI_C55_REG_GAMMA_OFFSETS_1			0x1c070
>  #define MALI_C55_GAMMA_OFFSET_R_MASK			GENMASK(11, 0)
>  #define MALI_C55_GAMMA_OFFSET_G_MASK			GENMASK(27, 16)
> +#define MALI_C55_GAMMA_OFFSET_G(x)				((x) << 16)

Nit: there is an extra tab before ((x) << 16) compared to the surrounding
defines. Please align this with the style of the nearby MALI_C55_GAMMA_GAIN_G()
macro.

>  #define MALI_C55_REG_GAMMA_OFFSETS_2			0x1c074
>  #define MALI_C55_GAMMA_OFFSET_B_MASK			GENMASK(11, 0)
>  
> @@ -441,6 +443,9 @@ enum mali_c55_interrupts {
>  #define MALI_C55_REG_FR_GAMMA_RGB_ENABLE		0x1c064
>  #define MALI_C55_REG_DS_GAMMA_RGB_ENABLE		0x1c1d8
>  
> +#define MALI_C55_REG_FR_GAMMA_RGB_MEM			0x18280
> +#define MALI_C55_REG_DS_GAMMA_RGB_MEM			0x18484
> +
>  #define MALI_C55_REG_FR_SCALER_HFILT			0x34a8
>  #define MALI_C55_REG_FR_SCALER_VFILT			0x44a8
>  #define MALI_C55_REG_DS_SCALER_HFILT			0x14a8
> diff --git a/include/uapi/linux/media/arm/mali-c55-config.h b/include/uapi/linux/media/arm/mali-c55-config.h
> index 0b2085eed81b..7e96564cb89c 100644
> --- a/include/uapi/linux/media/arm/mali-c55-config.h
> +++ b/include/uapi/linux/media/arm/mali-c55-config.h
> @@ -36,6 +36,9 @@
>   */
>  #define MALI_C55_MAX_ZONES	(15 * 15)
>  
> +/* Number of RGB gamma LUT entries. */
> +#define MALI_C55_NUM_GAMMA_LUT_ELEMENTS 129
> +
>  /**
>   * struct mali_c55_ae_1024bin_hist - Auto Exposure 1024-bin histogram statistics
>   *
> @@ -220,6 +223,8 @@ struct mali_c55_stats_buffer {
>   * @MALI_C55_PARAM_MESH_SHADING_CONFIG : Mesh shading tables configuration
>   * @MALI_C55_PARAM_MESH_SHADING_SELECTION: Mesh shading table selection
>   * @MALI_C55_PARAM_BLOCK_CCM: Colour correction matrix
> + * @MALI_C55_PARAM_BLOCK_GAMMA_FR: Gamma gain and offset for FR pipe
> + * @MALI_C55_PARAM_BLOCK_GAMMA_DS: Gamma gain and offset for DS pipe
>   */
>  enum mali_c55_param_block_type {
>  	MALI_C55_PARAM_BLOCK_SENSOR_OFFS,
> @@ -234,6 +239,8 @@ enum mali_c55_param_block_type {
>  	MALI_C55_PARAM_MESH_SHADING_CONFIG,
>  	MALI_C55_PARAM_MESH_SHADING_SELECTION,
>  	MALI_C55_PARAM_BLOCK_CCM,
> +	MALI_C55_PARAM_BLOCK_GAMMA_FR,
> +	MALI_C55_PARAM_BLOCK_GAMMA_DS,
>  };
>  
>  /**
> @@ -795,6 +802,40 @@ struct mali_c55_params_ccm {
>  	__u16 offs[3];
>  };
>  
> +/**
> + * struct mali_c55_params_gamma - RGB Gamma correction
> + *
> + * Gamma correction is used to program a standard gamma curve such as the sRGB
> + * one. It provides gains and offsets to implement contrast adjustments.
> + *
> + * Gamma correction is applied on both the FR and DS pipes separately in the RGB
> + * colour domain where the following operations take place:
> + * 1) An offset is subtracted from each colour channel
> + * 2) Each colour channel is multiplied by a gain
> + * 3) The Gamma LUT is applied to each colour channel
> + *
> + * The Gamma LUT has 129 entries where each node is an unsigned 12 bit number.
> + * It is expected that LUT[0]=0 and LUT[128]=0xffff, with the other 127 values

lut entries are documented as unsigned 12-bit values, so 0xffff looks
inconsistent. Should this be 0xfff instead?

> + * defining the Gamma correction curve.
> + *
> + * As one Gamma correction block is available on both the FR and DS pipes, the
> + * header.type field should be set to one of either
> + * MALI_C55_PARAM_BLOCK_GAMMA_FR or MALI_C55_PARAM_BLOCK_GAMMA_DS from
> + * :c:type:`mali_c55_param_block_type`.
> + *
> + * @header:	The Mali-C55 parameters block header
> + * @gains:	Gains for the red, green and blue channel in unsigned Q4.8 format
> + * @offs:	Offsets subtracted from the red, green and blue channels
> + *		in unsigned 12-bit format
> + * @lut:	129-node Gamma LUT in u0.12 format
> + */
> +struct mali_c55_params_gamma {
> +	struct v4l2_isp_params_block_header header;
> +	__u16 gains[3];
> +	__u16 offs[3];
> +	__u32 lut[MALI_C55_NUM_GAMMA_LUT_ELEMENTS];
> +};
> +
>  /**
>   * define MALI_C55_PARAMS_MAX_SIZE - Maximum size of all Mali C55 Parameters
>   *
> @@ -819,6 +860,8 @@ struct mali_c55_params_ccm {
>  	sizeof(struct mali_c55_params_awb_gains) +		\
>  	sizeof(struct mali_c55_params_mesh_shading_config) +	\
>  	sizeof(struct mali_c55_params_mesh_shading_selection) +	\
> -	sizeof(struct mali_c55_params_ccm))
> +	sizeof(struct mali_c55_params_ccm) +			\
> +	sizeof(struct mali_c55_params_gamma) +			\
> +	sizeof(struct mali_c55_params_gamma))
>  
>  #endif /* __UAPI_MALI_C55_CONFIG_H */
> 

-- 
Regards,
Vincenzo

Re: [PATCH v2 1/2] media: arm: mali-c55: Add support for CCM

[Jacopo Mondi]

Hi Vincenzo
   thanks for the review

On Fri, Jun 26, 2026 at 10:44:49AM +0100, Vincenzo Frascino wrote:
> Hi Jacopo,
>
> thank you for your patch!
>
> And sorry for the delay in my review.
>
> On 16/06/2026 15:36, Jacopo Mondi wrote:
> > From: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> >
> > Add support for the CCM (Color Correction Matrix) for the Mali C55 ISP.
> >
> > Define a new block in the uAPI using the extensible v4l2-isp format and
> > implement support for configuring the CCM parameters in the mali-c55
> > ISP driver.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
>
> I have a couple of questions. With this:
>
> Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>
> > ---
> >  .../media/platform/arm/mali-c55/mali-c55-params.c  | 52 ++++++++++++++++++++++
> >  include/uapi/linux/media/arm/mali-c55-config.h     | 41 ++++++++++++++++-
> >  2 files changed, 92 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> > index de0e9d898db7..96f1b28a6d77 100644
> > --- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> > +++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> > @@ -46,6 +46,7 @@
> >   * @awb_config:		For header->type == MALI_C55_PARAM_BLOCK_AWB_CONFIG
> >   * @shading_config:	For header->type == MALI_C55_PARAM_MESH_SHADING_CONFIG
> >   * @shading_selection:	For header->type == MALI_C55_PARAM_MESH_SHADING_SELECTION
> > + * @ccm:		For header->type == MALI_C55_PARAM_BLOCK_CCM
> >   * @data:		Allows easy initialisation of a union variable with a
> >   *			pointer into a __u8 array.
> >   */
> > @@ -59,6 +60,7 @@ union mali_c55_params_block {
> >  	const struct mali_c55_params_awb_config *awb_config;
> >  	const struct mali_c55_params_mesh_shading_config *shading_config;
> >  	const struct mali_c55_params_mesh_shading_selection *shading_selection;
> > +	const struct mali_c55_params_ccm *ccm;
> >  	const __u8 *data;
> >  };
> >
> > @@ -414,6 +416,52 @@ static void mali_c55_params_lsc_selection(struct mali_c55 *mali_c55,
> >  				 params->mesh_strength);
> >  }
> >
> > +static void mali_c55_params_ccm(struct mali_c55 *mali_c55,
> > +				union mali_c55_params_block block)
> > +{
> > +	const struct mali_c55_params_ccm *params = block.ccm;
> > +
> > +	if (block.header->flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE) {
> > +		mali_c55_ctx_write(mali_c55, MALI_C55_REG_CCM_ENABLE, 0);
> > +		return;
> > +	}
> > +
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_R_R,
> > +				 MALI_C55_CCM_COEF_MASK, params->coeffs[0][0]);
>
> Should values be validated or masked before programming? Since this is

Don't mali_c55_ctx_update_bits() does making already ?

> uAPI-controlled input, it may be worth rejecting values with bits outside
> MALI_C55_CCM_COEF_MASK rather than silently truncating them in update_bits().
>
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_R_G,
> > +				 MALI_C55_CCM_COEF_MASK, params->coeffs[0][1]);
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_R_B,
> > +				 MALI_C55_CCM_COEF_MASK, params->coeffs[0][2]);
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_G_R,
> > +				 MALI_C55_CCM_COEF_MASK, params->coeffs[1][0]);
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_G_G,
> > +				 MALI_C55_CCM_COEF_MASK, params->coeffs[1][1]);
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_G_B,
> > +				 MALI_C55_CCM_COEF_MASK, params->coeffs[1][2]);
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_B_R,
> > +				 MALI_C55_CCM_COEF_MASK, params->coeffs[2][0]);
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_B_G,
> > +				 MALI_C55_CCM_COEF_MASK, params->coeffs[2][1]);
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_COEF_B_B,
> > +				 MALI_C55_CCM_COEF_MASK, params->coeffs[2][2]);
> > +
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_GAIN_R,
> > +				 MALI_C55_CCM_ANTIFOG_GAIN_MASK, params->gains[0]);
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_GAIN_G,
> > +				 MALI_C55_CCM_ANTIFOG_GAIN_MASK, params->gains[1]);
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_GAIN_B,
> > +				 MALI_C55_CCM_ANTIFOG_GAIN_MASK, params->gains[2]);
> > +
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_OFFSET_R,
> > +				 MALI_C55_CCM_ANTIFOG_OFFSET_MASK, params->offs[0]);
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_OFFSET_G,
> > +				 MALI_C55_CCM_ANTIFOG_OFFSET_MASK, params->offs[1]);
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_CCM_ANTIFOG_OFFSET_B,
> > +				 MALI_C55_CCM_ANTIFOG_OFFSET_MASK, params->offs[2]);
> > +
> > +	mali_c55_ctx_write(mali_c55, MALI_C55_REG_CCM_ENABLE, 1);
> > +}
> > +
> >  static const mali_c55_params_handler mali_c55_params_handlers[] = {
> >  	[MALI_C55_PARAM_BLOCK_SENSOR_OFFS] = &mali_c55_params_sensor_offs,
> >  	[MALI_C55_PARAM_BLOCK_AEXP_HIST] = &mali_c55_params_aexp_hist,
> > @@ -426,6 +474,7 @@ static const mali_c55_params_handler mali_c55_params_handlers[] = {
> >  	[MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP] = &mali_c55_params_awb_gains,
> >  	[MALI_C55_PARAM_MESH_SHADING_CONFIG] = &mali_c55_params_lsc_config,
> >  	[MALI_C55_PARAM_MESH_SHADING_SELECTION] = &mali_c55_params_lsc_selection,
> > +	[MALI_C55_PARAM_BLOCK_CCM] = &mali_c55_params_ccm,
> >  };
> >
> >  static const struct v4l2_isp_params_block_type_info
> > @@ -463,6 +512,9 @@ mali_c55_params_block_types_info[] = {
> >  	[MALI_C55_PARAM_MESH_SHADING_SELECTION] = {
> >  		.size = sizeof(struct mali_c55_params_mesh_shading_selection),
> >  	},
> > +	[MALI_C55_PARAM_BLOCK_CCM] = {
> > +		.size = sizeof(struct mali_c55_params_ccm),
> > +	},
> >  };
> >
> >  static_assert(ARRAY_SIZE(mali_c55_params_handlers) ==
> > diff --git a/include/uapi/linux/media/arm/mali-c55-config.h b/include/uapi/linux/media/arm/mali-c55-config.h
> > index 3d335f950eeb..0b2085eed81b 100644
> > --- a/include/uapi/linux/media/arm/mali-c55-config.h
> > +++ b/include/uapi/linux/media/arm/mali-c55-config.h
> > @@ -219,6 +219,7 @@ struct mali_c55_stats_buffer {
> >   * @MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP: Auto-white balance gains for AEXP-0 tap
> >   * @MALI_C55_PARAM_MESH_SHADING_CONFIG : Mesh shading tables configuration
> >   * @MALI_C55_PARAM_MESH_SHADING_SELECTION: Mesh shading table selection
> > + * @MALI_C55_PARAM_BLOCK_CCM: Colour correction matrix
> >   */
> >  enum mali_c55_param_block_type {
> >  	MALI_C55_PARAM_BLOCK_SENSOR_OFFS,
> > @@ -232,6 +233,7 @@ enum mali_c55_param_block_type {
> >  	MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP,
> >  	MALI_C55_PARAM_MESH_SHADING_CONFIG,
> >  	MALI_C55_PARAM_MESH_SHADING_SELECTION,
> > +	MALI_C55_PARAM_BLOCK_CCM,
> >  };
> >
> >  /**
> > @@ -757,6 +759,42 @@ struct mali_c55_params_mesh_shading_selection {
> >  	__u16 mesh_strength;
> >  };
> >
> > +/**
> > + * struct mali_c55_params_ccm - Coefficients, offsets and gains for the colour
> > + *				correction matrix
> > + *
> > + * The colour correction module converts images data from a sensor-specific
> > + * colour space to known one.
> > + *
> > + * Colour correction is applied after demosaicing and each pixel is represented
> > + * as a column vector of the three RGB colour channels on which the following
> > + * operations take place:
> > + * 1) An offset is subtracted from each colour channel
> > + * 2) Each colour channel is multiplied by a gain
> > + * 3) The pixel column vector is multiplied by the colour correction matrix
> > + *
> > + * This struct allows users to configure the coefficients for CCM and the
> > + * per-channel offsets and gains. The nine matrix coefficients are expressed as
> > + * signed Q4.8 Sign/Magnitude fixed-point numbers, the three gain multipliers
> > + * are expressed as unsigned Q4.8 fixed-point numbers and the three offsets are
> > + * expressed as a 12-bit unsigned integers.
> > + *
> > + * header.type should be set to MALI_C55_PARAM_BLOCK_CCM from
> > + * :c:type:`mali_c55_param_block_type`.
> > + *
> > + * @header:	The Mali-C55 parameters block header
> > + * @coeffs:	3x3 color conversion matrix coefficients in sign/magnitude
> > + *		Q4.8 format
> > + * @gains:	Gains for red, green and blue channels in unsigned Q4.8 format
> > + * @offs:	Offsets for red, green and blue channels
> > + */
> > +struct mali_c55_params_ccm {
> > +	struct v4l2_isp_params_block_header header;
> > +	__u16 coeffs[3][3];
> > +	__u16 gains[3];
> > +	__u16 offs[3];
>
> The comment says offsets are 12-bit unsigned integers. Is there a reason why
> instead of validation in the params parser so userspace gets an error for values
> above 4095, we rely on register masking?
>

If we want per-block validation of values they should be implemented
on top of https://patchwork.linuxtv.org/project/linux-media/patch/20260505-extensible-stats-v1-4-e16f326b8dad@ideasonboard.com/
using the new block_validate() callback.

I can do so by listing
https://patchwork.linuxtv.org/project/linux-media/list/?series=24772
as a pre-requisite of this patch

> > +};
> > +
> >  /**
> >   * define MALI_C55_PARAMS_MAX_SIZE - Maximum size of all Mali C55 Parameters
> >   *
> > @@ -780,6 +818,7 @@ struct mali_c55_params_mesh_shading_selection {
> >  	sizeof(struct mali_c55_params_awb_config) +		\
> >  	sizeof(struct mali_c55_params_awb_gains) +		\
> >  	sizeof(struct mali_c55_params_mesh_shading_config) +	\
> > -	sizeof(struct mali_c55_params_mesh_shading_selection))
> > +	sizeof(struct mali_c55_params_mesh_shading_selection) +	\
> > +	sizeof(struct mali_c55_params_ccm))
> >
> >  #endif /* __UAPI_MALI_C55_CONFIG_H */
> >
>
> --
> Regards,
> Vincenzo
>
>

Re: [PATCH v2 2/2] media: arm: mali-c55: Add support for RGB Gamma

[Jacopo Mondi]

Hi Linus,
  thanks for the review

On Fri, Jun 26, 2026 at 12:53:45AM +0200, Linus Walleij wrote:
> Hi Jacopo,
>
> thanks for your patch!
>
> On Tue, Jun 16, 2026 at 4:36 PM Jacopo Mondi
> <jacopo.mondi@ideasonboard.com> wrote:
>
> > From: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> >
> > Add support for Gamma curve correction for the Mali C55 ISP.
> >
> > Define a new block in the uAPI using the extensible v4l2-isp format and
> > implement support for configuring the RGB Gamma parameters in the
> > mali-c55 parameters handler.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> (...)
>
> > +static void mali_c55_params_gamma(struct mali_c55 *mali_c55,
> > +                                 union mali_c55_params_block block,
> > +                                 __u32 offset, __u32 lut_base)
> > +{
> > +       const struct mali_c55_params_gamma *params = block.gamma;
> > +
> > +       if (block.header->flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE) {
> > +               mali_c55_ctx_update_bits(mali_c55,
> > +                                        MALI_C55_REG_GAMMA_RGB_ENABLE + offset,
> > +                                        MALI_C55_GAMMA_ENABLE_MASK, 0x00);
> > +               return;
> > +       }
> > +
> > +       mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_GAMMA_GAINS_1 + offset,
> > +                                MALI_C55_GAMMA_GAIN_R_MASK, params->gains[0]);
> > +       mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_GAMMA_GAINS_1 + offset,
> > +                                MALI_C55_GAMMA_GAIN_G_MASK,
> > +                                MALI_C55_GAMMA_GAIN_G(params->gains[1]));
>
> It is a bit of confusion for the head when GAINS_1 is indexed to
> gains[0] and gains[1] because the register i split.
>

See below

> > +       /* We cannot apply parameters to DS if it is not fitted. */
> > +       if (!(mali_c55->capabilities & MALI_C55_GPS_DS_PIPE_FITTED))
> > +               return;
>
> I suppose this is a HW synthesis thin? Whether DS is fitted or not?
> (Just curious.)

I presume so ?

>
> > @@ -425,11 +425,13 @@ enum mali_c55_interrupts {
> >  #define MALI_C55_REG_GAMMA_GAINS_1                     0x1c068
> >  #define MALI_C55_GAMMA_GAIN_R_MASK                     GENMASK(11, 0)
> >  #define MALI_C55_GAMMA_GAIN_G_MASK                     GENMASK(27, 16)
>
> Because of my confusion I would rename *GAINS_1
> to *GAINS_RG..
>
> > +#define MALI_C55_GAMMA_GAIN_G(x)                       ((x) << 16)
> >  #define MALI_C55_REG_GAMMA_GAINS_2                     0x1c06c
> >  #define MALI_C55_GAMMA_GAIN_B_MASK                     GENMASK(11, 0)
>
> .. and *GAINS_2 to GAINS_B.
>
> This would make it clear what the registers are for.
>

I can certainly do so

> >  #define MALI_C55_REG_GAMMA_OFFSETS_1                   0x1c070
> >  #define MALI_C55_GAMMA_OFFSET_R_MASK                   GENMASK(11, 0)
> >  #define MALI_C55_GAMMA_OFFSET_G_MASK                   GENMASK(27, 16)
>
> Same here *GAMMA_OFFSETS_RG

Aren't the R and G masks different ?

Or are you suggesting
#define MALI_C55_REG_GAMMA_OFFSETS_RG                   0x1c070

>
> > +#define MALI_C55_GAMMA_OFFSET_G(x)                             ((x) << 16)
> >  #define MALI_C55_REG_GAMMA_OFFSETS_2                   0x1c074
> >  #define MALI_C55_GAMMA_OFFSET_B_MASK                   GENMASK(11, 0)
>
> Etc.
>
> You might have good reasons for this naming that I don't understand

I think there register names were already there before this patch
didn't they ?

> (like they are named like that in some documentation, I checked the
> register map document but it doesn't seem to name the individual registers
> but call them as a group "fr gamma rgb".
> Maybe I'm looking in the wrong place.
> so either way:
>
> Reviewed-by: Linus Walleij <linusw@kernel.org>

Thanks!

>
> Yours,
> Linus Walleij
>

Re: [PATCH v2 2/2] media: arm: mali-c55: Add support for RGB Gamma

[Jacopo Mondi]

Hi Vincenzo

On Fri, Jun 26, 2026 at 10:52:38AM +0100, Vincenzo Frascino wrote:
> Hi Jacopo,
>
> thank you for your patch!
>
> On 16/06/2026 15:36, Jacopo Mondi wrote:
> > From: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> >
> > Add support for Gamma curve correction for the Mali C55 ISP.
> >
> > Define a new block in the uAPI using the extensible v4l2-isp format and
> > implement support for configuring the RGB Gamma parameters in the
> > mali-c55 parameters handler.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
>
> I have a couple of comments in addition to what Linus mentioned in his review.
> With this:
>
> Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>
> >
> > ---
> > v2:
> > - Remove unused 'rgb_enable' member
> > - Address checkpatch issues
> > ---
> >  .../media/platform/arm/mali-c55/mali-c55-params.c  | 75 ++++++++++++++++++++++
> >  .../platform/arm/mali-c55/mali-c55-registers.h     |  5 ++
> >  include/uapi/linux/media/arm/mali-c55-config.h     | 45 ++++++++++++-
> >  3 files changed, 124 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> > index 96f1b28a6d77..21f031aaf595 100644
> > --- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> > +++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> > @@ -47,6 +47,8 @@
> >   * @shading_config:	For header->type == MALI_C55_PARAM_MESH_SHADING_CONFIG
> >   * @shading_selection:	For header->type == MALI_C55_PARAM_MESH_SHADING_SELECTION
> >   * @ccm:		For header->type == MALI_C55_PARAM_BLOCK_CCM
> > + * @gamma:		For header->type == MALI_C55_PARAM_BLOCK_GAMMA_FR and
> > + *			header->type = MALI_C55_PARAM_BLOCK_GAMMA_DS
> >   * @data:		Allows easy initialisation of a union variable with a
> >   *			pointer into a __u8 array.
> >   */
> > @@ -61,6 +63,7 @@ union mali_c55_params_block {
> >  	const struct mali_c55_params_mesh_shading_config *shading_config;
> >  	const struct mali_c55_params_mesh_shading_selection *shading_selection;
> >  	const struct mali_c55_params_ccm *ccm;
> > +	const struct mali_c55_params_gamma *gamma;
> >  	const __u8 *data;
> >  };
> >
> > @@ -462,6 +465,70 @@ static void mali_c55_params_ccm(struct mali_c55 *mali_c55,
> >  	mali_c55_ctx_write(mali_c55, MALI_C55_REG_CCM_ENABLE, 1);
> >  }
> >
> > +static void mali_c55_params_gamma(struct mali_c55 *mali_c55,
> > +				  union mali_c55_params_block block,
> > +				  __u32 offset, __u32 lut_base)
> > +{
> > +	const struct mali_c55_params_gamma *params = block.gamma;
> > +
> > +	if (block.header->flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE) {
> > +		mali_c55_ctx_update_bits(mali_c55,
> > +					 MALI_C55_REG_GAMMA_RGB_ENABLE + offset,
> > +					 MALI_C55_GAMMA_ENABLE_MASK, 0x00);
> > +		return;
> > +	}
> > +
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_GAMMA_GAINS_1 + offset,
> > +				 MALI_C55_GAMMA_GAIN_R_MASK, params->gains[0]);
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_GAMMA_GAINS_1 + offset,
> > +				 MALI_C55_GAMMA_GAIN_G_MASK,
> > +				 MALI_C55_GAMMA_GAIN_G(params->gains[1]));
> > +	mali_c55_ctx_update_bits(mali_c55, MALI_C55_REG_GAMMA_GAINS_2 + offset,
> > +				 MALI_C55_GAMMA_GAIN_B_MASK, params->gains[2]);
> > +	mali_c55_ctx_update_bits(mali_c55,
> > +				 MALI_C55_REG_GAMMA_OFFSETS_1 + offset,
> > +				 MALI_C55_GAMMA_OFFSET_R_MASK,
> > +				 params->offs[0]);
> > +	mali_c55_ctx_update_bits(mali_c55,
> > +				 MALI_C55_REG_GAMMA_OFFSETS_1 + offset,
> > +				 MALI_C55_GAMMA_OFFSET_G_MASK,
> > +				 MALI_C55_GAMMA_OFFSET_G(params->offs[1]));
> > +	mali_c55_ctx_update_bits(mali_c55,
> > +				 MALI_C55_REG_GAMMA_OFFSETS_2 + offset,
> > +				 MALI_C55_GAMMA_OFFSET_B_MASK,
> > +				 params->offs[2]);
> > +
> > +	for (unsigned int i = 0; i < MALI_C55_NUM_GAMMA_LUT_ELEMENTS; i++) {
> > +		__u32 addr = lut_base + (i * 4);
> > +
> > +		mali_c55_ctx_write(mali_c55, addr, params->lut[i]);
> > +	}
> > +
> > +	mali_c55_ctx_update_bits(mali_c55,
> > +				 MALI_C55_REG_GAMMA_RGB_ENABLE + offset,
> > +				 MALI_C55_GAMMA_ENABLE_MASK, 0x1);
> > +}
> > +
> > +static void mali_c55_params_gamma_fr(struct mali_c55 *mali_c55,
> > +				     union mali_c55_params_block block)
> > +{
> > +	return mali_c55_params_gamma(mali_c55, block,
> > +				     MALI_C55_CAP_DEV_FR_REG_OFFSET,
> > +				     MALI_C55_REG_FR_GAMMA_RGB_MEM);
> > +}
> > +
> > +static void mali_c55_params_gamma_ds(struct mali_c55 *mali_c55,
> > +				     union mali_c55_params_block block)
> > +{
> > +	/* We cannot apply parameters to DS if it is not fitted. */
> > +	if (!(mali_c55->capabilities & MALI_C55_GPS_DS_PIPE_FITTED))
> > +		return;
> > +
> > +	return mali_c55_params_gamma(mali_c55, block,
> > +				     MALI_C55_CAP_DEV_DS_REG_OFFSET,
> > +				     MALI_C55_REG_DS_GAMMA_RGB_MEM);
> > +}
> > +
> >  static const mali_c55_params_handler mali_c55_params_handlers[] = {
> >  	[MALI_C55_PARAM_BLOCK_SENSOR_OFFS] = &mali_c55_params_sensor_offs,
> >  	[MALI_C55_PARAM_BLOCK_AEXP_HIST] = &mali_c55_params_aexp_hist,
> > @@ -475,6 +542,8 @@ static const mali_c55_params_handler mali_c55_params_handlers[] = {
> >  	[MALI_C55_PARAM_MESH_SHADING_CONFIG] = &mali_c55_params_lsc_config,
> >  	[MALI_C55_PARAM_MESH_SHADING_SELECTION] = &mali_c55_params_lsc_selection,
> >  	[MALI_C55_PARAM_BLOCK_CCM] = &mali_c55_params_ccm,
> > +	[MALI_C55_PARAM_BLOCK_GAMMA_FR] = &mali_c55_params_gamma_fr,
> > +	[MALI_C55_PARAM_BLOCK_GAMMA_DS] = &mali_c55_params_gamma_ds,
> >  };
> >
> >  static const struct v4l2_isp_params_block_type_info
> > @@ -515,6 +584,12 @@ mali_c55_params_block_types_info[] = {
> >  	[MALI_C55_PARAM_BLOCK_CCM] = {
> >  		.size = sizeof(struct mali_c55_params_ccm),
> >  	},
> > +	[MALI_C55_PARAM_BLOCK_GAMMA_FR] = {
> > +		.size = sizeof(struct mali_c55_params_gamma),
> > +	},
> > +	[MALI_C55_PARAM_BLOCK_GAMMA_DS] = {
> > +		.size = sizeof(struct mali_c55_params_gamma),
> > +	},
> >  };
> >
> >  static_assert(ARRAY_SIZE(mali_c55_params_handlers) ==
> > diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-registers.h b/drivers/media/platform/arm/mali-c55/mali-c55-registers.h
> > index f098effde7b4..7a606bd2e843 100644
> > --- a/drivers/media/platform/arm/mali-c55/mali-c55-registers.h
> > +++ b/drivers/media/platform/arm/mali-c55/mali-c55-registers.h
> > @@ -425,11 +425,13 @@ enum mali_c55_interrupts {
> >  #define MALI_C55_REG_GAMMA_GAINS_1			0x1c068
> >  #define MALI_C55_GAMMA_GAIN_R_MASK			GENMASK(11, 0)
> >  #define MALI_C55_GAMMA_GAIN_G_MASK			GENMASK(27, 16)
> > +#define MALI_C55_GAMMA_GAIN_G(x)			((x) << 16)
> >  #define MALI_C55_REG_GAMMA_GAINS_2			0x1c06c
> >  #define MALI_C55_GAMMA_GAIN_B_MASK			GENMASK(11, 0)
> >  #define MALI_C55_REG_GAMMA_OFFSETS_1			0x1c070
> >  #define MALI_C55_GAMMA_OFFSET_R_MASK			GENMASK(11, 0)
> >  #define MALI_C55_GAMMA_OFFSET_G_MASK			GENMASK(27, 16)
> > +#define MALI_C55_GAMMA_OFFSET_G(x)				((x) << 16)
>
> Nit: there is an extra tab before ((x) << 16) compared to the surrounding
> defines. Please align this with the style of the nearby MALI_C55_GAMMA_GAIN_G()
> macro.
>

Good catch, thanks

> >  #define MALI_C55_REG_GAMMA_OFFSETS_2			0x1c074
> >  #define MALI_C55_GAMMA_OFFSET_B_MASK			GENMASK(11, 0)
> >
> > @@ -441,6 +443,9 @@ enum mali_c55_interrupts {
> >  #define MALI_C55_REG_FR_GAMMA_RGB_ENABLE		0x1c064
> >  #define MALI_C55_REG_DS_GAMMA_RGB_ENABLE		0x1c1d8
> >
> > +#define MALI_C55_REG_FR_GAMMA_RGB_MEM			0x18280
> > +#define MALI_C55_REG_DS_GAMMA_RGB_MEM			0x18484
> > +
> >  #define MALI_C55_REG_FR_SCALER_HFILT			0x34a8
> >  #define MALI_C55_REG_FR_SCALER_VFILT			0x44a8
> >  #define MALI_C55_REG_DS_SCALER_HFILT			0x14a8
> > diff --git a/include/uapi/linux/media/arm/mali-c55-config.h b/include/uapi/linux/media/arm/mali-c55-config.h
> > index 0b2085eed81b..7e96564cb89c 100644
> > --- a/include/uapi/linux/media/arm/mali-c55-config.h
> > +++ b/include/uapi/linux/media/arm/mali-c55-config.h
> > @@ -36,6 +36,9 @@
> >   */
> >  #define MALI_C55_MAX_ZONES	(15 * 15)
> >
> > +/* Number of RGB gamma LUT entries. */
> > +#define MALI_C55_NUM_GAMMA_LUT_ELEMENTS 129
> > +
> >  /**
> >   * struct mali_c55_ae_1024bin_hist - Auto Exposure 1024-bin histogram statistics
> >   *
> > @@ -220,6 +223,8 @@ struct mali_c55_stats_buffer {
> >   * @MALI_C55_PARAM_MESH_SHADING_CONFIG : Mesh shading tables configuration
> >   * @MALI_C55_PARAM_MESH_SHADING_SELECTION: Mesh shading table selection
> >   * @MALI_C55_PARAM_BLOCK_CCM: Colour correction matrix
> > + * @MALI_C55_PARAM_BLOCK_GAMMA_FR: Gamma gain and offset for FR pipe
> > + * @MALI_C55_PARAM_BLOCK_GAMMA_DS: Gamma gain and offset for DS pipe
> >   */
> >  enum mali_c55_param_block_type {
> >  	MALI_C55_PARAM_BLOCK_SENSOR_OFFS,
> > @@ -234,6 +239,8 @@ enum mali_c55_param_block_type {
> >  	MALI_C55_PARAM_MESH_SHADING_CONFIG,
> >  	MALI_C55_PARAM_MESH_SHADING_SELECTION,
> >  	MALI_C55_PARAM_BLOCK_CCM,
> > +	MALI_C55_PARAM_BLOCK_GAMMA_FR,
> > +	MALI_C55_PARAM_BLOCK_GAMMA_DS,
> >  };
> >
> >  /**
> > @@ -795,6 +802,40 @@ struct mali_c55_params_ccm {
> >  	__u16 offs[3];
> >  };
> >
> > +/**
> > + * struct mali_c55_params_gamma - RGB Gamma correction
> > + *
> > + * Gamma correction is used to program a standard gamma curve such as the sRGB
> > + * one. It provides gains and offsets to implement contrast adjustments.
> > + *
> > + * Gamma correction is applied on both the FR and DS pipes separately in the RGB
> > + * colour domain where the following operations take place:
> > + * 1) An offset is subtracted from each colour channel
> > + * 2) Each colour channel is multiplied by a gain
> > + * 3) The Gamma LUT is applied to each colour channel
> > + *
> > + * The Gamma LUT has 129 entries where each node is an unsigned 12 bit number.
> > + * It is expected that LUT[0]=0 and LUT[128]=0xffff, with the other 127 values
>
> lut entries are documented as unsigned 12-bit values, so 0xffff looks
> inconsistent. Should this be 0xfff instead?

Oh yes!

Thanks
   j

>
> > + * defining the Gamma correction curve.
> > + *
> > + * As one Gamma correction block is available on both the FR and DS pipes, the
> > + * header.type field should be set to one of either
> > + * MALI_C55_PARAM_BLOCK_GAMMA_FR or MALI_C55_PARAM_BLOCK_GAMMA_DS from
> > + * :c:type:`mali_c55_param_block_type`.
> > + *
> > + * @header:	The Mali-C55 parameters block header
> > + * @gains:	Gains for the red, green and blue channel in unsigned Q4.8 format
> > + * @offs:	Offsets subtracted from the red, green and blue channels
> > + *		in unsigned 12-bit format
> > + * @lut:	129-node Gamma LUT in u0.12 format
> > + */
> > +struct mali_c55_params_gamma {
> > +	struct v4l2_isp_params_block_header header;
> > +	__u16 gains[3];
> > +	__u16 offs[3];
> > +	__u32 lut[MALI_C55_NUM_GAMMA_LUT_ELEMENTS];
> > +};
> > +
> >  /**
> >   * define MALI_C55_PARAMS_MAX_SIZE - Maximum size of all Mali C55 Parameters
> >   *
> > @@ -819,6 +860,8 @@ struct mali_c55_params_ccm {
> >  	sizeof(struct mali_c55_params_awb_gains) +		\
> >  	sizeof(struct mali_c55_params_mesh_shading_config) +	\
> >  	sizeof(struct mali_c55_params_mesh_shading_selection) +	\
> > -	sizeof(struct mali_c55_params_ccm))
> > +	sizeof(struct mali_c55_params_ccm) +			\
> > +	sizeof(struct mali_c55_params_gamma) +			\
> > +	sizeof(struct mali_c55_params_gamma))
> >
> >  #endif /* __UAPI_MALI_C55_CONFIG_H */
> >
>
> --
> Regards,
> Vincenzo
>
>

Re: [PATCH v2 2/2] media: arm: mali-c55: Add support for RGB Gamma

[Linus Walleij]

Hi Jacopo,

On Fri, Jun 26, 2026 at 4:52 PM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:

> > >  #define MALI_C55_REG_GAMMA_OFFSETS_1                   0x1c070
> > >  #define MALI_C55_GAMMA_OFFSET_R_MASK                   GENMASK(11, 0)
> > >  #define MALI_C55_GAMMA_OFFSET_G_MASK                   GENMASK(27, 16)
> >
> > Same here *GAMMA_OFFSETS_RG
>
> Aren't the R and G masks different ?
>
> Or are you suggesting
> #define MALI_C55_REG_GAMMA_OFFSETS_RG                   0x1c070

Yes, exactly, then when reading I know "aha that register contains
R and G gamma".

Yours,
Linus Walleij