[PATCH v5 00/12] media: microchip-isc: fixes and enhancements

[PATCH v5 00/12] media: microchip-isc: fixes and enhancements

[Balakrishnan Sambath]

Bug fixes and feature additions for the Microchip ISC/XISC driver.

Fixes:
 - SBGGR10 Bayer pattern was mapped incorrectly (red/blue swap)
 - WB register fields corrupted by sign extension
 - Race between histogram IRQ and stream stop
 - PM runtime reference leak in AWB work handler

Features:
 - Driver documentation
 - Gamma 1.8/2.4 preset curves
 - Hue/saturation controls for SAMA7G5
 - Grey World AWB with EMA smoothing

Split from v1 per review. Histogram statistics support is being sent
as a separate follow-up series.

Tested on SAMA7G5-EK with IMX219 (RAW10 Bayer capture, AWB, controls
verified across 8 formats and 5 resolutions up to 3264x2464).

Based on v6.19 (e9ec05addd1a).

v1: https://lore.kernel.org/linux-media/20251009155251.102472-1-balamanikandan.gunasundar@microchip.com/
v2: https://lore.kernel.org/linux-media/20260512154339.210444-1-balakrishnan.s@microchip.com/
v3: https://lore.kernel.org/linux-media/20260513071742.97263-1-balakrishnan.s@microchip.com/
v4: https://lore.kernel.org/linux-media/20260518-balki-isc-series1-v4-v4-0-97f189185b7e@microchip.com/

v5:
 - Picked up Eugen's Reviewed-by on the PM runtime leak fix.
   The SBGGR10 fix is extended to SAMA5D2 in v5, so dropped his
   Reviewed-by from that patch pending re-review.
 - Fix SAMA7G5 pipeline mask: CBHS_ENABLE, not CBC_ENABLE
 - Per-platform gamma_default. v4 used 1 for both SoCs, which picks
   the wrong curve on SAMA5D2 (1/2.2 is at index 2 there, index 1
   on SAMA7G5)
 - Fix V4L2_CID_SATURATION range to 0..127 (Q4); update docs to match
 - Initialise hue/saturation at probe to avoid a grayscale first frame
 - Reset histogram stats and gain_smooth in isc_reset_awb_ctrls() so
   AWB does not consume stale state from a previous stream
 - Reword WB masking subject; expand commit bodies on the feature
   patches

v4:
 - Drop gamma LUT controls and CC matrix V4L2 controls patches (move
   to parameter buffer follow-up per Sakari's review)
 - Drop AWB enable pipeline reset patch (cleanup for the dropped
   controls, will return with the follow-up series)
 - Update documentation patch to remove references to dropped controls
 - Rebase on v6.19

v3:
 - Fix bisect failures (regmap declaration, gamma LUT macro ordering)
 - Fix Fixes: tag (use mainline commit, not staging)
 - Add Co-developed-by trailers for Balamanikandan Gunasundar

v2:
 - Split series (histogram stats moved to a separate follow-up series)
 - Reorder: bug fixes first, then features
 - Commit message cleanups
 - Rebase on v6.19-rc8

Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>

---
Balakrishnan Sambath (12):
      media: microchip-isc: fix SBGGR10 Bayer pattern
      media: microchip-isc: fix WB offset and gain register field masking
      media: microchip-isc: fix race condition on stream stop
      media: microchip-isc: fix PM runtime leak in AWB work handler
      media: microchip-isc: add driver documentation
      media: microchip-isc: set SAM9X7 maximum resolution to 2560x1920
      media: microchip-isc: configure DPC and pipeline for SAMA7G5
      media: microchip-isc: add gamma 1.8 and 2.4 correction curves
      media: microchip-isc: add SAMA7G5 hue and saturation controls
      media: microchip-isc: use weighted averages for Grey World AWB
      media: microchip-isc: smooth AWB gains with EMA filter
      media: microchip-isc: scale DPC black level to sensor bit depth

 .../userspace-api/media/drivers/index.rst          |   1 +
 .../userspace-api/media/drivers/microchip-isc.rst  |  68 +++++
 MAINTAINERS                                        |   1 +
 .../media/platform/microchip/microchip-isc-base.c  | 328 ++++++++++++++++-----
 .../media/platform/microchip/microchip-isc-regs.h  |  11 +-
 drivers/media/platform/microchip/microchip-isc.h   |   9 +-
 .../platform/microchip/microchip-sama5d2-isc.c     |   6 +-
 .../platform/microchip/microchip-sama7g5-isc.c     | 102 +++++--
 8 files changed, 426 insertions(+), 100 deletions(-)
---
base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
change-id: 20260525-microchip-isc-fixes-a2cdef7c3d6e

Best regards,
-- 
Balakrishnan Sambath <balakrishnan.s@microchip.com>

[PATCH v5 01/12] media: microchip-isc: fix SBGGR10 Bayer pattern

[Balakrishnan Sambath]

SBGGR10 was mapped to ISC_BAY_CFG_RGRG instead of ISC_BAY_CFG_BGBG,
causing red/blue channel swap.

Fixes: 91b4e487b0c6 ("media: microchip: add ISC driver as Microchip ISC")
Cc: stable@vger.kernel.org
Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 drivers/media/platform/microchip/microchip-sama5d2-isc.c | 2 +-
 drivers/media/platform/microchip/microchip-sama7g5-isc.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/microchip/microchip-sama5d2-isc.c b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
index 66d3d7891991..0ddff1e7b0a0 100644
--- a/drivers/media/platform/microchip/microchip-sama5d2-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
@@ -147,7 +147,7 @@ static struct isc_format sama5d2_formats_list[] = {
 		.fourcc		= V4L2_PIX_FMT_SBGGR10,
 		.mbus_code	= MEDIA_BUS_FMT_SBGGR10_1X10,
 		.pfe_cfg0_bps	= ISC_PFG_CFG0_BPS_TEN,
-		.cfa_baycfg	= ISC_BAY_CFG_RGRG,
+		.cfa_baycfg	= ISC_BAY_CFG_BGBG,
 	},
 	{
 		.fourcc		= V4L2_PIX_FMT_SGBRG10,
diff --git a/drivers/media/platform/microchip/microchip-sama7g5-isc.c b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
index b0302dfc3278..ca23e8adecbd 100644
--- a/drivers/media/platform/microchip/microchip-sama7g5-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
@@ -156,7 +156,7 @@ static struct isc_format sama7g5_formats_list[] = {
 		.fourcc		= V4L2_PIX_FMT_SBGGR10,
 		.mbus_code	= MEDIA_BUS_FMT_SBGGR10_1X10,
 		.pfe_cfg0_bps	= ISC_PFG_CFG0_BPS_TEN,
-		.cfa_baycfg	= ISC_BAY_CFG_RGRG,
+		.cfa_baycfg	= ISC_BAY_CFG_BGBG,
 	},
 	{
 		.fourcc		= V4L2_PIX_FMT_SGBRG10,

-- 
2.34.1

[PATCH v5 02/12] media: microchip-isc: fix WB offset and gain register field masking

[Balakrishnan Sambath]

ISC_WB_O_* and ISC_WB_G_* pack two 13-bit fields per register. Sign
extension from negative offsets corrupts the upper field. Mask both
fields to 13 bits before packing.

Fixes: 91b4e487b0c6 ("media: microchip: add ISC driver as Microchip ISC")
Cc: stable@vger.kernel.org
Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 .../media/platform/microchip/microchip-isc-base.c   | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index a7cdc743fda7..45b94f1e89d8 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -61,18 +61,23 @@ static inline void isc_update_awb_ctrls(struct isc_device *isc)
 
 	/* In here we set our actual hw pipeline config */
 
+	/*
+	 * Mask offset fields to 13 bits. Sign extension of negative s32
+	 * values would otherwise corrupt the adjacent field.
+	 */
 	regmap_write(isc->regmap, ISC_WB_O_RGR,
-		     ((ctrls->offset[ISC_HIS_CFG_MODE_R])) |
-		     ((ctrls->offset[ISC_HIS_CFG_MODE_GR]) << 16));
+		     ((u32)ctrls->offset[ISC_HIS_CFG_MODE_R] & GENMASK(12, 0)) |
+		     (((u32)ctrls->offset[ISC_HIS_CFG_MODE_GR] & GENMASK(12, 0)) << 16));
 	regmap_write(isc->regmap, ISC_WB_O_BGB,
-		     ((ctrls->offset[ISC_HIS_CFG_MODE_B])) |
-		     ((ctrls->offset[ISC_HIS_CFG_MODE_GB]) << 16));
+		     ((u32)ctrls->offset[ISC_HIS_CFG_MODE_B] & GENMASK(12, 0)) |
+		     (((u32)ctrls->offset[ISC_HIS_CFG_MODE_GB] & GENMASK(12, 0)) << 16));
+	/* Gains are 13-bit unsigned fields [12:0] and [28:16] */
 	regmap_write(isc->regmap, ISC_WB_G_RGR,
-		     ctrls->gain[ISC_HIS_CFG_MODE_R] |
-		     (ctrls->gain[ISC_HIS_CFG_MODE_GR] << 16));
+		     (ctrls->gain[ISC_HIS_CFG_MODE_R] & GENMASK(12, 0)) |
+		     ((ctrls->gain[ISC_HIS_CFG_MODE_GR] & GENMASK(12, 0)) << 16));
 	regmap_write(isc->regmap, ISC_WB_G_BGB,
-		     ctrls->gain[ISC_HIS_CFG_MODE_B] |
-		     (ctrls->gain[ISC_HIS_CFG_MODE_GB] << 16));
+		     (ctrls->gain[ISC_HIS_CFG_MODE_B] & GENMASK(12, 0)) |
+		     ((ctrls->gain[ISC_HIS_CFG_MODE_GB] & GENMASK(12, 0)) << 16));
 }
 
 static inline void isc_reset_awb_ctrls(struct isc_device *isc)

-- 
2.34.1

[PATCH v5 03/12] media: microchip-isc: fix race condition on stream stop

[Balakrishnan Sambath]

Disable histogram and drain AWB work queue before releasing DMA
buffers to prevent use-after-free if histogram IRQ fires during
stream stop.

Fixes: 91b4e487b0c6 ("media: microchip: add ISC driver as Microchip ISC")
Cc: stable@vger.kernel.org
Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 drivers/media/platform/microchip/microchip-isc-base.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index 45b94f1e89d8..b19c5a63b4bd 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -427,6 +427,14 @@ static void isc_stop_streaming(struct vb2_queue *vq)
 
 	mutex_unlock(&isc->awb_mutex);
 
+	/*
+	 * Disable the histogram so the ISR stops firing HISREQ, then drain
+	 * any work that was already queued before returning.  This must happen
+	 * after releasing awb_mutex because isc_awb_work also takes it.
+	 */
+	isc_set_histogram(isc, false);
+	cancel_work_sync(&isc->awb_work);
+
 	/* Disable DMA interrupt */
 	regmap_write(isc->regmap, ISC_INTDIS, ISC_INT_DDONE);
 
@@ -1519,10 +1527,17 @@ static int isc_s_awb_ctrl(struct v4l2_ctrl *ctrl)
 		}
 		mutex_unlock(&isc->awb_mutex);
 
-		/* if we have autowhitebalance on, start histogram procedure */
+		/*
+		 * If AWB auto mode is requested and we are streaming RAW,
+		 * start the histogram procedure, but only if it is not
+		 * already running. Repeated enable requests would reset
+		 * hist_id, preventing the 4-channel Bayer cycle from
+		 * completing.
+		 */
 		if (ctrls->awb == ISC_WB_AUTO &&
 		    vb2_is_streaming(&isc->vb2_vidq) &&
-		    ISC_IS_FORMAT_RAW(isc->config.sd_format->mbus_code))
+		    ISC_IS_FORMAT_RAW(isc->config.sd_format->mbus_code) &&
+		    ctrls->hist_stat != HIST_ENABLED)
 			isc_set_histogram(isc, true);
 
 		/*

-- 
2.34.1

[PATCH v5 04/12] media: microchip-isc: fix PM runtime leak in AWB work handler

[Balakrishnan Sambath]

Early return when streaming stops skips pm_runtime_put_sync(),
leaking the reference and preventing runtime suspend.

Fixes: 91b4e487b0c6 ("media: microchip: add ISC driver as Microchip ISC")
Cc: stable@vger.kernel.org
Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
Reviewed-by: Eugen Hristev <ehristev@kernel.org>
---
 drivers/media/platform/microchip/microchip-isc-base.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index b19c5a63b4bd..f61a5d5a3e04 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -1429,7 +1429,7 @@ static void isc_awb_work(struct work_struct *w)
 	/* streaming is not active anymore */
 	if (isc->stop) {
 		mutex_unlock(&isc->awb_mutex);
-		return;
+		goto out_pm_put;
 	}
 
 	isc_update_profile(isc);
