[PATCH 0/5] media: ov5647: Reintroduce 8bpp modes from R-Car BSP

[PATCH 0/5] media: ov5647: Reintroduce 8bpp modes from R-Car BSP

[Jacopo Mondi]

8 bpp mode was removed in commit 38c223081815 ("media: ov5647:
Remove 640x480 SBGGR8 mode") as it hangs the sensor and no streaming
was possible.

This series upports a few patches from Renesas R-Car 4.1.0 BSP which use
8 bpp modes for ADAS applications.

While at it, a few patches from the RPi BSP have been upported as well, as the
series has been tested on a Pi.

As reported in the commit message of 3/5, a register quirk allows to capture
8bpp modes with no hangs:

Quoting:

-------------------------------------------------------------------------------
Comparing the register table for the 10 bit full-size mode and the
register table for the there introduced 8 bit full size mode, the main
difference is in the value of register 0x3034, documented as:

0x3034: Bit[7]:   Not used
	Bit[6:4]: pll_charge_pump
	Bit[3:0]: mipi_bit_mode
		  0000: 8 bit mode
		  0001: 10 bit mode
		  Others: Reserved to future use

However the value currently assigned to the register in all 10 bits
modes contradicts the register description (0x3034=0x1a) suggesting that
the documentation is possibly wrong and the lower and higher 4 bits are
actually swapped.

In facts, the 8 bits mode as added in the BSP commit assigns to register
0x3034 the value 0x08, causing the sensor to hang.

This patch uses for the register the same value as the 10 bits mode with
BIT(4) cleared, resulting in correct streaming operations with the
expected 15 FPS frame rate.

pi@raspberrypi:~ $ v4l2-ctl --get-subdev-fmt pad=0 -d /dev/v4l-subdev0
pi@raspberrypi:~ $ yavta -s2592x1944 -fSGBRG8 --capture=10 --skip=7 -F /dev/video0
...
Captured 10 frames in 0.631383 seconds (15.838237 fps, 79806470.803431 B/s).
...
-------------------------------------------------------------------------------

As reported in the same commit messages, frames as captured from the Pi are
completely black, suggesting that some other setting is off.

However the sensor does not hang anymore and it is worth re-enabling the modes
as a base for further debugging.

Thanks
   j

David Plowman (1):
  media: ov5647: Support HFLIP and VFLIP

Jacopo Mondi (3):
  media: ov5647: Add 8 bit SGBRG8 full size mode
  media: ov5647: Reintroduce 8 bit 640x480
  media: ov5647: Add support for test patterns

Laurent Pinchart (1):
  media: ov5647: Parse and register properties

 drivers/media/i2c/ov5647.c | 411 +++++++++++++++++++++++++++++++++++--
 1 file changed, 393 insertions(+), 18 deletions(-)

--
2.35.1

[PATCH 1/5] media: ov5647: Parse and register properties

[Jacopo Mondi]

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Parse device properties and register controls for them using the V4L2
fwnode properties helpers.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
[increase number of controls reserved in the handler]
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index d346d18ce629..98b72094ef68 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -1265,12 +1265,13 @@ static const struct v4l2_ctrl_ops ov5647_ctrl_ops = {
 	.s_ctrl = ov5647_s_ctrl,
 };
 
-static int ov5647_init_controls(struct ov5647 *sensor)
+static int ov5647_init_controls(struct ov5647 *sensor, struct device *dev)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->sd);
 	int hblank, exposure_max, exposure_def;
+	struct v4l2_fwnode_device_properties props;
 
-	v4l2_ctrl_handler_init(&sensor->ctrls, 8);
+	v4l2_ctrl_handler_init(&sensor->ctrls, 10);
 
 	v4l2_ctrl_new_std(&sensor->ctrls, &ov5647_ctrl_ops,
 			  V4L2_CID_AUTOGAIN, 0, 1, 1, 0);
@@ -1314,6 +1315,11 @@ static int ov5647_init_controls(struct ov5647 *sensor)
 					   sensor->mode->vts -
 					   sensor->mode->format.height);
 
+	v4l2_fwnode_device_parse(dev, &props);
+
+	v4l2_ctrl_new_fwnode_properties(&sensor->ctrls, &ov5647_ctrl_ops,
+					&props);
+
 	if (sensor->ctrls.error)
 		goto handler_free;
 
@@ -1400,7 +1406,7 @@ static int ov5647_probe(struct i2c_client *client)
 
 	sensor->mode = OV5647_DEFAULT_MODE;
 
-	ret = ov5647_init_controls(sensor);
+	ret = ov5647_init_controls(sensor, dev);
 	if (ret)
 		goto mutex_destroy;
 
-- 
2.35.1

