All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI
@ 2022-02-23 14:55 Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 01/27] media: ov5640: Add pixel rate to modes Jacopo Mondi
                   ` (26 more replies)
  0 siblings, 27 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

v1:
https://patchwork.linuxtv.org/project/linux-media/list/?series=7249
v2:
https://patchwork.linuxtv.org/project/linux-media/list/?series=7311
v3:
https://patchwork.linuxtv.org/project/linux-media/list/?series=7385

A branch for testing based on the most recent media-master is available at
https://git.sr.ht/~jmondi_/linux #jmondi/media-master/ov5640-v4

The series is now almost fully reviewed and tested on 3 CSI-2 platforms.

The only missing review is on
"media: ov5640: Split DVP and CSI-2 timings"
for which a Tested-by: on a DVP setup would be ideal.

With that, I hope the series can be merged soon.

Thanks
   j

v4:
Very minor update. Added tags and reworked enum_mbus_format as suggested
by Laurent.

v3:
The series has now grown by 4 patches and the driver is now even larger
being the formats and the timings for DVP and CSI-2 defined separately.

Tested in CSI-2 mode with UYVY, RGB565, SBGGR and RGB24 in all supported modes.

Tested format and sizes enumeration with the new formats definition.

Tested frame rate handling:

	vblank = ( duration msec * pixe_rate MHz / htot - height)

  640x480 YUYV 15FPS (default 30 FPS)

	duration = 666666 msec
	pixel_rate = 48 Mhz
	htot = 1600
	vtot = 1999
	vblank = vtot - height = 1519

	$ v4l2-ctl -d /dev/v4l-subdev4 -c 0x009e0901=1519
	$ yavta -f YUYV -s 640x480 -c100 --skip 7 /dev/video0
	...
	10 (2) [-] any 11 614400 B 2189.317617 2189.317629 15.244 fps ts mono/EoF
	11 (3) [-] any 12 614400 B 2189.383212 2189.383224 15.245 fps ts mono/EoF
	12 (4) [-] any 13 614400 B 2189.448810 2189.448821 15.244 fps ts mono/EoF
	13 (5) [-] any 14 614400 B 2189.514405 2189.514417 15.245 fps ts mono/EoF
	14 (6) [-] any 15 614400 B 2189.580002 2189.580015 15.245 fps ts mono/EoF
	..

  2592x1944 YUVV 15 FPS (default)
	$ yavta -f YUYV -s 2592x1944 -c100 --skip 7 /dev/video0
	...
	6 (6) [-] any 7 10077696 B 2438.377592 2438.377605 15.009 fps ts mono/EoF
	7 (7) [-] any 8 10077696 B 2438.444219 2438.444233 15.009 fps ts mono/EoF
	8 (0) [-] any 9 10077696 B 2438.510846 2438.510860 15.009 fps ts mono/EoF
	9 (1) [-] any 10 10077696 B 2438.577474 2438.577488 15.009 fps ts mono/EoF
	10 (2) [-] any 11 10077696 B 2438.644101 2438.644116 15.009 fps ts mono/EoF
	11 (3) [-] any 12 10077696 B 2438.710727 2438.710740 15.009 fps ts mono/EoF
	12 (4) [-] any 13 10077696 B 2438.777358 2438.777370 15.008 fps ts mono/EoF
	13 (5) [-] any 14 10077696 B 2438.843984 2438.843998 15.009 fps ts mono/EoF
	14 (6) [-] any 15 10077696 B 2438.910611 2438.910623 15.009 fps ts mono/EoF
	15 (7) [-] any 16 10077696 B 2438.977238 2438.977252 15.009 fps ts mono/EoF
	16 (0) [-] any 17 10077696 B 2439.043865 2439.043877 15.009 fps ts mono
	...


To enable higher FPS the LINK_FREQ control should be made writable to increase
the pixel rate

  640x480 YUYV 60 FPS (pixel_rate = 96 Mhz)

	$ yavta -f YUYV -s 640x480 -c100 --skip 7 /dev/video0
 	...
	9 (1) [-] any 10 614400 B 57.098649 57.098667 59.995 fps ts mono/EoF
	10 (2) [-] any 11 614400 B 57.115314 57.115332 60.006 fps ts mono/EoF
	11 (3) [-] any 12 614400 B 57.131978 57.131994 60.010 fps ts mono/EoF
	12 (4) [-] any 13 614400 B 57.148645 57.148664 59.999 fps ts mono/EoF
	13 (5) [-] any 14 614400 B 57.165310 57.165328 60.006 fps ts mono/EoF
	14 (6) [-] any 15 614400 B 57.181977 57.181996 59.999 fps ts mono/EoF
	15 (7) [-] any 16 614400 B 57.198642 57.198660 60.006 fps ts mono/EoF

Changelog:

v2->v3:

- Eugen (thanks) reported regression in DVP mode :(
  To maintain the DVP timings un-changed in this version the mode definition now
  looks like

		/* 640x480 */
		.id		= OV5640_MODE_VGA_640_480,
		.dn_mode	= SUBSAMPLING,
		.pixel_rate	= OV5640_PIXEL_RATE_48M,
		.width		= 640,
		.height		= 480,
		.dvp_timings = {
			.analog_crop = {
				.left	= 0,
				.top	= 4,
				.width	= 2624,
				.height	= 1944,
			},
			.crop = {
				.left	= 16,
				.top	= 6,
				.width	= 640,
				.height	= 480,
			},
			.htot		= 1896,
			.vblank_def	= 600,
			.max_fps	= OV5640_60_FPS
		},
		.csi2_timings = {
			.analog_crop = {
				/* Feed the full valid pixel array to the ISP. */
				.left	= OV5640_PIXEL_ARRAY_LEFT,
				.top	= OV5640_PIXEL_ARRAY_TOP,
				.width	= OV5640_PIXEL_ARRAY_WIDTH,
				.height	= OV5640_PIXEL_ARRAY_HEIGHT,
			},
			.crop = {
				/* Maintain a minimum digital crop processing margins. */
				.left	= 2,
				.top	= 4,
				.width	= 640,
				.height	= 480,
			},
			.htot		= 1600,
			.vblank_def	= 520,
		},
		.reg_data	= ov5640_setting_low_res,
		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),

  with a .dvp_timings and a .csi2_timings members to separate the two.
  Is it nice ? No it's not, but it should help maintaining DVP users happy.

  Eugen: if you are willing to run another test round to confirm if this version
  does not regress DVP it would be great :)

- Split image formats between CSI-2 and DVP
- Remove RGB888 as per the CSIS discussion with Laurent
- Removed register tables for modes < 720 as they're all equal
- Minor fixes on Laurent's comments
- Add Adam's tag

v1 -> v2:
- rework the modes definition to process the full pixel array
- rework get_selection to report the correct BOUND and DEFAULT targets
- implement init_cfg
- minor style changes as suggested by Laurent
- test with 1 data lane

Jacopo Mondi (27):
  media: ov5640: Add pixel rate to modes
  media: ov5604: Re-arrange modes definition
  media: ov5640: Add ov5640_is_csi2() function
  media: ov5640: Associate bpp with formats
  media: ov5640: Add LINK_FREQ control
  media: ov5640: Update pixel_rate and link_freq
  media: ov5640: Rework CSI-2 clock tree
  media: ov5640: Rework timings programming
  media: ov5640: Fix 720x480 in RGB888 mode
  media: ov5640: Split DVP and CSI-2 timings
  media: ov5640: Provide timings accessor
  media: ov5640: Re-sort per-mode register tables
  media: ov5640: Remove duplicated mode settings
  media: ov5640: Remove ov5640_mode_init_data
  media: ov5640: Add HBLANK control
  media: ov5640: Add VBLANK control
  media: ov5640: Change CSI-2 timings to comply with FPS
  media: ov5640: Implement init_cfg
  media: ov5640: Implement get_selection
  media: ov5640: Limit frame_interval to DVP mode only
  media: ov5640: Register device properties
  media: ov5640: Add RGB565_1X16 format
  media: ov5640: Add BGR888 format
  media: ov5640: Restrict sizes to mbus code
  media: ov5640: Adjust format to bpp in s_fmt
  media: ov5640: Split DVP and CSI-2 formats
  media: ov5640: Move format mux config in format

 drivers/media/i2c/ov5640.c | 1620 ++++++++++++++++++++++++++----------
 1 file changed, 1160 insertions(+), 460 deletions(-)

--
2.35.0


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v4 01/27] media: ov5640: Add pixel rate to modes
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 02/27] media: ov5604: Re-arrange modes definition Jacopo Mondi
                   ` (25 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Add to each mode supported by the sensor the ideal pixel rate, as
defined by Table 2.1 in the chip manual.

The ideal pixel rate will be used to compute the MIPI CSI-2 clock tree.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Adam Ford <aford173@gmail.com> #imx8mm-beacon-kit
---
 drivers/media/i2c/ov5640.c | 44 +++++++++++++++++++++++++++++++++++---
 1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index db5a19babe67..01f4a075f86e 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -118,6 +118,29 @@ enum ov5640_frame_rate {
 	OV5640_NUM_FRAMERATES,
 };
 
+enum ov5640_pixel_rate_id {
+	OV5640_PIXEL_RATE_168M,
+	OV5640_PIXEL_RATE_148M,
+	OV5640_PIXEL_RATE_124M,
+	OV5640_PIXEL_RATE_96M,
+	OV5640_PIXEL_RATE_48M,
+	OV5640_NUM_PIXEL_RATES,
+};
+
+/*
+ * The chip manual suggests 24/48/96/192 MHz pixel clocks.
+ *
+ * 192MHz exceeds the sysclk limits; use 168MHz as maximum pixel rate for
+ * full resolution mode @15 FPS.
+ */
+static const u32 ov5640_pixel_rates[] = {
+	[OV5640_PIXEL_RATE_168M] = 168000000,
+	[OV5640_PIXEL_RATE_148M] = 148000000,
+	[OV5640_PIXEL_RATE_124M] = 124000000,
+	[OV5640_PIXEL_RATE_96M] = 96000000,
+	[OV5640_PIXEL_RATE_48M] = 48000000,
+};
+
 enum ov5640_format_mux {
 	OV5640_FMT_MUX_YUV422 = 0,
 	OV5640_FMT_MUX_RGB,
@@ -189,6 +212,7 @@ struct reg_value {
 struct ov5640_mode_info {
 	enum ov5640_mode_id id;
 	enum ov5640_downsize_mode dn_mode;
+	enum ov5640_pixel_rate_id pixel_rate;
 	u32 hact;
 	u32 htot;
 	u32 vact;
@@ -565,7 +589,9 @@ static const struct reg_value ov5640_setting_QSXGA_2592_1944[] = {
 
 /* power-on sensor init reg table */
 static const struct ov5640_mode_info ov5640_mode_init_data = {
-	0, SUBSAMPLING, 640, 1896, 480, 984,
+	0, SUBSAMPLING,
+	OV5640_PIXEL_RATE_96M,
+	640, 1896, 480, 984,
 	ov5640_init_setting_30fps_VGA,
 	ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
 	OV5640_30_FPS,
@@ -574,51 +600,61 @@ static const struct ov5640_mode_info ov5640_mode_init_data = {
 static const struct ov5640_mode_info
 ov5640_mode_data[OV5640_NUM_MODES] = {
 	{OV5640_MODE_QQVGA_160_120, SUBSAMPLING,
+	 OV5640_PIXEL_RATE_48M,
 	 160, 1896, 120, 984,
 	 ov5640_setting_QQVGA_160_120,
 	 ARRAY_SIZE(ov5640_setting_QQVGA_160_120),
 	 OV5640_30_FPS},
 	{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
+	 OV5640_PIXEL_RATE_48M,
 	 176, 1896, 144, 984,
 	 ov5640_setting_QCIF_176_144,
 	 ARRAY_SIZE(ov5640_setting_QCIF_176_144),
 	 OV5640_30_FPS},
 	{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
+	 OV5640_PIXEL_RATE_48M,
 	 320, 1896, 240, 984,
 	 ov5640_setting_QVGA_320_240,
 	 ARRAY_SIZE(ov5640_setting_QVGA_320_240),
 	 OV5640_30_FPS},
 	{OV5640_MODE_VGA_640_480, SUBSAMPLING,
+	 OV5640_PIXEL_RATE_48M,
 	 640, 1896, 480, 1080,
 	 ov5640_setting_VGA_640_480,
 	 ARRAY_SIZE(ov5640_setting_VGA_640_480),
 	 OV5640_60_FPS},
 	{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
+	 OV5640_PIXEL_RATE_96M,
 	 720, 1896, 480, 984,
 	 ov5640_setting_NTSC_720_480,
 	 ARRAY_SIZE(ov5640_setting_NTSC_720_480),
 	OV5640_30_FPS},
 	{OV5640_MODE_PAL_720_576, SUBSAMPLING,
+	 OV5640_PIXEL_RATE_96M,
 	 720, 1896, 576, 984,
 	 ov5640_setting_PAL_720_576,
 	 ARRAY_SIZE(ov5640_setting_PAL_720_576),
 	 OV5640_30_FPS},
 	{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
+	 OV5640_PIXEL_RATE_96M,
 	 1024, 1896, 768, 1080,
 	 ov5640_setting_XGA_1024_768,
 	 ARRAY_SIZE(ov5640_setting_XGA_1024_768),
 	 OV5640_30_FPS},
 	{OV5640_MODE_720P_1280_720, SUBSAMPLING,
+	 OV5640_PIXEL_RATE_124M,
 	 1280, 1892, 720, 740,
 	 ov5640_setting_720P_1280_720,
 	 ARRAY_SIZE(ov5640_setting_720P_1280_720),
 	 OV5640_30_FPS},
 	{OV5640_MODE_1080P_1920_1080, SCALING,
+	 OV5640_PIXEL_RATE_148M,
 	 1920, 2500, 1080, 1120,
 	 ov5640_setting_1080P_1920_1080,
 	 ARRAY_SIZE(ov5640_setting_1080P_1920_1080),
 	 OV5640_30_FPS},
 	{OV5640_MODE_QSXGA_2592_1944, SCALING,
+	 OV5640_PIXEL_RATE_168M,
 	 2592, 2844, 1944, 1968,
 	 ov5640_setting_QSXGA_2592_1944,
 	 ARRAY_SIZE(ov5640_setting_QSXGA_2592_1944),
@@ -2743,6 +2779,7 @@ static const struct v4l2_ctrl_ops ov5640_ctrl_ops = {
 
 static int ov5640_init_controls(struct ov5640_dev *sensor)
 {
+	const struct ov5640_mode_info *mode = sensor->current_mode;
 	const struct v4l2_ctrl_ops *ops = &ov5640_ctrl_ops;
 	struct ov5640_ctrls *ctrls = &sensor->ctrls;
 	struct v4l2_ctrl_handler *hdl = &ctrls->handler;
@@ -2755,8 +2792,9 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
 
 	/* Clock related controls */
 	ctrls->pixel_rate = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_PIXEL_RATE,
-					      0, INT_MAX, 1,
-					      ov5640_calc_pixel_rate(sensor));
+				      ov5640_pixel_rates[OV5640_NUM_PIXEL_RATES - 1],
+				      ov5640_pixel_rates[0], 1,
+				      ov5640_pixel_rates[mode->pixel_rate]);
 
 	/* Auto/manual white balance */
 	ctrls->auto_wb = v4l2_ctrl_new_std(hdl, ops,
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 02/27] media: ov5604: Re-arrange modes definition
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 01/27] media: ov5640: Add pixel rate to modes Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 03/27] media: ov5640: Add ov5640_is_csi2() function Jacopo Mondi
                   ` (24 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

The array of supported modes is close to unreadable.
Re-arrange it giving it some room to breath.

Cosmetic change only.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 141 +++++++++++++++++++++----------------
 1 file changed, 81 insertions(+), 60 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 01f4a075f86e..d8285be2bfa1 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -599,66 +599,87 @@ static const struct ov5640_mode_info ov5640_mode_init_data = {
 
 static const struct ov5640_mode_info
 ov5640_mode_data[OV5640_NUM_MODES] = {
-	{OV5640_MODE_QQVGA_160_120, SUBSAMPLING,
-	 OV5640_PIXEL_RATE_48M,
-	 160, 1896, 120, 984,
-	 ov5640_setting_QQVGA_160_120,
-	 ARRAY_SIZE(ov5640_setting_QQVGA_160_120),
-	 OV5640_30_FPS},
-	{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
-	 OV5640_PIXEL_RATE_48M,
-	 176, 1896, 144, 984,
-	 ov5640_setting_QCIF_176_144,
-	 ARRAY_SIZE(ov5640_setting_QCIF_176_144),
-	 OV5640_30_FPS},
-	{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
-	 OV5640_PIXEL_RATE_48M,
-	 320, 1896, 240, 984,
-	 ov5640_setting_QVGA_320_240,
-	 ARRAY_SIZE(ov5640_setting_QVGA_320_240),
-	 OV5640_30_FPS},
-	{OV5640_MODE_VGA_640_480, SUBSAMPLING,
-	 OV5640_PIXEL_RATE_48M,
-	 640, 1896, 480, 1080,
-	 ov5640_setting_VGA_640_480,
-	 ARRAY_SIZE(ov5640_setting_VGA_640_480),
-	 OV5640_60_FPS},
-	{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
-	 OV5640_PIXEL_RATE_96M,
-	 720, 1896, 480, 984,
-	 ov5640_setting_NTSC_720_480,
-	 ARRAY_SIZE(ov5640_setting_NTSC_720_480),
-	OV5640_30_FPS},
-	{OV5640_MODE_PAL_720_576, SUBSAMPLING,
-	 OV5640_PIXEL_RATE_96M,
-	 720, 1896, 576, 984,
-	 ov5640_setting_PAL_720_576,
-	 ARRAY_SIZE(ov5640_setting_PAL_720_576),
-	 OV5640_30_FPS},
-	{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
-	 OV5640_PIXEL_RATE_96M,
-	 1024, 1896, 768, 1080,
-	 ov5640_setting_XGA_1024_768,
-	 ARRAY_SIZE(ov5640_setting_XGA_1024_768),
-	 OV5640_30_FPS},
-	{OV5640_MODE_720P_1280_720, SUBSAMPLING,
-	 OV5640_PIXEL_RATE_124M,
-	 1280, 1892, 720, 740,
-	 ov5640_setting_720P_1280_720,
-	 ARRAY_SIZE(ov5640_setting_720P_1280_720),
-	 OV5640_30_FPS},
-	{OV5640_MODE_1080P_1920_1080, SCALING,
-	 OV5640_PIXEL_RATE_148M,
-	 1920, 2500, 1080, 1120,
-	 ov5640_setting_1080P_1920_1080,
-	 ARRAY_SIZE(ov5640_setting_1080P_1920_1080),
-	 OV5640_30_FPS},
-	{OV5640_MODE_QSXGA_2592_1944, SCALING,
-	 OV5640_PIXEL_RATE_168M,
-	 2592, 2844, 1944, 1968,
-	 ov5640_setting_QSXGA_2592_1944,
-	 ARRAY_SIZE(ov5640_setting_QSXGA_2592_1944),
-	 OV5640_15_FPS},
+	{
+		/* 160x120 */
+		OV5640_MODE_QQVGA_160_120, SUBSAMPLING,
+		OV5640_PIXEL_RATE_48M,
+		160, 1896, 120, 984,
+		ov5640_setting_QQVGA_160_120,
+		ARRAY_SIZE(ov5640_setting_QQVGA_160_120),
+		OV5640_30_FPS
+	}, {
+		/* 176x144 */
+		OV5640_MODE_QCIF_176_144, SUBSAMPLING,
+		OV5640_PIXEL_RATE_48M,
+		176, 1896, 144, 984,
+		ov5640_setting_QCIF_176_144,
+		ARRAY_SIZE(ov5640_setting_QCIF_176_144),
+		OV5640_30_FPS
+	}, {
+		/* 320x240 */
+		OV5640_MODE_QVGA_320_240, SUBSAMPLING,
+		OV5640_PIXEL_RATE_48M,
+		320, 1896, 240, 984,
+		ov5640_setting_QVGA_320_240,
+		ARRAY_SIZE(ov5640_setting_QVGA_320_240),
+		OV5640_30_FPS
+	}, {
+		/* 640x480 */
+		OV5640_MODE_VGA_640_480, SUBSAMPLING,
+		OV5640_PIXEL_RATE_48M,
+		640, 1896, 480, 1080,
+		ov5640_setting_VGA_640_480,
+		ARRAY_SIZE(ov5640_setting_VGA_640_480),
+		OV5640_60_FPS
+	}, {
+		/* 720x480 */
+		OV5640_MODE_NTSC_720_480, SUBSAMPLING,
+		OV5640_PIXEL_RATE_96M,
+		720, 1896, 480, 984,
+		ov5640_setting_NTSC_720_480,
+		ARRAY_SIZE(ov5640_setting_NTSC_720_480),
+		OV5640_30_FPS
+	}, {
+		/* 720x576 */
+		OV5640_MODE_PAL_720_576, SUBSAMPLING,
+		OV5640_PIXEL_RATE_96M,
+		720, 1896, 576, 984,
+		ov5640_setting_PAL_720_576,
+		ARRAY_SIZE(ov5640_setting_PAL_720_576),
+		OV5640_30_FPS
+	}, {
+		/* 1024x768 */
+		OV5640_MODE_XGA_1024_768, SUBSAMPLING,
+		OV5640_PIXEL_RATE_96M,
+		1024, 1896, 768, 1080,
+		ov5640_setting_XGA_1024_768,
+		ARRAY_SIZE(ov5640_setting_XGA_1024_768),
+		OV5640_30_FPS
+	}, {
+		/* 1280x720 */
+		OV5640_MODE_720P_1280_720, SUBSAMPLING,
+		OV5640_PIXEL_RATE_124M,
+		1280, 1892, 720, 740,
+		ov5640_setting_720P_1280_720,
+		ARRAY_SIZE(ov5640_setting_720P_1280_720),
+		OV5640_30_FPS
+	}, {
+		/* 1920x1080 */
+		OV5640_MODE_1080P_1920_1080, SCALING,
+		OV5640_PIXEL_RATE_148M,
+		1920, 2500, 1080, 1120,
+		ov5640_setting_1080P_1920_1080,
+		ARRAY_SIZE(ov5640_setting_1080P_1920_1080),
+		OV5640_30_FPS
+	}, {
+		/* 2592x1944 */
+		OV5640_MODE_QSXGA_2592_1944, SCALING,
+		OV5640_PIXEL_RATE_168M,
+		2592, 2844, 1944, 1968,
+		ov5640_setting_QSXGA_2592_1944,
+		ARRAY_SIZE(ov5640_setting_QSXGA_2592_1944),
+		OV5640_15_FPS
+	},
 };
 
 static int ov5640_init_slave_id(struct ov5640_dev *sensor)
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 03/27] media: ov5640: Add ov5640_is_csi2() function
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 01/27] media: ov5640: Add pixel rate to modes Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 02/27] media: ov5604: Re-arrange modes definition Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 04/27] media: ov5640: Associate bpp with formats Jacopo Mondi
                   ` (23 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Checking if the sensor is used in DVP or MIPI CSI-2 mode is a repeated
pattern which is about to be repeated more often.

Provide an inline function to shortcut that.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index d8285be2bfa1..238f7b182ed4 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -294,6 +294,11 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
 			     ctrls.handler)->sd;
 }
 
+static inline bool ov5640_is_csi2(const struct ov5640_dev *sensor)
+{
+	return sensor->ep.bus_type == V4L2_MBUS_CSI2_DPHY;
+}
+
 /*
  * FIXME: all of these register tables are likely filled with
  * entries that set the register to their power-on default values,
@@ -1208,7 +1213,7 @@ static int ov5640_load_regs(struct ov5640_dev *sensor,
 		/* remain in power down mode for DVP */
 		if (regs->reg_addr == OV5640_REG_SYS_CTRL0 &&
 		    val == OV5640_REG_SYS_CTRL0_SW_PWUP &&
-		    sensor->ep.bus_type != V4L2_MBUS_CSI2_DPHY)
+		    !ov5640_is_csi2(sensor))
 			continue;
 
 		if (mask)
@@ -1843,7 +1848,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor)
 	 * the same rate than YUV, so we can just use 16 bpp all the time.
 	 */
 	rate = ov5640_calc_pixel_rate(sensor) * 16;
-	if (sensor->ep.bus_type == V4L2_MBUS_CSI2_DPHY) {
+	if (ov5640_is_csi2(sensor)) {
 		rate = rate / sensor->ep.bus.mipi_csi2.num_data_lanes;
 		ret = ov5640_set_mipi_pclk(sensor, rate);
 	} else {
@@ -3020,7 +3025,7 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int enable)
 			sensor->pending_fmt_change = false;
 		}
 
-		if (sensor->ep.bus_type == V4L2_MBUS_CSI2_DPHY)
+		if (ov5640_is_csi2(sensor))
 			ret = ov5640_set_stream_mipi(sensor, enable);
 		else
 			ret = ov5640_set_stream_dvp(sensor, enable);
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 04/27] media: ov5640: Associate bpp with formats
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (2 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 03/27] media: ov5640: Add ov5640_is_csi2() function Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 05/27] media: ov5640: Add LINK_FREQ control Jacopo Mondi
                   ` (22 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Associate the bit depth to each format supported by the sensor.

The bpp will be used to calculate the line length.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 73 ++++++++++++++++++++++++++++++--------
 1 file changed, 59 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 238f7b182ed4..6ceca55e2d9d 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -150,24 +150,69 @@ enum ov5640_format_mux {
 	OV5640_FMT_MUX_RAW_CIP,
 };
 
-struct ov5640_pixfmt {
+static const struct ov5640_pixfmt {
 	u32 code;
 	u32 colorspace;
+	u8 bpp;
+} ov5640_formats[] = {
+	{
+		.code = MEDIA_BUS_FMT_JPEG_1X8,
+		.colorspace = V4L2_COLORSPACE_JPEG,
+		.bpp = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_UYVY8_2X8,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_UYVY8_1X16,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_YUYV8_2X8,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_YUYV8_1X16,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_RGB565_2X8_LE,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_RGB565_2X8_BE,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 8,
+	}, {
+		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 8
+	}, {
+		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 8,
+	}, {
+		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 8,
+	},
 };
 
-static const struct ov5640_pixfmt ov5640_formats[] = {
-	{ MEDIA_BUS_FMT_JPEG_1X8, V4L2_COLORSPACE_JPEG, },
-	{ MEDIA_BUS_FMT_UYVY8_2X8, V4L2_COLORSPACE_SRGB, },
-	{ MEDIA_BUS_FMT_UYVY8_1X16, V4L2_COLORSPACE_SRGB, },
-	{ MEDIA_BUS_FMT_YUYV8_2X8, V4L2_COLORSPACE_SRGB, },
-	{ MEDIA_BUS_FMT_YUYV8_1X16, V4L2_COLORSPACE_SRGB, },
-	{ MEDIA_BUS_FMT_RGB565_2X8_LE, V4L2_COLORSPACE_SRGB, },
-	{ MEDIA_BUS_FMT_RGB565_2X8_BE, V4L2_COLORSPACE_SRGB, },
-	{ MEDIA_BUS_FMT_SBGGR8_1X8, V4L2_COLORSPACE_SRGB, },
-	{ MEDIA_BUS_FMT_SGBRG8_1X8, V4L2_COLORSPACE_SRGB, },
-	{ MEDIA_BUS_FMT_SGRBG8_1X8, V4L2_COLORSPACE_SRGB, },
-	{ MEDIA_BUS_FMT_SRGGB8_1X8, V4L2_COLORSPACE_SRGB, },
-};
+static u32 ov5640_code_to_bpp(u32 code)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(ov5640_formats); ++i) {
+		if (ov5640_formats[i].code == code)
+			return ov5640_formats[i].bpp;
+	}
+
+	return 0;
+}
 
 /*
  * FIXME: remove this when a subdev API becomes available
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 05/27] media: ov5640: Add LINK_FREQ control
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (3 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 04/27] media: ov5640: Associate bpp with formats Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 06/27] media: ov5640: Update pixel_rate and link_freq Jacopo Mondi
                   ` (21 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Add the V4L2_CID_LINK_FREQ control to the ov5640 driver.
Make the control read-only for the moment.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 6ceca55e2d9d..b1a282f453e3 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -141,6 +141,24 @@ static const u32 ov5640_pixel_rates[] = {
 	[OV5640_PIXEL_RATE_48M] = 48000000,
 };
 
+/*
+ * MIPI CSI-2 link frequencies.
+ *
+ * Derived from the above defined pixel rate for bpp = (8, 16, 24) and
+ * data_lanes = (1, 2)
+ *
+ * link_freq = (pixel_rate * bpp) / (2 * data_lanes)
+ */
+static const s64 ov5640_csi2_link_freqs[] = {
+	992000000, 888000000, 768000000, 744000000, 672000000, 672000000,
+	592000000, 592000000, 576000000, 576000000, 496000000, 496000000,
+	384000000, 384000000, 384000000, 336000000, 296000000, 288000000,
+	248000000, 192000000, 192000000, 192000000, 96000000,
+};
+
+/* Link freq for default mode: UYVY 16 bpp, 2 data lanes. */
+#define OV5640_DEFAULT_LINK_FREQ	13
+
 enum ov5640_format_mux {
 	OV5640_FMT_MUX_YUV422 = 0,
 	OV5640_FMT_MUX_RGB,
@@ -270,6 +288,7 @@ struct ov5640_mode_info {
 struct ov5640_ctrls {
 	struct v4l2_ctrl_handler handler;
 	struct v4l2_ctrl *pixel_rate;
+	struct v4l2_ctrl *link_freq;
 	struct {
 		struct v4l2_ctrl *auto_exp;
 		struct v4l2_ctrl *exposure;
@@ -2867,6 +2886,12 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
 				      ov5640_pixel_rates[0], 1,
 				      ov5640_pixel_rates[mode->pixel_rate]);
 
+	ctrls->link_freq = v4l2_ctrl_new_int_menu(hdl, ops,
+					V4L2_CID_LINK_FREQ,
+					ARRAY_SIZE(ov5640_csi2_link_freqs) - 1,
+					OV5640_DEFAULT_LINK_FREQ,
+					ov5640_csi2_link_freqs);
+
 	/* Auto/manual white balance */
 	ctrls->auto_wb = v4l2_ctrl_new_std(hdl, ops,
 					   V4L2_CID_AUTO_WHITE_BALANCE,
@@ -2915,6 +2940,7 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
 	}
 
 	ctrls->pixel_rate->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+	ctrls->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 	ctrls->gain->flags |= V4L2_CTRL_FLAG_VOLATILE;
 	ctrls->exposure->flags |= V4L2_CTRL_FLAG_VOLATILE;
 
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 06/27] media: ov5640: Update pixel_rate and link_freq
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (4 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 05/27] media: ov5640: Add LINK_FREQ control Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 07/27] media: ov5640: Rework CSI-2 clock tree Jacopo Mondi
                   ` (20 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

After having set a new format re-calculate the pixel_rate and link_freq
control values and update them when in MIPI mode.

Take into account the limitation of the link frequency having to be
strictly smaller than 1GHz when computing the desired link_freq, and
adjust the resulting pixel_rate acounting for the clock tree
configuration.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 66 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index b1a282f453e3..17ad815f7bdf 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -31,6 +31,8 @@
 
 #define OV5640_DEFAULT_SLAVE_ID 0x3c
 
+#define OV5640_LINK_RATE_MAX		490000000U
+
 #define OV5640_REG_SYS_RESET02		0x3002
 #define OV5640_REG_SYS_CLOCK_ENABLE02	0x3006
 #define OV5640_REG_SYS_CTRL0		0x3008
@@ -2412,6 +2414,66 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
+{
+	const struct ov5640_mode_info *mode = sensor->current_mode;
+	struct v4l2_mbus_framefmt *fmt = &sensor->fmt;
+	enum ov5640_pixel_rate_id pixel_rate_id = mode->pixel_rate;
+	unsigned int i = 0;
+	u32 pixel_rate;
+	s64 link_freq;
+	u32 num_lanes;
+	u32 bpp;
+
+	/*
+	 * Update the pixel rate control value.
+	 *
+	 * For DVP mode, maintain the pixel rate calculation using fixed FPS.
+	 */
+	if (!ov5640_is_csi2(sensor)) {
+		__v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate,
+					 ov5640_calc_pixel_rate(sensor));
+
+		return 0;
+	}
+
+	/*
+	 * The MIPI CSI-2 link frequency should comply with the CSI-2
+	 * specification and be lower than 1GHz.
+	 *
+	 * Start from the suggested pixel_rate for the current mode and
+	 * progressively slow it down if it exceeds 1GHz.
+	 */
+	num_lanes = sensor->ep.bus.mipi_csi2.num_data_lanes;
+	bpp = ov5640_code_to_bpp(fmt->code);
+	do {
+		pixel_rate = ov5640_pixel_rates[pixel_rate_id];
+		link_freq = pixel_rate * bpp / (2 * num_lanes);
+	} while (link_freq >= 1000000000U &&
+		 ++pixel_rate_id < OV5640_NUM_PIXEL_RATES);
+
+	/*
+	 * Higher link rates require the clock tree to be programmed with
+	 * 'mipi_div' = 1; this has the effect of halving the actual output
+	 * pixel rate in the MIPI domain.
+	 *
+	 * Adjust the pixel rate control value to report it correctly to
+	 * userspace.
+	 */
+	if (link_freq > OV5640_LINK_RATE_MAX)
+		pixel_rate /= 2;
+
+	for (i = 0; i < ARRAY_SIZE(ov5640_csi2_link_freqs); ++i) {
+		if (ov5640_csi2_link_freqs[i] == link_freq)
+			break;
+	}
+
+	__v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate, pixel_rate);
+	__v4l2_ctrl_s_ctrl(sensor->ctrls.link_freq, i);
+
+	return 0;
+}
+
 static int ov5640_set_fmt(struct v4l2_subdev *sd,
 			  struct v4l2_subdev_state *sd_state,
 			  struct v4l2_subdev_format *format)
@@ -2451,8 +2513,8 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
 	/* update format even if code is unchanged, resolution might change */
 	sensor->fmt = *mbus_fmt;
 
-	__v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate,
-				 ov5640_calc_pixel_rate(sensor));
+	ov5640_update_pixel_rate(sensor);
+
 out:
 	mutex_unlock(&sensor->lock);
 	return ret;
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 07/27] media: ov5640: Rework CSI-2 clock tree
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (5 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 06/27] media: ov5640: Update pixel_rate and link_freq Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 08/27] media: ov5640: Rework timings programming Jacopo Mondi
                   ` (19 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Re-work the ov5640_set_mipi_pclk() function to calculate the
PLL configuration using the pixel_rate and link_freq values set at
s_fmt time.

Rework the DVP clock mode settings to calculate the pixel clock
internally and remove the assumption on the 16bpp format.

Tested in MIPI CSI-2 mode with 1 and 2 data lanes with:
- all the sensor supported resolutions in UYVY and RGB565 formats.
- resolutions >= 1280x720 in RAW Bayer format.
- resolutions < 1280x720 in RGB888 format.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 192 ++++++++++++++++++-------------------
 1 file changed, 92 insertions(+), 100 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 17ad815f7bdf..65851b27d830 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -90,6 +90,7 @@
 #define OV5640_REG_POLARITY_CTRL00	0x4740
 #define OV5640_REG_MIPI_CTRL00		0x4800
 #define OV5640_REG_DEBUG_MODE		0x4814
+#define OV5640_REG_PCLK_PERIOD		0x4837
 #define OV5640_REG_ISP_FORMAT_MUX_CTRL	0x501f
 #define OV5640_REG_PRE_ISP_TEST_SET1	0x503d
 #define OV5640_REG_SDE_CTRL0		0x5580
@@ -925,20 +926,10 @@ static int ov5640_mod_reg(struct ov5640_dev *sensor, u16 reg,
  *                                +-----+-----+
  *                                       +------------> PCLK
  *
- * This is deviating from the datasheet at least for the register
- * 0x3108, since it's said here that the PCLK would be clocked from
- * the PLL.
- *
- * There seems to be also (unverified) constraints:
+ * There seems to be also constraints:
  *  - the PLL pre-divider output rate should be in the 4-27MHz range
  *  - the PLL multiplier output rate should be in the 500-1000MHz range
  *  - PCLK >= SCLK * 2 in YUV, >= SCLK in Raw or JPEG
- *
- * In the two latter cases, these constraints are met since our
- * factors are hardcoded. If we were to change that, we would need to
- * take this into account. The only varying parts are the PLL
- * multiplier and the system clock divider, which are shared between
- * all these clocks so won't cause any issue.
  */
 
 /*
@@ -957,13 +948,6 @@ static int ov5640_mod_reg(struct ov5640_dev *sensor, u16 reg,
 #define OV5640_SYSDIV_MIN	1
 #define OV5640_SYSDIV_MAX	16
 
-/*
- * Hardcode these values for scaler and non-scaler modes.
- * FIXME: to be re-calcualted for 1 data lanes setups
- */
-#define OV5640_MIPI_DIV_PCLK	2
-#define OV5640_MIPI_DIV_SCLK	1
-
 /*
  * This is supposed to be ranging from 1 to 2, but the value is always
  * set to 2 in the vendor kernels.
@@ -1073,70 +1057,79 @@ static unsigned long ov5640_calc_sys_clk(struct ov5640_dev *sensor,
 /*
  * ov5640_set_mipi_pclk() - Calculate the clock tree configuration values
  *			    for the MIPI CSI-2 output.
- *
- * @rate: The requested bandwidth per lane in bytes per second.
- *	  'Bandwidth Per Lane' is calculated as:
- *	  bpl = HTOT * VTOT * FPS * bpp / num_lanes;
- *
- * This function use the requested bandwidth to calculate:
- * - sample_rate = bpl / (bpp / num_lanes);
- *	         = bpl / (PLL_RDIV * BIT_DIV * PCLK_DIV * MIPI_DIV / num_lanes);
- *
- * - mipi_sclk   = bpl / MIPI_DIV / 2; ( / 2 is for CSI-2 DDR)
- *
- * with these fixed parameters:
- *	PLL_RDIV	= 2;
- *	BIT_DIVIDER	= 2; (MIPI_BIT_MODE == 8 ? 2 : 2,5);
- *	PCLK_DIV	= 1;
- *
- * The MIPI clock generation differs for modes that use the scaler and modes
- * that do not. In case the scaler is in use, the MIPI_SCLK generates the MIPI
- * BIT CLk, and thus:
- *
- * - mipi_sclk = bpl / MIPI_DIV / 2;
- *   MIPI_DIV = 1;
- *
- * For modes that do not go through the scaler, the MIPI BIT CLOCK is generated
- * from the pixel clock, and thus:
- *
- * - sample_rate = bpl / (bpp / num_lanes);
- *	         = bpl / (2 * 2 * 1 * MIPI_DIV / num_lanes);
- *		 = bpl / (4 * MIPI_DIV / num_lanes);
- * - MIPI_DIV	 = bpp / (4 * num_lanes);
- *
- * FIXME: this have been tested with 16bpp and 2 lanes setup only.
- * MIPI_DIV is fixed to value 2, but it -might- be changed according to the
- * above formula for setups with 1 lane or image formats with different bpp.
- *
- * FIXME: this deviates from the sensor manual documentation which is quite
- * thin on the MIPI clock tree generation part.
  */
-static int ov5640_set_mipi_pclk(struct ov5640_dev *sensor,
-				unsigned long rate)
+static int ov5640_set_mipi_pclk(struct ov5640_dev *sensor)
 {
-	const struct ov5640_mode_info *mode = sensor->current_mode;
+	u8 bit_div, mipi_div, pclk_div, sclk_div, sclk2x_div, root_div;
+	struct v4l2_mbus_framefmt *fmt = &sensor->fmt;
 	u8 prediv, mult, sysdiv;
-	u8 mipi_div;
+	unsigned long sysclk;
+	unsigned long sample_rate;
+	u8 pclk_period;
+	s64 link_freq;
 	int ret;
 
+	/* Use the link frequency computed at ov5640_update_pixel_rate() time. */
+	link_freq = ov5640_csi2_link_freqs[sensor->ctrls.link_freq->cur.val];
+
 	/*
-	 * 1280x720 is reported to use 'SUBSAMPLING' only,
-	 * but according to the sensor manual it goes through the
-	 * scaler before subsampling.
+	 * - mipi_div - Additional divider for the MIPI lane clock.
+	 *
+	 * Higher link frequencies would make sysclk > 1GHz.
+	 * Keep the sysclk low and do not divide in the MIPI domain.
 	 */
-	if (mode->dn_mode == SCALING ||
-	   (mode->id == OV5640_MODE_720P_1280_720))
-		mipi_div = OV5640_MIPI_DIV_SCLK;
+	if (link_freq > OV5640_LINK_RATE_MAX)
+		mipi_div = 1;
 	else
-		mipi_div = OV5640_MIPI_DIV_PCLK;
+		mipi_div = 2;
 
-	ov5640_calc_sys_clk(sensor, rate, &prediv, &mult, &sysdiv);
+	sysclk = link_freq * mipi_div;
+	ov5640_calc_sys_clk(sensor, sysclk, &prediv, &mult, &sysdiv);
 
-	ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL0,
-			     0x0f, OV5640_PLL_CTRL0_MIPI_MODE_8BIT);
+	/*
+	 * Adjust PLL parameters to maintain the MIPI_SCLK-to-PCLK ratio.
+	 *
+	 * - root_div = 2 (fixed)
+	 * - bit_div : MIPI 8-bit = 2; MIPI 10-bit = 2.5
+	 * - pclk_div = 1 (fixed)
+	 * - p_div  = (2 lanes ? mipi_div : 2 * mipi_div)
+	 *
+	 * This results in the following MIPI_SCLK depending on the number
+	 * of lanes:
+	 *
+	 * - 2 lanes: MIPI_SCLK = (4 or 5) * PCLK
+	 * - 1 lanes: MIPI_SCLK = (8 or 10) * PCLK
+	 */
+	root_div = OV5640_PLL_CTRL3_PLL_ROOT_DIV_2;
+	bit_div =  OV5640_PLL_CTRL0_MIPI_MODE_8BIT;
+	pclk_div = ilog2(OV5640_PCLK_ROOT_DIV);
 
-	ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL1,
-			     0xff, sysdiv << 4 | mipi_div);
+	/*
+	 * Scaler clock:
+	 * - YUV: PCLK >= 2 * SCLK
+	 * - RAW or JPEG: PCLK >= SCLK
+	 * - sclk2x_div = sclk_div / 2
+	 *
+	 * TODO: test with JPEG.
+	 */
+	sclk_div = ilog2(OV5640_SCLK_ROOT_DIV);
+	sclk2x_div = ilog2(OV5640_SCLK2X_ROOT_DIV);
+
+	/*
+	 * Set the sample period expressed in ns with 1-bit decimal
+	 * (0x01=0.5ns).
+	 */
+	sample_rate = ov5640_pixel_rates[sensor->current_mode->pixel_rate]
+		    * (ov5640_code_to_bpp(fmt->code) / 8);
+	pclk_period = 2000000000U / sample_rate;
+
+	/* Program the clock tree registers. */
+	ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL0, 0x0f, bit_div);
+	if (ret)
+		return ret;
+
+	ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL1, 0xff,
+			     (sysdiv << 4) | mipi_div);
 	if (ret)
 		return ret;
 
@@ -1144,13 +1137,27 @@ static int ov5640_set_mipi_pclk(struct ov5640_dev *sensor,
 	if (ret)
 		return ret;
 
-	ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL3,
-			     0x1f, OV5640_PLL_CTRL3_PLL_ROOT_DIV_2 | prediv);
+	ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL3, 0x1f,
+			     root_div | prediv);
 	if (ret)
 		return ret;
 
-	return ov5640_mod_reg(sensor, OV5640_REG_SYS_ROOT_DIVIDER,
-			      0x30, OV5640_PLL_SYS_ROOT_DIVIDER_BYPASS);
+	ret = ov5640_mod_reg(sensor, OV5640_REG_SYS_ROOT_DIVIDER, 0x3f,
+			     (pclk_div << 4) | (sclk2x_div << 2) | sclk_div);
+	if (ret)
+		return ret;
+
+	return ov5640_write_reg(sensor, OV5640_REG_PCLK_PERIOD, pclk_period);
+}
+
+static u32 ov5640_calc_pixel_rate(struct ov5640_dev *sensor)
+{
+	u32 rate;
+
+	rate = sensor->current_mode->vtot * sensor->current_mode->htot;
+	rate *= ov5640_framerates[sensor->current_fr];
+
+	return rate;
 }
 
 static unsigned long ov5640_calc_pclk(struct ov5640_dev *sensor,
@@ -1170,11 +1177,16 @@ static unsigned long ov5640_calc_pclk(struct ov5640_dev *sensor,
 	return _rate / *pll_rdiv / *bit_div / *pclk_div;
 }
 
-static int ov5640_set_dvp_pclk(struct ov5640_dev *sensor, unsigned long rate)
+static int ov5640_set_dvp_pclk(struct ov5640_dev *sensor)
 {
 	u8 prediv, mult, sysdiv, pll_rdiv, bit_div, pclk_div;
+	u32 rate;
 	int ret;
 
+	rate = ov5640_calc_pixel_rate(sensor);
+	rate *= ov5640_code_to_bpp(sensor->fmt.code);
+	rate /= sensor->ep.bus.parallel.bus_width;
+
 	ov5640_calc_pclk(sensor, rate, &prediv, &mult, &sysdiv, &pll_rdiv,
 			 &bit_div, &pclk_div);
 
@@ -1699,16 +1711,6 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
 	return mode;
 }
 
-static u64 ov5640_calc_pixel_rate(struct ov5640_dev *sensor)
-{
-	u64 rate;
-
-	rate = sensor->current_mode->vtot * sensor->current_mode->htot;
-	rate *= ov5640_framerates[sensor->current_fr];
-
-	return rate;
-}
-
 /*
  * sensor changes between scaling and subsampling, go through
  * exposure calculation
@@ -1890,7 +1892,6 @@ static int ov5640_set_mode(struct ov5640_dev *sensor)
 	enum ov5640_downsize_mode dn_mode, orig_dn_mode;
 	bool auto_gain = sensor->ctrls.auto_gain->val == 1;
 	bool auto_exp =  sensor->ctrls.auto_exp->val == V4L2_EXPOSURE_AUTO;
-	unsigned long rate;
 	int ret;
 
 	dn_mode = mode->dn_mode;
@@ -1909,19 +1910,10 @@ static int ov5640_set_mode(struct ov5640_dev *sensor)
 			goto restore_auto_gain;
 	}
 
-	/*
-	 * All the formats we support have 16 bits per pixel, seems to require
-	 * the same rate than YUV, so we can just use 16 bpp all the time.
-	 */
-	rate = ov5640_calc_pixel_rate(sensor) * 16;
-	if (ov5640_is_csi2(sensor)) {
-		rate = rate / sensor->ep.bus.mipi_csi2.num_data_lanes;
-		ret = ov5640_set_mipi_pclk(sensor, rate);
-	} else {
-		rate = rate / sensor->ep.bus.parallel.bus_width;
-		ret = ov5640_set_dvp_pclk(sensor, rate);
-	}
-
+	if (ov5640_is_csi2(sensor))
+		ret = ov5640_set_mipi_pclk(sensor);
+	else
+		ret = ov5640_set_dvp_pclk(sensor);
 	if (ret < 0)
 		return 0;
 
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 08/27] media: ov5640: Rework timings programming
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (6 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 07/27] media: ov5640: Rework CSI-2 clock tree Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 09/27] media: ov5640: Fix 720x480 in RGB888 mode Jacopo Mondi
                   ` (18 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

The current definition of a sensor mode defines timings as follows:

- hact, vact: Visible width and height
- htot, vtot: Total sizes including blankings

This makes difficult to clearly separate the visible sizes from the
blankings and to make the vertical blanking programmable.

Rework the sensor modes sizes definition to:
- Report the analog crop sizes
- Report the visible crop size
- Report the total pixels per line as HBLANK is fixed
- Report the VBLANK value to make it programmable

Also modify the ov5640_set_timings() function to program all the
windowing registers are remove them from the per-mode register-value
tables.

Do not change the timing values from the ones reported in the register
tables to maintain bisectability.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 443 +++++++++++++++++++++++++------------
 1 file changed, 298 insertions(+), 145 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 65851b27d830..2198224e7439 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -61,10 +61,16 @@
 #define OV5640_REG_AEC_PK_MANUAL	0x3503
 #define OV5640_REG_AEC_PK_REAL_GAIN	0x350a
 #define OV5640_REG_AEC_PK_VTS		0x350c
+#define OV5640_REG_TIMING_HS		0x3800
+#define OV5640_REG_TIMING_VS		0x3802
+#define OV5640_REG_TIMING_HW		0x3804
+#define OV5640_REG_TIMING_VH		0x3806
 #define OV5640_REG_TIMING_DVPHO		0x3808
 #define OV5640_REG_TIMING_DVPVO		0x380a
 #define OV5640_REG_TIMING_HTS		0x380c
 #define OV5640_REG_TIMING_VTS		0x380e
+#define OV5640_REG_TIMING_HOFFS		0x3810
+#define OV5640_REG_TIMING_VOFFS		0x3812
 #define OV5640_REG_TIMING_TC_REG20	0x3820
 #define OV5640_REG_TIMING_TC_REG21	0x3821
 #define OV5640_REG_AEC_CTRL00		0x3a00
@@ -279,12 +285,17 @@ struct ov5640_mode_info {
 	enum ov5640_mode_id id;
 	enum ov5640_downsize_mode dn_mode;
 	enum ov5640_pixel_rate_id pixel_rate;
-	u32 hact;
+	/* Analog crop rectangle. */
+	struct v4l2_rect analog_crop;
+	/* Visibile crop: from analog crop top-left corner. */
+	struct v4l2_rect crop;
+	/* Total pixels per line: crop.width + fixed hblank. */
 	u32 htot;
-	u32 vact;
-	u32 vtot;
+	/* Default vertical blanking: frame height = crop.height + vblank. */
+	u32 vblank_def;
 	const struct reg_value *reg_data;
 	u32 reg_data_size;
+	/* DVP only; ignored in MIPI mode. */
 	u32 max_fps;
 };
 
@@ -390,11 +401,7 @@ static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
 	{0x3c06, 0x00, 0, 0}, {0x3c07, 0x08, 0, 0}, {0x3c08, 0x00, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
+	{0x3815, 0x31, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
 	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
@@ -462,11 +469,7 @@ static const struct reg_value ov5640_setting_VGA_640_480[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
+	{0x3815, 0x31, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
 	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
@@ -481,11 +484,7 @@ static const struct reg_value ov5640_setting_XGA_1024_768[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
+	{0x3815, 0x31, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
 	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
@@ -500,11 +499,7 @@ static const struct reg_value ov5640_setting_QVGA_320_240[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
+	{0x3815, 0x31, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
 	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
@@ -519,11 +514,7 @@ static const struct reg_value ov5640_setting_QQVGA_160_120[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
+	{0x3815, 0x31, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
 	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
@@ -537,11 +528,7 @@ static const struct reg_value ov5640_setting_QCIF_176_144[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
+	{0x3815, 0x31, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
 	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
@@ -556,11 +543,7 @@ static const struct reg_value ov5640_setting_NTSC_720_480[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x3c, 0, 0},
+	{0x3815, 0x31, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
 	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
@@ -575,11 +558,7 @@ static const struct reg_value ov5640_setting_PAL_720_576[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x38, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
+	{0x3815, 0x31, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
 	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
@@ -594,11 +573,7 @@ static const struct reg_value ov5640_setting_720P_1280_720[] = {
 	{0x3c07, 0x07, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0xfa, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x06, 0, 0}, {0x3807, 0xa9, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
+	{0x3815, 0x31, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x02, 0, 0},
 	{0x3a03, 0xe4, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0xbc, 0, 0},
@@ -613,11 +588,7 @@ static const struct reg_value ov5640_setting_1080P_1920_1080[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x11, 0, 0},
-	{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
+	{0x3815, 0x11, 0, 0},
 	{0x3618, 0x04, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x21, 0, 0},
 	{0x3709, 0x12, 0, 0}, {0x370c, 0x00, 0, 0}, {0x3a02, 0x03, 0, 0},
 	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
@@ -628,9 +599,6 @@ static const struct reg_value ov5640_setting_1080P_1920_1080[] = {
 	{0x3824, 0x02, 0, 0}, {0x5001, 0x83, 0, 0},
 	{0x3c07, 0x07, 0, 0}, {0x3c08, 0x00, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3800, 0x01, 0, 0}, {0x3801, 0x50, 0, 0}, {0x3802, 0x01, 0, 0},
-	{0x3803, 0xb2, 0, 0}, {0x3804, 0x08, 0, 0}, {0x3805, 0xef, 0, 0},
-	{0x3806, 0x05, 0, 0}, {0x3807, 0xf1, 0, 0},
 	{0x3612, 0x2b, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3a02, 0x04, 0, 0}, {0x3a03, 0x60, 0, 0}, {0x3a08, 0x01, 0, 0},
 	{0x3a09, 0x50, 0, 0}, {0x3a0a, 0x01, 0, 0}, {0x3a0b, 0x18, 0, 0},
@@ -644,11 +612,7 @@ static const struct reg_value ov5640_setting_QSXGA_2592_1944[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x11, 0, 0},
-	{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
+	{0x3815, 0x11, 0, 0},
 	{0x3618, 0x04, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x21, 0, 0},
 	{0x3709, 0x12, 0, 0}, {0x370c, 0x00, 0, 0}, {0x3a02, 0x03, 0, 0},
 	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
@@ -661,96 +625,250 @@ static const struct reg_value ov5640_setting_QSXGA_2592_1944[] = {
 
 /* power-on sensor init reg table */
 static const struct ov5640_mode_info ov5640_mode_init_data = {
-	0, SUBSAMPLING,
-	OV5640_PIXEL_RATE_96M,
-	640, 1896, 480, 984,
-	ov5640_init_setting_30fps_VGA,
-	ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
-	OV5640_30_FPS,
+		.id		= 0,
+		.dn_mode	= SUBSAMPLING,
+		.pixel_rate	= OV5640_PIXEL_RATE_96M,
+		.analog_crop = {
+			.left	= 0,
+			.top	= 4,
+			.width	= 2624,
+			.height	= 1944,
+		},
+		.crop = {
+			.left	= 16,
+			.top	= 6,
+			.width	= 640,
+			.height	= 480,
+		},
+		.htot		= 1896,
+		.vblank_def	= 504,
+		.reg_data	= ov5640_init_setting_30fps_VGA,
+		.reg_data_size	= ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
+		.max_fps	= OV5640_30_FPS
 };
 
 static const struct ov5640_mode_info
 ov5640_mode_data[OV5640_NUM_MODES] = {
 	{
 		/* 160x120 */
-		OV5640_MODE_QQVGA_160_120, SUBSAMPLING,
-		OV5640_PIXEL_RATE_48M,
-		160, 1896, 120, 984,
-		ov5640_setting_QQVGA_160_120,
-		ARRAY_SIZE(ov5640_setting_QQVGA_160_120),
-		OV5640_30_FPS
+		.id		= OV5640_MODE_QQVGA_160_120,
+		.dn_mode	= SUBSAMPLING,
+		.pixel_rate	= OV5640_PIXEL_RATE_48M,
+		.analog_crop = {
+			.left	= 0,
+			.top	= 4,
+			.width	= 2624,
+			.height	= 1944,
+		},
+		.crop = {
+			.left	= 16,
+			.top	= 6,
+			.width	= 160,
+			.height	= 120,
+		},
+		.htot		= 1896,
+		.vblank_def	= 864,
+		.reg_data	= ov5640_setting_QQVGA_160_120,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_QQVGA_160_120),
+		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 176x144 */
-		OV5640_MODE_QCIF_176_144, SUBSAMPLING,
-		OV5640_PIXEL_RATE_48M,
-		176, 1896, 144, 984,
-		ov5640_setting_QCIF_176_144,
-		ARRAY_SIZE(ov5640_setting_QCIF_176_144),
-		OV5640_30_FPS
+		.id		= OV5640_MODE_QCIF_176_144,
+		.dn_mode	= SUBSAMPLING,
+		.pixel_rate	= OV5640_PIXEL_RATE_48M,
+		.analog_crop = {
+			.left	= 0,
+			.top	= 4,
+			.width	= 2624,
+			.height	= 1944,
+		},
+		.crop = {
+			.left	= 16,
+			.top	= 6,
+			.width	= 176,
+			.height	= 144,
+		},
+		.htot		= 1896,
+		.vblank_def	= 840,
+		.reg_data	= ov5640_setting_QCIF_176_144,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_QCIF_176_144),
+		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 320x240 */
-		OV5640_MODE_QVGA_320_240, SUBSAMPLING,
-		OV5640_PIXEL_RATE_48M,
-		320, 1896, 240, 984,
-		ov5640_setting_QVGA_320_240,
-		ARRAY_SIZE(ov5640_setting_QVGA_320_240),
-		OV5640_30_FPS
+		.id		= OV5640_MODE_QVGA_320_240,
+		.dn_mode	= SUBSAMPLING,
+		.pixel_rate	= OV5640_PIXEL_RATE_48M,
+		.analog_crop = {
+			.left	= 0,
+			.top	= 4,
+			.width	= 2624,
+			.height	= 1944,
+		},
+		.crop = {
+			.left	= 16,
+			.top	= 6,
+			.width	= 320,
+			.height	= 240,
+		},
+		.htot		= 1896,
+		.vblank_def	= 744,
+		.reg_data	= ov5640_setting_QVGA_320_240,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_QVGA_320_240),
+		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 640x480 */
-		OV5640_MODE_VGA_640_480, SUBSAMPLING,
-		OV5640_PIXEL_RATE_48M,
-		640, 1896, 480, 1080,
-		ov5640_setting_VGA_640_480,
-		ARRAY_SIZE(ov5640_setting_VGA_640_480),
-		OV5640_60_FPS
+		.id		= OV5640_MODE_VGA_640_480,
+		.dn_mode	= SUBSAMPLING,
+		.pixel_rate	= OV5640_PIXEL_RATE_48M,
+		.analog_crop = {
+			.left	= 0,
+			.top	= 4,
+			.width	= 2624,
+			.height	= 1944,
+		},
+		.crop = {
+			.left	= 16,
+			.top	= 6,
+			.width	= 640,
+			.height	= 480,
+		},
+		.htot		= 1896,
+		.vblank_def	= 600,
+		.reg_data	= ov5640_setting_VGA_640_480,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_VGA_640_480),
+		.max_fps	= OV5640_60_FPS
 	}, {
 		/* 720x480 */
-		OV5640_MODE_NTSC_720_480, SUBSAMPLING,
-		OV5640_PIXEL_RATE_96M,
-		720, 1896, 480, 984,
-		ov5640_setting_NTSC_720_480,
-		ARRAY_SIZE(ov5640_setting_NTSC_720_480),
-		OV5640_30_FPS
+		.id		= OV5640_MODE_NTSC_720_480,
+		.dn_mode	= SUBSAMPLING,
+		.pixel_rate	= OV5640_PIXEL_RATE_96M,
+		.analog_crop = {
+			.left	= 0,
+			.top	= 4,
+			.width	= 2624,
+			.height	= 1944,
+		},
+		.crop = {
+			.left	= 16,
+			.top	= 60,
+			.width	= 720,
+			.height	= 480,
+		},
+		.htot		= 1896,
+		.vblank_def	= 504,
+		.reg_data	= ov5640_setting_NTSC_720_480,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_NTSC_720_480),
+		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 720x576 */
-		OV5640_MODE_PAL_720_576, SUBSAMPLING,
-		OV5640_PIXEL_RATE_96M,
-		720, 1896, 576, 984,
-		ov5640_setting_PAL_720_576,
-		ARRAY_SIZE(ov5640_setting_PAL_720_576),
-		OV5640_30_FPS
+		.id		= OV5640_MODE_PAL_720_576,
+		.dn_mode	= SUBSAMPLING,
+		.pixel_rate	= OV5640_PIXEL_RATE_96M,
+		.analog_crop = {
+			.left	= 0,
+			.top	= 4,
+			.width	= 2624,
+			.height	= 1944,
+		},
+		.crop = {
+			.left	= 56,
+			.top	= 6,
+			.width	= 720,
+			.height	= 576,
+		},
+		.htot		= 1896,
+		.vblank_def	= 408,
+		.reg_data	= ov5640_setting_PAL_720_576,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_PAL_720_576),
+		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 1024x768 */
-		OV5640_MODE_XGA_1024_768, SUBSAMPLING,
-		OV5640_PIXEL_RATE_96M,
-		1024, 1896, 768, 1080,
-		ov5640_setting_XGA_1024_768,
-		ARRAY_SIZE(ov5640_setting_XGA_1024_768),
-		OV5640_30_FPS
+		.id		= OV5640_MODE_XGA_1024_768,
+		.dn_mode	= SUBSAMPLING,
+		.pixel_rate	= OV5640_PIXEL_RATE_96M,
+		.analog_crop = {
+			.left	= 0,
+			.top	= 4,
+			.width	= 2624,
+			.height	= 1944,
+		},
+		.crop = {
+			.left	= 16,
+			.top	= 6,
+			.width	= 1024,
+			.height	= 768,
+		},
+		.htot		= 1896,
+		.vblank_def	= 312,
+		.reg_data	= ov5640_setting_XGA_1024_768,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_XGA_1024_768),
+		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 1280x720 */
-		OV5640_MODE_720P_1280_720, SUBSAMPLING,
-		OV5640_PIXEL_RATE_124M,
-		1280, 1892, 720, 740,
-		ov5640_setting_720P_1280_720,
-		ARRAY_SIZE(ov5640_setting_720P_1280_720),
-		OV5640_30_FPS
+		.id		= OV5640_MODE_720P_1280_720,
+		.dn_mode	= SUBSAMPLING,
+		.pixel_rate	= OV5640_PIXEL_RATE_124M,
+		.analog_crop = {
+			.left	= 0,
+			.top	= 250,
+			.width	= 2624,
+			.height	= 1456,
+		},
+		.crop = {
+			.left	= 16,
+			.top	= 4,
+			.width	= 1280,
+			.height	= 720,
+		},
+		.htot		= 1892,
+		.vblank_def	= 20,
+		.reg_data	= ov5640_setting_720P_1280_720,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_720P_1280_720),
+		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 1920x1080 */
-		OV5640_MODE_1080P_1920_1080, SCALING,
-		OV5640_PIXEL_RATE_148M,
-		1920, 2500, 1080, 1120,
-		ov5640_setting_1080P_1920_1080,
-		ARRAY_SIZE(ov5640_setting_1080P_1920_1080),
-		OV5640_30_FPS
+		.id		= OV5640_MODE_1080P_1920_1080,
+		.dn_mode	= SCALING,
+		.pixel_rate	= OV5640_PIXEL_RATE_148M,
+		.analog_crop = {
+			.left	= 336,
+			.top	= 434,
+			.width	= 1952,
+			.height	= 1088,
+		},
+		.crop = {
+			.left	= 16,
+			.top	= 4,
+			.width	= 1920,
+			.height	= 1080,
+		},
+		.htot		= 2500,
+		.vblank_def	= 40,
+		.reg_data	= ov5640_setting_1080P_1920_1080,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_1080P_1920_1080),
+		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 2592x1944 */
-		OV5640_MODE_QSXGA_2592_1944, SCALING,
-		OV5640_PIXEL_RATE_168M,
-		2592, 2844, 1944, 1968,
-		ov5640_setting_QSXGA_2592_1944,
-		ARRAY_SIZE(ov5640_setting_QSXGA_2592_1944),
-		OV5640_15_FPS
+		.id		= OV5640_MODE_QSXGA_2592_1944,
+		.dn_mode	= SCALING,
+		.pixel_rate	= OV5640_PIXEL_RATE_168M,
+		.analog_crop = {
+			.left	= 0,
+			.top	= 0,
+			.width	= 2624,
+			.height	= 1952,
+		},
+		.crop = {
+			.left	= 16,
+			.top	= 4,
+			.width	= 2592,
+			.height	= 1944,
+		},
+		.htot		= 2844,
+		.vblank_def	= 24,
+		.reg_data	= ov5640_setting_QSXGA_2592_1944,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_QSXGA_2592_1944),
+		.max_fps	= OV5640_15_FPS
 	},
 };
 
@@ -1154,7 +1272,8 @@ static u32 ov5640_calc_pixel_rate(struct ov5640_dev *sensor)
 {
 	u32 rate;
 
-	rate = sensor->current_mode->vtot * sensor->current_mode->htot;
+	rate = sensor->current_mode->htot
+	     * (sensor->current_mode->crop.height + sensor->current_mode->vblank_def);
 	rate *= ov5640_framerates[sensor->current_fr];
 
 	return rate;
@@ -1238,17 +1357,21 @@ static int ov5640_set_jpeg_timings(struct ov5640_dev *sensor,
 	if (ret < 0)
 		return ret;
 
-	ret = ov5640_write_reg16(sensor, OV5640_REG_VFIFO_HSIZE, mode->hact);
+	ret = ov5640_write_reg16(sensor, OV5640_REG_VFIFO_HSIZE,
+				 mode->crop.width);
 	if (ret < 0)
 		return ret;
 
-	return ov5640_write_reg16(sensor, OV5640_REG_VFIFO_VSIZE, mode->vact);
+	return ov5640_write_reg16(sensor, OV5640_REG_VFIFO_VSIZE,
+				  mode->crop.height);
 }
 
 /* download ov5640 settings to sensor through i2c */
 static int ov5640_set_timings(struct ov5640_dev *sensor,
 			      const struct ov5640_mode_info *mode)
 {
+	const struct v4l2_rect *analog_crop = &mode->analog_crop;
+	const struct v4l2_rect *crop = &mode->crop;
 	int ret;
 
 	if (sensor->fmt.code == MEDIA_BUS_FMT_JPEG_1X8) {
@@ -1257,11 +1380,37 @@ static int ov5640_set_timings(struct ov5640_dev *sensor,
 			return ret;
 	}
 
-	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_DVPHO, mode->hact);
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_HS, analog_crop->left);
+	if (ret < 0)
+		return ret;
+
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_VS, analog_crop->top);
+	if (ret < 0)
+		return ret;
+
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_HW,
+				 analog_crop->left + analog_crop->width - 1);
+	if (ret < 0)
+		return ret;
+
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_VH,
+				 analog_crop->top + analog_crop->height - 1);
+	if (ret < 0)
+		return ret;
+
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_HOFFS, crop->left);
+	if (ret < 0)
+		return ret;
+
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_VOFFS, crop->top);
+	if (ret < 0)
+		return ret;
+
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_DVPHO, crop->width);
 	if (ret < 0)
 		return ret;
 
-	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_DVPVO, mode->vact);
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_DVPVO, crop->height);
 	if (ret < 0)
 		return ret;
 
@@ -1269,7 +1418,12 @@ static int ov5640_set_timings(struct ov5640_dev *sensor,
 	if (ret < 0)
 		return ret;
 
-	return ov5640_write_reg16(sensor, OV5640_REG_TIMING_VTS, mode->vtot);
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_VTS,
+				 crop->height + mode->vblank_def);
+	if (ret < 0)
+		return ret;
+
+	return 0;
 }
 
 static int ov5640_load_regs(struct ov5640_dev *sensor,
@@ -1697,11 +1851,11 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
 
 	mode = v4l2_find_nearest_size(ov5640_mode_data,
 				      ARRAY_SIZE(ov5640_mode_data),
-				      hact, vact,
-				      width, height);
+				      crop.width, crop.height, width, height);
 
 	if (!mode ||
-	    (!nearest && (mode->hact != width || mode->vact != height)))
+	    (!nearest &&
+	     (mode->crop.width != width || mode->crop.height != height)))
 		return NULL;
 
 	/* Check to see if the current mode exceeds the max frame rate */
@@ -2385,8 +2539,8 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev *sd,
 	mode = ov5640_find_mode(sensor, fr, fmt->width, fmt->height, true);
 	if (!mode)
 		return -EINVAL;
-	fmt->width = mode->hact;
-	fmt->height = mode->vact;
+	fmt->width = mode->crop.width;
+	fmt->height = mode->crop.height;
 
 	if (new_mode)
 		*new_mode = mode;
@@ -3019,11 +3173,9 @@ static int ov5640_enum_frame_size(struct v4l2_subdev *sd,
 	if (fse->index >= OV5640_NUM_MODES)
 		return -EINVAL;
 
-	fse->min_width =
-		ov5640_mode_data[fse->index].hact;
+	fse->min_width = ov5640_mode_data[fse->index].crop.width;
 	fse->max_width = fse->min_width;
-	fse->min_height =
-		ov5640_mode_data[fse->index].vact;
+	fse->min_height = ov5640_mode_data[fse->index].crop.height;
 	fse->max_height = fse->min_height;
 
 	return 0;
@@ -3087,15 +3239,16 @@ static int ov5640_s_frame_interval(struct v4l2_subdev *sd,
 	mode = sensor->current_mode;
 
 	frame_rate = ov5640_try_frame_interval(sensor, &fi->interval,
-					       mode->hact, mode->vact);
+					       mode->crop.width,
+					       mode->crop.height);
 	if (frame_rate < 0) {
 		/* Always return a valid frame interval value */
 		fi->interval = sensor->frame_interval;
 		goto out;
 	}
 
-	mode = ov5640_find_mode(sensor, frame_rate, mode->hact,
-				mode->vact, true);
+	mode = ov5640_find_mode(sensor, frame_rate, mode->crop.width,
+				mode->crop.height, true);
 	if (!mode) {
 		ret = -EINVAL;
 		goto out;
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 09/27] media: ov5640: Fix 720x480 in RGB888 mode
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (7 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 08/27] media: ov5640: Rework timings programming Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 10/27] media: ov5640: Split DVP and CSI-2 timings Jacopo Mondi
                   ` (17 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Adjust the left crop of 720x480 to enable capture in RGB888 format,
which is otherwise broken.

The 56 pixels alignment has been copied from the 720x576 mode.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 2198224e7439..594adaf93c9d 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -749,7 +749,7 @@ ov5640_mode_data[OV5640_NUM_MODES] = {
 			.height	= 1944,
 		},
 		.crop = {
-			.left	= 16,
+			.left	= 56,
 			.top	= 60,
 			.width	= 720,
 			.height	= 480,
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 10/27] media: ov5640: Split DVP and CSI-2 timings
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (8 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 09/27] media: ov5640: Fix 720x480 in RGB888 mode Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 11/27] media: ov5640: Provide timings accessor Jacopo Mondi
                   ` (16 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Separate the timings for the DVP mode from the timings for the CSI-2
mode in the ov5640 modes definition.

The CSI-2 timings will deviate from the DVP ones as the clock tree is
calculated differently.

In CSI-2 mode change the analog crop rectangles as the OV5640 pixel
array is composed as:
- vertically: 16 dummy columns, 2592 valid ones and 16 dummy columns for
  a total of 2624 columns
- horizontally: 8 optical black lines, 6 dummy ones, 1944 valid and 6
  dummies for a total of 1964 lines

Adjust the analog crop rectangle in CSI-2 mode to:
- Skip the first 16 dummy columns
- Skip the first 14 black/dummy lines
- Pass the whole valid pixel array size to the ISP for all modes except
  1024x768, 720p and 1080p which are obtained by cropping the valid pixel
  array.

Tested in RGB565, UYVY and RGB888 modes in CSI-2 mode.

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

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 594adaf93c9d..4040ace2fd55 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -29,6 +29,13 @@
 #define OV5640_XCLK_MIN  6000000
 #define OV5640_XCLK_MAX 54000000
 
+#define OV5640_NATIVE_WIDTH		2624
+#define OV5640_NATIVE_HEIGHT		1964
+#define OV5640_PIXEL_ARRAY_TOP		14
+#define OV5640_PIXEL_ARRAY_LEFT		16
+#define OV5640_PIXEL_ARRAY_WIDTH	2592
+#define OV5640_PIXEL_ARRAY_HEIGHT	1944
+
 #define OV5640_DEFAULT_SLAVE_ID 0x3c
 
 #define OV5640_LINK_RATE_MAX		490000000U
@@ -281,24 +288,34 @@ struct reg_value {
 	u32 delay_ms;
 };
 
-struct ov5640_mode_info {
-	enum ov5640_mode_id id;
-	enum ov5640_downsize_mode dn_mode;
-	enum ov5640_pixel_rate_id pixel_rate;
+struct ov5640_timings {
 	/* Analog crop rectangle. */
 	struct v4l2_rect analog_crop;
 	/* Visibile crop: from analog crop top-left corner. */
 	struct v4l2_rect crop;
-	/* Total pixels per line: crop.width + fixed hblank. */
+	/* Total pixels per line: width + fixed hblank. */
 	u32 htot;
-	/* Default vertical blanking: frame height = crop.height + vblank. */
+	/* Default vertical blanking: frame height = height + vblank. */
 	u32 vblank_def;
-	const struct reg_value *reg_data;
-	u32 reg_data_size;
 	/* DVP only; ignored in MIPI mode. */
 	u32 max_fps;
 };
 
+struct ov5640_mode_info {
+	enum ov5640_mode_id id;
+	enum ov5640_downsize_mode dn_mode;
+	enum ov5640_pixel_rate_id pixel_rate;
+
+	unsigned int width;
+	unsigned int height;
+
+	struct ov5640_timings dvp_timings;
+	struct ov5640_timings csi2_timings;
+
+	const struct reg_value *reg_data;
+	u32 reg_data_size;
+};
+
 struct ov5640_ctrls {
 	struct v4l2_ctrl_handler handler;
 	struct v4l2_ctrl *pixel_rate;
@@ -628,247 +645,481 @@ static const struct ov5640_mode_info ov5640_mode_init_data = {
 		.id		= 0,
 		.dn_mode	= SUBSAMPLING,
 		.pixel_rate	= OV5640_PIXEL_RATE_96M,
-		.analog_crop = {
-			.left	= 0,
-			.top	= 4,
-			.width	= 2624,
-			.height	= 1944,
+		.width	= 640,
+		.height	= 480,
+		.dvp_timings = {
+			.analog_crop = {
+				.left	= 0,
+				.top	= 4,
+				.width	= 2624,
+				.height	= 1944,
+			},
+			.crop = {
+				.left	= 16,
+				.top	= 6,
+				.width	= 640,
+				.height	= 480,
+			},
+			.htot		= 1896,
+			.vblank_def	= 504,
+			.max_fps	= OV5640_30_FPS
 		},
-		.crop = {
-			.left	= 16,
-			.top	= 6,
-			.width	= 640,
-			.height	= 480,
+		.csi2_timings = {
+			.analog_crop = {
+				.left	= OV5640_PIXEL_ARRAY_LEFT,
+				.top	= OV5640_PIXEL_ARRAY_TOP,
+				.width	= OV5640_PIXEL_ARRAY_WIDTH,
+				.height	= OV5640_PIXEL_ARRAY_HEIGHT,
+			},
+			.crop = {
+				.left	= 2,
+				.top	= 4,
+				.width	= 640,
+				.height	= 480,
+			},
+			.htot		= 1896,
+			.vblank_def	= 504,
 		},
-		.htot		= 1896,
-		.vblank_def	= 504,
 		.reg_data	= ov5640_init_setting_30fps_VGA,
 		.reg_data_size	= ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
-		.max_fps	= OV5640_30_FPS
 };
 
-static const struct ov5640_mode_info
-ov5640_mode_data[OV5640_NUM_MODES] = {
+static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 	{
 		/* 160x120 */
 		.id		= OV5640_MODE_QQVGA_160_120,
 		.dn_mode	= SUBSAMPLING,
 		.pixel_rate	= OV5640_PIXEL_RATE_48M,
-		.analog_crop = {
-			.left	= 0,
-			.top	= 4,
-			.width	= 2624,
-			.height	= 1944,
+		.width		= 160,
+		.height		= 120,
+		.dvp_timings = {
+			.analog_crop = {
+				.left	= 0,
+				.top	= 4,
+				.width	= 2624,
+				.height	= 1944,
+			},
+			.crop = {
+				.left	= 16,
+				.top	= 6,
+				.width	= 160,
+				.height	= 120,
+			},
+			.htot		= 1896,
+			.vblank_def	= 864,
+			.max_fps	= OV5640_30_FPS
 		},
-		.crop = {
-			.left	= 16,
-			.top	= 6,
-			.width	= 160,
-			.height	= 120,
+		.csi2_timings = {
+			.analog_crop = {
+				/* Feed the full valid pixel array to the ISP. */
+				.left	= OV5640_PIXEL_ARRAY_LEFT,
+				.top	= OV5640_PIXEL_ARRAY_TOP,
+				.width	= OV5640_PIXEL_ARRAY_WIDTH,
+				.height	= OV5640_PIXEL_ARRAY_HEIGHT,
+			},
+			.crop = {
+				/* Maintain a minimum digital crop processing margins. */
+				.left	= 2,
+				.top	= 4,
+				.width	= 160,
+				.height	= 120,
+			},
+			.htot		= 1896,
+			.vblank_def	= 864,
 		},
-		.htot		= 1896,
-		.vblank_def	= 864,
 		.reg_data	= ov5640_setting_QQVGA_160_120,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_QQVGA_160_120),
-		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 176x144 */
 		.id		= OV5640_MODE_QCIF_176_144,
 		.dn_mode	= SUBSAMPLING,
 		.pixel_rate	= OV5640_PIXEL_RATE_48M,
-		.analog_crop = {
-			.left	= 0,
-			.top	= 4,
-			.width	= 2624,
-			.height	= 1944,
+		.width		= 176,
+		.height		= 144,
+		.dvp_timings = {
+			.analog_crop = {
+				.left	= 0,
+				.top	= 4,
+				.width	= 2624,
+				.height	= 1944,
+			},
+			.crop = {
+				.left	= 16,
+				.top	= 6,
+				.width	= 176,
+				.height	= 144,
+			},
+			.htot		= 1896,
+			.vblank_def	= 840,
+			.max_fps	= OV5640_30_FPS
 		},
-		.crop = {
-			.left	= 16,
-			.top	= 6,
-			.width	= 176,
-			.height	= 144,
+		.csi2_timings = {
+			.analog_crop = {
+				/* Feed the full valid pixel array to the ISP. */
+				.left	= OV5640_PIXEL_ARRAY_LEFT,
+				.top	= OV5640_PIXEL_ARRAY_TOP,
+				.width	= OV5640_PIXEL_ARRAY_WIDTH,
+				.height	= OV5640_PIXEL_ARRAY_HEIGHT,
+			},
+			.crop = {
+				/* Maintain a minimum digital crop processing margins. */
+				.left	= 2,
+				.top	= 4,
+				.width	= 176,
+				.height	= 144,
+			},
+			.htot		= 1896,
+			.vblank_def	= 840,
 		},
-		.htot		= 1896,
-		.vblank_def	= 840,
 		.reg_data	= ov5640_setting_QCIF_176_144,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_QCIF_176_144),
-		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 320x240 */
 		.id		= OV5640_MODE_QVGA_320_240,
 		.dn_mode	= SUBSAMPLING,
+		.width		= 320,
+		.height		= 240,
 		.pixel_rate	= OV5640_PIXEL_RATE_48M,
-		.analog_crop = {
-			.left	= 0,
-			.top	= 4,
-			.width	= 2624,
-			.height	= 1944,
+		.dvp_timings = {
+			.analog_crop = {
+				.left	= 0,
+				.top	= 4,
+				.width	= 2624,
+				.height	= 1944,
+			},
+			.crop = {
+				.left	= 16,
+				.top	= 6,
+				.width	= 320,
+				.height	= 240,
+			},
+			.htot		= 1896,
+			.vblank_def	= 744,
+			.max_fps	= OV5640_30_FPS
 		},
-		.crop = {
-			.left	= 16,
-			.top	= 6,
-			.width	= 320,
-			.height	= 240,
+		.csi2_timings = {
+			.analog_crop = {
+				/* Feed the full valid pixel array to the ISP. */
+				.left	= OV5640_PIXEL_ARRAY_LEFT,
+				.top	= OV5640_PIXEL_ARRAY_TOP,
+				.width	= OV5640_PIXEL_ARRAY_WIDTH,
+				.height	= OV5640_PIXEL_ARRAY_HEIGHT,
+			},
+			.crop = {
+				/* Maintain a minimum digital crop processing margins. */
+				.left	= 2,
+				.top	= 4,
+				.width	= 320,
+				.height	= 240,
+			},
+			.htot		= 1896,
+			.vblank_def	= 744,
 		},
-		.htot		= 1896,
-		.vblank_def	= 744,
 		.reg_data	= ov5640_setting_QVGA_320_240,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_QVGA_320_240),
-		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 640x480 */
 		.id		= OV5640_MODE_VGA_640_480,
 		.dn_mode	= SUBSAMPLING,
 		.pixel_rate	= OV5640_PIXEL_RATE_48M,
-		.analog_crop = {
-			.left	= 0,
-			.top	= 4,
-			.width	= 2624,
-			.height	= 1944,
+		.width		= 640,
+		.height		= 480,
+		.dvp_timings = {
+			.analog_crop = {
+				.left	= 0,
+				.top	= 4,
+				.width	= 2624,
+				.height	= 1944,
+			},
+			.crop = {
+				.left	= 16,
+				.top	= 6,
+				.width	= 640,
+				.height	= 480,
+			},
+			.htot		= 1896,
+			.vblank_def	= 600,
+			.max_fps	= OV5640_60_FPS
 		},
-		.crop = {
-			.left	= 16,
-			.top	= 6,
-			.width	= 640,
-			.height	= 480,
+		.csi2_timings = {
+			.analog_crop = {
+				/* Feed the full valid pixel array to the ISP. */
+				.left	= OV5640_PIXEL_ARRAY_LEFT,
+				.top	= OV5640_PIXEL_ARRAY_TOP,
+				.width	= OV5640_PIXEL_ARRAY_WIDTH,
+				.height	= OV5640_PIXEL_ARRAY_HEIGHT,
+			},
+			.crop = {
+				/* Maintain a minimum digital crop processing margins. */
+				.left	= 2,
+				.top	= 4,
+				.width	= 640,
+				.height	= 480,
+			},
+			.htot		= 1896,
+			.vblank_def	= 600,
 		},
-		.htot		= 1896,
-		.vblank_def	= 600,
 		.reg_data	= ov5640_setting_VGA_640_480,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_VGA_640_480),
-		.max_fps	= OV5640_60_FPS
 	}, {
 		/* 720x480 */
 		.id		= OV5640_MODE_NTSC_720_480,
 		.dn_mode	= SUBSAMPLING,
+		.width		= 720,
+		.height		= 480,
 		.pixel_rate	= OV5640_PIXEL_RATE_96M,
-		.analog_crop = {
-			.left	= 0,
-			.top	= 4,
-			.width	= 2624,
-			.height	= 1944,
+		.dvp_timings = {
+			.analog_crop = {
+				.left	= 0,
+				.top	= 4,
+				.width	= 2624,
+				.height	= 1944,
+			},
+			.crop = {
+				.left	= 56,
+				.top	= 60,
+				.width	= 720,
+				.height	= 480,
+			},
+			.htot		= 1896,
+			.vblank_def	= 504,
+			.max_fps	= OV5640_30_FPS
 		},
-		.crop = {
-			.left	= 56,
-			.top	= 60,
-			.width	= 720,
-			.height	= 480,
+		.csi2_timings = {
+			.analog_crop = {
+				/* Feed the full valid pixel array to the ISP. */
+				.left	= OV5640_PIXEL_ARRAY_LEFT,
+				.top	= OV5640_PIXEL_ARRAY_TOP,
+				.width	= OV5640_PIXEL_ARRAY_WIDTH,
+				.height	= OV5640_PIXEL_ARRAY_HEIGHT,
+			},
+			.crop = {
+				/* Maintain a minimum digital crop processing margins. */
+				.left	= 56,
+				.top	= 60,
+				.width	= 720,
+				.height	= 480,
+			},
+			.htot		= 1896,
+			.vblank_def	= 504,
 		},
-		.htot		= 1896,
-		.vblank_def	= 504,
 		.reg_data	= ov5640_setting_NTSC_720_480,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_NTSC_720_480),
-		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 720x576 */
 		.id		= OV5640_MODE_PAL_720_576,
 		.dn_mode	= SUBSAMPLING,
+		.width		= 720,
+		.height		= 576,
 		.pixel_rate	= OV5640_PIXEL_RATE_96M,
-		.analog_crop = {
-			.left	= 0,
-			.top	= 4,
-			.width	= 2624,
-			.height	= 1944,
+		.dvp_timings = {
+			.analog_crop = {
+				.left	= 0,
+				.top	= 4,
+				.width	= 2624,
+				.height	= 1944,
+			},
+			.crop = {
+				.left	= 56,
+				.top	= 6,
+				.width	= 720,
+				.height	= 576,
+			},
+			.htot		= 1896,
+			.vblank_def	= 408,
+			.max_fps	= OV5640_30_FPS
 		},
-		.crop = {
-			.left	= 56,
-			.top	= 6,
-			.width	= 720,
-			.height	= 576,
+		.csi2_timings = {
+			.analog_crop = {
+				/* Feed the full valid pixel array to the ISP. */
+				.left	= OV5640_PIXEL_ARRAY_LEFT,
+				.top	= OV5640_PIXEL_ARRAY_TOP,
+				.width	= OV5640_PIXEL_ARRAY_WIDTH,
+				.height	= OV5640_PIXEL_ARRAY_HEIGHT,
+			},
+			.crop = {
+				/* Maintain a minimum digital crop processing margins. */
+				.left	= 56,
+				.top	= 6,
+				.width	= 720,
+				.height	= 576,
+			},
+			.htot		= 1896,
+			.vblank_def	= 408,
 		},
-		.htot		= 1896,
-		.vblank_def	= 408,
 		.reg_data	= ov5640_setting_PAL_720_576,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_PAL_720_576),
-		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 1024x768 */
 		.id		= OV5640_MODE_XGA_1024_768,
 		.dn_mode	= SUBSAMPLING,
 		.pixel_rate	= OV5640_PIXEL_RATE_96M,
-		.analog_crop = {
-			.left	= 0,
-			.top	= 4,
-			.width	= 2624,
-			.height	= 1944,
+		.width		= 1024,
+		.height		= 768,
+		.dvp_timings = {
+			.analog_crop = {
+				.left	= 0,
+				.top	= 4,
+				.width	= 2624,
+				.height	= 1944,
+			},
+			.crop = {
+				.left	= 16,
+				.top	= 6,
+				.width	= 1024,
+				.height	= 768,
+			},
+			.htot		= 1896,
+			.vblank_def	= 312,
+			.max_fps	= OV5640_30_FPS
 		},
-		.crop = {
-			.left	= 16,
-			.top	= 6,
-			.width	= 1024,
-			.height	= 768,
+		.csi2_timings = {
+			.analog_crop = {
+				.left	= 0,
+				.top	= 4,
+				.width	= OV5640_NATIVE_WIDTH,
+				.height	= OV5640_PIXEL_ARRAY_HEIGHT,
+			},
+			.crop = {
+				.left	= 16,
+				.top	= 6,
+				.width	= 1024,
+				.height	= 768,
+			},
+			.htot		= 1896,
+			.vblank_def	= 312,
 		},
-		.htot		= 1896,
-		.vblank_def	= 312,
 		.reg_data	= ov5640_setting_XGA_1024_768,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_XGA_1024_768),
-		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 1280x720 */
 		.id		= OV5640_MODE_720P_1280_720,
 		.dn_mode	= SUBSAMPLING,
 		.pixel_rate	= OV5640_PIXEL_RATE_124M,
-		.analog_crop = {
-			.left	= 0,
-			.top	= 250,
-			.width	= 2624,
-			.height	= 1456,
+		.width		= 1280,
+		.height		= 720,
+		.dvp_timings = {
+			.analog_crop = {
+				.left	= 0,
+				.top	= 250,
+				.width	= 2624,
+				.height	= 1456,
+			},
+			.crop = {
+				.left	= 16,
+				.top	= 4,
+				.width	= 1280,
+				.height	= 720,
+			},
+			.htot		= 1892,
+			.vblank_def	= 20,
+			.max_fps	= OV5640_30_FPS
 		},
-		.crop = {
-			.left	= 16,
-			.top	= 4,
-			.width	= 1280,
-			.height	= 720,
+		.csi2_timings = {
+			.analog_crop = {
+				.left	= 0,
+				.top	= 250,
+				.width	= 2624,
+				.height	= 1456,
+			},
+			.crop = {
+				.left	= 16,
+				.top	= 4,
+				.width	= 1280,
+				.height	= 720,
+			},
+			.htot		= 1892,
+			.vblank_def	= 20,
 		},
-		.htot		= 1892,
-		.vblank_def	= 20,
 		.reg_data	= ov5640_setting_720P_1280_720,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_720P_1280_720),
-		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 1920x1080 */
 		.id		= OV5640_MODE_1080P_1920_1080,
 		.dn_mode	= SCALING,
 		.pixel_rate	= OV5640_PIXEL_RATE_148M,
-		.analog_crop = {
-			.left	= 336,
-			.top	= 434,
-			.width	= 1952,
-			.height	= 1088,
+		.width		= 1920,
+		.height		= 1080,
+		.dvp_timings = {
+			.analog_crop = {
+				.left	= 336,
+				.top	= 434,
+				.width	= 1952,
+				.height	= 1088,
+			},
+			.crop = {
+				.left	= 16,
+				.top	= 4,
+				.width	= 1920,
+				.height	= 1080,
+			},
+			.htot		= 2500,
+			.vblank_def	= 40,
+			.max_fps	= OV5640_30_FPS
 		},
-		.crop = {
-			.left	= 16,
-			.top	= 4,
-			.width	= 1920,
-			.height	= 1080,
+		.csi2_timings = {
+			.analog_crop = {
+				/* Crop the full valid pixel array in the center. */
+				.left	= 336,
+				.top	= 434,
+				.width	= 1952,
+				.height	= 1088,
+			},
+			.crop = {
+				/* Maintain a larger digital crop processing margins. */
+				.left	= 16,
+				.top	= 4,
+				.width	= 1920,
+				.height	= 1080,
+			},
+			.htot		= 2500,
+			.vblank_def	= 40,
 		},
-		.htot		= 2500,
-		.vblank_def	= 40,
 		.reg_data	= ov5640_setting_1080P_1920_1080,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_1080P_1920_1080),
-		.max_fps	= OV5640_30_FPS
 	}, {
 		/* 2592x1944 */
 		.id		= OV5640_MODE_QSXGA_2592_1944,
 		.dn_mode	= SCALING,
 		.pixel_rate	= OV5640_PIXEL_RATE_168M,
-		.analog_crop = {
-			.left	= 0,
-			.top	= 0,
-			.width	= 2624,
-			.height	= 1952,
+		.width		= OV5640_PIXEL_ARRAY_WIDTH,
+		.height		= OV5640_PIXEL_ARRAY_HEIGHT,
+		.dvp_timings = {
+			.analog_crop = {
+				.left	= 0,
+				.top	= 0,
+				.width	= 2624,
+				.height	= 1952,
+			},
+			.crop = {
+				.left	= 16,
+				.top	= 4,
+				.width	= 2592,
+				.height	= 1944,
+			},
+			.htot		= 2844,
+			.vblank_def	= 24,
+			.max_fps	= OV5640_15_FPS
 		},
-		.crop = {
-			.left	= 16,
-			.top	= 4,
-			.width	= 2592,
-			.height	= 1944,
+		.csi2_timings = {
+			.analog_crop = {
+				/* Give more processing margin to full resolution. */
+				.left	= 0,
+				.top	= 0,
+				.width	= OV5640_NATIVE_WIDTH,
+				.height	= 1952,
+			},
+			.crop = {
+				.left	= 16,
+				.top	= 4,
+				.width	= 2592,
+				.height	= 1944,
+			},
+			.htot		= 2844,
+			.vblank_def	= 24,
 		},
-		.htot		= 2844,
-		.vblank_def	= 24,
 		.reg_data	= ov5640_setting_QSXGA_2592_1944,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_QSXGA_2592_1944),
-		.max_fps	= OV5640_15_FPS
 	},
 };
 
@@ -1270,10 +1521,10 @@ static int ov5640_set_mipi_pclk(struct ov5640_dev *sensor)
 
 static u32 ov5640_calc_pixel_rate(struct ov5640_dev *sensor)
 {
+	const struct ov5640_timings *timings = &sensor->current_mode->dvp_timings;
 	u32 rate;
 
-	rate = sensor->current_mode->htot
-	     * (sensor->current_mode->crop.height + sensor->current_mode->vblank_def);
+	rate = timings->htot * (timings->crop.height + timings->vblank_def);
 	rate *= ov5640_framerates[sensor->current_fr];
 
 	return rate;
@@ -1357,21 +1608,20 @@ static int ov5640_set_jpeg_timings(struct ov5640_dev *sensor,
 	if (ret < 0)
 		return ret;
 
-	ret = ov5640_write_reg16(sensor, OV5640_REG_VFIFO_HSIZE,
-				 mode->crop.width);
+	ret = ov5640_write_reg16(sensor, OV5640_REG_VFIFO_HSIZE, mode->width);
 	if (ret < 0)
 		return ret;
 
-	return ov5640_write_reg16(sensor, OV5640_REG_VFIFO_VSIZE,
-				  mode->crop.height);
+	return ov5640_write_reg16(sensor, OV5640_REG_VFIFO_VSIZE, mode->height);
 }
 
 /* download ov5640 settings to sensor through i2c */
 static int ov5640_set_timings(struct ov5640_dev *sensor,
 			      const struct ov5640_mode_info *mode)
 {
-	const struct v4l2_rect *analog_crop = &mode->analog_crop;
-	const struct v4l2_rect *crop = &mode->crop;
+	const struct ov5640_timings *timings;
+	const struct v4l2_rect *analog_crop;
+	const struct v4l2_rect *crop;
 	int ret;
 
 	if (sensor->fmt.code == MEDIA_BUS_FMT_JPEG_1X8) {
@@ -1380,6 +1630,14 @@ static int ov5640_set_timings(struct ov5640_dev *sensor,
 			return ret;
 	}
 
+	if (ov5640_is_csi2(sensor))
+		timings = &mode->csi2_timings;
+	else
+		timings = &mode->dvp_timings;
+
+	analog_crop = &timings->analog_crop;
+	crop = &timings->crop;
+
 	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_HS, analog_crop->left);
 	if (ret < 0)
 		return ret;
@@ -1406,20 +1664,20 @@ static int ov5640_set_timings(struct ov5640_dev *sensor,
 	if (ret < 0)
 		return ret;
 
-	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_DVPHO, crop->width);
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_DVPHO, mode->width);
 	if (ret < 0)
 		return ret;
 
-	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_DVPVO, crop->height);
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_DVPVO, mode->height);
 	if (ret < 0)
 		return ret;
 
-	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_HTS, mode->htot);
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_HTS, timings->htot);
 	if (ret < 0)
 		return ret;
 
 	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_VTS,
-				 crop->height + mode->vblank_def);
+				 mode->height + timings->vblank_def);
 	if (ret < 0)
 		return ret;
 
@@ -1851,15 +2109,15 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
 
 	mode = v4l2_find_nearest_size(ov5640_mode_data,
 				      ARRAY_SIZE(ov5640_mode_data),
-				      crop.width, crop.height, width, height);
+				      width, height, width, height);
 
 	if (!mode ||
 	    (!nearest &&
-	     (mode->crop.width != width || mode->crop.height != height)))
+	     (mode->width != width || mode->height != height)))
 		return NULL;
 
 	/* Check to see if the current mode exceeds the max frame rate */
-	if (ov5640_framerates[fr] > ov5640_framerates[mode->max_fps])
+	if (ov5640_framerates[fr] > ov5640_framerates[mode->dvp_timings.max_fps])
 		return NULL;
 
 	return mode;
@@ -2539,8 +2797,8 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev *sd,
 	mode = ov5640_find_mode(sensor, fr, fmt->width, fmt->height, true);
 	if (!mode)
 		return -EINVAL;
-	fmt->width = mode->crop.width;
-	fmt->height = mode->crop.height;
+	fmt->width = mode->width;
+	fmt->height = mode->height;
 
 	if (new_mode)
 		*new_mode = mode;
@@ -3173,9 +3431,9 @@ static int ov5640_enum_frame_size(struct v4l2_subdev *sd,
 	if (fse->index >= OV5640_NUM_MODES)
 		return -EINVAL;
 
-	fse->min_width = ov5640_mode_data[fse->index].crop.width;
+	fse->min_width = ov5640_mode_data[fse->index].width;
 	fse->max_width = fse->min_width;
-	fse->min_height = ov5640_mode_data[fse->index].crop.height;
+	fse->min_height = ov5640_mode_data[fse->index].height;
 	fse->max_height = fse->min_height;
 
 	return 0;
@@ -3239,16 +3497,16 @@ static int ov5640_s_frame_interval(struct v4l2_subdev *sd,
 	mode = sensor->current_mode;
 
 	frame_rate = ov5640_try_frame_interval(sensor, &fi->interval,
-					       mode->crop.width,
-					       mode->crop.height);
+					       mode->width,
+					       mode->height);
 	if (frame_rate < 0) {
 		/* Always return a valid frame interval value */
 		fi->interval = sensor->frame_interval;
 		goto out;
 	}
 
-	mode = ov5640_find_mode(sensor, frame_rate, mode->crop.width,
-				mode->crop.height, true);
+	mode = ov5640_find_mode(sensor, frame_rate, mode->width,
+				mode->height, true);
 	if (!mode) {
 		ret = -EINVAL;
 		goto out;
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 11/27] media: ov5640: Provide timings accessor
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (9 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 10/27] media: ov5640: Split DVP and CSI-2 timings Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 12/27] media: ov5640: Re-sort per-mode register tables Jacopo Mondi
                   ` (15 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Provide a function to shortcut access to the correct timings definition
to avoid repeating the same pattern when accessing the sensor timings.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 4040ace2fd55..1232646bc7bb 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1123,6 +1123,15 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 	},
 };
 