@@ -1440,6 +1440,7 @@ static void isc_awb_work(struct work_struct *w)
 	if (ctrls->awb)
 		regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_HISREQ);
 
+out_pm_put:
 	pm_runtime_put_sync(isc->dev);
 }
 

-- 
2.34.1

[PATCH v5 05/12] media: microchip-isc: add driver documentation

[Balakrishnan Sambath]

Document the driver topology, supported formats, controls, the AWB
algorithm, the gamma table layout, and the Microchip-specific custom
controls exposed via atmel-isc-media.h.

Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 .../userspace-api/media/drivers/index.rst          |  1 +
 .../userspace-api/media/drivers/microchip-isc.rst  | 68 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 3 files changed, 70 insertions(+)

diff --git a/Documentation/userspace-api/media/drivers/index.rst b/Documentation/userspace-api/media/drivers/index.rst
index 02967c9b18d6..65ef6ba3523e 100644
--- a/Documentation/userspace-api/media/drivers/index.rst
+++ b/Documentation/userspace-api/media/drivers/index.rst
@@ -34,6 +34,7 @@ For more details see the file COPYING in the source distribution of Linux.
 	imx-uapi
 	mali-c55
 	max2175
+	microchip-isc
 	npcm-video
 	omap3isp-uapi
 	thp7312
diff --git a/Documentation/userspace-api/media/drivers/microchip-isc.rst b/Documentation/userspace-api/media/drivers/microchip-isc.rst
new file mode 100644
index 000000000000..7272ccec0169
--- /dev/null
+++ b/Documentation/userspace-api/media/drivers/microchip-isc.rst
@@ -0,0 +1,68 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Microchip ISC/XISC Driver
+=========================
+
+The Image Sensor Controller (ISC) on SAMA5D2 and eXtended ISC (XISC) on
+SAMA7G5/SAM9X7 provide camera capture with hardware image processing.
+
+Supported Hardware
+------------------
+
+==========  ==========  ==============  ================  ===============
+SoC         Controller  Max Resolution  Interface         Hue/Saturation
+==========  ==========  ==============  ================  ===============
+SAMA5D2     ISC         2592x1944       12-bit parallel   No
+SAMA7G5     XISC        3264x2464       12-bit + CSI-2    Yes
+SAM9X7      XISC        2560x1920       12-bit + CSI-2    Yes
+==========  ==========  ==============  ================  ===============
+
+SAM9X7 shares the XISC pipeline with SAMA7G5 but has a smaller internal
+line buffer, limiting horizontal resolution to 2560 pixels.
+
+Controls
+--------
+
+Standard V4L2 controls:
+
+* ``V4L2_CID_BRIGHTNESS``: -1024..1023, default 0
+* ``V4L2_CID_CONTRAST``: -2048..2047, default 256 (1.0x)
+* ``V4L2_CID_GAMMA``: 0..2 selects a preset curve. Indices differ
+  per SoC: SAMA7G5/SAM9X7 use 0=1/2.4, 1=1/2.2 (default), 2=1/1.8;
+  SAMA5D2 uses 0=1/1.8, 1=1/2.0, 2=1/2.2 (default).
+* ``V4L2_CID_AUTO_WHITE_BALANCE``: Enable kernel Grey World AWB
+* ``V4L2_CID_DO_WHITE_BALANCE``: Trigger one-shot AWB
+
+SAMA7G5/SAM9X7 add:
+
+* ``V4L2_CID_HUE``: -180..180 degrees
+* ``V4L2_CID_SATURATION``: 0..127, default 16 (Q4 fixed-point, 16 = 1.0x)
+
+Custom controls (defined in ``atmel-isc-media.h``):
+
+* ``ISC_CID_R_GAIN``, ``ISC_CID_B_GAIN``, ``ISC_CID_GR_GAIN``,
+  ``ISC_CID_GB_GAIN``: WB gains, 0..8191, Q2.9 (512 = 1.0x)
+* ``ISC_CID_R_OFFSET``, ``ISC_CID_B_OFFSET``, ``ISC_CID_GR_OFFSET``,
+  ``ISC_CID_GB_OFFSET``: WB offsets, -4096..4095
+
+Pipeline
+--------
+
+Pipeline modules: DPC -> WB -> CFA -> CC -> GAM -> CBHS/CBC -> CSC -> SUB
+
+* DPC: Defective Pixel Correction (XISC only), black level subtraction
+  to sensor bit depth, green disparity correction
+* WB: White Balance gains/offsets
+* CFA: Color Filter Array interpolation (demosaic)
+* CC: Color Correction matrix
+* GAM: Gamma correction (preset)
+* CBHS: Contrast/Brightness/Hue/Saturation (XISC only)
+* CBC: Contrast/Brightness (ISC only)
+* CSC: Color Space Conversion (RGB to YCbCr)
+* SUB: Chroma subsampling (4:2:2, 4:2:0)
+
+Pipeline usage depends on input and output formats:
+
+* Raw Bayer input, RGB output: DPC, WB, CFA, CC, GAM
+* Raw Bayer input, YUV output: Full pipeline including CSC, CBHS/CBC, SUB
+* Non-RAW input (YUV/RGB sensor): Pipeline bypassed
diff --git a/MAINTAINERS b/MAINTAINERS
index e08767323763..d4aa7e86e2bd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17057,6 +17057,7 @@ L:	linux-media@vger.kernel.org
 S:	Supported
 F:	Documentation/devicetree/bindings/media/atmel,isc.yaml
 F:	Documentation/devicetree/bindings/media/microchip,xisc.yaml
+F:	Documentation/userspace-api/media/drivers/microchip-isc.rst
 F:	drivers/media/platform/microchip/microchip-isc*
 F:	drivers/media/platform/microchip/microchip-sama*-isc*
 F:	drivers/staging/media/deprecated/atmel/atmel-isc*

-- 
2.34.1

[PATCH v5 06/12] media: microchip-isc: set SAM9X7 maximum resolution to 2560x1920

[Balakrishnan Sambath]

SAM9X7 XISC uses the same image processing pipeline as SAMA7G5 but has
a smaller internal line buffer. The reduced RAM constrains the maximum
horizontal resolution to 2560 pixels (compared to 3264 on SAMA7G5),
resulting in a maximum capture resolution of 2560x1920.

Co-developed-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 drivers/media/platform/microchip/microchip-sama7g5-isc.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/microchip/microchip-sama7g5-isc.c b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
index ca23e8adecbd..4119cfe12cdf 100644
--- a/drivers/media/platform/microchip/microchip-sama7g5-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
@@ -55,6 +55,9 @@
 #define ISC_SAMA7G5_MAX_SUPPORT_WIDTH   3264
 #define ISC_SAMA7G5_MAX_SUPPORT_HEIGHT  2464
 
+#define ISC_SAM9X7_MAX_SUPPORT_WIDTH    2560
+#define ISC_SAM9X7_MAX_SUPPORT_HEIGHT   1920
+
 #define ISC_SAMA7G5_PIPELINE \
 	(WB_ENABLE | CFA_ENABLE | CC_ENABLE | GAM_ENABLES | CSC_ENABLE | \
 	CBC_ENABLE | SUB422_ENABLE | SUB420_ENABLE)
@@ -432,8 +435,13 @@ static int microchip_xisc_probe(struct platform_device *pdev)
 	isc->gamma_table = isc_sama7g5_gamma_table;
 	isc->gamma_max = 0;
 
-	isc->max_width = ISC_SAMA7G5_MAX_SUPPORT_WIDTH;
-	isc->max_height = ISC_SAMA7G5_MAX_SUPPORT_HEIGHT;
+	if (of_machine_is_compatible("microchip,sam9x7")) {
+		isc->max_width = ISC_SAM9X7_MAX_SUPPORT_WIDTH;
+		isc->max_height = ISC_SAM9X7_MAX_SUPPORT_HEIGHT;
+	} else {
+		isc->max_width = ISC_SAMA7G5_MAX_SUPPORT_WIDTH;
+		isc->max_height = ISC_SAMA7G5_MAX_SUPPORT_HEIGHT;
+	}
 
 	isc->config_dpc = isc_sama7g5_config_dpc;
 	isc->config_csc = isc_sama7g5_config_csc;

-- 
2.34.1

[PATCH v5 07/12] media: microchip-isc: configure DPC and pipeline for SAMA7G5

[Balakrishnan Sambath]

Enable DPC black level and green disparity correction for raw Bayer
to RGB conversion. Bypass the pipeline for raw Bayer output so
software ISPs (libcamera) receive unmodified sensor data.

Rename CBC_ENABLE to CBHS_ENABLE to match the SAMA7G5 block name;
SAMA5D2's CBC sub-block uses the same bitmap bit.

Co-developed-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 drivers/media/platform/microchip/microchip-isc-base.c    | 15 ++++++---------
 drivers/media/platform/microchip/microchip-isc.h         |  2 +-
 drivers/media/platform/microchip/microchip-sama5d2-isc.c |  2 +-
 drivers/media/platform/microchip/microchip-sama7g5-isc.c |  5 +++--
 4 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index f61a5d5a3e04..ff920019fe37 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -800,7 +800,7 @@ static int isc_try_configure_pipeline(struct isc_device *isc)
 		if (ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code)) {
 			isc->try_config.bits_pipeline = CFA_ENABLE |
 				WB_ENABLE | GAM_ENABLES | DPC_BLCENABLE |
-				CC_ENABLE;
+				DPC_GDCENABLE | CC_ENABLE;
 		} else {
 			isc->try_config.bits_pipeline = 0x0;
 		}
@@ -810,7 +810,7 @@ static int isc_try_configure_pipeline(struct isc_device *isc)
 		if (ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code)) {
 			isc->try_config.bits_pipeline = CFA_ENABLE |
 				CSC_ENABLE | GAM_ENABLES | WB_ENABLE |
-				SUB420_ENABLE | SUB422_ENABLE | CBC_ENABLE |
+				SUB420_ENABLE | SUB422_ENABLE | CBHS_ENABLE |
 				DPC_BLCENABLE;
 		} else {
 			isc->try_config.bits_pipeline = 0x0;
@@ -821,7 +821,7 @@ static int isc_try_configure_pipeline(struct isc_device *isc)
 		if (ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code)) {
 			isc->try_config.bits_pipeline = CFA_ENABLE |
 				CSC_ENABLE | WB_ENABLE | GAM_ENABLES |
-				SUB422_ENABLE | CBC_ENABLE | DPC_BLCENABLE;
+				SUB422_ENABLE | CBHS_ENABLE | DPC_BLCENABLE;
 		} else {
 			isc->try_config.bits_pipeline = 0x0;
 		}
@@ -833,7 +833,7 @@ static int isc_try_configure_pipeline(struct isc_device *isc)
 		if (ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code)) {
 			isc->try_config.bits_pipeline = CFA_ENABLE |
 				CSC_ENABLE | WB_ENABLE | GAM_ENABLES |
-				SUB422_ENABLE | CBC_ENABLE | DPC_BLCENABLE;
+				SUB422_ENABLE | CBHS_ENABLE | DPC_BLCENABLE;
 		} else {
 			isc->try_config.bits_pipeline = 0x0;
 		}
@@ -844,16 +844,13 @@ static int isc_try_configure_pipeline(struct isc_device *isc)
 		if (ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code)) {
 			isc->try_config.bits_pipeline = CFA_ENABLE |
 				CSC_ENABLE | WB_ENABLE | GAM_ENABLES |
-				CBC_ENABLE | DPC_BLCENABLE;
+				CBHS_ENABLE | DPC_BLCENABLE;
 		} else {
 			isc->try_config.bits_pipeline = 0x0;
 		}
 		break;
 	default:
-		if (ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code))
-			isc->try_config.bits_pipeline = WB_ENABLE | DPC_BLCENABLE;
-		else
-			isc->try_config.bits_pipeline = 0x0;
+		isc->try_config.bits_pipeline = 0x0;
 	}
 
 	/* Tune the pipeline to product specific */