[PATCH 2/5] media: ov5647: Support HFLIP and VFLIP

[Jacopo Mondi]

From: David Plowman <david.plowman@raspberrypi.com>

Add these missing V4L2 controls. Tested binned and full resolution
modes in all four orientations using Raspberry Pi running libcamera.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 72 ++++++++++++++++++++++++++++++++++----
 1 file changed, 66 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index 98b72094ef68..c292e5d09eab 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -54,6 +54,8 @@
 #define OV5647_REG_GAIN_LO		0x350b
 #define OV5647_REG_VTS_HI		0x380e
 #define OV5647_REG_VTS_LO		0x380f
+#define OV5647_REG_VFLIP		0x3820
+#define OV5647_REG_HFLIP		0x3821
 #define OV5647_REG_FRAME_OFF_NUMBER	0x4202
 #define OV5647_REG_MIPI_CTRL00		0x4800
 #define OV5647_REG_MIPI_CTRL14		0x4814
@@ -108,6 +110,8 @@ struct ov5647 {
 	struct v4l2_ctrl		*hblank;
 	struct v4l2_ctrl		*vblank;
 	struct v4l2_ctrl		*exposure;
+	struct v4l2_ctrl		*hflip;
+	struct v4l2_ctrl		*vflip;
 	bool				streaming;
 };
 