+static const struct ov5640_timings *
+ov5640_timings(const struct ov5640_dev *sensor, const struct ov5640_mode_info *mode)
+{
+	if (ov5640_is_csi2(sensor))
+		return &mode->csi2_timings;
+
+	return &mode->dvp_timings;
+}
+
 static int ov5640_init_slave_id(struct ov5640_dev *sensor)
 {
 	struct i2c_client *client = sensor->i2c_client;
@@ -1630,11 +1639,7 @@ static int ov5640_set_timings(struct ov5640_dev *sensor,
 			return ret;
 	}
 
-	if (ov5640_is_csi2(sensor))
-		timings = &mode->csi2_timings;
-	else
-		timings = &mode->dvp_timings;
-
+	timings = ov5640_timings(sensor, mode);
 	analog_crop = &timings->analog_crop;
 	crop = &timings->crop;
 
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 12/27] media: ov5640: Re-sort per-mode register tables
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (10 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 11/27] media: ov5640: Provide timings accessor Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 13/27] media: ov5640: Remove duplicated mode settings Jacopo Mondi
                   ` (14 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

The per-mode register tables are not sorted by size. Fix it.

Cosmetic change only.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1232646bc7bb..b2ed6df098c6 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -482,7 +482,7 @@ static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
 	{0x3a1f, 0x14, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3c00, 0x04, 0, 300},
 };
 
-static const struct reg_value ov5640_setting_VGA_640_480[] = {
+static const struct reg_value ov5640_setting_QQVGA_160_120[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
@@ -493,11 +493,10 @@ static const struct reg_value ov5640_setting_VGA_640_480[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
 
-static const struct reg_value ov5640_setting_XGA_1024_768[] = {
+static const struct reg_value ov5640_setting_QCIF_176_144[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
@@ -527,7 +526,7 @@ static const struct reg_value ov5640_setting_QVGA_320_240[] = {
 	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
 
-static const struct reg_value ov5640_setting_QQVGA_160_120[] = {
+static const struct reg_value ov5640_setting_VGA_640_480[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
@@ -538,10 +537,11 @@ static const struct reg_value ov5640_setting_QQVGA_160_120[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x5001, 0xa3, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
+	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
 
-static const struct reg_value ov5640_setting_QCIF_176_144[] = {
+static const struct reg_value ov5640_setting_NTSC_720_480[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
@@ -556,7 +556,7 @@ static const struct reg_value ov5640_setting_QCIF_176_144[] = {
 	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
 
-static const struct reg_value ov5640_setting_NTSC_720_480[] = {
+static const struct reg_value ov5640_setting_PAL_720_576[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
@@ -571,7 +571,7 @@ static const struct reg_value ov5640_setting_NTSC_720_480[] = {
 	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
 
-static const struct reg_value ov5640_setting_PAL_720_576[] = {
+static const struct reg_value ov5640_setting_XGA_1024_768[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 13/27] media: ov5640: Remove duplicated mode settings
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (11 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 12/27] media: ov5640: Re-sort per-mode register tables Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 14/27] media: ov5640: Remove ov5640_mode_init_data Jacopo Mondi
                   ` (13 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

The register tables for all resolutions smaller than 720p are now
identical.

Remove the duplicated ones and create a single ov5640_setting_low_res[]
register table.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 120 +++++--------------------------------
 1 file changed, 15 insertions(+), 105 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index b2ed6df098c6..30c4c5388a87 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -482,7 +482,7 @@ static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
 	{0x3a1f, 0x14, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3c00, 0x04, 0, 300},
 };
 