diff --git a/drivers/media/platform/microchip/microchip-isc.h b/drivers/media/platform/microchip/microchip-isc.h
index ad4e98a1dd8f..a943b072f6be 100644
--- a/drivers/media/platform/microchip/microchip-isc.h
+++ b/drivers/media/platform/microchip/microchip-isc.h
@@ -88,7 +88,7 @@ struct isc_format {
 #define GAM_RENABLE	BIT(9)
 #define VHXS_ENABLE	BIT(10)
 #define CSC_ENABLE	BIT(11)
-#define CBC_ENABLE	BIT(12)
+#define CBHS_ENABLE	BIT(12)
 #define SUB422_ENABLE	BIT(13)
 #define SUB420_ENABLE	BIT(14)
 
diff --git a/drivers/media/platform/microchip/microchip-sama5d2-isc.c b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
index 0ddff1e7b0a0..71609a93358a 100644
--- a/drivers/media/platform/microchip/microchip-sama5d2-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
@@ -54,7 +54,7 @@
 
 #define ISC_SAMA5D2_PIPELINE \
 	(WB_ENABLE | CFA_ENABLE | CC_ENABLE | GAM_ENABLES | CSC_ENABLE | \
-	CBC_ENABLE | SUB422_ENABLE | SUB420_ENABLE)
+	CBHS_ENABLE | SUB422_ENABLE | SUB420_ENABLE)
 
 /* This is a list of the formats that the ISC can *output* */
 static const struct isc_format sama5d2_controller_formats[] = {
diff --git a/drivers/media/platform/microchip/microchip-sama7g5-isc.c b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
index 4119cfe12cdf..daa3978a89bd 100644
--- a/drivers/media/platform/microchip/microchip-sama7g5-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
@@ -59,8 +59,9 @@
 #define ISC_SAM9X7_MAX_SUPPORT_HEIGHT   1920
 
 #define ISC_SAMA7G5_PIPELINE \
-	(WB_ENABLE | CFA_ENABLE | CC_ENABLE | GAM_ENABLES | CSC_ENABLE | \
-	CBC_ENABLE | SUB422_ENABLE | SUB420_ENABLE)
+	(DPC_DPCENABLE | DPC_GDCENABLE | DPC_BLCENABLE | \
+	 WB_ENABLE | CFA_ENABLE | CC_ENABLE | GAM_ENABLES | CSC_ENABLE | \
+	 CBHS_ENABLE | SUB422_ENABLE | SUB420_ENABLE)
 
 /* This is a list of the formats that the ISC can *output* */
 static const struct isc_format sama7g5_controller_formats[] = {

-- 
2.34.1

[PATCH v5 08/12] media: microchip-isc: add gamma 1.8 and 2.4 correction curves

[Balakrishnan Sambath]

Display profiles for older macOS content (gamma 1.8) and HDR
pipelines (gamma 2.4) need curves not covered by the existing sRGB
2.2 default. Add the two extra curves to the SAMA7G5 table so
userspace can pick a curve matching the target display profile.

The two SoCs put gamma 1/2.2 at different indices in their tables
(SAMA5D2 at index 2, SAMA7G5 at index 1), so introduce a
gamma_default field on struct isc_device and let each platform set
it. The SAMA5D2 default of index 2 matches the historical behaviour.

Co-developed-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 .../media/platform/microchip/microchip-isc-base.c  |  2 +-
 drivers/media/platform/microchip/microchip-isc.h   |  1 +
 .../platform/microchip/microchip-sama5d2-isc.c     |  2 +
 .../platform/microchip/microchip-sama7g5-isc.c     | 56 ++++++++++++++++------
 4 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index ff920019fe37..04187127070d 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -1648,7 +1648,7 @@ static int isc_ctrl_init(struct isc_device *isc)
 
 	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_BRIGHTNESS, -1024, 1023, 1, 0);
 	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_GAMMA, 0, isc->gamma_max, 1,
-			  isc->gamma_max);
+			  isc->gamma_default);
 	isc->awb_ctrl = v4l2_ctrl_new_std(hdl, &isc_awb_ops,
 					  V4L2_CID_AUTO_WHITE_BALANCE,
 					  0, 1, 1, 1);
diff --git a/drivers/media/platform/microchip/microchip-isc.h b/drivers/media/platform/microchip/microchip-isc.h
index a943b072f6be..2282ef7dd596 100644
--- a/drivers/media/platform/microchip/microchip-isc.h
+++ b/drivers/media/platform/microchip/microchip-isc.h
@@ -342,6 +342,7 @@ struct isc_device {
 	/* pointer to the defined gamma table */
 	const u32	(*gamma_table)[GAMMA_ENTRIES];
 	u32		gamma_max;
+	u32		gamma_default;
 
 	u32		max_width;
 	u32		max_height;
diff --git a/drivers/media/platform/microchip/microchip-sama5d2-isc.c b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
index 71609a93358a..9fa8413c74c7 100644
--- a/drivers/media/platform/microchip/microchip-sama5d2-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama5d2-isc.c
@@ -442,6 +442,8 @@ static int microchip_isc_probe(struct platform_device *pdev)
 
 	isc->gamma_table = isc_sama5d2_gamma_table;
 	isc->gamma_max = 2;
+	/* Index 2 in the SAMA5D2 table is gamma 1/2.2 (sRGB). */
+	isc->gamma_default = 2;
 
 	isc->max_width = ISC_SAMA5D2_MAX_SUPPORT_WIDTH;
 	isc->max_height = ISC_SAMA5D2_MAX_SUPPORT_HEIGHT;
diff --git a/drivers/media/platform/microchip/microchip-sama7g5-isc.c b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
index daa3978a89bd..06aa801b88f9 100644
--- a/drivers/media/platform/microchip/microchip-sama7g5-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
@@ -320,21 +320,47 @@ static void isc_sama7g5_adapt_pipeline(struct isc_device *isc)
 	isc->try_config.bits_pipeline &= ISC_SAMA7G5_PIPELINE;
 }
 
-/* Gamma table with gamma 1/2.2 */
+/* Gamma tables with gamma values 0.42, 0.45(Default), 0.56 */
 static const u32 isc_sama7g5_gamma_table[][GAMMA_ENTRIES] = {
-	/* index 0 --> gamma bipartite */
+	/* index 0 --> gamma bipartite 1/2.4(=0.42) */
 	{
-	      0x980,  0x4c0320,  0x650260,  0x7801e0,  0x8701a0,  0x940180,
-	   0xa00160,  0xab0120,  0xb40120,  0xbd0120,  0xc60100,  0xce0100,
-	   0xd600e0,  0xdd00e0,  0xe400e0,  0xeb00c0,  0xf100c0,  0xf700c0,
-	   0xfd00c0, 0x10300a0, 0x10800c0, 0x10e00a0, 0x11300a0, 0x11800a0,
-	  0x11d00a0, 0x12200a0, 0x12700a0, 0x12c0080, 0x13000a0, 0x1350080,
-	  0x13900a0, 0x13e0080, 0x1420076, 0x17d0062, 0x1ae0054, 0x1d8004a,
-	  0x1fd0044, 0x21f003e, 0x23e003a, 0x25b0036, 0x2760032, 0x28f0030,
-	  0x2a7002e, 0x2be002c, 0x2d4002c, 0x2ea0028, 0x2fe0028, 0x3120026,
-	  0x3250024, 0x3370024, 0x3490022, 0x35a0022, 0x36b0020, 0x37b0020,
-	  0x38b0020, 0x39b001e, 0x3aa001e, 0x3b9001c, 0x3c7001c, 0x3d5001c,
-	  0x3e3001c, 0x3f1001c, 0x3ff001a, 0x40c001a },
+	      0x940,  0x4b0310,  0x630250,  0x7601d0,  0x840190,  0x910170,
+	   0x9d0150,  0xa80110,  0xb10110,  0xba0110,  0xc300f0,  0xcb00f0,
+	   0xd300e0,  0xda00e0,  0xe100c0,  0xe800c0,  0xee00c0,  0xf400c0,
+	   0xfa00a0, 0x10000a0, 0x10500a0, 0x10b00a0, 0x11000a0, 0x11500a0,
+	  0x11a0080, 0x11f0080, 0x1240080, 0x1290080, 0x12e0080, 0x1330070,
+	  0x1380070, 0x13c0070, 0x1410070, 0x17a0060, 0x1aa0052, 0x1d40046,
+	  0x1f90042, 0x21b003c, 0x23a0038, 0x2570034, 0x2720030, 0x28b002e,
+	  0x2a3002c, 0x2ba002a, 0x2d0002a, 0x2e60028, 0x2fa0026, 0x30e0026,
+	  0x3210024, 0x3330022, 0x3450022, 0x3560020, 0x3670020, 0x3770020,
+	  0x387001e, 0x396001e, 0x3a5001c, 0x3b3001c, 0x3c1001c, 0x3cf001a,
+	  0x3dd001a, 0x3eb0018, 0x3f90018, 0x4070016 },
+	/* index 1 --> gamma bipartite 1/2.2(=0.45) */
+	{
+	     0x980,  0x4c0320,  0x650260,  0x7801e0,  0x8701a0,  0x940180,
+	  0xa00160,  0xab0120,  0xb40120,  0xbd0120,  0xc60100,  0xce0100,
+	  0xd600e0,  0xdd00e0,  0xe400e0,  0xeb00c0,  0xf100c0,  0xf700c0,
+	  0xfd00c0, 0x10300a0, 0x10800c0, 0x10e00a0, 0x11300a0, 0x11800a0,
+	 0x11d00a0, 0x12200a0, 0x12700a0, 0x12c0080, 0x13000a0, 0x1350080,
+	 0x13900a0, 0x13e0080, 0x1420076, 0x17d0062, 0x1ae0054, 0x1d8004a,
+	 0x1fd0044, 0x21f003e, 0x23e003a, 0x25b0036, 0x2760032, 0x28f0030,
+	 0x2a7002e, 0x2be002c, 0x2d4002c, 0x2ea0028, 0x2fe0028, 0x3120026,
+	 0x3250024, 0x3370024, 0x3490022, 0x35a0022, 0x36b0020, 0x37b0020,
+	 0x38b0020, 0x39b001e, 0x3aa001e, 0x3b9001c, 0x3c7001c, 0x3d5001c,
+	 0x3e3001c, 0x3f1001c, 0x3ff001a, 0x40c001a },
+	/* index 2 --> gamma bipartite 1/1.8(=0.56) */
+	{
+	      0xa62,  0x4f0350,  0x680280,  0x7e0200,  0x8d01c0,  0x9a01a0,
+	   0xa50180,  0xb00140,  0xb90140,  0xc20120,  0xcb0120,  0xd30100,
+	   0xdb0100,  0xe300e0,  0xea00e0,  0xf100e0,  0xf700c0,  0xfd00c0,
+	  0x10300c0, 0x10900a0, 0x10e00a0, 0x11400a0, 0x11900a0, 0x11e00a0,
+	  0x12300a0, 0x12800a0, 0x12d0080, 0x1320080, 0x1370080, 0x13c0080,
+	  0x1410080, 0x1460080, 0x14a0070, 0x1830060, 0x1b40052, 0x1df0048,
+	  0x2040042, 0x2250040, 0x2440038, 0x2600036, 0x27b0032, 0x2940030,
+	  0x2ac002e, 0x2c4002c, 0x2da002a, 0x2f0002a, 0x3050028, 0x3190026,
+	  0x32c0026, 0x33e0024, 0x3500024, 0x3610022, 0x3720020, 0x3820020,
+	  0x3920020, 0x3a2001e, 0x3b1001e, 0x3c0001c, 0x3ce001c, 0x3dc001c,
+	  0x3ea001a, 0x3f8001a, 0x4060018, 0x4130018 },
 };
 
 static int xisc_parse_dt(struct device *dev, struct isc_device *isc)
@@ -434,7 +460,9 @@ static int microchip_xisc_probe(struct platform_device *pdev)
 	}
 
 	isc->gamma_table = isc_sama7g5_gamma_table;