@@ -136,7 +140,7 @@ static struct regval_list ov5647_2592x1944_10bpp[] = {
 	{0x3036, 0x69},
 	{0x303c, 0x11},
 	{0x3106, 0xf5},
-	{0x3821, 0x06},
+	{0x3821, 0x00},
 	{0x3820, 0x00},
 	{0x3827, 0xec},
 	{0x370c, 0x03},
@@ -225,7 +229,7 @@ static struct regval_list ov5647_1080p30_10bpp[] = {
 	{0x3036, 0x62},
 	{0x303c, 0x11},
 	{0x3106, 0xf5},
-	{0x3821, 0x06},
+	{0x3821, 0x00},
 	{0x3820, 0x00},
 	{0x3827, 0xec},
 	{0x370c, 0x03},
@@ -389,7 +393,7 @@ static struct regval_list ov5647_2x2binned_10bpp[] = {
 	{0x4800, 0x24},
 	{0x3503, 0x03},
 	{0x3820, 0x41},
-	{0x3821, 0x07},
+	{0x3821, 0x01},
 	{0x350a, 0x00},
 	{0x350b, 0x10},
 	{0x3500, 0x00},
@@ -405,7 +409,7 @@ static struct regval_list ov5647_640x480_10bpp[] = {
 	{0x3035, 0x11},
 	{0x3036, 0x46},
 	{0x303c, 0x11},
-	{0x3821, 0x07},
+	{0x3821, 0x01},
 	{0x3820, 0x41},
 	{0x370c, 0x03},
 	{0x3612, 0x59},
@@ -918,6 +922,21 @@ static const struct v4l2_subdev_video_ops ov5647_subdev_video_ops = {
 	.s_stream =		ov5647_s_stream,
 };
 
+static u32 ov5647_get_mbus_code(struct v4l2_subdev *sd)
+{
+	struct ov5647 *sensor = to_sensor(sd);
+	int index = sensor->hflip->val | (sensor->vflip->val << 1);
+
+	static const u32 codes[4] = {
+		MEDIA_BUS_FMT_SGBRG10_1X10,
+		MEDIA_BUS_FMT_SBGGR10_1X10,
+		MEDIA_BUS_FMT_SRGGB10_1X10,
+		MEDIA_BUS_FMT_SGRBG10_1X10
+	};
+
+	return codes[index];
+}
+
 static int ov5647_enum_mbus_code(struct v4l2_subdev *sd,
 				 struct v4l2_subdev_state *sd_state,
 				 struct v4l2_subdev_mbus_code_enum *code)
@@ -925,7 +944,7 @@ static int ov5647_enum_mbus_code(struct v4l2_subdev *sd,
 	if (code->index > 0)
 		return -EINVAL;
 
-	code->code = MEDIA_BUS_FMT_SBGGR10_1X10;
+	code->code = ov5647_get_mbus_code(sd);
 
 	return 0;
 }
@@ -936,7 +955,7 @@ static int ov5647_enum_frame_size(struct v4l2_subdev *sd,
 {
 	const struct v4l2_mbus_framefmt *fmt;
 
-	if (fse->code != MEDIA_BUS_FMT_SBGGR10_1X10 ||
+	if (fse->code != ov5647_get_mbus_code(sd) ||
 	    fse->index >= ARRAY_SIZE(ov5647_modes))
 		return -EINVAL;
 
@@ -969,6 +988,8 @@ static int ov5647_get_pad_fmt(struct v4l2_subdev *sd,
 	}
 
 	*fmt = *sensor_format;
+	/* The code we pass back must reflect the current h/vflips. */
+	fmt->code = ov5647_get_mbus_code(sd);
 	mutex_unlock(&sensor->lock);
 
 	return 0;
@@ -1016,6 +1037,8 @@ static int ov5647_set_pad_fmt(struct v4l2_subdev *sd,
 					 exposure_def);
 	}
 	*fmt = mode->format;
+	/* The code we pass back must reflect the current h/vflips. */
+	fmt->code = ov5647_get_mbus_code(sd);
 	mutex_unlock(&sensor->lock);
 
 	return 0;
@@ -1191,6 +1214,25 @@ static int ov5647_s_exposure(struct v4l2_subdev *sd, u32 val)
 	return ov5647_write(sd, OV5647_REG_EXP_LO, (val & 0xf) << 4);
 }
 
+static int ov5647_s_flip(struct v4l2_subdev *sd, u16 reg, u32 ctrl_val)
+{
+	u8 reg_val;
+	int ret;
+
+	/* Set or clear bit 1 and leave everything else alone. */
+	ret = ov5647_read(sd, reg, &reg_val);
+	if (ret == 0) {
+		if (ctrl_val)
+			reg_val |= 2;
+		else
+			reg_val &= ~2;
+
+		ret = ov5647_write(sd, reg, reg_val);
+	}
+
+	return ret;
+}
+
 static int ov5647_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct ov5647 *sensor = container_of(ctrl->handler,
@@ -1249,6 +1291,14 @@ static int ov5647_s_ctrl(struct v4l2_ctrl *ctrl)
 		/* Read-only, but we adjust it based on mode. */
 		break;
 
+	case V4L2_CID_HFLIP:
+		/* There's an in-built hflip in the sensor, so account for that here. */
+		ov5647_s_flip(sd, OV5647_REG_HFLIP, !ctrl->val);
+		break;
+	case V4L2_CID_VFLIP:
+		ov5647_s_flip(sd, OV5647_REG_VFLIP, ctrl->val);
+		break;
+
 	default:
 		dev_info(&client->dev,
 			 "Control (id:0x%x, val:0x%x) not supported\n",
@@ -1315,6 +1365,16 @@ static int ov5647_init_controls(struct ov5647 *sensor, struct device *dev)
 					   sensor->mode->vts -
 					   sensor->mode->format.height);
 
+	sensor->hflip = v4l2_ctrl_new_std(&sensor->ctrls, &ov5647_ctrl_ops,
+					  V4L2_CID_HFLIP, 0, 1, 1, 0);
+	if (sensor->hflip)
+		sensor->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
+
+	sensor->vflip = v4l2_ctrl_new_std(&sensor->ctrls, &ov5647_ctrl_ops,
+					  V4L2_CID_VFLIP, 0, 1, 1, 0);
+	if (sensor->vflip)
+		sensor->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
+
 	v4l2_fwnode_device_parse(dev, &props);
 
 	v4l2_ctrl_new_fwnode_properties(&sensor->ctrls, &ov5647_ctrl_ops,
-- 
2.35.1

[PATCH 3/5] media: ov5647: Add 8 bit SGBRG8 full size mode

[Jacopo Mondi]

VGA 8 bpp mode was removed in commit 38c223081815 ("media: ov5647:
Remove 640x480 SBGGR8 mode") as it hangs the sensor and no streaming
was possible.

The Renesas BSP rcar-4.1.0 re-introduces full-size SBGGR8 mode in patch
e95242f52cac ("media: ov5647: Add 2592x1944 8-bit SBGGR mode") but again
the mode as-is does hang and does not provide any frame when tested on a
raspberry pi.

Comparing the register table for the 10 bit full-size mode and the
register table for the there introduced 8 bit full size mode, the main
difference is in the value of register 0x3034, documented as:

0x3034: Bit[7]:   Not used
	Bit[6:4]: pll_charge_pump
	Bit[3:0]: mipi_bit_mode
		  0000: 8 bit mode
		  0001: 10 bit mode
		  Others: Reserved to future use

However the value currently assigned to the register in all 10 bits
modes contradicts the register description (0x3034=0x1a) suggesting that
the documentation is possibly wrong and the lower and higher 4 bits are
actually swapped.

In facts, the 8 bits mode as added in the BSP commit assigns to register
0x3034 the value 0x08, causing the sensor to hang.

This patch uses for the register the same value as the 10 bits mode with
BIT(4) cleared, resulting in correct streaming operations with the
expected 15 FPS frame rate.

pi@raspberrypi:~ $ v4l2-ctl --get-subdev-fmt pad=0 -d /dev/v4l-subdev0
pi@raspberrypi:~ $ yavta -s2592x1944 -fSGBRG8 --capture=10 --skip=7 -F /dev/video0
...
Captured 10 frames in 0.631383 seconds (15.838237 fps, 79806470.803431 B/s).
...

However, the images as captured from the raspberry pi are completely
black suggesting gain or exposure time should probably be tuned
differently. It is anyway worth to re-introduce the mode as the sensor
seems to stream correctly.

No regressions detected when testing the existing 10 bits mode.

Based on a patch from:
Valentine Barshak <valentine.barshak@cogentembedded.com>
in Renesas R-Car 4.1.0 BSP.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 204 ++++++++++++++++++++++++++++++++++---
 1 file changed, 189 insertions(+), 15 deletions(-)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index c292e5d09eab..e0a693640661 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -493,7 +493,102 @@ static struct regval_list ov5647_640x480_10bpp[] = {
 	{0x0100, 0x01},
 };
 
-static const struct ov5647_mode ov5647_modes[] = {
+static struct regval_list ov5647_2592x1944_8bpp[] = {
+	{0x0100, 0x00},
+	{0x0103, 0x01},
+	{0x3034, 0x0a},
+	{0x3035, 0x21},
+	{0x3036, 0x69},
+	{0x303c, 0x11},
+	{0x3106, 0xf5},
+	{0x3821, 0x00},
+	{0x3820, 0x00},
+	{0x3827, 0xec},
+	{0x370c, 0x03},
+	{0x3612, 0x5b},
+	{0x3618, 0x04},
+	{0x5000, 0x06},
+	{0x5002, 0x41},
+	{0x5003, 0x08},
+	{0x5a00, 0x08},
+	{0x3000, 0x00},
+	{0x3001, 0x00},
+	{0x3002, 0x00},
+	{0x3016, 0x08},
+	{0x3017, 0xe0},
+	{0x3018, 0x44},
+	{0x301c, 0xf8},
+	{0x301d, 0xf0},
+	{0x3a18, 0x00},
+	{0x3a19, 0xf8},
+	{0x3c01, 0x80},
+	{0x3b07, 0x0c},
+	{0x380c, 0x0b},
+	{0x380d, 0x1c},
+	{0x3814, 0x11},
+	{0x3815, 0x11},
+	{0x3708, 0x64},
+	{0x3709, 0x12},
+	{0x3808, 0x0a},
+	{0x3809, 0x20},
+	{0x380a, 0x07},
+	{0x380b, 0x98},
+	{0x3800, 0x00},
+	{0x3801, 0x00},
+	{0x3802, 0x00},
+	{0x3803, 0x00},
+	{0x3804, 0x0a},
+	{0x3805, 0x3f},
+	{0x3806, 0x07},
+	{0x3807, 0xa3},
+	{0x3811, 0x10},
+	{0x3813, 0x06},
+	{0x3630, 0x2e},
+	{0x3632, 0xe2},
+	{0x3633, 0x23},
+	{0x3634, 0x44},
+	{0x3636, 0x06},
+	{0x3620, 0x64},
+	{0x3621, 0xe0},
+	{0x3600, 0x37},
+	{0x3704, 0xa0},
+	{0x3703, 0x5a},
+	{0x3715, 0x78},
+	{0x3717, 0x01},
+	{0x3731, 0x02},
+	{0x370b, 0x60},
+	{0x3705, 0x1a},
+	{0x3f05, 0x02},
+	{0x3f06, 0x10},
+	{0x3f01, 0x0a},
+	{0x3a08, 0x01},
+	{0x3a09, 0x28},
+	{0x3a0a, 0x00},
+	{0x3a0b, 0xf6},
+	{0x3a0d, 0x08},
+	{0x3a0e, 0x06},
+	{0x3a0f, 0x58},
+	{0x3a10, 0x50},
+	{0x3a1b, 0x58},
+	{0x3a1e, 0x50},
+	{0x3a11, 0x60},
+	{0x3a1f, 0x28},
+	{0x4001, 0x02},
+	{0x4004, 0x04},
+	{0x4000, 0x09},
+	{0x4837, 0x19},
+	{0x4800, 0x24},
+	{0x3503, 0x03},
+	{0x5001, 0x00},	/* White balance off */
+	{0x350a, 0x00},	/* Analogue gain [9:8] */
+	{0x350b, 0x20},	/* Analogue gain [7:0] */
+	{0x3500, 0x00},	/* Exposure [19:16] */
+	{0x3501, 0x3e},	/* Exposure [15:8] */
+	{0x3502, 0x80},	/* Exposure [7:0] */
+	{0x0100, 0x01},
+};
+
+static const struct ov5647_mode ov5647_10_bpp_modes[] = {
 	/* 2592x1944 full resolution full FOV 10-bit mode. */
 	{
 		.format = {
@@ -580,9 +675,33 @@ static const struct ov5647_mode ov5647_modes[] = {
 	},
 };
 
+static const struct ov5647_mode ov5647_8_bpp_modes[] = {
+	/* 2592x1944 full resolution full FOV 8-bit mode. */
+	{
+		.format = {
+			.code		= MEDIA_BUS_FMT_SBGGR8_1X8,
+			.colorspace	= V4L2_COLORSPACE_RAW,
+			.field		= V4L2_FIELD_NONE,
+			.width		= 2592,
+			.height		= 1944
+		},
+		.crop = {
+			.left		= OV5647_PIXEL_ARRAY_LEFT,
+			.top		= OV5647_PIXEL_ARRAY_TOP,
+			.width		= 2592,
+			.height		= 1944
+		},
+		.pixel_rate	= 69600000,
+		.hts		= 2844,
+		.vts		= 0x7b0,
+		.reg_list	= ov5647_2592x1944_8bpp,
+		.num_regs	= ARRAY_SIZE(ov5647_2592x1944_8bpp)
+	},
+};
+
 /* Default sensor mode is 2x2 binned 640x480 SBGGR10_1X10. */
-#define OV5647_DEFAULT_MODE	(&ov5647_modes[3])
-#define OV5647_DEFAULT_FORMAT	(ov5647_modes[3].format)
+#define OV5647_DEFAULT_MODE	(&ov5647_10_bpp_modes[3])
+#define OV5647_DEFAULT_FORMAT	(ov5647_10_bpp_modes[3].format)
 
 static int ov5647_write16(struct v4l2_subdev *sd, u16 reg, u16 val)
 {
@@ -922,17 +1041,33 @@ static const struct v4l2_subdev_video_ops ov5647_subdev_video_ops = {
 	.s_stream =		ov5647_s_stream,
 };
 
-static u32 ov5647_get_mbus_code(struct v4l2_subdev *sd)
+static unsigned int ov5647_code_to_bpp(unsigned int code)
+{
+	return code == MEDIA_BUS_FMT_SGBRG8_1X8 ||
+	       code == MEDIA_BUS_FMT_SBGGR8_1X8 ||
+	       code == MEDIA_BUS_FMT_SRGGB8_1X8 ||
+	       code == MEDIA_BUS_FMT_SGRBG8_1X8 ?
+	       8 : 10;
+}
+
+static u32 ov5647_get_mbus_code(struct v4l2_subdev *sd, unsigned int bpp)
 {
 	struct ov5647 *sensor = to_sensor(sd);
 	int index = sensor->hflip->val | (sensor->vflip->val << 1);
 
-	static const u32 codes[4] = {
+	static const u32 codes_10bpp[4] = {
 		MEDIA_BUS_FMT_SGBRG10_1X10,
 		MEDIA_BUS_FMT_SBGGR10_1X10,
 		MEDIA_BUS_FMT_SRGGB10_1X10,
 		MEDIA_BUS_FMT_SGRBG10_1X10
 	};
+	static const u32 codes_8bpp[4] = {
+		MEDIA_BUS_FMT_SGBRG8_1X8,
+		MEDIA_BUS_FMT_SBGGR8_1X8,
+		MEDIA_BUS_FMT_SRGGB8_1X8,
+		MEDIA_BUS_FMT_SGRBG8_1X8
+	};
+	const u32 *codes = bpp == 8 ? codes_8bpp : codes_10bpp;
 
 	return codes[index];
 }
@@ -941,10 +1076,10 @@ static int ov5647_enum_mbus_code(struct v4l2_subdev *sd,
 				 struct v4l2_subdev_state *sd_state,
 				 struct v4l2_subdev_mbus_code_enum *code)
 {
-	if (code->index > 0)
+	if (code->index > 1)
 		return -EINVAL;
 
-	code->code = ov5647_get_mbus_code(sd);
+	code->code = ov5647_get_mbus_code(sd, code->index == 0 ? 8 : 10);
 
 	return 0;
 }
@@ -954,12 +1089,42 @@ static int ov5647_enum_frame_size(struct v4l2_subdev *sd,
 				  struct v4l2_subdev_frame_size_enum *fse)
 {
 	const struct v4l2_mbus_framefmt *fmt;
+	const struct ov5647_mode *modes;
+	unsigned int num_modes;
+
+	switch (fse->code) {
+	case MEDIA_BUS_FMT_SGBRG10_1X10:
+		fallthrough;
+	case MEDIA_BUS_FMT_SBGGR10_1X10:
+		fallthrough;
+	case MEDIA_BUS_FMT_SRGGB10_1X10:
+		fallthrough;
+	case MEDIA_BUS_FMT_SGRBG10_1X10:
+		fallthrough;
+	case MEDIA_BUS_FMT_SGBRG8_1X8:
+		fallthrough;
+	case MEDIA_BUS_FMT_SBGGR8_1X8:
+		fallthrough;
+	case MEDIA_BUS_FMT_SRGGB8_1X8:
+		fallthrough;
+	case MEDIA_BUS_FMT_SGRBG8_1X8:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (ov5647_code_to_bpp(fse->code) == 10) {
+		modes = ov5647_10_bpp_modes;
+		num_modes = ARRAY_SIZE(ov5647_10_bpp_modes);
+	} else {
+		modes = ov5647_8_bpp_modes;
+		num_modes = ARRAY_SIZE(ov5647_8_bpp_modes);
+	}
 
-	if (fse->code != ov5647_get_mbus_code(sd) ||
-	    fse->index >= ARRAY_SIZE(ov5647_modes))
+	if (fse->index >= num_modes)
 		return -EINVAL;
 
-	fmt = &ov5647_modes[fse->index].format;
+	fmt = &modes[fse->index].format;
 	fse->min_width = fmt->width;
 	fse->max_width = fmt->width;
 	fse->min_height = fmt->height;
@@ -989,7 +1154,7 @@ static int ov5647_get_pad_fmt(struct v4l2_subdev *sd,
 
 	*fmt = *sensor_format;
 	/* The code we pass back must reflect the current h/vflips. */
-	fmt->code = ov5647_get_mbus_code(sd);
+	fmt->code = ov5647_get_mbus_code(sd, ov5647_code_to_bpp(fmt->code));
 	mutex_unlock(&sensor->lock);
 
 	return 0;
@@ -1001,11 +1166,20 @@ static int ov5647_set_pad_fmt(struct v4l2_subdev *sd,
 {
 	struct v4l2_mbus_framefmt *fmt = &format->format;
 	struct ov5647 *sensor = to_sensor(sd);
+	const struct ov5647_mode *modes;
 	const struct ov5647_mode *mode;
+	unsigned int num_modes;
+
+	if (ov5647_code_to_bpp(format->format.code) == 8) {
+		modes = ov5647_8_bpp_modes;
+		num_modes = ARRAY_SIZE(ov5647_8_bpp_modes);
+	} else {
+		modes = ov5647_10_bpp_modes;
+		num_modes = ARRAY_SIZE(ov5647_10_bpp_modes);
+	}
 
-	mode = v4l2_find_nearest_size(ov5647_modes, ARRAY_SIZE(ov5647_modes),
-				      format.width, format.height,
-				      fmt->width, fmt->height);
+	mode = v4l2_find_nearest_size(modes, num_modes, format.width,
+				      format.height, fmt->width, fmt->height);
 
 	/* Update the sensor mode and apply at it at streamon time. */
 	mutex_lock(&sensor->lock);
@@ -1038,7 +1212,7 @@ static int ov5647_set_pad_fmt(struct v4l2_subdev *sd,
 	}
 	*fmt = mode->format;
 	/* The code we pass back must reflect the current h/vflips. */
-	fmt->code = ov5647_get_mbus_code(sd);
+	fmt->code = ov5647_get_mbus_code(sd, ov5647_code_to_bpp(fmt->code));
 	mutex_unlock(&sensor->lock);
 
 	return 0;
-- 
2.35.1

[PATCH 4/5] media: ov5647: Reintroduce 8 bit 640x480

[Jacopo Mondi]

VGA 8 bpp mode was removed in commit 38c223081815 ("media: ov5647:
Remove 640x480 SBGGR8 mode") as it hangs the sensor and no streaming was
possible.

This is a partial revert of that commit as it re-introduces the mode
with the value of register 0x3034 modified.

Streaming operations are correctly working
pi@raspberrypi:~ $ v4l2-ctl --get-subdev-fmt pad=0 -d /dev/v4l-subdev0
pi@raspberrypi:~ $ yavta -s640x480 -fSGBRG8 --capture=10 --skip=7 -F /dev/video0
...
Captured 10 frames in 0.319589 seconds (31.290098 fps, 9612318.005293 B/s).
...

Frames when captured on a raspberry pi result in being completely
black. It is worth re-introducing the mode as compared to the previous
version it does not hang the sensor anymore and can be used for further
improvements.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 109 +++++++++++++++++++++++++++++++++++++
 1 file changed, 109 insertions(+)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index e0a693640661..0a3e4acec036 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -588,6 +588,94 @@ static struct regval_list ov5647_2592x1944_8bpp[] = {
 	{0x0100, 0x01},
 };
 
+static const struct regval_list ov5647_640x480_8bpp[] = {
+	{0x0100, 0x00},
+	{0x0103, 0x01},
+	{0x3034, 0x0a},
+	{0x3035, 0x21},
+	{0x3036, 0x46},
+	{0x303c, 0x11},
+	{0x3106, 0xf5},
+	{0x3821, 0x01},
+	{0x3820, 0x41},
+	{0x3827, 0xec},
+	{0x370c, 0x0f},
+	{0x3612, 0x59},
+	{0x3618, 0x00},
+	{0x5000, 0x06},
+	{0x5002, 0x41},
+	{0x5003, 0x08},
+	{0x5a00, 0x08},
+	{0x3000, 0x00},
+	{0x3001, 0x00},
+	{0x3002, 0x00},
+	{0x3016, 0x08},
+	{0x3017, 0xe0},
+	{0x3018, 0x44},
+	{0x301c, 0xf8},
+	{0x301d, 0xf0},
+	{0x3a18, 0x00},
+	{0x3a19, 0xf8},
+	{0x3c01, 0x80},
+	{0x3b07, 0x0c},
+	{0x380c, 0x07},
+	{0x380d, 0x68},
+	{0x3814, 0x31},
+	{0x3815, 0x31},
+	{0x3708, 0x64},
+	{0x3709, 0x52},
+	{0x3808, 0x02},
+	{0x3809, 0x80},
+	{0x380a, 0x01},
+	{0x380b, 0xe0},
+	{0x3801, 0x00},
+	{0x3802, 0x00},
+	{0x3803, 0x00},
+	{0x3804, 0x0a},
+	{0x3805, 0x3f},
+	{0x3806, 0x07},
+	{0x3807, 0xa1},
+	{0x3811, 0x08},
+	{0x3813, 0x02},
+	{0x3630, 0x2e},
+	{0x3632, 0xe2},
+	{0x3633, 0x23},
+	{0x3634, 0x44},
+	{0x3636, 0x06},
+	{0x3620, 0x64},
+	{0x3621, 0xe0},
+	{0x3600, 0x37},
+	{0x3704, 0xa0},
+	{0x3703, 0x5a},
+	{0x3715, 0x78},
+	{0x3717, 0x01},
+	{0x3731, 0x02},
+	{0x370b, 0x60},
+	{0x3705, 0x1a},
+	{0x3f05, 0x02},
+	{0x3f06, 0x10},
+	{0x3f01, 0x0a},
+	{0x3a08, 0x01},
+	{0x3a09, 0x27},
+	{0x3a0a, 0x00},
+	{0x3a0b, 0xf6},
+	{0x3a0d, 0x04},
+	{0x3a0e, 0x03},
+	{0x3a0f, 0x58},
+	{0x3a10, 0x50},
+	{0x3a1b, 0x58},
+	{0x3a1e, 0x50},
+	{0x3a11, 0x60},
+	{0x3a1f, 0x28},
+	{0x4001, 0x02},
+	{0x4004, 0x02},
+	{0x4000, 0x09},
+	{0x4837, 0x24},
+	{0x4050, 0x6e},
+	{0x4051, 0x8f},
+	{0x0100, 0x01},
+};
+
 static const struct ov5647_mode ov5647_10_bpp_modes[] = {
 	/* 2592x1944 full resolution full FOV 10-bit mode. */
 	{
@@ -697,6 +785,27 @@ static const struct ov5647_mode ov5647_8_bpp_modes[] = {
 		.reg_list	= ov5647_2592x1944_8bpp,
 		.num_regs	= ARRAY_SIZE(ov5647_2592x1944_8bpp)
 	},
+	/* 8-bit VGA mode: Uncentred crop 2x2 binned 1296x972 image. */
+	{
+		.format = {
+			.code           = MEDIA_BUS_FMT_SBGGR8_1X8,
+			.colorspace     = V4L2_COLORSPACE_SRGB,
+			.field          = V4L2_FIELD_NONE,
+			.width          = 640,
+			.height         = 480
+		},
+		.crop = {
+			.left           = OV5647_PIXEL_ARRAY_LEFT,
+			.top            = OV5647_PIXEL_ARRAY_TOP,
+			.width          = 1280,
+			.height         = 960,
+		},
+		.pixel_rate     = 77291670,
+		.hts            = 1896,
+		.vts            = 0x3d8,
+		.reg_list       = ov5647_640x480_8bpp,
+		.num_regs       = ARRAY_SIZE(ov5647_640x480_8bpp)
+	},
 };
 
 /* Default sensor mode is 2x2 binned 640x480 SBGGR10_1X10. */
-- 
2.35.1

[PATCH 5/5] media: ov5647: Add support for test patterns

[Jacopo Mondi]

Add support for V4L2_CID_TEST_PATTERN.

Based on a patch from Renesas R-Car BSP 4.1.0 from
Valentine Barshak <valentine.barshak@cogentembedded.com>

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index 0a3e4acec036..68e56b0d8153 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -60,6 +60,7 @@
 #define OV5647_REG_MIPI_CTRL00		0x4800
 #define OV5647_REG_MIPI_CTRL14		0x4814
 #define OV5647_REG_AWB			0x5001
+#define OV5647_REG_ISP_CTRL3D		0x503d
 
 #define REG_TERM 0xfffe
 #define VAL_TERM 0xfe
@@ -812,6 +813,22 @@ static const struct ov5647_mode ov5647_8_bpp_modes[] = {
 #define OV5647_DEFAULT_MODE	(&ov5647_10_bpp_modes[3])
 #define OV5647_DEFAULT_FORMAT	(ov5647_10_bpp_modes[3].format)
 
+static const char * const ov5647_test_pattern_menu[] = {
+	"Disabled",
+	"Color Bars",
+	"Color Squares",
+	"Random Data",
+	"Input Data"
+};
+
+static u8 ov5647_test_pattern_val[] = {
+	0x00,	/* Disabled */
+	0x80,	/* Color Bars */
+	0x82,	/* Color Squares */
+	0x81,	/* Random Data */
+	0x83,	/* Input Data */
+};
+
 static int ov5647_write16(struct v4l2_subdev *sd, u16 reg, u16 val)
 {
 	unsigned char data[4] = { reg >> 8, reg & 0xff, val >> 8, val & 0xff};
@@ -1582,6 +1599,10 @@ static int ov5647_s_ctrl(struct v4l2_ctrl *ctrl)
 		ov5647_s_flip(sd, OV5647_REG_VFLIP, ctrl->val);
 		break;
 
+	case V4L2_CID_TEST_PATTERN:
+		ret = ov5647_write(sd, OV5647_REG_ISP_CTRL3D,
+				   ov5647_test_pattern_val[ctrl->val]);
+		break;
 	default:
 		dev_info(&client->dev,
 			 "Control (id:0x%x, val:0x%x) not supported\n",
@@ -1604,7 +1625,7 @@ static int ov5647_init_controls(struct ov5647 *sensor, struct device *dev)
 	int hblank, exposure_max, exposure_def;
 	struct v4l2_fwnode_device_properties props;
 
-	v4l2_ctrl_handler_init(&sensor->ctrls, 10);
+	v4l2_ctrl_handler_init(&sensor->ctrls, 11);
 
 	v4l2_ctrl_new_std(&sensor->ctrls, &ov5647_ctrl_ops,
 			  V4L2_CID_AUTOGAIN, 0, 1, 1, 0);
@@ -1658,6 +1679,11 @@ static int ov5647_init_controls(struct ov5647 *sensor, struct device *dev)
 	if (sensor->vflip)
 		sensor->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
 
+	v4l2_ctrl_new_std_menu_items(&sensor->ctrls, &ov5647_ctrl_ops,
+				     V4L2_CID_TEST_PATTERN,
+				     ARRAY_SIZE(ov5647_test_pattern_menu) - 1,
+				     0, 0, ov5647_test_pattern_menu);
+
 	v4l2_fwnode_device_parse(dev, &props);
 
 	v4l2_ctrl_new_fwnode_properties(&sensor->ctrls, &ov5647_ctrl_ops,
-- 
2.35.1

Re: [PATCH 2/5] media: ov5647: Support HFLIP and VFLIP

[Sakari Ailus]

Hi Jacopo,

Thanks for the set.

On Wed, Jun 15, 2022 at 05:14:54PM +0200, Jacopo Mondi wrote:
> From: David Plowman <david.plowman@raspberrypi.com>
> 
> Add these missing V4L2 controls. Tested binned and full resolution
> modes in all four orientations using Raspberry Pi running libcamera.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  drivers/media/i2c/ov5647.c | 72 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 66 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> index 98b72094ef68..c292e5d09eab 100644
> --- a/drivers/media/i2c/ov5647.c
> +++ b/drivers/media/i2c/ov5647.c
> @@ -136,7 +140,7 @@ static struct regval_list ov5647_2592x1944_10bpp[] = {
>  	{0x3036, 0x69},
>  	{0x303c, 0x11},
>  	{0x3106, 0xf5},
> -	{0x3821, 0x06},
> +	{0x3821, 0x00},
>  	{0x3820, 0x00},

As you're setting the values through the control handler, the registers
should be removed from register lists.

>  	{0x3827, 0xec},
>  	{0x370c, 0x03},

-- 
Regards,

Sakari Ailus