Re: [PATCH 10/11] media: i2c: ov5640: Disable ISP for raw output

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

Hi Kieran

On Fri, May 01, 2026 at 04:39:12PM +0100, Kieran Bingham wrote:
> The OV5640 has ISP operations that can run even when outputting RAW
> bayer data.  These include the Lens Shading Correction, Gamma and Auto
> white balance which need to be disabled when performing module
> calibration using RAW data.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  drivers/media/i2c/ov5640.c | 45 ++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index fd369a13463e..f63d81640f54 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -280,28 +280,28 @@ static const struct ov5640_pixfmt ov5640_dvp_formats[] = {
>  	}, {
>  		/* Raw, BGBG... / GRGR... */
>  		.code		= MEDIA_BUS_FMT_SBGGR8_1X8,
> -		.colorspace	= V4L2_COLORSPACE_SRGB,
> +		.colorspace	= V4L2_COLORSPACE_RAW,
>  		.bpp		= 8,
>  		.ctrl00		= 0x00,
>  		.mux		= OV5640_FMT_MUX_RAW_DPC,
>  	}, {
>  		/* Raw bayer, GBGB... / RGRG... */
>  		.code		= MEDIA_BUS_FMT_SGBRG8_1X8,
> -		.colorspace	= V4L2_COLORSPACE_SRGB,
> +		.colorspace	= V4L2_COLORSPACE_RAW,
>  		.bpp		= 8,
>  		.ctrl00		= 0x01,
>  		.mux		= OV5640_FMT_MUX_RAW_DPC,
>  	}, {
>  		/* Raw bayer, GRGR... / BGBG... */
>  		.code		= MEDIA_BUS_FMT_SGRBG8_1X8,
> -		.colorspace	= V4L2_COLORSPACE_SRGB,
> +		.colorspace	= V4L2_COLORSPACE_RAW,
>  		.bpp		= 8,
>  		.ctrl00		= 0x02,
>  		.mux		= OV5640_FMT_MUX_RAW_DPC,
>  	}, {
>  		/* Raw bayer, RGRG... / GBGB... */
>  		.code		= MEDIA_BUS_FMT_SRGGB8_1X8,
> -		.colorspace	= V4L2_COLORSPACE_SRGB,
> +		.colorspace	= V4L2_COLORSPACE_RAW,
>  		.bpp		= 8,
>  		.ctrl00		= 0x03,
>  		.mux		= OV5640_FMT_MUX_RAW_DPC,
> @@ -348,7 +348,7 @@ static const struct ov5640_pixfmt ov5640_csi2_formats[] = {
>  	}, {
>  		/* Raw, BGBG... / GRGR... */
>  		.code		= MEDIA_BUS_FMT_SBGGR8_1X8,
> -		.colorspace	= V4L2_COLORSPACE_SRGB,
> +		.colorspace	= V4L2_COLORSPACE_RAW,
>  		.bpp		= 8,
>  		.ctrl00		= 0x00,
>  		.mux		= OV5640_FMT_MUX_RAW_DPC,
> @@ -473,6 +473,7 @@ struct ov5640_dev {
>
>  	struct v4l2_mbus_framefmt fmt;
>  	bool pending_fmt_change;
> +	bool is_raw;
>
>  	const struct ov5640_mode_info *current_mode;
>  	const struct ov5640_mode_info *last_mode;
> @@ -618,8 +619,13 @@ static const struct reg_value ov5640_init_setting[] = {
>  	{0x501f, 0x00, 0, 0}, {0x440e, 0x00, 0, 0}, {0x4837, 0x0a, 0, 0},
>
>  	/* ISP Control */
> -	{OV5640_REG_ISP_CTRL00, 0xa7, 0, 0},
> -	{OV5640_REG_ISP_CTRL01, 0xa3, 0, 0},
> +	{OV5640_REG_ISP_CTRL00, OV5640_ISP_00_LENC_ENABLE | OV5640_ISP_00_GMA_ENABLE |
> +				OV5640_ISP_00_BPC_ENABLE | OV5640_ISP_00_WPC_ENABLE |
> +				OV5640_ISP_00_CIP_ENABLE, 0, 0},
> +
> +	/* OV5640_ISP_01_UVA_ENABLE is not enabled */
> +	{OV5640_REG_ISP_CTRL01, OV5640_ISP_01_SDE_ENABLE | OV5640_ISP_01_SCL_ENABLE |
> +				OV5640_ISP_01_CMX_ENABLE | OV5640_ISP_01_AWB_ENABLE, 0, 0},

Should we avoid writing these registers at all in the init sequence ?

>
>  	/* AWB Control */
>  	{OV5640_REG_AWB_CONTROL_00, 0xff, 0, 0}, /* AWB B Block */
> @@ -3116,6 +3122,31 @@ static int ov5640_set_framefmt(struct ov5640_dev *sensor,
>  	if (ret)
>  		return ret;
>
> +	/*
> +	 * Disable all ISP image processing (Lens Shading, Gamma, AWB...) for
> +	 * RAW modes to facilitate module tuning.
> +	 */
> +	sensor->is_raw = pixfmt->colorspace == V4L2_COLORSPACE_RAW;

Unless it is used later on, the 'is_raw' flag can be kept a local
variable.

> +	if (sensor->is_raw) {
> +		ret = ov5640_write_reg(sensor, OV5640_REG_ISP_CTRL00, 0);
> +		if (ret)
> +			return ret;
> +	} else {
> +		ret = ov5640_write_reg(sensor, OV5640_REG_ISP_CTRL00,
> +				       OV5640_ISP_00_LENC_ENABLE | OV5640_ISP_00_GMA_ENABLE |
> +				       OV5640_ISP_00_BPC_ENABLE | OV5640_ISP_00_WPC_ENABLE |
> +				       OV5640_ISP_00_CIP_ENABLE);
> +		if (ret)
> +			return ret;
> +
> +		/* OV5640_ISP_01_UVA_ENABLE is not enabled */
> +		ret = ov5640_write_reg(sensor, OV5640_REG_ISP_CTRL01,
> +				       OV5640_ISP_01_SDE_ENABLE | OV5640_ISP_01_SCL_ENABLE |
> +				       OV5640_ISP_01_CMX_ENABLE | OV5640_ISP_01_AWB_ENABLE);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	/*
>  	 * TIMING TC REG21:
>  	 * - [5]:	JPEG enable
>
> --
> 2.52.0
>
>