-static const struct reg_value ov5640_setting_QQVGA_160_120[] = {
+static const struct reg_value ov5640_setting_low_res[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3814, 0x31, 0, 0},
@@ -496,96 +496,6 @@ static const struct reg_value ov5640_setting_QQVGA_160_120[] = {
 	{0x4407, 0x04, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
 
-static const struct reg_value ov5640_setting_QCIF_176_144[] = {
-	{0x3c07, 0x08, 0, 0},
-	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0},
-	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
-	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
-	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
-	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
-};
-
-static const struct reg_value ov5640_setting_QVGA_320_240[] = {
-	{0x3c07, 0x08, 0, 0},
-	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0},
-	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
-	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
-	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
-	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
-};
-
-static const struct reg_value ov5640_setting_VGA_640_480[] = {
-	{0x3c07, 0x08, 0, 0},
-	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0},
-	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
-	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
-	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
-	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
-};
-
-static const struct reg_value ov5640_setting_NTSC_720_480[] = {
-	{0x3c07, 0x08, 0, 0},
-	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0},
-	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
-	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
-	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
-	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
-};
-
-static const struct reg_value ov5640_setting_PAL_720_576[] = {
-	{0x3c07, 0x08, 0, 0},
-	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0},
-	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
-	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
-	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
-	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
-};
-
-static const struct reg_value ov5640_setting_XGA_1024_768[] = {
-	{0x3c07, 0x08, 0, 0},
-	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0},
-	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
-	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
-	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
-	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
-};
-
 static const struct reg_value ov5640_setting_720P_1280_720[] = {
 	{0x3c07, 0x07, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
@@ -727,8 +637,8 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 			.htot		= 1896,
 			.vblank_def	= 864,
 		},
-		.reg_data	= ov5640_setting_QQVGA_160_120,
-		.reg_data_size	= ARRAY_SIZE(ov5640_setting_QQVGA_160_120),
+		.reg_data	= ov5640_setting_low_res,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
 	}, {
 		/* 176x144 */
 		.id		= OV5640_MODE_QCIF_176_144,
@@ -771,8 +681,8 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 			.htot		= 1896,
 			.vblank_def	= 840,
 		},