-	isc->gamma_max = 0;
+	isc->gamma_max = 2;
+	/* Index 1 in the SAMA7G5 table is gamma 1/2.2 (sRGB). */
+	isc->gamma_default = 1;
 
 	if (of_machine_is_compatible("microchip,sam9x7")) {
 		isc->max_width = ISC_SAM9X7_MAX_SUPPORT_WIDTH;

-- 
2.34.1

[PATCH v5 09/12] media: microchip-isc: add SAMA7G5 hue and saturation controls

[Balakrishnan Sambath]

The CBHS (Contrast/Brightness/Hue/Saturation) block on SAMA7G5
operates in YCbCr space; expose hue and saturation as V4L2 controls
for the YUV/RGB output paths only. The SAMA5D2 has only the CBC
block (no hue/saturation), so the controls are gated on a new
has_cbhs flag.

Saturation uses the Q4 fixed-point range 0..127 with default 16
(1.0x) directly matching the CBHS_SAT register field. The control
state is initialised to neutral at probe so the first config_cbc()
write after streaming starts does not produce a grayscale image.

Co-developed-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 .../media/platform/microchip/microchip-isc-base.c  | 75 ++++++++++++++++++++++
 .../media/platform/microchip/microchip-isc-regs.h  | 11 ++--
 drivers/media/platform/microchip/microchip-isc.h   |  3 +
 .../platform/microchip/microchip-sama7g5-isc.c     |  6 +-
 4 files changed, 88 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index 04187127070d..cb338133d03f 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -859,6 +859,56 @@ static int isc_try_configure_pipeline(struct isc_device *isc)
 	return 0;
 }
 
+static bool isc_format_has_chroma(u32 fourcc)
+{
+	switch (fourcc) {
+	case V4L2_PIX_FMT_YUV420:
+	case V4L2_PIX_FMT_YUV422P:
+	case V4L2_PIX_FMT_YUYV:
+	case V4L2_PIX_FMT_UYVY:
+	case V4L2_PIX_FMT_VYUY:
+		return true;
+	default:
+		return false;
+	}
+}
+
+/*
+ * isc_update_cbc_ctrl_activity() - Activate/deactivate CBC controls
+ *
+ * Called from isc_set_fmt(), isc_link_validate(), and isc_ctrl_init().
+ * At isc_ctrl_init() time isc->config.bits_pipeline is zero (no format
+ * has been negotiated yet), so all CBC controls are initially marked
+ * inactive.  They become active once a format that includes CBHS in the
+ * pipeline is configured via VIDIOC_S_FMT or link validation.
+ */
+static void isc_update_cbc_ctrl_activity(struct isc_device *isc)
+{
+	struct v4l2_ctrl_handler *hdl = &isc->ctrls.handler;
+	struct v4l2_ctrl *brightness;
+	struct v4l2_ctrl *contrast;
+	struct v4l2_ctrl *hue;
+	struct v4l2_ctrl *saturation;
+	bool cbc_active = isc->config.bits_pipeline & CBHS_ENABLE;
+	bool chroma_active = cbc_active && isc_format_has_chroma(isc->config.fourcc);
+
+	brightness = v4l2_ctrl_find(hdl, V4L2_CID_BRIGHTNESS);
+	if (brightness)
+		v4l2_ctrl_activate(brightness, cbc_active);
+
+	contrast = v4l2_ctrl_find(hdl, V4L2_CID_CONTRAST);
+	if (contrast)
+		v4l2_ctrl_activate(contrast, cbc_active);
+
+	hue = v4l2_ctrl_find(hdl, V4L2_CID_HUE);
+	if (hue)
+		v4l2_ctrl_activate(hue, chroma_active);
+
+	saturation = v4l2_ctrl_find(hdl, V4L2_CID_SATURATION);
+	if (saturation)
+		v4l2_ctrl_activate(saturation, chroma_active);
+}
+
 static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f)
 {
 	struct v4l2_pix_format *pixfmt = &f->fmt.pix;
@@ -902,6 +952,7 @@ static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f)
 	/* make the try configuration active */
 	isc->config = isc->try_config;
 	isc->fmt = isc->try_fmt;
+	isc_update_cbc_ctrl_activity(isc);
 
 	dev_dbg(isc->dev, "ISC set_fmt to %.4s @%dx%d\n",
 		(char *)&f->fmt.pix.pixelformat,
@@ -989,6 +1040,7 @@ static int isc_link_validate(struct media_link *link)
 		return ret;
 
 	isc->config = isc->try_config;
+	isc_update_cbc_ctrl_activity(isc);
 
 	dev_dbg(isc->dev, "New ISC configuration in place\n");
 
@@ -1457,6 +1509,14 @@ static int isc_s_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_CONTRAST:
 		ctrls->contrast = ctrl->val & ISC_CBC_CONTRAST_MASK;
 		break;
+	case V4L2_CID_HUE:
+		if (isc->has_cbhs)
+			ctrls->hue = ctrl->val & ISC_CBHS_HUE_MASK;
+		break;
+	case V4L2_CID_SATURATION:
+		if (isc->has_cbhs)
+			ctrls->saturation = ctrl->val & ISC_CBHS_SAT_MASK;
+		break;
 	case V4L2_CID_GAMMA:
 		ctrls->gamma_index = ctrl->val;
 		break;
@@ -1464,6 +1524,7 @@ static int isc_s_ctrl(struct v4l2_ctrl *ctrl)
 		return -EINVAL;
 	}
 
+	/* config_cbc() flushes ctrls to hardware at stream start. */
 	return 0;
 }
 
@@ -1647,6 +1708,19 @@ static int isc_ctrl_init(struct isc_device *isc)
 	ctrls->brightness = 0;
 
 	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_BRIGHTNESS, -1024, 1023, 1, 0);
+	if (isc->has_cbhs) {
+		/*
+		 * CBHS_HUE is a signed 9-bit value in degrees.
+		 * CBHS_SAT is Q4 unsigned 7-bit, 16 = 1.0x.
+		 * Initialize the kernel-side state to neutral here so the
+		 * first config_cbc() call after streaming starts does not
+		 * write zero (grayscale) to the hardware.
+		 */
+		ctrls->hue = 0;
+		ctrls->saturation = 16;
+		v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HUE, -180, 180, 1, 0);
+		v4l2_ctrl_new_std(hdl, ops, V4L2_CID_SATURATION, 0, 127, 1, 16);
+	}
 	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_GAMMA, 0, isc->gamma_max, 1,
 			  isc->gamma_default);
 	isc->awb_ctrl = v4l2_ctrl_new_std(hdl, &isc_awb_ops,
@@ -1665,6 +1739,7 @@ static int isc_ctrl_init(struct isc_device *isc)
 	}
 
 	v4l2_ctrl_activate(isc->do_wb_ctrl, false);
+	isc_update_cbc_ctrl_activity(isc);
 
 	isc->r_gain_ctrl = v4l2_ctrl_new_custom(hdl, &isc_r_gain_ctrl, NULL);
 	isc->b_gain_ctrl = v4l2_ctrl_new_custom(hdl, &isc_b_gain_ctrl, NULL);
diff --git a/drivers/media/platform/microchip/microchip-isc-regs.h b/drivers/media/platform/microchip/microchip-isc-regs.h
index e77e1d9a1db8..7f5c2e50e74b 100644
--- a/drivers/media/platform/microchip/microchip-isc-regs.h
+++ b/drivers/media/platform/microchip/microchip-isc-regs.h
@@ -268,10 +268,13 @@
 #define ISC_CBC_CONTRAST	0x000003c0
 #define ISC_CBC_CONTRAST_MASK	GENMASK(11, 0)
 
-/* Hue Register */
-#define ISC_CBCHS_HUE	0x4e0
-/* Saturation Register */
-#define ISC_CBCHS_SAT	0x4e4
+/* Hue Register: signed 9-bit two's complement, covers -180 to +180 degrees */
+#define ISC_CBHS_HUE		0x4e0
+#define ISC_CBHS_HUE_MASK	GENMASK(8, 0)
+
+/* Saturation Register: unsigned Q4 fixed-point (1.0 = 16, V4L2 range 0..127) */
+#define ISC_CBHS_SAT		0x4e4
+#define ISC_CBHS_SAT_MASK	GENMASK(6, 0)
 
 /* Offset for SUB422 register specific to sama5d2 product */
 #define ISC_SAMA5D2_SUB422_OFFSET	0
diff --git a/drivers/media/platform/microchip/microchip-isc.h b/drivers/media/platform/microchip/microchip-isc.h
index 2282ef7dd596..36a9c0cb241f 100644
--- a/drivers/media/platform/microchip/microchip-isc.h
+++ b/drivers/media/platform/microchip/microchip-isc.h
@@ -139,6 +139,8 @@ struct isc_ctrls {
 
 	u32 brightness;
 	u32 contrast;
+	u32 hue;
+	u32 saturation;
 	u8 gamma_index;
 #define ISC_WB_NONE	0
 #define ISC_WB_AUTO	1
@@ -343,6 +345,7 @@ struct isc_device {
 	const u32	(*gamma_table)[GAMMA_ENTRIES];
 	u32		gamma_max;
 	u32		gamma_default;
+	bool		has_cbhs;
 
 	u32		max_width;
 	u32		max_height;
diff --git a/drivers/media/platform/microchip/microchip-sama7g5-isc.c b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
index 06aa801b88f9..f51c7cac25df 100644
--- a/drivers/media/platform/microchip/microchip-sama7g5-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
@@ -257,9 +257,8 @@ static void isc_sama7g5_config_cbc(struct isc_device *isc)
 	/* Configure what is set via v4l2 ctrls */
 	regmap_write(regmap, ISC_CBC_BRIGHT + isc->offsets.cbc, isc->ctrls.brightness);
 	regmap_write(regmap, ISC_CBC_CONTRAST + isc->offsets.cbc, isc->ctrls.contrast);
-	/* Configure Hue and Saturation as neutral midpoint */
-	regmap_write(regmap, ISC_CBCHS_HUE, 0);
-	regmap_write(regmap, ISC_CBCHS_SAT, (1 << 4));
+	regmap_write(regmap, ISC_CBHS_HUE, isc->ctrls.hue);
+	regmap_write(regmap, ISC_CBHS_SAT, isc->ctrls.saturation);
 }
 
 static void isc_sama7g5_config_cc(struct isc_device *isc)
@@ -463,6 +462,7 @@ static int microchip_xisc_probe(struct platform_device *pdev)
 	isc->gamma_max = 2;
 	/* Index 1 in the SAMA7G5 table is gamma 1/2.2 (sRGB). */
 	isc->gamma_default = 1;
+	isc->has_cbhs = true;
 
 	if (of_machine_is_compatible("microchip,sam9x7")) {
 		isc->max_width = ISC_SAM9X7_MAX_SUPPORT_WIDTH;

-- 
2.34.1

[PATCH v5 10/12] media: microchip-isc: use weighted averages for Grey World AWB

[Balakrishnan Sambath]

Bright highlights dominate the unweighted pixel-count average and
bias grey-world estimates toward overexposed regions. Replace pixel
counts with intensity-weighted averages and add 2% outlier rejection
at the histogram tails so saturated highlights and the noise floor
do not dominate the gain calculation.

Also reset the new per-channel histogram statistics in
isc_reset_awb_ctrls() so a fresh streaming session does not feed
the first AWB cycle with stale values from a previous session.

Co-developed-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 .../media/platform/microchip/microchip-isc-base.c  | 180 +++++++++++++++------
 drivers/media/platform/microchip/microchip-isc.h   |   2 +
 2 files changed, 134 insertions(+), 48 deletions(-)

diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index cb338133d03f..a2719830d39b 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -39,6 +39,12 @@
 	(((mbus_code) == MEDIA_BUS_FMT_Y10_1X10) | \
 	(((mbus_code) == MEDIA_BUS_FMT_Y8_1X8)))
 
+/* 4.0 in Q9 fixed-point: cap grey-world correction at 4x. */
+#define ISC_AWB_GW_GAIN_MAX	(4u << 9)
+
+/* Outlier rejection: skip darkest/brightest 2% of histogram. */
+#define ISC_AWB_OUTLIER_DIV	50
+
 static inline void isc_update_v4l2_ctrls(struct isc_device *isc)
 {
 	struct isc_ctrls *ctrls = &isc->ctrls;
@@ -82,14 +88,24 @@ static inline void isc_update_awb_ctrls(struct isc_device *isc)
 
 static inline void isc_reset_awb_ctrls(struct isc_device *isc)
 {
+	struct isc_ctrls *ctrls = &isc->ctrls;
 	unsigned int c;
 
 	for (c = ISC_HIS_CFG_MODE_GR; c <= ISC_HIS_CFG_MODE_B; c++) {
 		/* gains have a fixed point at 9 decimals */
-		isc->ctrls.gain[c] = 1 << 9;
+		ctrls->gain[c] = 1 << 9;
 		/* offsets are in 2's complements */
-		isc->ctrls.offset[c] = 0;
+		ctrls->offset[c] = 0;
 	}
+
+	/*
+	 * Reset histogram statistics so the first AWB cycle of a new
+	 * streaming session does not feed isc_wb_update with stale
+	 * values left over from a previous session.
+	 */
+	memset(ctrls->channel_avg, 0, sizeof(ctrls->channel_avg));
+	memset(ctrls->total_pixels, 0, sizeof(ctrls->total_pixels));
+	memset(ctrls->hist_minmax, 0, sizeof(ctrls->hist_minmax));
 }
 
 static int isc_queue_setup(struct vb2_queue *vq,
@@ -1286,6 +1302,11 @@ static void isc_hist_count(struct isc_device *isc, u32 *min, u32 *max)
 	u32 *hist_count = &ctrls->hist_count[ctrls->hist_id];
 	u32 *hist_entry = &ctrls->hist_entry[0];
 	u32 i;
+	u32 total_pixels;
+	u32 dark_threshold, bright_threshold;
+	u32 cumulative;
+	u64 weighted_sum;
+	u32 pixel_count;
 
 	*min = 0;
 	*max = HIST_ENTRIES;
@@ -1293,44 +1314,98 @@ static void isc_hist_count(struct isc_device *isc, u32 *min, u32 *max)
 	regmap_bulk_read(regmap, ISC_HIS_ENTRY + isc->offsets.his_entry,
 			 hist_entry, HIST_ENTRIES);
 
-	*hist_count = 0;
-	/*
-	 * we deliberately ignore the end of the histogram,
-	 * the most white pixels
-	 */
+	/* Calculate total pixels */
+	total_pixels = 0;
+	for (i = 0; i < HIST_ENTRIES; i++)
+		total_pixels += hist_entry[i];
+
+	/* Handle empty histogram case */
+	if (total_pixels == 0) {
+		*hist_count = 0;
+		ctrls->channel_avg[ctrls->hist_id] = 256; /* Default middle value */
+		ctrls->total_pixels[ctrls->hist_id] = 0;
+		*min = 1;
+		*max = HIST_ENTRIES - 1;
+		dev_dbg(isc->dev,
+			"isc wb: no pixels in histogram for channel %u\n",
+			ctrls->hist_id);
+		return;
+	}
+
+	/* Outlier rejection: skip darkest/brightest 2% of histogram */
+	dark_threshold = total_pixels / ISC_AWB_OUTLIER_DIV;
+	bright_threshold = total_pixels / ISC_AWB_OUTLIER_DIV;
+	cumulative = 0;
+
+	/* Find effective minimum (skip dark noise) */
+	*min = 1;
 	for (i = 1; i < HIST_ENTRIES; i++) {
-		if (*hist_entry && !*min)
+		cumulative += hist_entry[i];
+		if (cumulative > dark_threshold) {
 			*min = i;
-		if (*hist_entry)
+			break;
+		}
+	}
+
+	/* Find effective maximum (skip bright saturation) */
+	cumulative = 0;
+	*max = HIST_ENTRIES - 1;
+	for (i = HIST_ENTRIES - 1; i > *min; i--) {
+		cumulative += hist_entry[i];
+		if (cumulative > bright_threshold) {
 			*max = i;
-		*hist_count += i * (*hist_entry++);
+			break;
+		}
 	}
 
-	if (!*min)
-		*min = 1;
+	/* Ensure reasonable range */
+	if (*max <= *min) {
+		*min = HIST_ENTRIES / 4;
+		*max = (HIST_ENTRIES * 3) / 4;
+	}
+
+	/* Calculate both pixel count and weighted average for useful range */
+	*hist_count = 0;
+	weighted_sum = 0;
+
+	for (i = *min; i <= *max; i++) {
+		pixel_count = hist_entry[i];
+		*hist_count += pixel_count;
+		weighted_sum += (u64)i * pixel_count;
+	}
 
-	dev_dbg(isc->dev, "isc wb: hist_id %u, hist_count %u",
-		ctrls->hist_id, *hist_count);
+	/* Store total useful pixels for this channel */
+	ctrls->total_pixels[ctrls->hist_id] = *hist_count;
+
+	if (*hist_count > 0)
+		ctrls->channel_avg[ctrls->hist_id] =
+			div64_u64(weighted_sum, *hist_count);
+	else
+		ctrls->channel_avg[ctrls->hist_id] = 256;
+
+	dev_dbg(isc->dev,
+		"isc wb: hist_id %u, avg %u, count %u, range [%u,%u], total %u\n",
+		ctrls->hist_id, ctrls->channel_avg[ctrls->hist_id],
+		*hist_count, *min, *max, total_pixels);
 }
 
 static void isc_wb_update(struct isc_ctrls *ctrls)
 {
 	struct isc_device *isc = container_of(ctrls, struct isc_device, ctrls);
-	u32 *hist_count = &ctrls->hist_count[0];
 	u32 c, offset[4];
 	u64 avg = 0;
-	/* We compute two gains, stretch gain and grey world gain */
-	u32 s_gain[4], gw_gain[4];
+	u32 gain, gw_gain, s_gain;
+	u32 min_pixels;
+	u32 frame_pixels;
 
 	/*
 	 * According to Grey World, we need to set gains for R/B to normalize
 	 * them towards the green channel.
-	 * Thus we want to keep Green as fixed and adjust only Red/Blue
-	 * Compute the average of the both green channels first
+	 * Thus we want to keep Green as fixed and adjust only Red/Blue.
+	 * Compute the average of the both green channels first.
 	 */
-	avg = (u64)hist_count[ISC_HIS_CFG_MODE_GR] +
-		(u64)hist_count[ISC_HIS_CFG_MODE_GB];
-	avg >>= 1;
+	avg = (ctrls->channel_avg[ISC_HIS_CFG_MODE_GR] +
+		ctrls->channel_avg[ISC_HIS_CFG_MODE_GB]) >> 1;
 
 	dev_dbg(isc->dev, "isc wb: green components average %llu\n", avg);
 
@@ -1338,7 +1413,23 @@ static void isc_wb_update(struct isc_ctrls *ctrls)
 	if (!avg)
 		return;
 
+	/*
+	 * Require a minimum pixel count for both black-level offset and
+	 * grey-world gain: 1/64 of the frame area, which equals ~6.25% of
+	 * one Bayer channel's expected pixel count.  This scales with sensor
+	 * resolution and prevents noise-dominated histograms (from very small
+	 * crops or a nearly-empty frame) from producing wild corrections.
+	 * A floor of 64 ensures the guard is non-zero for tiny crops.
+	 */
+	frame_pixels = isc->fmt.fmt.pix.width * isc->fmt.fmt.pix.height;
+	min_pixels = frame_pixels ? max(frame_pixels >> 6, 64u) : 64u;
+
 	for (c = ISC_HIS_CFG_MODE_GR; c <= ISC_HIS_CFG_MODE_B; c++) {
+		u32 hist_min = ctrls->hist_minmax[c][HIST_MIN_INDEX];
+		u32 hist_max = ctrls->hist_minmax[c][HIST_MAX_INDEX];
+		u32 channel_avg = ctrls->channel_avg[c];
+		u32 total_pixels = ctrls->total_pixels[c];
+
 		/*
 		 * the color offset is the minimum value of the histogram.
 		 * we stretch this color to the full range by substracting
@@ -1364,40 +1455,33 @@ static void isc_wb_update(struct isc_ctrls *ctrls)
 		ctrls->offset[c] = -ctrls->offset[c];
 
 		/*
-		 * the stretch gain is the total number of histogram bins
-		 * divided by the actual range of color component (Max - Min)
-		 * If we compute gain like this, the actual color component
-		 * will be stretched to the full histogram.
-		 * We need to shift 9 bits for precision, we have 9 bits for
-		 * decimals
+		 * Stretch gain: scale the histogram range [hist_min, hist_max]
+		 * to the full 512-bin span.  Result is in Q9 fixed-point
+		 * (1.0 = 512).
 		 */
-		s_gain[c] = (HIST_ENTRIES << 9) /
-			(ctrls->hist_minmax[c][HIST_MAX_INDEX] -
-			ctrls->hist_minmax[c][HIST_MIN_INDEX] + 1);
+		s_gain = (HIST_ENTRIES << 9) / (hist_max - hist_min + 1);
 
 		/*
-		 * Now we have to compute the gain w.r.t. the average.
-		 * Add/lose gain to the component towards the average.
-		 * If it happens that the component is zero, use the
-		 * fixed point value : 1.0 gain.
+		 * Grey-world gain: scale each channel towards the green
+		 * average.  Require a minimum pixel count so noise-dominated
+		 * channels do not produce wild corrections.
 		 */
-		if (hist_count[c])
-			gw_gain[c] = div_u64(avg << 9, hist_count[c]);
+		if (channel_avg > 0 && total_pixels >= min_pixels)
+			gw_gain = div64_u64((avg << 9), channel_avg);
 		else
-			gw_gain[c] = 1 << 9;
+			gw_gain = 1 << 9;
 
-		dev_dbg(isc->dev,
-			"isc wb: component %d, s_gain %u, gw_gain %u\n",
-			c, s_gain[c], gw_gain[c]);
-		/* multiply both gains and adjust for decimals */
-		ctrls->gain[c] = s_gain[c] * gw_gain[c];
-		ctrls->gain[c] >>= 9;
+		/* Cap grey-world correction at 4x to avoid over-amplification. */
+		gw_gain = min_t(u32, gw_gain, ISC_AWB_GW_GAIN_MAX);
 
-		/* make sure we are not out of range */
-		ctrls->gain[c] = clamp_val(ctrls->gain[c], 0, GENMASK(12, 0));
+		/* Combine stretch and grey-world gains; result stays in Q9. */
+		gain = (s_gain * gw_gain) >> 9;
 
-		dev_dbg(isc->dev, "isc wb: component %d, final gain %u\n",
-			c, ctrls->gain[c]);
+		ctrls->gain[c] = clamp_val(gain, 0, GENMASK(12, 0));
+
+		dev_dbg(isc->dev,
+			"isc wb: c=%u black=%u avg=%u s_gain=%u gw_gain=%u gain=%u",
+			c, hist_min, channel_avg, s_gain, gw_gain, gain);
 	}
 }
 
diff --git a/drivers/media/platform/microchip/microchip-isc.h b/drivers/media/platform/microchip/microchip-isc.h
index 36a9c0cb241f..45168c62e3bc 100644
--- a/drivers/media/platform/microchip/microchip-isc.h
+++ b/drivers/media/platform/microchip/microchip-isc.h
@@ -158,6 +158,8 @@ struct isc_ctrls {
 #define HIST_MIN_INDEX		0
 #define HIST_MAX_INDEX		1
 	u32 hist_minmax[HIST_BAYER][2];
+	u32 channel_avg[HIST_BAYER];
+	u32 total_pixels[HIST_BAYER];
 };
 
 #define ISC_PIPE_LINE_NODE_NUM	15

-- 
2.34.1

[PATCH v5 11/12] media: microchip-isc: smooth AWB gains with EMA filter

[Balakrishnan Sambath]

Apply exponential moving average (alpha=0.25) to reduce per-frame
flicker from sensor noise.

Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 drivers/media/platform/microchip/microchip-isc-base.c | 19 ++++++++++++++++---
 drivers/media/platform/microchip/microchip-isc.h      |  1 +
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
index a2719830d39b..d07ea2fa33c6 100644
--- a/drivers/media/platform/microchip/microchip-isc-base.c
+++ b/drivers/media/platform/microchip/microchip-isc-base.c
@@ -94,6 +94,7 @@ static inline void isc_reset_awb_ctrls(struct isc_device *isc)
 	for (c = ISC_HIS_CFG_MODE_GR; c <= ISC_HIS_CFG_MODE_B; c++) {
 		/* gains have a fixed point at 9 decimals */
 		ctrls->gain[c] = 1 << 9;
+		ctrls->gain_smooth[c] = 1 << 9;
 		/* offsets are in 2's complements */
 		ctrls->offset[c] = 0;
 	}
@@ -1477,11 +1478,23 @@ static void isc_wb_update(struct isc_ctrls *ctrls)
 		/* Combine stretch and grey-world gains; result stays in Q9. */
 		gain = (s_gain * gw_gain) >> 9;
 
-		ctrls->gain[c] = clamp_val(gain, 0, GENMASK(12, 0));
+		/*
+		 * Smooth gain updates with an exponential weighted average
+		 * to suppress per-frame flicker:
+		 *   smooth[n] = (3 * smooth[n-1] + gain) / 4
+		 * Clamp to the hardware register width to prevent unbounded
+		 * accumulation under degenerate (near-empty histogram) inputs.
+		 */
+		ctrls->gain_smooth[c] = (3 * ctrls->gain_smooth[c] + gain) / 4;
+		ctrls->gain_smooth[c] = min_t(u32, ctrls->gain_smooth[c],
+					      GENMASK(12, 0));
+
+		ctrls->gain[c] = ctrls->gain_smooth[c];
 
 		dev_dbg(isc->dev,
-			"isc wb: c=%u black=%u avg=%u s_gain=%u gw_gain=%u gain=%u",
-			c, hist_min, channel_avg, s_gain, gw_gain, gain);
+			"isc wb: c=%u black=%u avg=%u s_gain=%u gw_gain=%u gain=%u smooth=%u\n",
+			c, hist_min, channel_avg, s_gain, gw_gain, gain,
+			ctrls->gain_smooth[c]);
 	}
 }
 