-		.reg_data	= ov5640_setting_QCIF_176_144,
-		.reg_data_size	= ARRAY_SIZE(ov5640_setting_QCIF_176_144),
+		.reg_data	= ov5640_setting_low_res,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
 	}, {
 		/* 320x240 */
 		.id		= OV5640_MODE_QVGA_320_240,
@@ -815,8 +725,8 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 			.htot		= 1896,
 			.vblank_def	= 744,
 		},
-		.reg_data	= ov5640_setting_QVGA_320_240,
-		.reg_data_size	= ARRAY_SIZE(ov5640_setting_QVGA_320_240),
+		.reg_data	= ov5640_setting_low_res,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
 	}, {
 		/* 640x480 */
 		.id		= OV5640_MODE_VGA_640_480,
@@ -859,8 +769,8 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 			.htot		= 1896,
 			.vblank_def	= 600,
 		},
-		.reg_data	= ov5640_setting_VGA_640_480,
-		.reg_data_size	= ARRAY_SIZE(ov5640_setting_VGA_640_480),
+		.reg_data	= ov5640_setting_low_res,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
 	}, {
 		/* 720x480 */
 		.id		= OV5640_MODE_NTSC_720_480,
@@ -903,8 +813,8 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 			.htot		= 1896,
 			.vblank_def	= 504,
 		},