diff --git a/drivers/media/platform/microchip/microchip-isc.h b/drivers/media/platform/microchip/microchip-isc.h
index 45168c62e3bc..0ae9b4e8f32d 100644
--- a/drivers/media/platform/microchip/microchip-isc.h
+++ b/drivers/media/platform/microchip/microchip-isc.h
@@ -149,6 +149,7 @@ struct isc_ctrls {
 
 	/* one for each component : GR, R, GB, B */
 	u32 gain[HIST_BAYER];
+	u32 gain_smooth[HIST_BAYER];
 	s32 offset[HIST_BAYER];
 
 	u32 hist_entry[HIST_ENTRIES];

-- 
2.34.1

[PATCH v5 12/12] media: microchip-isc: scale DPC black level to sensor bit depth

[Balakrishnan Sambath]

The DPC_BLCFG black level register expects counts in the sensor's
native bit depth. The previous fixed 10-bit value (64 counts) under-
corrects 12-bit sensors and over-corrects 8-bit ones, producing an
incorrect black point. Scale the nominal 10-bit value to match the
8/10/12-bit sensor bus width derived from pfe_cfg0_bps.

Co-developed-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
---
 .../media/platform/microchip/microchip-sama7g5-isc.c  | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/microchip/microchip-sama7g5-isc.c b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
index f51c7cac25df..067a6e1558d3 100644
--- a/drivers/media/platform/microchip/microchip-sama7g5-isc.c
+++ b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
@@ -26,6 +26,7 @@
  * HIS: Histogram module performs statistic counters on the frames
  */
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/clkdev.h>
 #include <linux/clk-provider.h>
@@ -289,9 +290,25 @@ static void isc_sama7g5_config_dpc(struct isc_device *isc)
 {
 	u32 bay_cfg = isc->config.sd_format->cfa_baycfg;
 	struct regmap *regmap = isc->regmap;
+	u32 bps, bloff;
+
+	/*
+	 * Scale the nominal 10-bit black level offset (64 counts) to the
+	 * actual sensor bus width.
+	 * ISC_PFE_CFG0_BPS encodes (12 - bit_depth) / 2 in bits[30:28]:
+	 *   BPS_EIGHT  = 4  ->  8-bit  -> bloff = 64 >> 2 = 16
+	 *   BPS_TEN    = 2  -> 10-bit  -> bloff = 64
+	 *   BPS_TWELVE = 0  -> 12-bit  -> bloff = min(64 << 2, 255) = 255
+	 * The BLOFF hardware field is 8-bit so values are clamped to 255.
+	 */
+	bps = FIELD_GET(ISC_PFE_CFG0_BPS_MASK, isc->config.sd_format->pfe_cfg0_bps);
+	if (bps >= 2)
+		bloff = 64u >> (bps - 2);
+	else
+		bloff = min(64u << (2 - bps), 255u);
 
 	regmap_update_bits(regmap, ISC_DPC_CFG, ISC_DPC_CFG_BLOFF_MASK,
-			   (64 << ISC_DPC_CFG_BLOFF_SHIFT));
+			   (bloff << ISC_DPC_CFG_BLOFF_SHIFT));
 	regmap_update_bits(regmap, ISC_DPC_CFG, ISC_DPC_CFG_BAYCFG_MASK,
 			   (bay_cfg << ISC_DPC_CFG_BAYCFG_SHIFT));
 }

-- 
2.34.1

Re: [PATCH v5 01/12] media: microchip-isc: fix SBGGR10 Bayer pattern

[Eugen Hristev]

On 5/27/26 14:07, Balakrishnan Sambath wrote:
> SBGGR10 was mapped to ISC_BAY_CFG_RGRG instead of ISC_BAY_CFG_BGBG,
> causing red/blue channel swap.
> 
> Fixes: 91b4e487b0c6 ("media: microchip: add ISC driver as Microchip ISC")
> Cc: stable@vger.kernel.org
> Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
> ---

Reviewed-by: Eugen Hristev <ehristev@kernel.org>

Re: [PATCH v5 11/12] media: microchip-isc: smooth AWB gains with EMA filter

[Eugen Hristev]

On 5/27/26 14:07, Balakrishnan Sambath wrote:
> Apply exponential moving average (alpha=0.25) to reduce per-frame
> flicker from sensor noise.
> 
> Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
> ---
>  drivers/media/platform/microchip/microchip-isc-base.c | 19 ++++++++++++++++---
>  drivers/media/platform/microchip/microchip-isc.h      |  1 +
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
> index a2719830d39b..d07ea2fa33c6 100644
> --- a/drivers/media/platform/microchip/microchip-isc-base.c
> +++ b/drivers/media/platform/microchip/microchip-isc-base.c
> @@ -94,6 +94,7 @@ static inline void isc_reset_awb_ctrls(struct isc_device *isc)
>  	for (c = ISC_HIS_CFG_MODE_GR; c <= ISC_HIS_CFG_MODE_B; c++) {
>  		/* gains have a fixed point at 9 decimals */
>  		ctrls->gain[c] = 1 << 9;
> +		ctrls->gain_smooth[c] = 1 << 9;
>  		/* offsets are in 2's complements */
>  		ctrls->offset[c] = 0;
>  	}
> @@ -1477,11 +1478,23 @@ static void isc_wb_update(struct isc_ctrls *ctrls)
>  		/* Combine stretch and grey-world gains; result stays in Q9. */
>  		gain = (s_gain * gw_gain) >> 9;
>  
> -		ctrls->gain[c] = clamp_val(gain, 0, GENMASK(12, 0));
> +		/*
> +		 * Smooth gain updates with an exponential weighted average
> +		 * to suppress per-frame flicker:
> +		 *   smooth[n] = (3 * smooth[n-1] + gain) / 4
> +		 * Clamp to the hardware register width to prevent unbounded
> +		 * accumulation under degenerate (near-empty histogram) inputs.
> +		 */
> +		ctrls->gain_smooth[c] = (3 * ctrls->gain_smooth[c] + gain) / 4;
> +		ctrls->gain_smooth[c] = min_t(u32, ctrls->gain_smooth[c],
> +					      GENMASK(12, 0));
> +
> +		ctrls->gain[c] = ctrls->gain_smooth[c];

If now 'gain' becomes 'gain_smooth' , what is the purpose of still
having 'gain' at all ?
Does it make sense to just recompute gain in the new way ?