-		.reg_data	= ov5640_setting_NTSC_720_480,
-		.reg_data_size	= ARRAY_SIZE(ov5640_setting_NTSC_720_480),
+		.reg_data	= ov5640_setting_low_res,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
 	}, {
 		/* 720x576 */
 		.id		= OV5640_MODE_PAL_720_576,
@@ -947,8 +857,8 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 			.htot		= 1896,
 			.vblank_def	= 408,
 		},
-		.reg_data	= ov5640_setting_PAL_720_576,
-		.reg_data_size	= ARRAY_SIZE(ov5640_setting_PAL_720_576),
+		.reg_data	= ov5640_setting_low_res,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
 	}, {
 		/* 1024x768 */
 		.id		= OV5640_MODE_XGA_1024_768,
@@ -989,8 +899,8 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 			.htot		= 1896,
 			.vblank_def	= 312,
 		},
-		.reg_data	= ov5640_setting_XGA_1024_768,
-		.reg_data_size	= ARRAY_SIZE(ov5640_setting_XGA_1024_768),
+		.reg_data	= ov5640_setting_low_res,
+		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
 	}, {
 		/* 1280x720 */
 		.id		= OV5640_MODE_720P_1280_720,
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 14/27] media: ov5640: Remove ov5640_mode_init_data
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (12 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 13/27] media: ov5640: Remove duplicated mode settings Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 15/27] media: ov5640: Add HBLANK control Jacopo Mondi
                   ` (12 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

The ov5640_mode_init_data is a fictional sensor more which is used to
program the initial sensor settings.

It is only used to initialize the sensor and can be replaced
it with a throw-away mode which just wraps the register table.

Also rename the register table to drop the format from the name to make
it clear an actual sensor mode has to be applied after the initial
programming.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 67 ++++++--------------------------------
 1 file changed, 10 insertions(+), 57 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 30c4c5388a87..593e702e9658 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -402,7 +402,7 @@ static inline bool ov5640_is_csi2(const struct ov5640_dev *sensor)
  * over i2c.
  */
 /* YUV422 UYVY VGA@30fps */
-static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
+static const struct reg_value ov5640_init_setting[] = {
 	{0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
 	{0x3103, 0x03, 0, 0}, {0x3630, 0x36, 0, 0},
 	{0x3631, 0x0e, 0, 0}, {0x3632, 0xe2, 0, 0}, {0x3633, 0x12, 0, 0},
@@ -550,50 +550,6 @@ static const struct reg_value ov5640_setting_QSXGA_2592_1944[] = {
 	{0x3824, 0x02, 0, 0}, {0x5001, 0x83, 0, 70},
 };
 
-/* power-on sensor init reg table */
-static const struct ov5640_mode_info ov5640_mode_init_data = {
-		.id		= 0,
-		.dn_mode	= SUBSAMPLING,
-		.pixel_rate	= OV5640_PIXEL_RATE_96M,
-		.width	= 640,
-		.height	= 480,
-		.dvp_timings = {
-			.analog_crop = {
-				.left	= 0,
-				.top	= 4,
-				.width	= 2624,
-				.height	= 1944,
-			},
-			.crop = {
-				.left	= 16,
-				.top	= 6,
-				.width	= 640,
-				.height	= 480,
-			},
-			.htot		= 1896,
-			.vblank_def	= 504,
-			.max_fps	= OV5640_30_FPS
-		},
-		.csi2_timings = {
-			.analog_crop = {
-				.left	= OV5640_PIXEL_ARRAY_LEFT,
-				.top	= OV5640_PIXEL_ARRAY_TOP,
-				.width	= OV5640_PIXEL_ARRAY_WIDTH,
-				.height	= OV5640_PIXEL_ARRAY_HEIGHT,
-			},
-			.crop = {
-				.left	= 2,
-				.top	= 4,
-				.width	= 640,
-				.height	= 480,
-			},
-			.htot		= 1896,
-			.vblank_def	= 504,
-		},
-		.reg_data	= ov5640_init_setting_30fps_VGA,
-		.reg_data_size	= ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
-};
-
 static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 	{
 		/* 160x120 */
@@ -1599,17 +1555,16 @@ static int ov5640_set_timings(struct ov5640_dev *sensor,
 	return 0;
 }
 
-static int ov5640_load_regs(struct ov5640_dev *sensor,
-			    const struct ov5640_mode_info *mode)
+static void ov5640_load_regs(struct ov5640_dev *sensor,
+			     const struct reg_value *regs, unsigned int regnum)
 {
-	const struct reg_value *regs = mode->reg_data;
 	unsigned int i;
 	u32 delay_ms;
 	u16 reg_addr;
 	u8 mask, val;
 	int ret = 0;
 
-	for (i = 0; i < mode->reg_data_size; ++i, ++regs) {
+	for (i = 0; i < regnum; ++i, ++regs) {
 		delay_ms = regs->delay_ms;
 		reg_addr = regs->reg_addr;
 		val = regs->val;
@@ -1631,8 +1586,6 @@ static int ov5640_load_regs(struct ov5640_dev *sensor,
 		if (delay_ms)
 			usleep_range(1000 * delay_ms, 1000 * delay_ms + 100);
 	}
-
-	return ov5640_set_timings(sensor, mode);
 }
 
 static int ov5640_set_autoexposure(struct ov5640_dev *sensor, bool on)
@@ -2085,7 +2038,8 @@ static int ov5640_set_mode_exposure_calc(struct ov5640_dev *sensor,
 		return ret;
 
 	/* Write capture setting */
-	ret = ov5640_load_regs(sensor, mode);
+	ov5640_load_regs(sensor, mode->reg_data, mode->reg_data_size);
+	ret = ov5640_set_timings(sensor, mode);
 	if (ret < 0)
 		return ret;
 
@@ -2209,7 +2163,8 @@ static int ov5640_set_mode_direct(struct ov5640_dev *sensor,
 		return -EINVAL;
 
 	/* Write capture setting */
-	return ov5640_load_regs(sensor, mode);
+	ov5640_load_regs(sensor, mode->reg_data, mode->reg_data_size);
+	return ov5640_set_timings(sensor, mode);
 }
 
 static int ov5640_set_mode(struct ov5640_dev *sensor)
@@ -2307,10 +2262,8 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
 	int ret;
 
 	/* first load the initial register values */
-	ret = ov5640_load_regs(sensor, &ov5640_mode_init_data);
-	if (ret < 0)
-		return ret;
-	sensor->last_mode = &ov5640_mode_init_data;
+	ov5640_load_regs(sensor, ov5640_init_setting,
+			 ARRAY_SIZE(ov5640_init_setting));
 
 	ret = ov5640_mod_reg(sensor, OV5640_REG_SYS_ROOT_DIVIDER, 0x3f,
 			     (ilog2(OV5640_SCLK2X_ROOT_DIV) << 2) |
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 15/27] media: ov5640: Add HBLANK control
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (13 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 14/27] media: ov5640: Remove ov5640_mode_init_data Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 16/27] media: ov5640: Add VBLANK control Jacopo Mondi
                   ` (11 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Add the HBLANK control as read-only.

The hblank value is fixed in the mode definition and is updated
everytime a new format is applied.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 593e702e9658..bf84f32436b1 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -320,6 +320,7 @@ struct ov5640_ctrls {
 	struct v4l2_ctrl_handler handler;
 	struct v4l2_ctrl *pixel_rate;
 	struct v4l2_ctrl *link_freq;
+	struct v4l2_ctrl *hblank;
 	struct {
 		struct v4l2_ctrl *auto_exp;
 		struct v4l2_ctrl *exposure;
@@ -2691,6 +2692,8 @@ static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
 	const struct ov5640_mode_info *mode = sensor->current_mode;
 	struct v4l2_mbus_framefmt *fmt = &sensor->fmt;
 	enum ov5640_pixel_rate_id pixel_rate_id = mode->pixel_rate;
+	const struct ov5640_timings *timings;
+	unsigned int hblank;
 	unsigned int i = 0;
 	u32 pixel_rate;
 	s64 link_freq;
@@ -2743,6 +2746,11 @@ static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
 	__v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate, pixel_rate);
 	__v4l2_ctrl_s_ctrl(sensor->ctrls.link_freq, i);
 
+	timings = ov5640_timings(sensor, mode);
+	hblank = timings->htot - mode->width;
+	__v4l2_ctrl_modify_range(sensor->ctrls.hblank,
+				 hblank, hblank, 1, hblank);
+
 	return 0;
 }
 
@@ -3207,6 +3215,8 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
 	const struct v4l2_ctrl_ops *ops = &ov5640_ctrl_ops;
 	struct ov5640_ctrls *ctrls = &sensor->ctrls;
 	struct v4l2_ctrl_handler *hdl = &ctrls->handler;
+	const struct ov5640_timings *timings;
+	unsigned int hblank;
 	int ret;
 
 	v4l2_ctrl_handler_init(hdl, 32);
@@ -3226,6 +3236,11 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
 					OV5640_DEFAULT_LINK_FREQ,
 					ov5640_csi2_link_freqs);
 
+	timings = ov5640_timings(sensor, mode);
+	hblank = timings->htot - mode->width;
+	ctrls->hblank = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HBLANK, hblank,
+					  hblank, 1, hblank);
+
 	/* Auto/manual white balance */
 	ctrls->auto_wb = v4l2_ctrl_new_std(hdl, ops,
 					   V4L2_CID_AUTO_WHITE_BALANCE,
@@ -3275,6 +3290,7 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
 
 	ctrls->pixel_rate->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 	ctrls->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+	ctrls->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 	ctrls->gain->flags |= V4L2_CTRL_FLAG_VOLATILE;
 	ctrls->exposure->flags |= V4L2_CTRL_FLAG_VOLATILE;
 
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 16/27] media: ov5640: Add VBLANK control
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (14 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 15/27] media: ov5640: Add HBLANK control Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 20:23   ` Sakari Ailus
  2022-02-23 14:55 ` [PATCH v4 17/27] media: ov5640: Change CSI-2 timings to comply with FPS Jacopo Mondi
                   ` (10 subsequent siblings)
  26 siblings, 1 reply; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Add the VBLANK control which allows to select the duration of the
frame vertical blankings and allows to control the framerate.

The VBLANK control also modifies the exposure time range, which cannot
exceed the maximum frame length.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 57 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index bf84f32436b1..134129fc13ae 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -36,6 +36,10 @@
 #define OV5640_PIXEL_ARRAY_WIDTH	2592
 #define OV5640_PIXEL_ARRAY_HEIGHT	1944
 
+/* FIXME: not documented. */
+#define OV5640_MIN_VBLANK	24
+#define OV5640_MAX_VTS		1968
+
 #define OV5640_DEFAULT_SLAVE_ID 0x3c
 
 #define OV5640_LINK_RATE_MAX		490000000U
@@ -321,6 +325,7 @@ struct ov5640_ctrls {
 	struct v4l2_ctrl *pixel_rate;
 	struct v4l2_ctrl *link_freq;
 	struct v4l2_ctrl *hblank;
+	struct v4l2_ctrl *vblank;
 	struct {
 		struct v4l2_ctrl *auto_exp;
 		struct v4l2_ctrl *exposure;
@@ -2693,6 +2698,7 @@ static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
 	struct v4l2_mbus_framefmt *fmt = &sensor->fmt;
 	enum ov5640_pixel_rate_id pixel_rate_id = mode->pixel_rate;
 	const struct ov5640_timings *timings;
+	s64 exposure_val, exposure_max;
 	unsigned int hblank;
 	unsigned int i = 0;
 	u32 pixel_rate;
@@ -2751,6 +2757,20 @@ static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
 	__v4l2_ctrl_modify_range(sensor->ctrls.hblank,
 				 hblank, hblank, 1, hblank);
 
+	__v4l2_ctrl_modify_range(sensor->ctrls.vblank,
+				 OV5640_MIN_VBLANK,
+				 OV5640_MAX_VTS - mode->height, 1,
+				 timings->vblank_def);
+	__v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, timings->vblank_def);
+
+	exposure_max = timings->crop.height + timings->vblank_def - 4;
+	exposure_val = clamp((s64)sensor->ctrls.exposure->val,
+			     (s64)sensor->ctrls.exposure->minimum,
+			     (s64)exposure_max);
+	__v4l2_ctrl_modify_range(sensor->ctrls.exposure,
+				 sensor->ctrls.exposure->minimum,
+				 exposure_max, 1, exposure_val);
+
 	return 0;
 }
 
@@ -3123,6 +3143,15 @@ static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
 			      (BIT(2) | BIT(1)) : 0);
 }
 
+static int ov5640_set_ctrl_vblank(struct ov5640_dev *sensor, int value)
+{
+	const struct ov5640_mode_info *mode = sensor->current_mode;
+
+	/* Update the VTOT timing register value. */
+	return ov5640_write_reg16(sensor, OV5640_REG_TIMING_VTS,
+				  mode->height + value);
+}
+
 static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
@@ -3153,10 +3182,29 @@ static int ov5640_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
 	struct ov5640_dev *sensor = to_ov5640_dev(sd);
+	const struct ov5640_mode_info *mode = sensor->current_mode;
+	const struct ov5640_timings *timings;
+	unsigned int exp_max;
 	int ret;
 
 	/* v4l2_ctrl_lock() locks our own mutex */
 
+	if (ov5640_is_csi2(sensor))
+		timings = &mode->csi2_timings;
+	else
+		timings = &mode->dvp_timings;
+
+	switch (ctrl->id) {
+	case V4L2_CID_VBLANK:
+		/* Update the exposure range to the newly programmed vblank. */
+		exp_max = mode->height + ctrl->val - 4;
+		__v4l2_ctrl_modify_range(sensor->ctrls.exposure,
+					 sensor->ctrls.exposure->minimum,
+					 exp_max, sensor->ctrls.exposure->step,
+					 timings->vblank_def);
+		break;
+	}
+
 	/*
 	 * If the device is not powered up by the host driver do
 	 * not apply any controls to H/W at this time. Instead
@@ -3196,6 +3244,9 @@ static int ov5640_s_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_VFLIP:
 		ret = ov5640_set_ctrl_vflip(sensor, ctrl->val);
 		break;
+	case V4L2_CID_VBLANK:
+		ret = ov5640_set_ctrl_vblank(sensor, ctrl->val);
+		break;
 	default:
 		ret = -EINVAL;
 		break;
@@ -3216,6 +3267,7 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
 	struct ov5640_ctrls *ctrls = &sensor->ctrls;
 	struct v4l2_ctrl_handler *hdl = &ctrls->handler;
 	const struct ov5640_timings *timings;
+	unsigned int max_vblank;
 	unsigned int hblank;
 	int ret;
 
@@ -3241,6 +3293,11 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
 	ctrls->hblank = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HBLANK, hblank,
 					  hblank, 1, hblank);
 
+	max_vblank = OV5640_MAX_VTS - mode->height;
+	ctrls->vblank = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VBLANK,
+					  OV5640_MIN_VBLANK, max_vblank,
+					  1, timings->vblank_def);
+
 	/* Auto/manual white balance */
 	ctrls->auto_wb = v4l2_ctrl_new_std(hdl, ops,
 					   V4L2_CID_AUTO_WHITE_BALANCE,
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 17/27] media: ov5640: Change CSI-2 timings to comply with FPS
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (15 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 16/27] media: ov5640: Add VBLANK control Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 18/27] media: ov5640: Implement init_cfg Jacopo Mondi
                   ` (9 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Now that the frame duration can be controlled by tuning the VBLANK
duration in CSI-2 mode, fix all modes definitions to comply with the
reported FPS.

All modes run at 30 FPS except for full-resolution mode 2592x1944
which runs at 15FPS.

Tested on a 2 data lanes setup in UYVY and RGB565 modes in CSI-2 mode.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 134129fc13ae..6c2c76b61140 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -596,8 +596,8 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 				.width	= 160,
 				.height	= 120,
 			},