>  
>  		dev_dbg(isc->dev,
> -			"isc wb: c=%u black=%u avg=%u s_gain=%u gw_gain=%u gain=%u",
> -			c, hist_min, channel_avg, s_gain, gw_gain, gain);
> +			"isc wb: c=%u black=%u avg=%u s_gain=%u gw_gain=%u gain=%u smooth=%u\n",
> +			c, hist_min, channel_avg, s_gain, gw_gain, gain,
> +			ctrls->gain_smooth[c]);
>  	}
>  }
>  
> diff --git a/drivers/media/platform/microchip/microchip-isc.h b/drivers/media/platform/microchip/microchip-isc.h
> index 45168c62e3bc..0ae9b4e8f32d 100644
> --- a/drivers/media/platform/microchip/microchip-isc.h
> +++ b/drivers/media/platform/microchip/microchip-isc.h
> @@ -149,6 +149,7 @@ struct isc_ctrls {
>  
>  	/* one for each component : GR, R, GB, B */
>  	u32 gain[HIST_BAYER];
> +	u32 gain_smooth[HIST_BAYER];
>  	s32 offset[HIST_BAYER];
>  
>  	u32 hist_entry[HIST_ENTRIES];
> 

Re: [PATCH v5 11/12] media: microchip-isc: smooth AWB gains with EMA filter

[Balakrishnan.S]

Hi Eugen,

On 29/05/26 1:50 am, Eugen Hristev wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 5/27/26 14:07, Balakrishnan Sambath wrote:
>> Apply exponential moving average (alpha=0.25) to reduce per-frame
>> flicker from sensor noise.
>>
>> Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
>> ---
>>   drivers/media/platform/microchip/microchip-isc-base.c | 19 ++++++++++++++++---
>>   drivers/media/platform/microchip/microchip-isc.h      |  1 +
>>   2 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
>> index a2719830d39b..d07ea2fa33c6 100644
>> --- a/drivers/media/platform/microchip/microchip-isc-base.c
>> +++ b/drivers/media/platform/microchip/microchip-isc-base.c
>> @@ -94,6 +94,7 @@ static inline void isc_reset_awb_ctrls(struct isc_device *isc)
>>        for (c = ISC_HIS_CFG_MODE_GR; c <= ISC_HIS_CFG_MODE_B; c++) {
>>                /* gains have a fixed point at 9 decimals */
>>                ctrls->gain[c] = 1 << 9;
>> +             ctrls->gain_smooth[c] = 1 << 9;
>>                /* offsets are in 2's complements */
>>                ctrls->offset[c] = 0;
>>        }
>> @@ -1477,11 +1478,23 @@ static void isc_wb_update(struct isc_ctrls *ctrls)
>>                /* Combine stretch and grey-world gains; result stays in Q9. */
>>                gain = (s_gain * gw_gain) >> 9;
>>
>> -             ctrls->gain[c] = clamp_val(gain, 0, GENMASK(12, 0));
>> +             /*
>> +              * Smooth gain updates with an exponential weighted average
>> +              * to suppress per-frame flicker:
>> +              *   smooth[n] = (3 * smooth[n-1] + gain) / 4
>> +              * Clamp to the hardware register width to prevent unbounded
>> +              * accumulation under degenerate (near-empty histogram) inputs.
>> +              */
>> +             ctrls->gain_smooth[c] = (3 * ctrls->gain_smooth[c] + gain) / 4;
>> +             ctrls->gain_smooth[c] = min_t(u32, ctrls->gain_smooth[c],
>> +                                           GENMASK(12, 0));
>> +
>> +             ctrls->gain[c] = ctrls->gain_smooth[c];
> 
> If now 'gain' becomes 'gain_smooth' , what is the purpose of still
> having 'gain' at all ?
> Does it make sense to just recompute gain in the new way ?

I had kept gain_smooth[] separate so that isc_s_awb_ctrl, which
also writes ctrls->gain[], would not touch the EMA state.

Going back through the cluster setup, v4l2_ctrl_auto_cluster()
grabs the slave gain controls while AWB is in AUTO mode. The
v4l2 framework actually rejects user writes to those slaves before they
reach isc_s_awb_ctrl which I overlooked, so the case I had in mind 
cannot happen.

Thanks for the valuable insight.

Will collapse gain_smooth[] into gain[] in v6:

     ctrls->gain[c] = (3 * ctrls->gain[c] + gain) / 4;
     ctrls->gain[c] = min_t(u32, ctrls->gain[c], GENMASK(12, 0));

Best Regards,
Balakrishnan

> 
> 
>>
>>                dev_dbg(isc->dev,
>> -                     "isc wb: c=%u black=%u avg=%u s_gain=%u gw_gain=%u gain=%u",
>> -                     c, hist_min, channel_avg, s_gain, gw_gain, gain);
>> +                     "isc wb: c=%u black=%u avg=%u s_gain=%u gw_gain=%u gain=%u smooth=%u\n",
>> +                     c, hist_min, channel_avg, s_gain, gw_gain, gain,
>> +                     ctrls->gain_smooth[c]);
>>        }
>>   }
>>
>> diff --git a/drivers/media/platform/microchip/microchip-isc.h b/drivers/media/platform/microchip/microchip-isc.h
>> index 45168c62e3bc..0ae9b4e8f32d 100644
>> --- a/drivers/media/platform/microchip/microchip-isc.h
>> +++ b/drivers/media/platform/microchip/microchip-isc.h
>> @@ -149,6 +149,7 @@ struct isc_ctrls {
>>
>>        /* one for each component : GR, R, GB, B */
>>        u32 gain[HIST_BAYER];
>> +     u32 gain_smooth[HIST_BAYER];
>>        s32 offset[HIST_BAYER];
>>
>>        u32 hist_entry[HIST_ENTRIES];
>>
> 

Re: [PATCH v5 09/12] media: microchip-isc: add SAMA7G5 hue and saturation controls

[Eugen Hristev]

On 5/27/26 14:07, Balakrishnan Sambath wrote:
> The CBHS (Contrast/Brightness/Hue/Saturation) block on SAMA7G5
> operates in YCbCr space; expose hue and saturation as V4L2 controls
> for the YUV/RGB output paths only. The SAMA5D2 has only the CBC
> block (no hue/saturation), so the controls are gated on a new
> has_cbhs flag.
> 
> Saturation uses the Q4 fixed-point range 0..127 with default 16
> (1.0x) directly matching the CBHS_SAT register field. The control
> state is initialised to neutral at probe so the first config_cbc()
> write after streaming starts does not produce a grayscale image.
> 
> Co-developed-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
> Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
> Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
> ---
>  .../media/platform/microchip/microchip-isc-base.c  | 75 ++++++++++++++++++++++
>  .../media/platform/microchip/microchip-isc-regs.h  | 11 ++--
>  drivers/media/platform/microchip/microchip-isc.h   |  3 +
>  .../platform/microchip/microchip-sama7g5-isc.c     |  6 +-
>  4 files changed, 88 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
> index 04187127070d..cb338133d03f 100644
> --- a/drivers/media/platform/microchip/microchip-isc-base.c
> +++ b/drivers/media/platform/microchip/microchip-isc-base.c
> @@ -859,6 +859,56 @@ static int isc_try_configure_pipeline(struct isc_device *isc)
>  	return 0;
>  }
>  
> +static bool isc_format_has_chroma(u32 fourcc)
> +{
> +	switch (fourcc) {
> +	case V4L2_PIX_FMT_YUV420:
> +	case V4L2_PIX_FMT_YUV422P:
> +	case V4L2_PIX_FMT_YUYV:
> +	case V4L2_PIX_FMT_UYVY:
> +	case V4L2_PIX_FMT_VYUY:
> +		return true;
RGB565 doesn't have chroma for example ?

> +	default:
> +		return false;
> +	}
> +}
> +
> +/*
> + * isc_update_cbc_ctrl_activity() - Activate/deactivate CBC controls

What is the purpose of 'activity' in the name ? is a bit misleading for me.
> + *
> + * Called from isc_set_fmt(), isc_link_validate(), and isc_ctrl_init().
> + * At isc_ctrl_init() time isc->config.bits_pipeline is zero (no format
> + * has been negotiated yet), so all CBC controls are initially marked
> + * inactive.  They become active once a format that includes CBHS in the
> + * pipeline is configured via VIDIOC_S_FMT or link validation.
> + */
> +static void isc_update_cbc_ctrl_activity(struct isc_device *isc)
> +{
> +	struct v4l2_ctrl_handler *hdl = &isc->ctrls.handler;
> +	struct v4l2_ctrl *brightness;
> +	struct v4l2_ctrl *contrast;
> +	struct v4l2_ctrl *hue;
> +	struct v4l2_ctrl *saturation;
> +	bool cbc_active = isc->config.bits_pipeline & CBHS_ENABLE;
> +	bool chroma_active = cbc_active && isc_format_has_chroma(isc->config.fourcc);
> +
> +	brightness = v4l2_ctrl_find(hdl, V4L2_CID_BRIGHTNESS);
> +	if (brightness)
> +		v4l2_ctrl_activate(brightness, cbc_active);
> +
> +	contrast = v4l2_ctrl_find(hdl, V4L2_CID_CONTRAST);
> +	if (contrast)
> +		v4l2_ctrl_activate(contrast, cbc_active);
> +
> +	hue = v4l2_ctrl_find(hdl, V4L2_CID_HUE);
> +	if (hue)
> +		v4l2_ctrl_activate(hue, chroma_active);
> +
> +	saturation = v4l2_ctrl_find(hdl, V4L2_CID_SATURATION);
> +	if (saturation)
> +		v4l2_ctrl_activate(saturation, chroma_active);

Usage of v4l2_ctrl_find is a bit odd, don't you have the ctrls already
in the isc struct ? e.g. isc->ctrls.brightness, etc.

> +}
> +
>  static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f)
>  {
>  	struct v4l2_pix_format *pixfmt = &f->fmt.pix;
> @@ -902,6 +952,7 @@ static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f)
>  	/* make the try configuration active */
>  	isc->config = isc->try_config;
>  	isc->fmt = isc->try_fmt;
> +	isc_update_cbc_ctrl_activity(isc);
>  
>  	dev_dbg(isc->dev, "ISC set_fmt to %.4s @%dx%d\n",
>  		(char *)&f->fmt.pix.pixelformat,
> @@ -989,6 +1040,7 @@ static int isc_link_validate(struct media_link *link)
>  		return ret;
>  
>  	isc->config = isc->try_config;
> +	isc_update_cbc_ctrl_activity(isc);
>  
>  	dev_dbg(isc->dev, "New ISC configuration in place\n");
>  
> @@ -1457,6 +1509,14 @@ static int isc_s_ctrl(struct v4l2_ctrl *ctrl)
>  	case V4L2_CID_CONTRAST:
>  		ctrls->contrast = ctrl->val & ISC_CBC_CONTRAST_MASK;
>  		break;
> +	case V4L2_CID_HUE:
> +		if (isc->has_cbhs)
> +			ctrls->hue = ctrl->val & ISC_CBHS_HUE_MASK;
> +		break;
> +	case V4L2_CID_SATURATION:
> +		if (isc->has_cbhs)
> +			ctrls->saturation = ctrl->val & ISC_CBHS_SAT_MASK;
> +		break;
>  	case V4L2_CID_GAMMA:
>  		ctrls->gamma_index = ctrl->val;
>  		break;
> @@ -1464,6 +1524,7 @@ static int isc_s_ctrl(struct v4l2_ctrl *ctrl)
>  		return -EINVAL;
>  	}
>  
> +	/* config_cbc() flushes ctrls to hardware at stream start. */

What's the meaning of this comment here . s_ctrl configures multiple
controls, including gamma, etc, so why mention config_cbc() ?