-			.htot		= 1896,
-			.vblank_def	= 864,
+			.htot		= 1600,
+			.vblank_def	= 878,
 		},
 		.reg_data	= ov5640_setting_low_res,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
@@ -640,8 +640,8 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 				.width	= 176,
 				.height	= 144,
 			},
-			.htot		= 1896,
-			.vblank_def	= 840,
+			.htot		= 1600,
+			.vblank_def	= 854,
 		},
 		.reg_data	= ov5640_setting_low_res,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
@@ -684,8 +684,8 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 				.width	= 320,
 				.height	= 240,
 			},
-			.htot		= 1896,
-			.vblank_def	= 744,
+			.htot		= 1600,
+			.vblank_def	= 760,
 		},
 		.reg_data	= ov5640_setting_low_res,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
@@ -728,8 +728,8 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 				.width	= 640,
 				.height	= 480,
 			},
-			.htot		= 1896,
-			.vblank_def	= 600,
+			.htot		= 1600,
+			.vblank_def	= 520,
 		},
 		.reg_data	= ov5640_setting_low_res,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
@@ -773,7 +773,7 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 				.height	= 480,
 			},
 			.htot		= 1896,
-			.vblank_def	= 504,
+			.vblank_def	= 1206,
 		},
 		.reg_data	= ov5640_setting_low_res,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
@@ -817,7 +817,7 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 				.height	= 576,
 			},
 			.htot		= 1896,
-			.vblank_def	= 408,
+			.vblank_def	= 1110,
 		},
 		.reg_data	= ov5640_setting_low_res,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
@@ -859,7 +859,7 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 				.height	= 768,
 			},
 			.htot		= 1896,
-			.vblank_def	= 312,
+			.vblank_def	= 918,
 		},
 		.reg_data	= ov5640_setting_low_res,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_low_res),
@@ -900,8 +900,8 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 				.width	= 1280,
 				.height	= 720,
 			},
-			.htot		= 1892,
-			.vblank_def	= 20,
+			.htot		= 1600,
+			.vblank_def	= 560,
 		},
 		.reg_data	= ov5640_setting_720P_1280_720,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_720P_1280_720),
@@ -944,8 +944,8 @@ static const struct ov5640_mode_info ov5640_mode_data[OV5640_NUM_MODES] = {
 				.width	= 1920,
 				.height	= 1080,
 			},
-			.htot		= 2500,
-			.vblank_def	= 40,
+			.htot		= 2234,
+			.vblank_def	= 24,
 		},
 		.reg_data	= ov5640_setting_1080P_1920_1080,
 		.reg_data_size	= ARRAY_SIZE(ov5640_setting_1080P_1920_1080),
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 18/27] media: ov5640: Implement init_cfg
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (16 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 17/27] media: ov5640: Change CSI-2 timings to comply with FPS Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 20:25   ` Sakari Ailus
  2022-02-23 14:55 ` [PATCH v4 19/27] media: ov5640: Implement get_selection Jacopo Mondi
                   ` (8 subsequent siblings)
  26 siblings, 1 reply; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Implement the init_cfg pad operation to initialize the subdev state
format to the default one.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 6c2c76b61140..c1fc2ab1e0cf 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -408,6 +408,18 @@ static inline bool ov5640_is_csi2(const struct ov5640_dev *sensor)
  * over i2c.
  */
 /* YUV422 UYVY VGA@30fps */
+
+static const struct v4l2_mbus_framefmt ov5640_default_fmt = {
+	.code = MEDIA_BUS_FMT_UYVY8_2X8,
+	.width = 640,
+	.height = 480,
+	.colorspace = V4L2_COLORSPACE_SRGB,
+	.ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SRGB),
+	.quantization = V4L2_QUANTIZATION_FULL_RANGE,
+	.xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SRGB),
+	.field = V4L2_FIELD_NONE,
+};
+
 static const struct reg_value ov5640_init_setting[] = {
 	{0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
 	{0x3103, 0x03, 0, 0}, {0x3630, 0x36, 0, 0},
@@ -3515,6 +3527,16 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int enable)
 	return ret;
 }
 
+static int ov5640_init_cfg(struct v4l2_subdev *sd,
+			   struct v4l2_subdev_state *state)
+{
+	struct v4l2_mbus_framefmt *fmt = v4l2_subdev_get_try_format(sd, state, 0);
+
+	*fmt = ov5640_default_fmt;
+
+	return 0;
+}
+
 static const struct v4l2_subdev_core_ops ov5640_core_ops = {
 	.s_power = ov5640_s_power,
 	.log_status = v4l2_ctrl_subdev_log_status,
@@ -3529,6 +3551,7 @@ static const struct v4l2_subdev_video_ops ov5640_video_ops = {
 };
 
 static const struct v4l2_subdev_pad_ops ov5640_pad_ops = {
+	.init_cfg = ov5640_init_cfg,
 	.enum_mbus_code = ov5640_enum_mbus_code,
 	.get_fmt = ov5640_get_fmt,
 	.set_fmt = ov5640_set_fmt,
@@ -3587,7 +3610,6 @@ static int ov5640_probe(struct i2c_client *client)
 	struct device *dev = &client->dev;
 	struct fwnode_handle *endpoint;
 	struct ov5640_dev *sensor;
-	struct v4l2_mbus_framefmt *fmt;
 	u32 rotation;
 	int ret;
 
@@ -3601,15 +3623,7 @@ static int ov5640_probe(struct i2c_client *client)
 	 * default init sequence initialize sensor to
 	 * YUV422 UYVY VGA@30fps
 	 */
-	fmt = &sensor->fmt;
-	fmt->code = MEDIA_BUS_FMT_UYVY8_2X8;
-	fmt->colorspace = V4L2_COLORSPACE_SRGB;
-	fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
-	fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
-	fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
-	fmt->width = 640;
-	fmt->height = 480;
-	fmt->field = V4L2_FIELD_NONE;
+	sensor->fmt = ov5640_default_fmt;
 	sensor->frame_interval.numerator = 1;
 	sensor->frame_interval.denominator = ov5640_framerates[OV5640_30_FPS];
 	sensor->current_fr = OV5640_30_FPS;
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 19/27] media: ov5640: Implement get_selection
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (17 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 18/27] media: ov5640: Implement init_cfg Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 20/27] media: ov5640: Limit frame_interval to DVP mode only Jacopo Mondi
                   ` (7 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Implement the get_selection pad operation for the OV5640 sensor driver.

The supported targets report the sensor's native size, the active pixel
array size and the analog crop rectangle from which the image is
produced.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 46 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index c1fc2ab1e0cf..88662972abb0 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2832,6 +2832,45 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
 	return ret;
 }
 