>  	return 0;
>  }
>  
> @@ -1647,6 +1708,19 @@ static int isc_ctrl_init(struct isc_device *isc)
>  	ctrls->brightness = 0;
>  
>  	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_BRIGHTNESS, -1024, 1023, 1, 0);
> +	if (isc->has_cbhs) {
> +		/*
> +		 * CBHS_HUE is a signed 9-bit value in degrees.
> +		 * CBHS_SAT is Q4 unsigned 7-bit, 16 = 1.0x.
> +		 * Initialize the kernel-side state to neutral here so the
> +		 * first config_cbc() call after streaming starts does not
> +		 * write zero (grayscale) to the hardware.
> +		 */
> +		ctrls->hue = 0;
> +		ctrls->saturation = 16;
> +		v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HUE, -180, 180, 1, 0);
> +		v4l2_ctrl_new_std(hdl, ops, V4L2_CID_SATURATION, 0, 127, 1, 16);
> +	}
>  	v4l2_ctrl_new_std(hdl, ops, V4L2_CID_GAMMA, 0, isc->gamma_max, 1,
>  			  isc->gamma_default);
>  	isc->awb_ctrl = v4l2_ctrl_new_std(hdl, &isc_awb_ops,
> @@ -1665,6 +1739,7 @@ static int isc_ctrl_init(struct isc_device *isc)
>  	}
>  
>  	v4l2_ctrl_activate(isc->do_wb_ctrl, false);
> +	isc_update_cbc_ctrl_activity(isc);
>  
>  	isc->r_gain_ctrl = v4l2_ctrl_new_custom(hdl, &isc_r_gain_ctrl, NULL);
>  	isc->b_gain_ctrl = v4l2_ctrl_new_custom(hdl, &isc_b_gain_ctrl, NULL);
> diff --git a/drivers/media/platform/microchip/microchip-isc-regs.h b/drivers/media/platform/microchip/microchip-isc-regs.h
> index e77e1d9a1db8..7f5c2e50e74b 100644
> --- a/drivers/media/platform/microchip/microchip-isc-regs.h
> +++ b/drivers/media/platform/microchip/microchip-isc-regs.h
> @@ -268,10 +268,13 @@
>  #define ISC_CBC_CONTRAST	0x000003c0
>  #define ISC_CBC_CONTRAST_MASK	GENMASK(11, 0)
>  
> -/* Hue Register */
> -#define ISC_CBCHS_HUE	0x4e0
> -/* Saturation Register */
> -#define ISC_CBCHS_SAT	0x4e4
> +/* Hue Register: signed 9-bit two's complement, covers -180 to +180 degrees */
> +#define ISC_CBHS_HUE		0x4e0
> +#define ISC_CBHS_HUE_MASK	GENMASK(8, 0)
> +
> +/* Saturation Register: unsigned Q4 fixed-point (1.0 = 16, V4L2 range 0..127) */
> +#define ISC_CBHS_SAT		0x4e4
> +#define ISC_CBHS_SAT_MASK	GENMASK(6, 0)
>  
>  /* Offset for SUB422 register specific to sama5d2 product */
>  #define ISC_SAMA5D2_SUB422_OFFSET	0
> diff --git a/drivers/media/platform/microchip/microchip-isc.h b/drivers/media/platform/microchip/microchip-isc.h
> index 2282ef7dd596..36a9c0cb241f 100644
> --- a/drivers/media/platform/microchip/microchip-isc.h
> +++ b/drivers/media/platform/microchip/microchip-isc.h
> @@ -139,6 +139,8 @@ struct isc_ctrls {
>  
>  	u32 brightness;
>  	u32 contrast;
> +	u32 hue;
> +	u32 saturation;
>  	u8 gamma_index;
>  #define ISC_WB_NONE	0
>  #define ISC_WB_AUTO	1
> @@ -343,6 +345,7 @@ struct isc_device {
>  	const u32	(*gamma_table)[GAMMA_ENTRIES];
>  	u32		gamma_max;
>  	u32		gamma_default;
> +	bool		has_cbhs;
>  
>  	u32		max_width;
>  	u32		max_height;
> diff --git a/drivers/media/platform/microchip/microchip-sama7g5-isc.c b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
> index 06aa801b88f9..f51c7cac25df 100644
> --- a/drivers/media/platform/microchip/microchip-sama7g5-isc.c
> +++ b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
> @@ -257,9 +257,8 @@ static void isc_sama7g5_config_cbc(struct isc_device *isc)
>  	/* Configure what is set via v4l2 ctrls */
>  	regmap_write(regmap, ISC_CBC_BRIGHT + isc->offsets.cbc, isc->ctrls.brightness);
>  	regmap_write(regmap, ISC_CBC_CONTRAST + isc->offsets.cbc, isc->ctrls.contrast);
> -	/* Configure Hue and Saturation as neutral midpoint */
> -	regmap_write(regmap, ISC_CBCHS_HUE, 0);
> -	regmap_write(regmap, ISC_CBCHS_SAT, (1 << 4));
> +	regmap_write(regmap, ISC_CBHS_HUE, isc->ctrls.hue);
> +	regmap_write(regmap, ISC_CBHS_SAT, isc->ctrls.saturation);
>  }
>  
>  static void isc_sama7g5_config_cc(struct isc_device *isc)
> @@ -463,6 +462,7 @@ static int microchip_xisc_probe(struct platform_device *pdev)
>  	isc->gamma_max = 2;
>  	/* Index 1 in the SAMA7G5 table is gamma 1/2.2 (sRGB). */
>  	isc->gamma_default = 1;
> +	isc->has_cbhs = true;
>  
>  	if (of_machine_is_compatible("microchip,sam9x7")) {
>  		isc->max_width = ISC_SAM9X7_MAX_SUPPORT_WIDTH;
> 

Re: [PATCH v5 09/12] media: microchip-isc: add SAMA7G5 hue and saturation controls

[Balakrishnan.S]

Hi Eugen,

On 29/05/26 8:50 pm, Eugen Hristev wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 5/27/26 14:07, Balakrishnan Sambath wrote:
>> The CBHS (Contrast/Brightness/Hue/Saturation) block on SAMA7G5
>> operates in YCbCr space; expose hue and saturation as V4L2 controls
>> for the YUV/RGB output paths only. The SAMA5D2 has only the CBC
>> block (no hue/saturation), so the controls are gated on a new
>> has_cbhs flag.
>>
>> Saturation uses the Q4 fixed-point range 0..127 with default 16
>> (1.0x) directly matching the CBHS_SAT register field. The control
>> state is initialised to neutral at probe so the first config_cbc()
>> write after streaming starts does not produce a grayscale image.
>>
>> Co-developed-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
>> Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
>> Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
>> ---
>>   .../media/platform/microchip/microchip-isc-base.c  | 75 ++++++++++++++++++++++
>>   .../media/platform/microchip/microchip-isc-regs.h  | 11 ++--
>>   drivers/media/platform/microchip/microchip-isc.h   |  3 +
>>   .../platform/microchip/microchip-sama7g5-isc.c     |  6 +-
>>   4 files changed, 88 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
>> index 04187127070d..cb338133d03f 100644
>> --- a/drivers/media/platform/microchip/microchip-isc-base.c
>> +++ b/drivers/media/platform/microchip/microchip-isc-base.c
>> @@ -859,6 +859,56 @@ static int isc_try_configure_pipeline(struct isc_device *isc)
>>        return 0;
>>   }
>>
>> +static bool isc_format_has_chroma(u32 fourcc)
>> +{
>> +     switch (fourcc) {
>> +     case V4L2_PIX_FMT_YUV420:
>> +     case V4L2_PIX_FMT_YUV422P:
>> +     case V4L2_PIX_FMT_YUYV:
>> +     case V4L2_PIX_FMT_UYVY:
>> +     case V4L2_PIX_FMT_VYUY:
>> +             return true;
> RGB565 doesn't have chroma for example ?

Good point. CBHS sits after CSC in the pipeline, so it only
applies to YUV outputs. The hardware cannot do hue/saturation on
the RGB path. Filtering itself is correct, the name "has_chroma"
is misleading though. Will rename to isc_format_is_yuv() if it's
okay ?

> 
>> +     default:
>> +             return false;
>> +     }
>> +}
>> +
>> +/*
>> + * isc_update_cbc_ctrl_activity() - Activate/deactivate CBC controls
> 
> What is the purpose of 'activity' in the name ? is a bit misleading for me.

Agreed. Will rename to isc_update_cbhs_ctrls() to match the existing
isc_update_awb_ctrls() in next version.

>> + *
>> + * Called from isc_set_fmt(), isc_link_validate(), and isc_ctrl_init().
>> + * At isc_ctrl_init() time isc->config.bits_pipeline is zero (no format
>> + * has been negotiated yet), so all CBC controls are initially marked
>> + * inactive.  They become active once a format that includes CBHS in the
>> + * pipeline is configured via VIDIOC_S_FMT or link validation.
>> + */
>> +static void isc_update_cbc_ctrl_activity(struct isc_device *isc)
>> +{
>> +     struct v4l2_ctrl_handler *hdl = &isc->ctrls.handler;
>> +     struct v4l2_ctrl *brightness;
>> +     struct v4l2_ctrl *contrast;
>> +     struct v4l2_ctrl *hue;
>> +     struct v4l2_ctrl *saturation;
>> +     bool cbc_active = isc->config.bits_pipeline & CBHS_ENABLE;
>> +     bool chroma_active = cbc_active && isc_format_has_chroma(isc->config.fourcc);
>> +
>> +     brightness = v4l2_ctrl_find(hdl, V4L2_CID_BRIGHTNESS);
>> +     if (brightness)
>> +             v4l2_ctrl_activate(brightness, cbc_active);
>> +
>> +     contrast = v4l2_ctrl_find(hdl, V4L2_CID_CONTRAST);
>> +     if (contrast)
>> +             v4l2_ctrl_activate(contrast, cbc_active);
>> +
>> +     hue = v4l2_ctrl_find(hdl, V4L2_CID_HUE);
>> +     if (hue)
>> +             v4l2_ctrl_activate(hue, chroma_active);
>> +
>> +     saturation = v4l2_ctrl_find(hdl, V4L2_CID_SATURATION);
>> +     if (saturation)
>> +             v4l2_ctrl_activate(saturation, chroma_active);
> 
> Usage of v4l2_ctrl_find is a bit odd, don't you have the ctrls already
> in the isc struct ? e.g. isc->ctrls.brightness, etc.

You are right, I missed it. AWB controls already have direct
pointers, CBHS ones (brightness, contrast, hue, saturation) don't.
I went with the existing brightness/contrast pattern when adding
hue and saturation. Will add direct pointers for all four of them and 
use it here.

> 
>> +}
>> +
>>   static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f)
>>   {
>>        struct v4l2_pix_format *pixfmt = &f->fmt.pix;
>> @@ -902,6 +952,7 @@ static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f)
>>        /* make the try configuration active */
>>        isc->config = isc->try_config;
>>        isc->fmt = isc->try_fmt;
>> +     isc_update_cbc_ctrl_activity(isc);
>>
>>        dev_dbg(isc->dev, "ISC set_fmt to %.4s @%dx%d\n",
>>                (char *)&f->fmt.pix.pixelformat,
>> @@ -989,6 +1040,7 @@ static int isc_link_validate(struct media_link *link)
>>                return ret;
>>
>>        isc->config = isc->try_config;
>> +     isc_update_cbc_ctrl_activity(isc);
>>
>>        dev_dbg(isc->dev, "New ISC configuration in place\n");
>>
>> @@ -1457,6 +1509,14 @@ static int isc_s_ctrl(struct v4l2_ctrl *ctrl)
>>        case V4L2_CID_CONTRAST:
>>                ctrls->contrast = ctrl->val & ISC_CBC_CONTRAST_MASK;
>>                break;
>> +     case V4L2_CID_HUE:
>> +             if (isc->has_cbhs)
>> +                     ctrls->hue = ctrl->val & ISC_CBHS_HUE_MASK;
>> +             break;
>> +     case V4L2_CID_SATURATION:
>> +             if (isc->has_cbhs)
>> +                     ctrls->saturation = ctrl->val & ISC_CBHS_SAT_MASK;
>> +             break;
>>        case V4L2_CID_GAMMA:
>>                ctrls->gamma_index = ctrl->val;
>>                break;
>> @@ -1464,6 +1524,7 @@ static int isc_s_ctrl(struct v4l2_ctrl *ctrl)
>>                return -EINVAL;
>>        }
>>
>> +     /* config_cbc() flushes ctrls to hardware at stream start. */
> 
> What's the meaning of this comment here . s_ctrl configures multiple
> controls, including gamma, etc, so why mention config_cbc() ?

In fact looks stale for me too will fix it in next version.
> 
>>        return 0;
>>   }
>>
>> @@ -1647,6 +1708,19 @@ static int isc_ctrl_init(struct isc_device *isc)
>>        ctrls->brightness = 0;
>>
>>        v4l2_ctrl_new_std(hdl, ops, V4L2_CID_BRIGHTNESS, -1024, 1023, 1, 0);
>> +     if (isc->has_cbhs) {
>> +             /*
>> +              * CBHS_HUE is a signed 9-bit value in degrees.
>> +              * CBHS_SAT is Q4 unsigned 7-bit, 16 = 1.0x.
>> +              * Initialize the kernel-side state to neutral here so the
>> +              * first config_cbc() call after streaming starts does not
>> +              * write zero (grayscale) to the hardware.
>> +              */
>> +             ctrls->hue = 0;
>> +             ctrls->saturation = 16;
>> +             v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HUE, -180, 180, 1, 0);
>> +             v4l2_ctrl_new_std(hdl, ops, V4L2_CID_SATURATION, 0, 127, 1, 16);
>> +     }
>>        v4l2_ctrl_new_std(hdl, ops, V4L2_CID_GAMMA, 0, isc->gamma_max, 1,
>>                          isc->gamma_default);
>>        isc->awb_ctrl = v4l2_ctrl_new_std(hdl, &isc_awb_ops,
>> @@ -1665,6 +1739,7 @@ static int isc_ctrl_init(struct isc_device *isc)
>>        }
>>
>>        v4l2_ctrl_activate(isc->do_wb_ctrl, false);
>> +     isc_update_cbc_ctrl_activity(isc);
>>
>>        isc->r_gain_ctrl = v4l2_ctrl_new_custom(hdl, &isc_r_gain_ctrl, NULL);
>>        isc->b_gain_ctrl = v4l2_ctrl_new_custom(hdl, &isc_b_gain_ctrl, NULL);
>> diff --git a/drivers/media/platform/microchip/microchip-isc-regs.h b/drivers/media/platform/microchip/microchip-isc-regs.h
>> index e77e1d9a1db8..7f5c2e50e74b 100644
>> --- a/drivers/media/platform/microchip/microchip-isc-regs.h
>> +++ b/drivers/media/platform/microchip/microchip-isc-regs.h
>> @@ -268,10 +268,13 @@
>>   #define ISC_CBC_CONTRAST     0x000003c0
>>   #define ISC_CBC_CONTRAST_MASK        GENMASK(11, 0)
>>
>> -/* Hue Register */
>> -#define ISC_CBCHS_HUE        0x4e0
>> -/* Saturation Register */
>> -#define ISC_CBCHS_SAT        0x4e4
>> +/* Hue Register: signed 9-bit two's complement, covers -180 to +180 degrees */
>> +#define ISC_CBHS_HUE         0x4e0
>> +#define ISC_CBHS_HUE_MASK    GENMASK(8, 0)
>> +
>> +/* Saturation Register: unsigned Q4 fixed-point (1.0 = 16, V4L2 range 0..127) */
>> +#define ISC_CBHS_SAT         0x4e4
>> +#define ISC_CBHS_SAT_MASK    GENMASK(6, 0)
>>
>>   /* Offset for SUB422 register specific to sama5d2 product */
>>   #define ISC_SAMA5D2_SUB422_OFFSET    0
>> diff --git a/drivers/media/platform/microchip/microchip-isc.h b/drivers/media/platform/microchip/microchip-isc.h
>> index 2282ef7dd596..36a9c0cb241f 100644
>> --- a/drivers/media/platform/microchip/microchip-isc.h
>> +++ b/drivers/media/platform/microchip/microchip-isc.h
>> @@ -139,6 +139,8 @@ struct isc_ctrls {
>>
>>        u32 brightness;
>>        u32 contrast;
>> +     u32 hue;
>> +     u32 saturation;
>>        u8 gamma_index;
>>   #define ISC_WB_NONE  0
>>   #define ISC_WB_AUTO  1
>> @@ -343,6 +345,7 @@ struct isc_device {
>>        const u32       (*gamma_table)[GAMMA_ENTRIES];
>>        u32             gamma_max;
>>        u32             gamma_default;
>> +     bool            has_cbhs;
>>
>>        u32             max_width;
>>        u32             max_height;
>> diff --git a/drivers/media/platform/microchip/microchip-sama7g5-isc.c b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
>> index 06aa801b88f9..f51c7cac25df 100644
>> --- a/drivers/media/platform/microchip/microchip-sama7g5-isc.c
>> +++ b/drivers/media/platform/microchip/microchip-sama7g5-isc.c
>> @@ -257,9 +257,8 @@ static void isc_sama7g5_config_cbc(struct isc_device *isc)
>>        /* Configure what is set via v4l2 ctrls */
>>        regmap_write(regmap, ISC_CBC_BRIGHT + isc->offsets.cbc, isc->ctrls.brightness);
>>        regmap_write(regmap, ISC_CBC_CONTRAST + isc->offsets.cbc, isc->ctrls.contrast);
>> -     /* Configure Hue and Saturation as neutral midpoint */
>> -     regmap_write(regmap, ISC_CBCHS_HUE, 0);
>> -     regmap_write(regmap, ISC_CBCHS_SAT, (1 << 4));
>> +     regmap_write(regmap, ISC_CBHS_HUE, isc->ctrls.hue);
>> +     regmap_write(regmap, ISC_CBHS_SAT, isc->ctrls.saturation);
>>   }
>>
>>   static void isc_sama7g5_config_cc(struct isc_device *isc)
>> @@ -463,6 +462,7 @@ static int microchip_xisc_probe(struct platform_device *pdev)
>>        isc->gamma_max = 2;
>>        /* Index 1 in the SAMA7G5 table is gamma 1/2.2 (sRGB). */
>>        isc->gamma_default = 1;
>> +     isc->has_cbhs = true;
>>
>>        if (of_machine_is_compatible("microchip,sam9x7")) {
>>                isc->max_width = ISC_SAM9X7_MAX_SUPPORT_WIDTH;
>>
>