+static int ov5640_get_selection(struct v4l2_subdev *sd,
+				struct v4l2_subdev_state *sd_state,
+				struct v4l2_subdev_selection *sel)
+{
+	struct ov5640_dev *sensor = to_ov5640_dev(sd);
+	const struct ov5640_mode_info *mode = sensor->current_mode;
+	const struct ov5640_timings *timings;
+
+	switch (sel->target) {
+	case V4L2_SEL_TGT_CROP: {
+		mutex_lock(&sensor->lock);
+		timings = ov5640_timings(sensor, mode);
+		sel->r = timings->analog_crop;
+		mutex_unlock(&sensor->lock);
+
+		return 0;
+	}
+
+	case V4L2_SEL_TGT_NATIVE_SIZE:
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+		sel->r.top = 0;
+		sel->r.left = 0;
+		sel->r.width = OV5640_NATIVE_WIDTH;
+		sel->r.height = OV5640_NATIVE_HEIGHT;
+
+		return 0;
+
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+		sel->r.top = OV5640_PIXEL_ARRAY_TOP;
+		sel->r.left = OV5640_PIXEL_ARRAY_LEFT;
+		sel->r.width = OV5640_PIXEL_ARRAY_WIDTH;
+		sel->r.height = OV5640_PIXEL_ARRAY_HEIGHT;
+
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 static int ov5640_set_framefmt(struct ov5640_dev *sensor,
 			       struct v4l2_mbus_framefmt *format)
 {
@@ -3531,9 +3570,15 @@ static int ov5640_init_cfg(struct v4l2_subdev *sd,
 			   struct v4l2_subdev_state *state)
 {
 	struct v4l2_mbus_framefmt *fmt = v4l2_subdev_get_try_format(sd, state, 0);
+	struct v4l2_rect *crop = v4l2_subdev_get_try_crop(sd, state, 0);
 
 	*fmt = ov5640_default_fmt;
 
+	crop->left = OV5640_PIXEL_ARRAY_LEFT;
+	crop->top = OV5640_PIXEL_ARRAY_TOP;
+	crop->width = OV5640_PIXEL_ARRAY_WIDTH;
+	crop->height = OV5640_PIXEL_ARRAY_HEIGHT;
+
 	return 0;
 }
 
@@ -3555,6 +3600,7 @@ static const struct v4l2_subdev_pad_ops ov5640_pad_ops = {
 	.enum_mbus_code = ov5640_enum_mbus_code,
 	.get_fmt = ov5640_get_fmt,
 	.set_fmt = ov5640_set_fmt,
+	.get_selection = ov5640_get_selection,
 	.enum_frame_size = ov5640_enum_frame_size,
 	.enum_frame_interval = ov5640_enum_frame_interval,
 };
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 20/27] media: ov5640: Limit frame_interval to DVP mode only
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (18 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 19/27] media: ov5640: Implement get_selection Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 21/27] media: ov5640: Register device properties Jacopo Mondi
                   ` (6 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

In MIPI mode the frame rate control is performed by adjusting the
frame blankings and the s_frame_interval function is not used anymore.

Only check for the per-mode supported frame rate in DVP mode and do not
restrict MIPI mode.

Disallow enum/s/g_frame_interval if the chip is used in MIPI mode.

While at it re-indent one function which whose parameters were wrongly
aligned.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 88662972abb0..178e203bfcca 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2002,8 +2002,13 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
 	     (mode->width != width || mode->height != height)))
 		return NULL;
 
-	/* Check to see if the current mode exceeds the max frame rate */
-	if (ov5640_framerates[fr] > ov5640_framerates[mode->dvp_timings.max_fps])
+	/*
+	 * Check to see if the current mode exceeds the max frame rate.
+	 * Only DVP mode uses the frame rate set by s_frame_interval, MIPI
+	 * mode controls framerate by setting blankings.
+	 */
+	if (!ov5640_is_csi2(sensor) &&
+	    ov5640_framerates[fr] > ov5640_framerates[mode->dvp_timings.max_fps])
 		return NULL;
 
 	return mode;
@@ -3431,15 +3436,17 @@ static int ov5640_enum_frame_size(struct v4l2_subdev *sd,
 	return 0;
 }
 
-static int ov5640_enum_frame_interval(
-	struct v4l2_subdev *sd,
-	struct v4l2_subdev_state *sd_state,
-	struct v4l2_subdev_frame_interval_enum *fie)
+static int ov5640_enum_frame_interval(struct v4l2_subdev *sd,
+				      struct v4l2_subdev_state *sd_state,
+				      struct v4l2_subdev_frame_interval_enum *fie)
 {
 	struct ov5640_dev *sensor = to_ov5640_dev(sd);
 	struct v4l2_fract tpf;
 	int ret;
 
+	if (ov5640_is_csi2(sensor))
+		return -EINVAL;
+
 	if (fie->pad != 0)
 		return -EINVAL;
 	if (fie->index >= OV5640_NUM_FRAMERATES)
@@ -3462,6 +3469,9 @@ static int ov5640_g_frame_interval(struct v4l2_subdev *sd,
 {
 	struct ov5640_dev *sensor = to_ov5640_dev(sd);
 
+	if (ov5640_is_csi2(sensor))
+		return -EINVAL;
+
 	mutex_lock(&sensor->lock);
 	fi->interval = sensor->frame_interval;
 	mutex_unlock(&sensor->lock);
@@ -3476,6 +3486,9 @@ static int ov5640_s_frame_interval(struct v4l2_subdev *sd,
 	const struct ov5640_mode_info *mode;
 	int frame_rate, ret = 0;
 
+	if (ov5640_is_csi2(sensor))
+		return -EINVAL;
+
 	if (fi->pad != 0)
 		return -EINVAL;
 
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 21/27] media: ov5640: Register device properties
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (19 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 20/27] media: ov5640: Limit frame_interval to DVP mode only Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 22/27] media: ov5640: Add RGB565_1X16 format Jacopo Mondi
                   ` (5 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Parse the device properties and register the rotation and orientation
V4L2 controls using v4l2_ctrl_new_fwnode_properties().

Remove the open-coded parsing of the rotation property and assume the
DTS is correct is providing either <0> or <180> as possible rotations.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 178e203bfcca..3c1f45610e77 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -3322,6 +3322,7 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
 	const struct v4l2_ctrl_ops *ops = &ov5640_ctrl_ops;
 	struct ov5640_ctrls *ctrls = &sensor->ctrls;
 	struct v4l2_ctrl_handler *hdl = &ctrls->handler;
+	struct v4l2_fwnode_device_properties props;
 	const struct ov5640_timings *timings;
 	unsigned int max_vblank;
 	unsigned int hblank;
@@ -3401,6 +3402,17 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
 		goto free_ctrls;
 	}
 
+	ret = v4l2_fwnode_device_parse(&sensor->i2c_client->dev, &props);
+	if (ret)
+		goto free_ctrls;
+
+	if (props.rotation == 180)
+		sensor->upside_down = true;
+
+	ret = v4l2_ctrl_new_fwnode_properties(hdl, ops, &props);
+	if (ret)
+		goto free_ctrls;
+
 	ctrls->pixel_rate->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 	ctrls->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 	ctrls->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
@@ -3669,7 +3681,6 @@ static int ov5640_probe(struct i2c_client *client)
 	struct device *dev = &client->dev;
 	struct fwnode_handle *endpoint;
 	struct ov5640_dev *sensor;
-	u32 rotation;
 	int ret;
 
 	sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
@@ -3692,22 +3703,6 @@ static int ov5640_probe(struct i2c_client *client)
 
 	sensor->ae_target = 52;
 
-	/* optional indication of physical rotation of sensor */
-	ret = fwnode_property_read_u32(dev_fwnode(&client->dev), "rotation",
-				       &rotation);
-	if (!ret) {
-		switch (rotation) {
-		case 180:
-			sensor->upside_down = true;
-			fallthrough;
-		case 0:
-			break;
-		default:
-			dev_warn(dev, "%u degrees rotation is not supported, ignoring...\n",
-				 rotation);
-		}
-	}
-
 	endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
 						  NULL);
 	if (!endpoint) {
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 22/27] media: ov5640: Add RGB565_1X16 format
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (20 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 21/27] media: ov5640: Register device properties Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:55 ` [PATCH v4 23/27] media: ov5640: Add BGR888 format Jacopo Mondi
                   ` (4 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

The driver already supports the 2X8_[LE|BE] variants of RGB565
formats.

As for CSI-2 the 2X8 variants do not apply, add RGB565_1X16 variant
with little-endian ordering of components as required by the CSI-2
specifications.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 3c1f45610e77..ae7bff76cfb4 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -221,6 +221,10 @@ static const struct ov5640_pixfmt {
 		.code = MEDIA_BUS_FMT_RGB565_2X8_BE,
 		.colorspace = V4L2_COLORSPACE_SRGB,
 		.bpp = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_RGB565_1X16,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 16,
 	}, {
 		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
 		.colorspace = V4L2_COLORSPACE_SRGB,
@@ -2897,6 +2901,7 @@ static int ov5640_set_framefmt(struct ov5640_dev *sensor,
 		mux = OV5640_FMT_MUX_YUV422;
 		break;
 	case MEDIA_BUS_FMT_RGB565_2X8_LE:
+	case MEDIA_BUS_FMT_RGB565_1X16:
 		/* RGB565 {g[2:0],b[4:0]},{r[4:0],g[5:3]} */
 		fmt = 0x6F;
 		mux = OV5640_FMT_MUX_RGB;
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 23/27] media: ov5640: Add BGR888 format
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (21 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 22/27] media: ov5640: Add RGB565_1X16 format Jacopo Mondi
@ 2022-02-23 14:55 ` Jacopo Mondi
  2022-02-23 14:56 ` [PATCH v4 24/27] media: ov5640: Restrict sizes to mbus code Jacopo Mondi
                   ` (3 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:55 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Add support for BGR888 image format.

No existing media bus codes describe exactly the way data is transferred
on the CSI-2 bus. This is not a new issue, the CSI-2 YUV422 8-bit format
is described by MEDIA_BUS_FMT_UYVY8_1X16 which is an arbitrary
convention and not an exact match. Use the MEDIA_BUS_FMT_BGR888_1X24 to
follow the same convention, based on the order in which bits are
transmitted over the CSI-2 bus when producing images in RGB24 format.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index ae7bff76cfb4..0e9f04fdcee2 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -225,6 +225,10 @@ static const struct ov5640_pixfmt {
 		.code = MEDIA_BUS_FMT_RGB565_1X16,
 		.colorspace = V4L2_COLORSPACE_SRGB,
 		.bpp = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_BGR888_1X24,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 24,
 	}, {
 		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
 		.colorspace = V4L2_COLORSPACE_SRGB,
@@ -2911,6 +2915,11 @@ static int ov5640_set_framefmt(struct ov5640_dev *sensor,
 		fmt = 0x61;
 		mux = OV5640_FMT_MUX_RGB;
 		break;
+	case MEDIA_BUS_FMT_BGR888_1X24:
+		/* BGR888: RGB */
+		fmt = 0x23;
+		mux = OV5640_FMT_MUX_RGB;
+		break;
 	case MEDIA_BUS_FMT_JPEG_1X8:
 		/* YUV422, YUYV */
 		fmt = 0x30;
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 24/27] media: ov5640: Restrict sizes to mbus code
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (22 preceding siblings ...)
  2022-02-23 14:55 ` [PATCH v4 23/27] media: ov5640: Add BGR888 format Jacopo Mondi
@ 2022-02-23 14:56 ` Jacopo Mondi
  2022-02-23 14:56 ` [PATCH v4 25/27] media: ov5640: Adjust format to bpp in s_fmt Jacopo Mondi
                   ` (2 subsequent siblings)
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:56 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

The ov5640 driver supports different sizes for different mbus_codes.
In particular:

- 8bpp modes: high resolution sizes (>= 1280x720)
- 16bpp modes: all sizes
- 24bpp modes: low resolutions sizes (< 1280x720)

Restrict the frame sizes enumerations to the above constraints.

While at it, make sure the fse->mbus_code parameter is valid, and return
-EINVAL if it's not.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 0e9f04fdcee2..d29613b746f0 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -3449,14 +3449,28 @@ static int ov5640_enum_frame_size(struct v4l2_subdev *sd,
 				  struct v4l2_subdev_state *sd_state,
 				  struct v4l2_subdev_frame_size_enum *fse)
 {
+	u32 bpp = ov5640_code_to_bpp(fse->code);
+	unsigned int index = fse->index;
+
 	if (fse->pad != 0)
 		return -EINVAL;
-	if (fse->index >= OV5640_NUM_MODES)
+	if (!bpp)
+		return -EINVAL;
+
+	/* Only low-resolution modes are supported for 24bpp formats. */
+	if (bpp == 24 && index >= OV5640_MODE_720P_1280_720)
+		return -EINVAL;
+
+	/* FIXME: Low resolution modes don't work in 8bpp formats. */
+	if (bpp == 8)
+		index += OV5640_MODE_720P_1280_720;
+
+	if (index >= OV5640_NUM_MODES)
 		return -EINVAL;
 
-	fse->min_width = ov5640_mode_data[fse->index].width;
+	fse->min_width = ov5640_mode_data[index].width;
 	fse->max_width = fse->min_width;
-	fse->min_height = ov5640_mode_data[fse->index].height;
+	fse->min_height = ov5640_mode_data[index].height;
 	fse->max_height = fse->min_height;
 
 	return 0;
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 25/27] media: ov5640: Adjust format to bpp in s_fmt
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (23 preceding siblings ...)
  2022-02-23 14:56 ` [PATCH v4 24/27] media: ov5640: Restrict sizes to mbus code Jacopo Mondi
@ 2022-02-23 14:56 ` Jacopo Mondi
  2022-02-23 14:56 ` [PATCH v4 26/27] media: ov5640: Split DVP and CSI-2 formats Jacopo Mondi
  2022-02-23 14:56 ` [PATCH v4 27/27] media: ov5640: Move format mux config in format Jacopo Mondi
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:56 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

The ov5640 driver supports different sizes for different mbus_codes.
In particular:

- 8bpp modes: high resolution sizes (>= 1280x720)
- 16bpp modes: all sizes
- 24bpp modes: low resolutions sizes (< 1280x720)

Adjust the image sizes according to the above constraints.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index d29613b746f0..683f5795132f 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2689,6 +2689,7 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev *sd,
 				   enum ov5640_frame_rate fr,
 				   const struct ov5640_mode_info **new_mode)
 {
+	unsigned int bpp = ov5640_code_to_bpp(fmt->code);
 	struct ov5640_dev *sensor = to_ov5640_dev(sd);
 	const struct ov5640_mode_info *mode;
 	int i;
@@ -2696,6 +2697,17 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev *sd,
 	mode = ov5640_find_mode(sensor, fr, fmt->width, fmt->height, true);
 	if (!mode)
 		return -EINVAL;
+
+	/*
+	 * Adjust mode according to bpp:
+	 * - 8bpp modes work for resolution >= 1280x720
+	 * - 24bpp modes work resolution < 1280x720
+	 */
+	if (bpp == 8 && mode->width < 1280)
+		mode = &ov5640_mode_data[OV5640_MODE_720P_1280_720];
+	else if (bpp == 24 && mode->width > 1024)
+		mode = &ov5640_mode_data[OV5640_MODE_XGA_1024_768];
+
 	fmt->width = mode->width;
 	fmt->height = mode->height;
 
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 26/27] media: ov5640: Split DVP and CSI-2 formats
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (24 preceding siblings ...)
  2022-02-23 14:56 ` [PATCH v4 25/27] media: ov5640: Adjust format to bpp in s_fmt Jacopo Mondi
@ 2022-02-23 14:56 ` Jacopo Mondi
  2022-02-23 14:56 ` [PATCH v4 27/27] media: ov5640: Move format mux config in format Jacopo Mondi
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:56 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

The format enumeration list is shared between CSI-2 and DVP modes.
This lead to the enumeration of unsupported format variants in both
modes.

Separate the list of DVP and CSI-2 formats and create helpers to access
the correct one.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 125 ++++++++++++++++++++++++++-----------
 1 file changed, 88 insertions(+), 37 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 683f5795132f..2e86171f587d 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -188,11 +188,13 @@ enum ov5640_format_mux {
 	OV5640_FMT_MUX_RAW_CIP,
 };
 
-static const struct ov5640_pixfmt {
+struct ov5640_pixfmt {
 	u32 code;
 	u32 colorspace;
 	u8 bpp;
-} ov5640_formats[] = {
+};
+
+static const struct ov5640_pixfmt ov5640_dvp_formats[] = {
 	{
 		.code = MEDIA_BUS_FMT_JPEG_1X8,
 		.colorspace = V4L2_COLORSPACE_JPEG,
@@ -202,23 +204,48 @@ static const struct ov5640_pixfmt {
 		.colorspace = V4L2_COLORSPACE_SRGB,
 		.bpp = 16,
 	}, {
-		.code = MEDIA_BUS_FMT_UYVY8_1X16,
+		.code = MEDIA_BUS_FMT_YUYV8_2X8,
 		.colorspace = V4L2_COLORSPACE_SRGB,
 		.bpp = 16,
 	}, {
-		.code = MEDIA_BUS_FMT_YUYV8_2X8,
+		.code = MEDIA_BUS_FMT_RGB565_2X8_LE,
 		.colorspace = V4L2_COLORSPACE_SRGB,
 		.bpp = 16,
 	}, {
-		.code = MEDIA_BUS_FMT_YUYV8_1X16,
+		.code = MEDIA_BUS_FMT_RGB565_2X8_BE,
 		.colorspace = V4L2_COLORSPACE_SRGB,
 		.bpp = 16,
 	}, {
-		.code = MEDIA_BUS_FMT_RGB565_2X8_LE,
+		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 8,
+	}, {
+		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 8
+	}, {
+		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
 		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 8,
+	}, {
+		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 8,
+	},
+	{ /* sentinel */ }
+};
+
+static const struct ov5640_pixfmt ov5640_csi2_formats[] = {
+	{
+		.code = MEDIA_BUS_FMT_JPEG_1X8,
+		.colorspace = V4L2_COLORSPACE_JPEG,
 		.bpp = 16,
 	}, {
-		.code = MEDIA_BUS_FMT_RGB565_2X8_BE,
+		.code = MEDIA_BUS_FMT_UYVY8_1X16,
+		.colorspace = V4L2_COLORSPACE_SRGB,
+		.bpp = 16,
+	}, {
+		.code = MEDIA_BUS_FMT_YUYV8_1X16,
 		.colorspace = V4L2_COLORSPACE_SRGB,
 		.bpp = 16,
 	}, {
@@ -246,20 +273,9 @@ static const struct ov5640_pixfmt {
 		.colorspace = V4L2_COLORSPACE_SRGB,
 		.bpp = 8,
 	},
+	{ /* sentinel */ }
 };
 
-static u32 ov5640_code_to_bpp(u32 code)
-{
-	unsigned int i;
-
-	for (i = 0; i < ARRAY_SIZE(ov5640_formats); ++i) {
-		if (ov5640_formats[i].code == code)
-			return ov5640_formats[i].bpp;
-	}
-
-	return 0;
-}
-
 /*
  * FIXME: remove this when a subdev API becomes available
  * to set the MIPI CSI-2 virtual channel.
@@ -408,6 +424,32 @@ static inline bool ov5640_is_csi2(const struct ov5640_dev *sensor)
 	return sensor->ep.bus_type == V4L2_MBUS_CSI2_DPHY;
 }
 
+static inline const struct ov5640_pixfmt *ov5640_formats(struct ov5640_dev *sensor)
+{
+	return ov5640_is_csi2(sensor) ? ov5640_csi2_formats : ov5640_dvp_formats;
+}
+
+static const struct ov5640_pixfmt *ov5640_code_to_pixfmt(struct ov5640_dev *sensor,
+							 u32 code)
+{
+	const struct ov5640_pixfmt *formats = ov5640_formats(sensor);
+	unsigned int i;
+
+	for (i = 0; formats[i].code; ++i) {
+		if (formats[i].code == code)
+			return &formats[i];
+	}
+
+	return &formats[0];
+}
+
+static u32 ov5640_code_to_bpp(struct ov5640_dev *sensor, u32 code)
+{
+	const struct ov5640_pixfmt *format = ov5640_code_to_pixfmt(sensor, code);
+
+	return format->bpp;
+}
+
 /*
  * FIXME: all of these register tables are likely filled with
  * entries that set the register to their power-on default values,
@@ -1390,7 +1432,7 @@ static int ov5640_set_mipi_pclk(struct ov5640_dev *sensor)
 	 * (0x01=0.5ns).
 	 */
 	sample_rate = ov5640_pixel_rates[sensor->current_mode->pixel_rate]
-		    * (ov5640_code_to_bpp(fmt->code) / 8);
+		    * (ov5640_code_to_bpp(sensor, fmt->code) / 8);
 	pclk_period = 2000000000U / sample_rate;
 
 	/* Program the clock tree registers. */
@@ -1455,7 +1497,7 @@ static int ov5640_set_dvp_pclk(struct ov5640_dev *sensor)
 	int ret;
 
 	rate = ov5640_calc_pixel_rate(sensor);
-	rate *= ov5640_code_to_bpp(sensor->fmt.code);
+	rate *= ov5640_code_to_bpp(sensor, sensor->fmt.code);
 	rate /= sensor->ep.bus.parallel.bus_width;
 
 	ov5640_calc_pclk(sensor, rate, &prediv, &mult, &sysdiv, &pll_rdiv,
@@ -2689,15 +2731,18 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev *sd,
 				   enum ov5640_frame_rate fr,
 				   const struct ov5640_mode_info **new_mode)
 {
-	unsigned int bpp = ov5640_code_to_bpp(fmt->code);
 	struct ov5640_dev *sensor = to_ov5640_dev(sd);
 	const struct ov5640_mode_info *mode;
-	int i;
+	const struct ov5640_pixfmt *pixfmt;
+	unsigned int bpp;
 
 	mode = ov5640_find_mode(sensor, fr, fmt->width, fmt->height, true);
 	if (!mode)
 		return -EINVAL;
 
+	pixfmt = ov5640_code_to_pixfmt(sensor, fmt->code);
+	bpp = pixfmt->bpp;
+
 	/*
 	 * Adjust mode according to bpp:
 	 * - 8bpp modes work for resolution >= 1280x720
@@ -2714,14 +2759,8 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev *sd,
 	if (new_mode)
 		*new_mode = mode;
 
-	for (i = 0; i < ARRAY_SIZE(ov5640_formats); i++)
-		if (ov5640_formats[i].code == fmt->code)
-			break;
-	if (i >= ARRAY_SIZE(ov5640_formats))
-		i = 0;
-
-	fmt->code = ov5640_formats[i].code;
-	fmt->colorspace = ov5640_formats[i].colorspace;
+	fmt->code = pixfmt->code;
+	fmt->colorspace = pixfmt->colorspace;
 	fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
 	fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
 	fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
@@ -2763,7 +2802,7 @@ static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
 	 * progressively slow it down if it exceeds 1GHz.
 	 */
 	num_lanes = sensor->ep.bus.mipi_csi2.num_data_lanes;
-	bpp = ov5640_code_to_bpp(fmt->code);
+	bpp = ov5640_code_to_bpp(sensor, fmt->code);
 	do {
 		pixel_rate = ov5640_pixel_rates[pixel_rate_id];
 		link_freq = pixel_rate * bpp / (2 * num_lanes);
@@ -3461,7 +3500,8 @@ static int ov5640_enum_frame_size(struct v4l2_subdev *sd,
 				  struct v4l2_subdev_state *sd_state,
 				  struct v4l2_subdev_frame_size_enum *fse)
 {
-	u32 bpp = ov5640_code_to_bpp(fse->code);
+	struct ov5640_dev *sensor = to_ov5640_dev(sd);
+	u32 bpp = ov5640_code_to_bpp(sensor, fse->code);
 	unsigned int index = fse->index;
 
 	if (fse->pad != 0)
@@ -3588,12 +3628,23 @@ static int ov5640_enum_mbus_code(struct v4l2_subdev *sd,
 				 struct v4l2_subdev_state *sd_state,
 				 struct v4l2_subdev_mbus_code_enum *code)
 {
-	if (code->pad != 0)
-		return -EINVAL;
-	if (code->index >= ARRAY_SIZE(ov5640_formats))
+	struct ov5640_dev *sensor = to_ov5640_dev(sd);
+	const struct ov5640_pixfmt *formats;
+	unsigned int num_formats;
+
+	if (ov5640_is_csi2(sensor)) {
+		formats = ov5640_csi2_formats;
+		num_formats = ARRAY_SIZE(ov5640_csi2_formats) - 1;
+	} else {
+		formats = ov5640_dvp_formats;
+		num_formats = ARRAY_SIZE(ov5640_dvp_formats) - 1;
+	}
+
+	if (code->index >= num_formats)
 		return -EINVAL;
 
-	code->code = ov5640_formats[code->index].code;
+	code->code = formats[code->index].code;
+
 	return 0;
 }
 
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 27/27] media: ov5640: Move format mux config in format
  2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
                   ` (25 preceding siblings ...)
  2022-02-23 14:56 ` [PATCH v4 26/27] media: ov5640: Split DVP and CSI-2 formats Jacopo Mondi
@ 2022-02-23 14:56 ` Jacopo Mondi
  26 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-23 14:56 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Jacopo Mondi, laurent.pinchart, sakari.ailus, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

The image format produced by the sensor is controlled by two registers,
whose values computation is open coded in ov5640_set_framefmt().

As we have a list of formats already, move the OV5640_REG_FORMAT_CONTROL00
and OV5640_REG_ISP_FORMAT_MUX_CTRL register values to the static list
of formats instead of open coding it.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5640.c | 234 ++++++++++++++++++-------------------
 1 file changed, 117 insertions(+), 117 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 2e86171f587d..9351a4b7ef8e 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -192,86 +192,142 @@ struct ov5640_pixfmt {
 	u32 code;
 	u32 colorspace;
 	u8 bpp;
+	u8 ctrl00;
+	enum ov5640_format_mux mux;
 };
 
 static const struct ov5640_pixfmt ov5640_dvp_formats[] = {
 	{
-		.code = MEDIA_BUS_FMT_JPEG_1X8,
-		.colorspace = V4L2_COLORSPACE_JPEG,
-		.bpp = 16,
+		/* YUV422, YUYV */
+		.code		= MEDIA_BUS_FMT_JPEG_1X8,
+		.colorspace	= V4L2_COLORSPACE_JPEG,
+		.bpp		= 16,
+		.ctrl00		= 0x30,
+		.mux		= OV5640_FMT_MUX_YUV422,
 	}, {
-		.code = MEDIA_BUS_FMT_UYVY8_2X8,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 16,
+		/* YUV422, UYVY */
+		.code		= MEDIA_BUS_FMT_UYVY8_2X8,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 16,
+		.ctrl00		= 0x3f,
+		.mux		= OV5640_FMT_MUX_YUV422,
 	}, {
-		.code = MEDIA_BUS_FMT_YUYV8_2X8,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 16,
+		/* YUV422, YUYV */
+		.code		= MEDIA_BUS_FMT_YUYV8_2X8,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 16,
+		.ctrl00		= 0x30,
+		.mux		= OV5640_FMT_MUX_YUV422,
 	}, {
-		.code = MEDIA_BUS_FMT_RGB565_2X8_LE,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 16,
+		/* RGB565 {g[2:0],b[4:0]},{r[4:0],g[5:3]} */
+		.code		= MEDIA_BUS_FMT_RGB565_2X8_LE,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 16,
+		.ctrl00		= 0x6f,
+		.mux		= OV5640_FMT_MUX_RGB,
 	}, {
-		.code = MEDIA_BUS_FMT_RGB565_2X8_BE,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 16,
+		/* RGB565 {r[4:0],g[5:3]},{g[2:0],b[4:0]} */
+		.code		= MEDIA_BUS_FMT_RGB565_2X8_BE,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 16,
+		.ctrl00		= 0x61,
+		.mux		= OV5640_FMT_MUX_RGB,
 	}, {
-		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 8,
+		/* Raw, BGBG... / GRGR... */
+		.code		= MEDIA_BUS_FMT_SBGGR8_1X8,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 8,
+		.ctrl00		= 0x00,
+		.mux		= OV5640_FMT_MUX_RAW_DPC,
 	}, {
-		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 8
+		/* Raw bayer, GBGB... / RGRG... */
+		.code		= MEDIA_BUS_FMT_SGBRG8_1X8,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 8,
+		.ctrl00		= 0x01,
+		.mux		= OV5640_FMT_MUX_RAW_DPC,
 	}, {
-		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 8,
+		/* Raw bayer, GRGR... / BGBG... */
+		.code		= MEDIA_BUS_FMT_SGRBG8_1X8,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 8,
+		.ctrl00		= 0x02,
+		.mux		= OV5640_FMT_MUX_RAW_DPC,
 	}, {
-		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 8,
+		/* Raw bayer, RGRG... / GBGB... */
+		.code		= MEDIA_BUS_FMT_SRGGB8_1X8,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 8,
+		.ctrl00		= 0x03,
+		.mux		= OV5640_FMT_MUX_RAW_DPC,
 	},
 	{ /* sentinel */ }
 };
 
 static const struct ov5640_pixfmt ov5640_csi2_formats[] = {
 	{
-		.code = MEDIA_BUS_FMT_JPEG_1X8,
-		.colorspace = V4L2_COLORSPACE_JPEG,
-		.bpp = 16,
+		/* YUV422, YUYV */
+		.code		= MEDIA_BUS_FMT_JPEG_1X8,
+		.colorspace	= V4L2_COLORSPACE_JPEG,
+		.bpp		= 16,
+		.ctrl00		= 0x30,
+		.mux		= OV5640_FMT_MUX_YUV422,
 	}, {
-		.code = MEDIA_BUS_FMT_UYVY8_1X16,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 16,
+		/* YUV422, UYVY */
+		.code		= MEDIA_BUS_FMT_UYVY8_1X16,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 16,
+		.ctrl00		= 0x3f,
+		.mux		= OV5640_FMT_MUX_YUV422,
 	}, {
-		.code = MEDIA_BUS_FMT_YUYV8_1X16,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 16,
+		/* YUV422, YUYV */
+		.code		= MEDIA_BUS_FMT_YUYV8_1X16,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 16,
+		.ctrl00		= 0x30,
+		.mux		= OV5640_FMT_MUX_YUV422,
 	}, {
-		.code = MEDIA_BUS_FMT_RGB565_1X16,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 16,
+		/* RGB565 {g[2:0],b[4:0]},{r[4:0],g[5:3]} */
+		.code		= MEDIA_BUS_FMT_RGB565_1X16,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 16,
+		.ctrl00		= 0x6f,
+		.mux		= OV5640_FMT_MUX_RGB,
 	}, {
-		.code = MEDIA_BUS_FMT_BGR888_1X24,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 24,
+		/* BGR888: RGB */
+		.code		= MEDIA_BUS_FMT_BGR888_1X24,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 24,
+		.ctrl00		= 0x23,
+		.mux		= OV5640_FMT_MUX_RGB,
 	}, {
-		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 8,
+		/* Raw, BGBG... / GRGR... */
+		.code		= MEDIA_BUS_FMT_SBGGR8_1X8,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 8,
+		.ctrl00		= 0x00,
+		.mux		= OV5640_FMT_MUX_RAW_DPC,
 	}, {
-		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 8
+		/* Raw bayer, GBGB... / RGRG... */
+		.code		= MEDIA_BUS_FMT_SGBRG8_1X8,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 8,
+		.ctrl00		= 0x01,
+		.mux		= OV5640_FMT_MUX_RAW_DPC,
 	}, {
-		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 8,
+		/* Raw bayer, GRGR... / BGBG... */
+		.code		= MEDIA_BUS_FMT_SGRBG8_1X8,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 8,
+		.ctrl00		= 0x02,
+		.mux		= OV5640_FMT_MUX_RAW_DPC,
 	}, {
-		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
-		.colorspace = V4L2_COLORSPACE_SRGB,
-		.bpp = 8,
+		/* Raw bayer, RGRG... / GBGB... */
+		.code		= MEDIA_BUS_FMT_SRGGB8_1X8,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.bpp		= 8,
+		.ctrl00		= 0x03,
+		.mux		= OV5640_FMT_MUX_RAW_DPC,
 	},
 	{ /* sentinel */ }
 };
@@ -2938,76 +2994,20 @@ static int ov5640_get_selection(struct v4l2_subdev *sd,
 static int ov5640_set_framefmt(struct ov5640_dev *sensor,
 			       struct v4l2_mbus_framefmt *format)
 {
+	const struct ov5640_pixfmt *pixfmt = ov5640_code_to_pixfmt(sensor,
+								   format->code);
+	bool is_jpeg = format->code == MEDIA_BUS_FMT_JPEG_1X8;
 	int ret = 0;
-	bool is_jpeg = false;
-	u8 fmt, mux;
-
-	switch (format->code) {
-	case MEDIA_BUS_FMT_UYVY8_1X16:
-	case MEDIA_BUS_FMT_UYVY8_2X8:
-		/* YUV422, UYVY */
-		fmt = 0x3f;
-		mux = OV5640_FMT_MUX_YUV422;
-		break;
-	case MEDIA_BUS_FMT_YUYV8_1X16:
-	case MEDIA_BUS_FMT_YUYV8_2X8:
-		/* YUV422, YUYV */
-		fmt = 0x30;
-		mux = OV5640_FMT_MUX_YUV422;
-		break;
-	case MEDIA_BUS_FMT_RGB565_2X8_LE:
-	case MEDIA_BUS_FMT_RGB565_1X16:
-		/* RGB565 {g[2:0],b[4:0]},{r[4:0],g[5:3]} */
-		fmt = 0x6F;
-		mux = OV5640_FMT_MUX_RGB;
-		break;
-	case MEDIA_BUS_FMT_RGB565_2X8_BE:
-		/* RGB565 {r[4:0],g[5:3]},{g[2:0],b[4:0]} */
-		fmt = 0x61;
-		mux = OV5640_FMT_MUX_RGB;
-		break;
-	case MEDIA_BUS_FMT_BGR888_1X24:
-		/* BGR888: RGB */
-		fmt = 0x23;
-		mux = OV5640_FMT_MUX_RGB;
-		break;
-	case MEDIA_BUS_FMT_JPEG_1X8:
-		/* YUV422, YUYV */
-		fmt = 0x30;
-		mux = OV5640_FMT_MUX_YUV422;
-		is_jpeg = true;
-		break;
-	case MEDIA_BUS_FMT_SBGGR8_1X8:
-		/* Raw, BGBG... / GRGR... */
-		fmt = 0x00;
-		mux = OV5640_FMT_MUX_RAW_DPC;
-		break;
-	case MEDIA_BUS_FMT_SGBRG8_1X8:
-		/* Raw bayer, GBGB... / RGRG... */
-		fmt = 0x01;
-		mux = OV5640_FMT_MUX_RAW_DPC;
-		break;
-	case MEDIA_BUS_FMT_SGRBG8_1X8:
-		/* Raw bayer, GRGR... / BGBG... */
-		fmt = 0x02;
-		mux = OV5640_FMT_MUX_RAW_DPC;
-		break;
-	case MEDIA_BUS_FMT_SRGGB8_1X8:
-		/* Raw bayer, RGRG... / GBGB... */
-		fmt = 0x03;
-		mux = OV5640_FMT_MUX_RAW_DPC;
-		break;
-	default:
-		return -EINVAL;
-	}
 
 	/* FORMAT CONTROL00: YUV and RGB formatting */
-	ret = ov5640_write_reg(sensor, OV5640_REG_FORMAT_CONTROL00, fmt);
+	ret = ov5640_write_reg(sensor, OV5640_REG_FORMAT_CONTROL00,
+			       pixfmt->ctrl00);
 	if (ret)
 		return ret;
 
 	/* FORMAT MUX CONTROL: ISP YUV or RGB */
-	ret = ov5640_write_reg(sensor, OV5640_REG_ISP_FORMAT_MUX_CTRL, mux);
+	ret = ov5640_write_reg(sensor, OV5640_REG_ISP_FORMAT_MUX_CTRL,
+			       pixfmt->mux);
 	if (ret)
 		return ret;
 
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 16/27] media: ov5640: Add VBLANK control
  2022-02-23 14:55 ` [PATCH v4 16/27] media: ov5640: Add VBLANK control Jacopo Mondi
@ 2022-02-23 20:23   ` Sakari Ailus
  2022-02-24  8:58     ` Jacopo Mondi
  0 siblings, 1 reply; 33+ messages in thread
From: Sakari Ailus @ 2022-02-23 20:23 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: Steve Longerbeam, laurent.pinchart, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Hi Jacopo,

On Wed, Feb 23, 2022 at 03:55:52PM +0100, Jacopo Mondi wrote:
> Add the VBLANK control which allows to select the duration of the
> frame vertical blankings and allows to control the framerate.
> 
> The VBLANK control also modifies the exposure time range, which cannot
> exceed the maximum frame length.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/i2c/ov5640.c | 57 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index bf84f32436b1..134129fc13ae 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -36,6 +36,10 @@
>  #define OV5640_PIXEL_ARRAY_WIDTH	2592
>  #define OV5640_PIXEL_ARRAY_HEIGHT	1944
>  
> +/* FIXME: not documented. */
> +#define OV5640_MIN_VBLANK	24
> +#define OV5640_MAX_VTS		1968
> +
>  #define OV5640_DEFAULT_SLAVE_ID 0x3c
>  
>  #define OV5640_LINK_RATE_MAX		490000000U
> @@ -321,6 +325,7 @@ struct ov5640_ctrls {
>  	struct v4l2_ctrl *pixel_rate;
>  	struct v4l2_ctrl *link_freq;
>  	struct v4l2_ctrl *hblank;
> +	struct v4l2_ctrl *vblank;
>  	struct {
>  		struct v4l2_ctrl *auto_exp;
>  		struct v4l2_ctrl *exposure;
> @@ -2693,6 +2698,7 @@ static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
>  	struct v4l2_mbus_framefmt *fmt = &sensor->fmt;
>  	enum ov5640_pixel_rate_id pixel_rate_id = mode->pixel_rate;
>  	const struct ov5640_timings *timings;
> +	s64 exposure_val, exposure_max;
>  	unsigned int hblank;
>  	unsigned int i = 0;
>  	u32 pixel_rate;
> @@ -2751,6 +2757,20 @@ static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
>  	__v4l2_ctrl_modify_range(sensor->ctrls.hblank,
>  				 hblank, hblank, 1, hblank);
>  
> +	__v4l2_ctrl_modify_range(sensor->ctrls.vblank,
> +				 OV5640_MIN_VBLANK,
> +				 OV5640_MAX_VTS - mode->height, 1,
> +				 timings->vblank_def);
> +	__v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, timings->vblank_def);
> +
> +	exposure_max = timings->crop.height + timings->vblank_def - 4;
> +	exposure_val = clamp((s64)sensor->ctrls.exposure->val,
> +			     (s64)sensor->ctrls.exposure->minimum,

No need for casting here.

Do you need 64-bit values here? The control value is a signed 32-bit
number.

> +			     (s64)exposure_max);
> +	__v4l2_ctrl_modify_range(sensor->ctrls.exposure,
> +				 sensor->ctrls.exposure->minimum,
> +				 exposure_max, 1, exposure_val);
> +
>  	return 0;
>  }
>  
> @@ -3123,6 +3143,15 @@ static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
>  			      (BIT(2) | BIT(1)) : 0);
>  }
>  
> +static int ov5640_set_ctrl_vblank(struct ov5640_dev *sensor, int value)
> +{
> +	const struct ov5640_mode_info *mode = sensor->current_mode;
> +
> +	/* Update the VTOT timing register value. */
> +	return ov5640_write_reg16(sensor, OV5640_REG_TIMING_VTS,
> +				  mode->height + value);
> +}
> +
>  static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
>  {
>  	struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
> @@ -3153,10 +3182,29 @@ static int ov5640_s_ctrl(struct v4l2_ctrl *ctrl)
>  {
>  	struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
>  	struct ov5640_dev *sensor = to_ov5640_dev(sd);
> +	const struct ov5640_mode_info *mode = sensor->current_mode;
> +	const struct ov5640_timings *timings;
> +	unsigned int exp_max;
>  	int ret;
>  
>  	/* v4l2_ctrl_lock() locks our own mutex */
>  
> +	if (ov5640_is_csi2(sensor))
> +		timings = &mode->csi2_timings;
> +	else
> +		timings = &mode->dvp_timings;

Could you use ov5640_timings() here?

> +
> +	switch (ctrl->id) {
> +	case V4L2_CID_VBLANK:
> +		/* Update the exposure range to the newly programmed vblank. */
> +		exp_max = mode->height + ctrl->val - 4;
> +		__v4l2_ctrl_modify_range(sensor->ctrls.exposure,
> +					 sensor->ctrls.exposure->minimum,
> +					 exp_max, sensor->ctrls.exposure->step,
> +					 timings->vblank_def);
> +		break;
> +	}
> +
>  	/*
>  	 * If the device is not powered up by the host driver do
>  	 * not apply any controls to H/W at this time. Instead
> @@ -3196,6 +3244,9 @@ static int ov5640_s_ctrl(struct v4l2_ctrl *ctrl)
>  	case V4L2_CID_VFLIP:
>  		ret = ov5640_set_ctrl_vflip(sensor, ctrl->val);
>  		break;
> +	case V4L2_CID_VBLANK:
> +		ret = ov5640_set_ctrl_vblank(sensor, ctrl->val);
> +		break;
>  	default:
>  		ret = -EINVAL;
>  		break;
> @@ -3216,6 +3267,7 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
>  	struct ov5640_ctrls *ctrls = &sensor->ctrls;
>  	struct v4l2_ctrl_handler *hdl = &ctrls->handler;
>  	const struct ov5640_timings *timings;
> +	unsigned int max_vblank;
>  	unsigned int hblank;
>  	int ret;
>  
> @@ -3241,6 +3293,11 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
>  	ctrls->hblank = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HBLANK, hblank,
>  					  hblank, 1, hblank);
>  
> +	max_vblank = OV5640_MAX_VTS - mode->height;
> +	ctrls->vblank = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VBLANK,
> +					  OV5640_MIN_VBLANK, max_vblank,
> +					  1, timings->vblank_def);
> +
>  	/* Auto/manual white balance */
>  	ctrls->auto_wb = v4l2_ctrl_new_std(hdl, ops,
>  					   V4L2_CID_AUTO_WHITE_BALANCE,
> -- 
> 2.35.0
> 

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 18/27] media: ov5640: Implement init_cfg
  2022-02-23 14:55 ` [PATCH v4 18/27] media: ov5640: Implement init_cfg Jacopo Mondi
@ 2022-02-23 20:25   ` Sakari Ailus
  2022-02-24  8:08     ` Jacopo Mondi
  0 siblings, 1 reply; 33+ messages in thread
From: Sakari Ailus @ 2022-02-23 20:25 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: Steve Longerbeam, laurent.pinchart, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Hi Jacopo,

On Wed, Feb 23, 2022 at 03:55:54PM +0100, Jacopo Mondi wrote:
> Implement the init_cfg pad operation to initialize the subdev state
> format to the default one.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/i2c/ov5640.c | 34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 6c2c76b61140..c1fc2ab1e0cf 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -408,6 +408,18 @@ static inline bool ov5640_is_csi2(const struct ov5640_dev *sensor)
>   * over i2c.
>   */
>  /* YUV422 UYVY VGA@30fps */
> +
> +static const struct v4l2_mbus_framefmt ov5640_default_fmt = {
> +	.code = MEDIA_BUS_FMT_UYVY8_2X8,
> +	.width = 640,
> +	.height = 480,
> +	.colorspace = V4L2_COLORSPACE_SRGB,
> +	.ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SRGB),
> +	.quantization = V4L2_QUANTIZATION_FULL_RANGE,
> +	.xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SRGB),
> +	.field = V4L2_FIELD_NONE,
> +};
> +
>  static const struct reg_value ov5640_init_setting[] = {
>  	{0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
>  	{0x3103, 0x03, 0, 0}, {0x3630, 0x36, 0, 0},
> @@ -3515,6 +3527,16 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int enable)
>  	return ret;
>  }
>  
> +static int ov5640_init_cfg(struct v4l2_subdev *sd,
> +			   struct v4l2_subdev_state *state)
> +{
> +	struct v4l2_mbus_framefmt *fmt = v4l2_subdev_get_try_format(sd, state, 0);

Could you run

	$ ./scripts/checkpatch.pl --strict --max-line-length=80

on the entire set, and see what else is trivial to fix?

> +
> +	*fmt = ov5640_default_fmt;
> +
> +	return 0;
> +}
> +
>  static const struct v4l2_subdev_core_ops ov5640_core_ops = {
>  	.s_power = ov5640_s_power,
>  	.log_status = v4l2_ctrl_subdev_log_status,

-- 
Regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 18/27] media: ov5640: Implement init_cfg
  2022-02-23 20:25   ` Sakari Ailus
@ 2022-02-24  8:08     ` Jacopo Mondi
  2022-02-24  9:20       ` Sakari Ailus
  0 siblings, 1 reply; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-24  8:08 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Steve Longerbeam, laurent.pinchart, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Hi Sakari,

On Wed, Feb 23, 2022 at 10:25:14PM +0200, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Wed, Feb 23, 2022 at 03:55:54PM +0100, Jacopo Mondi wrote:
> > Implement the init_cfg pad operation to initialize the subdev state
> > format to the default one.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/media/i2c/ov5640.c | 34 ++++++++++++++++++++++++----------
> >  1 file changed, 24 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > index 6c2c76b61140..c1fc2ab1e0cf 100644
> > --- a/drivers/media/i2c/ov5640.c
> > +++ b/drivers/media/i2c/ov5640.c
> > @@ -408,6 +408,18 @@ static inline bool ov5640_is_csi2(const struct ov5640_dev *sensor)
> >   * over i2c.
> >   */
> >  /* YUV422 UYVY VGA@30fps */
> > +
> > +static const struct v4l2_mbus_framefmt ov5640_default_fmt = {
> > +	.code = MEDIA_BUS_FMT_UYVY8_2X8,
> > +	.width = 640,
> > +	.height = 480,
> > +	.colorspace = V4L2_COLORSPACE_SRGB,
> > +	.ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SRGB),
> > +	.quantization = V4L2_QUANTIZATION_FULL_RANGE,
> > +	.xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SRGB),
> > +	.field = V4L2_FIELD_NONE,
> > +};
> > +
> >  static const struct reg_value ov5640_init_setting[] = {
> >  	{0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
> >  	{0x3103, 0x03, 0, 0}, {0x3630, 0x36, 0, 0},
> > @@ -3515,6 +3527,16 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int enable)
> >  	return ret;
> >  }
> >
> > +static int ov5640_init_cfg(struct v4l2_subdev *sd,
> > +			   struct v4l2_subdev_state *state)
> > +{
> > +	struct v4l2_mbus_framefmt *fmt = v4l2_subdev_get_try_format(sd, state, 0);
>
> Could you run
>
> 	$ ./scripts/checkpatch.pl --strict --max-line-length=80
>
> on the entire set, and see what else is trivial to fix?
>

I was asked the same by Laurent, and I was honestly surprised as I
thought from our last discussions that 100 cols limit was encouraged.

Oh well, I suspect that' the price for having dropped the ball with
"[PATCH] media: Document coding style requirements"

I'll check with 80 cols limits.

Thanks
   j

> > +
> > +	*fmt = ov5640_default_fmt;
> > +
> > +	return 0;
> > +}
> > +
> >  static const struct v4l2_subdev_core_ops ov5640_core_ops = {
> >  	.s_power = ov5640_s_power,
> >  	.log_status = v4l2_ctrl_subdev_log_status,
>
> --
> Regards,
>
> Sakari Ailus

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 16/27] media: ov5640: Add VBLANK control
  2022-02-23 20:23   ` Sakari Ailus
@ 2022-02-24  8:58     ` Jacopo Mondi
  0 siblings, 0 replies; 33+ messages in thread
From: Jacopo Mondi @ 2022-02-24  8:58 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Steve Longerbeam, laurent.pinchart, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Hi Sakari

On Wed, Feb 23, 2022 at 10:23:19PM +0200, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Wed, Feb 23, 2022 at 03:55:52PM +0100, Jacopo Mondi wrote:
> > Add the VBLANK control which allows to select the duration of the
> > frame vertical blankings and allows to control the framerate.
> >
> > The VBLANK control also modifies the exposure time range, which cannot
> > exceed the maximum frame length.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/media/i2c/ov5640.c | 57 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> >
> > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > index bf84f32436b1..134129fc13ae 100644
> > --- a/drivers/media/i2c/ov5640.c
> > +++ b/drivers/media/i2c/ov5640.c
> > @@ -36,6 +36,10 @@
> >  #define OV5640_PIXEL_ARRAY_WIDTH	2592
> >  #define OV5640_PIXEL_ARRAY_HEIGHT	1944
> >
> > +/* FIXME: not documented. */
> > +#define OV5640_MIN_VBLANK	24
> > +#define OV5640_MAX_VTS		1968
> > +
> >  #define OV5640_DEFAULT_SLAVE_ID 0x3c
> >
> >  #define OV5640_LINK_RATE_MAX		490000000U
> > @@ -321,6 +325,7 @@ struct ov5640_ctrls {
> >  	struct v4l2_ctrl *pixel_rate;
> >  	struct v4l2_ctrl *link_freq;
> >  	struct v4l2_ctrl *hblank;
> > +	struct v4l2_ctrl *vblank;
> >  	struct {
> >  		struct v4l2_ctrl *auto_exp;
> >  		struct v4l2_ctrl *exposure;
> > @@ -2693,6 +2698,7 @@ static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
> >  	struct v4l2_mbus_framefmt *fmt = &sensor->fmt;
> >  	enum ov5640_pixel_rate_id pixel_rate_id = mode->pixel_rate;
> >  	const struct ov5640_timings *timings;
> > +	s64 exposure_val, exposure_max;
> >  	unsigned int hblank;
> >  	unsigned int i = 0;
> >  	u32 pixel_rate;
> > @@ -2751,6 +2757,20 @@ static int ov5640_update_pixel_rate(struct ov5640_dev *sensor)
> >  	__v4l2_ctrl_modify_range(sensor->ctrls.hblank,
> >  				 hblank, hblank, 1, hblank);
> >
> > +	__v4l2_ctrl_modify_range(sensor->ctrls.vblank,
> > +				 OV5640_MIN_VBLANK,
> > +				 OV5640_MAX_VTS - mode->height, 1,
> > +				 timings->vblank_def);
> > +	__v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, timings->vblank_def);
> > +
> > +	exposure_max = timings->crop.height + timings->vblank_def - 4;
> > +	exposure_val = clamp((s64)sensor->ctrls.exposure->val,
> > +			     (s64)sensor->ctrls.exposure->minimum,
>
> No need for casting here.
>
> Do you need 64-bit values here? The control value is a signed 32-bit
> number.

I need the cast, exposure->minimum is an s64 while exposure->val is an s32.
I'll cast everything to (s32) though

        struct v4l2_ctrl {
                ...
                s64 minimum, maximum, default_value;
                ...
                s32 val;
                ...
        };

>
> > +			     (s64)exposure_max);
> > +	__v4l2_ctrl_modify_range(sensor->ctrls.exposure,
> > +				 sensor->ctrls.exposure->minimum,
> > +				 exposure_max, 1, exposure_val);
> > +
> >  	return 0;
> >  }
> >
> > @@ -3123,6 +3143,15 @@ static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
> >  			      (BIT(2) | BIT(1)) : 0);
> >  }
> >
> > +static int ov5640_set_ctrl_vblank(struct ov5640_dev *sensor, int value)
> > +{
> > +	const struct ov5640_mode_info *mode = sensor->current_mode;
> > +
> > +	/* Update the VTOT timing register value. */
> > +	return ov5640_write_reg16(sensor, OV5640_REG_TIMING_VTS,
> > +				  mode->height + value);
> > +}
> > +
> >  static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> >  {
> >  	struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
> > @@ -3153,10 +3182,29 @@ static int ov5640_s_ctrl(struct v4l2_ctrl *ctrl)
> >  {
> >  	struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
> >  	struct ov5640_dev *sensor = to_ov5640_dev(sd);
> > +	const struct ov5640_mode_info *mode = sensor->current_mode;
> > +	const struct ov5640_timings *timings;
> > +	unsigned int exp_max;
> >  	int ret;
> >
> >  	/* v4l2_ctrl_lock() locks our own mutex */
> >
> > +	if (ov5640_is_csi2(sensor))
> > +		timings = &mode->csi2_timings;
> > +	else
> > +		timings = &mode->dvp_timings;
>
> Could you use ov5640_timings() here?
>

I should indeed

Thanks
  j

> > +
> > +	switch (ctrl->id) {
> > +	case V4L2_CID_VBLANK:
> > +		/* Update the exposure range to the newly programmed vblank. */
> > +		exp_max = mode->height + ctrl->val - 4;
> > +		__v4l2_ctrl_modify_range(sensor->ctrls.exposure,
> > +					 sensor->ctrls.exposure->minimum,
> > +					 exp_max, sensor->ctrls.exposure->step,
> > +					 timings->vblank_def);
> > +		break;
> > +	}
> > +
> >  	/*
> >  	 * If the device is not powered up by the host driver do
> >  	 * not apply any controls to H/W at this time. Instead
> > @@ -3196,6 +3244,9 @@ static int ov5640_s_ctrl(struct v4l2_ctrl *ctrl)
> >  	case V4L2_CID_VFLIP:
> >  		ret = ov5640_set_ctrl_vflip(sensor, ctrl->val);
> >  		break;
> > +	case V4L2_CID_VBLANK:
> > +		ret = ov5640_set_ctrl_vblank(sensor, ctrl->val);
> > +		break;
> >  	default:
> >  		ret = -EINVAL;
> >  		break;
> > @@ -3216,6 +3267,7 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
> >  	struct ov5640_ctrls *ctrls = &sensor->ctrls;
> >  	struct v4l2_ctrl_handler *hdl = &ctrls->handler;
> >  	const struct ov5640_timings *timings;
> > +	unsigned int max_vblank;
> >  	unsigned int hblank;
> >  	int ret;
> >
> > @@ -3241,6 +3293,11 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
> >  	ctrls->hblank = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HBLANK, hblank,
> >  					  hblank, 1, hblank);
> >
> > +	max_vblank = OV5640_MAX_VTS - mode->height;
> > +	ctrls->vblank = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VBLANK,
> > +					  OV5640_MIN_VBLANK, max_vblank,
> > +					  1, timings->vblank_def);
> > +
> >  	/* Auto/manual white balance */
> >  	ctrls->auto_wb = v4l2_ctrl_new_std(hdl, ops,
> >  					   V4L2_CID_AUTO_WHITE_BALANCE,
> > --
> > 2.35.0
> >
>
> --
> Sakari Ailus

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 18/27] media: ov5640: Implement init_cfg
  2022-02-24  8:08     ` Jacopo Mondi
@ 2022-02-24  9:20       ` Sakari Ailus
  0 siblings, 0 replies; 33+ messages in thread
From: Sakari Ailus @ 2022-02-24  9:20 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: Steve Longerbeam, laurent.pinchart, hverkuil-cisco,
	mirela.rabulea, xavier.roumegue, tomi.valkeinen, hugues.fruchet,
	prabhakar.mahadev-lad.rj, aford173, festevam, Eugen.Hristev,
	jbrunet, paul.elder, Mauro Carvalho Chehab, linux-media

Hi Jacopo,

On Thu, Feb 24, 2022 at 09:08:34AM +0100, Jacopo Mondi wrote:
> Hi Sakari,
> 
> On Wed, Feb 23, 2022 at 10:25:14PM +0200, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Wed, Feb 23, 2022 at 03:55:54PM +0100, Jacopo Mondi wrote:
> > > Implement the init_cfg pad operation to initialize the subdev state
> > > format to the default one.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  drivers/media/i2c/ov5640.c | 34 ++++++++++++++++++++++++----------
> > >  1 file changed, 24 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > > index 6c2c76b61140..c1fc2ab1e0cf 100644
> > > --- a/drivers/media/i2c/ov5640.c
> > > +++ b/drivers/media/i2c/ov5640.c
> > > @@ -408,6 +408,18 @@ static inline bool ov5640_is_csi2(const struct ov5640_dev *sensor)
> > >   * over i2c.
> > >   */
> > >  /* YUV422 UYVY VGA@30fps */
> > > +
> > > +static const struct v4l2_mbus_framefmt ov5640_default_fmt = {
> > > +	.code = MEDIA_BUS_FMT_UYVY8_2X8,
> > > +	.width = 640,
> > > +	.height = 480,
> > > +	.colorspace = V4L2_COLORSPACE_SRGB,
> > > +	.ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(V4L2_COLORSPACE_SRGB),
> > > +	.quantization = V4L2_QUANTIZATION_FULL_RANGE,
> > > +	.xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SRGB),
> > > +	.field = V4L2_FIELD_NONE,
> > > +};
> > > +
> > >  static const struct reg_value ov5640_init_setting[] = {
> > >  	{0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
> > >  	{0x3103, 0x03, 0, 0}, {0x3630, 0x36, 0, 0},
> > > @@ -3515,6 +3527,16 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int enable)
> > >  	return ret;
> > >  }
> > >
> > > +static int ov5640_init_cfg(struct v4l2_subdev *sd,
> > > +			   struct v4l2_subdev_state *state)
> > > +{
> > > +	struct v4l2_mbus_framefmt *fmt = v4l2_subdev_get_try_format(sd, state, 0);
> >
> > Could you run
> >
> > 	$ ./scripts/checkpatch.pl --strict --max-line-length=80
> >
> > on the entire set, and see what else is trivial to fix?
> >
> 
> I was asked the same by Laurent, and I was honestly surprised as I
> thought from our last discussions that 100 cols limit was encouraged.

At least I didn't draw that conclusion from the discussion. It was rather
that people had somewhat differing opinions what else should be documented
besides what's there right now.

> 
> Oh well, I suspect that' the price for having dropped the ball with
> "[PATCH] media: Document coding style requirements"
> 
> I'll check with 80 cols limits.

Thank you.

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2022-02-24  9:20 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-23 14:55 [PATCH v3 00/27] media: ov5640: Rework the clock tree programming for MIPI Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 01/27] media: ov5640: Add pixel rate to modes Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 02/27] media: ov5604: Re-arrange modes definition Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 03/27] media: ov5640: Add ov5640_is_csi2() function Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 04/27] media: ov5640: Associate bpp with formats Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 05/27] media: ov5640: Add LINK_FREQ control Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 06/27] media: ov5640: Update pixel_rate and link_freq Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 07/27] media: ov5640: Rework CSI-2 clock tree Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 08/27] media: ov5640: Rework timings programming Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 09/27] media: ov5640: Fix 720x480 in RGB888 mode Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 10/27] media: ov5640: Split DVP and CSI-2 timings Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 11/27] media: ov5640: Provide timings accessor Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 12/27] media: ov5640: Re-sort per-mode register tables Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 13/27] media: ov5640: Remove duplicated mode settings Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 14/27] media: ov5640: Remove ov5640_mode_init_data Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 15/27] media: ov5640: Add HBLANK control Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 16/27] media: ov5640: Add VBLANK control Jacopo Mondi
2022-02-23 20:23   ` Sakari Ailus
2022-02-24  8:58     ` Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 17/27] media: ov5640: Change CSI-2 timings to comply with FPS Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 18/27] media: ov5640: Implement init_cfg Jacopo Mondi
2022-02-23 20:25   ` Sakari Ailus
2022-02-24  8:08     ` Jacopo Mondi
2022-02-24  9:20       ` Sakari Ailus
2022-02-23 14:55 ` [PATCH v4 19/27] media: ov5640: Implement get_selection Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 20/27] media: ov5640: Limit frame_interval to DVP mode only Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 21/27] media: ov5640: Register device properties Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 22/27] media: ov5640: Add RGB565_1X16 format Jacopo Mondi
2022-02-23 14:55 ` [PATCH v4 23/27] media: ov5640: Add BGR888 format Jacopo Mondi
2022-02-23 14:56 ` [PATCH v4 24/27] media: ov5640: Restrict sizes to mbus code Jacopo Mondi
2022-02-23 14:56 ` [PATCH v4 25/27] media: ov5640: Adjust format to bpp in s_fmt Jacopo Mondi
2022-02-23 14:56 ` [PATCH v4 26/27] media: ov5640: Split DVP and CSI-2 formats Jacopo Mondi
2022-02-23 14:56 ` [PATCH v4 27/27] media: ov5640: Move format mux config in format Jacopo Mondi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.