drivers: video: ov7670: introduce driver for ov7670 camera #72826

danieldegrasse · 2024-05-15T22:50:24Z

Introduce driver for ov7670 camera, supporting QCIF,QVGA,CIF, and VGA
resolution in YUV and RGB mode.

loicpoulain

neat! For the record, could you add in the commit message the board/devkit with wich you tested this.

danieldegrasse · 2024-05-20T16:58:07Z

neat! For the record, could you add in the commit message the board/devkit with wich you tested this.

Sure- it was tested on the FRDM-MCXN947, but that support requires PR #72827, which is dependent on this driver. I can add something like the following to the commit message:

Support was verified on the FRDM-MCXN947, using the SmartDMA camera engine, which is enabled in the following PR: #72827

Would this work?

loicpoulain · 2024-05-20T17:21:45Z

Support was verified on the FRDM-MCXN947, using the SmartDMA camera engine, which is enabled in the following PR: #72827

Would this work?

Sure.

danieldegrasse · 2024-05-31T18:03:56Z

@loicpoulain could you revisit this PR when you get a chance? I've updated the commit message as requested

loicpoulain

LGTM! Thanks

danieldegrasse · 2024-05-31T20:26:28Z

@ngphibang or @josuah, FYI in case either of you would like to provide feedback.

danieldegrasse · 2024-05-31T20:27:27Z

@decsny would you be willing to review this when you have a chance (or @galak, I'd just like someone to take a look at this from the DTS side based on the assignee the bot selected)

decsny · 2024-05-31T20:32:14Z

the binding is fairly trivial, reassigning to video area

drivers/video/ov7670.c

dts/bindings/video/ovti,ov7670.yaml

drivers/video/ov7670.c

Introduce driver for ov7670 camera, supporting QCIF,QVGA,CIF, and VGA resolution in YUV and RGB mode. Support was verified on the FRDM-MCXN947, using the SmartDMA camera engine, which is enabled in the following PR: zephyrproject-rtos#72827 Signed-off-by: Daniel DeGrasse <[email protected]>

Add ov7670 camera driver to build test for video drivers Signed-off-by: Daniel DeGrasse <[email protected]>

ngphibang · 2024-06-03T15:55:54Z

LGTM ! +1

dleach02 · 2024-06-04T20:49:40Z

@loicpoulain can you return to this PR for final review

loicpoulain

LGTM

josuah

(note: this is purely informative: not collaborator, not maintainer, just an extra pair of eyeballs)

Good to have this starting point!

For reference, although I did not read them when reviewing:
https://web.mit.edu/6.111/www/f2016/tools/OV7670_2006.pdf
https://www.electronicscomp.com/datasheet/ov7670-sensor-datasheet.pdf

josuah · 2024-06-06T13:42:11Z

drivers/video/ov7670.c

+	if (!memcmp(&data->fmt, fmt, sizeof(data->fmt))) {
+		/* nothing to do */
+		return 0;
+	}


Just curious: Is this a performance optimization to reduce I2C traffic?
Is setting the format happening often in some cases?

Yeah, it is a performance optimization. Other drivers in tree have the same code, so I figured it would make sense to include the feature here as well

Thank you this makes sense!

Note to self: also add it to #73013

josuah · 2024-06-06T13:47:38Z

drivers/video/ov7670.c

+	memcpy(&data->fmt, fmt, sizeof(data->fmt));
+
+	if (fmt->pixelformat == VIDEO_PIX_FMT_RGB565) {
+		com7 |= 0x4;


For reference if anyone wonders, in Linux driver:

#define COM7_RGB 0x04 /* bits 0 and 2 - RGB format */

josuah · 2024-06-06T13:50:01Z

drivers/video/CMakeLists.txt

@@ -11,3 +11,4 @@ zephyr_library_sources_ifdef(CONFIG_VIDEO_OV7725	ov7725.c)
 zephyr_library_sources_ifdef(CONFIG_VIDEO_OV2640	ov2640.c)
 zephyr_library_sources_ifdef(CONFIG_VIDEO_STM32_DCMI	video_stm32_dcmi.c)
 zephyr_library_sources_ifdef(CONFIG_VIDEO_OV5640	ov5640.c)
+zephyr_library_sources_ifdef(CONFIG_VIDEO_OV7670	ov7670.c)


#73013 will fix the sorting later

josuah · 2024-06-06T13:55:18Z

drivers/video/ov7670.c

+static const struct video_format_cap fmts[] = {
+	OV7670_VIDEO_FORMAT_CAP(176, 144, VIDEO_PIX_FMT_RGB565), /* QCIF  */
+	OV7670_VIDEO_FORMAT_CAP(320, 240, VIDEO_PIX_FMT_RGB565), /* QVGA  */
+	OV7670_VIDEO_FORMAT_CAP(352, 288, VIDEO_PIX_FMT_RGB565), /* CIF   */
+	OV7670_VIDEO_FORMAT_CAP(640, 480, VIDEO_PIX_FMT_RGB565), /* VGA   */
+	OV7670_VIDEO_FORMAT_CAP(176, 144, VIDEO_PIX_FMT_YUYV),   /* QCIF  */
+	OV7670_VIDEO_FORMAT_CAP(320, 240, VIDEO_PIX_FMT_YUYV),   /* QVGA  */
+	OV7670_VIDEO_FORMAT_CAP(352, 288, VIDEO_PIX_FMT_YUYV),   /* CIF   */
+	OV7670_VIDEO_FORMAT_CAP(640, 480, VIDEO_PIX_FMT_YUYV),   /* VGA   */
+	{0}};


This looks convenient and easy to review.
This might be a good addition to #73867 later.

josuah · 2024-06-06T14:00:34Z

drivers/video/ov7670.c

+	if (fmt->pixelformat != VIDEO_PIX_FMT_RGB565 && fmt->pixelformat != VIDEO_PIX_FMT_YUYV) {
+		LOG_ERR("Only RGB565 and YUYV supported!");
+		return -ENOTSUP;
+	}


Is this not already handled below?
https://github.com/zephyrproject-rtos/zephyr/pull/72826/files#diff-1fe02c52aeadf74decf28b87839e35abfd3f96e3548f5d1518276ca2439bd6afR353-R354

No harm done checking twice in practice though...

Yes, it is-the advantage to this check is that skip the array search for formats we know we don't support. We could also manually check resolution using if statements like these, but that would be trickier.

Thank you! It makes sense, and brings a better error log message too when the wrong format gets sent.

zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding area: Video Video subsystem labels May 15, 2024

zephyrbot requested review from decsny, galak and loicpoulain May 15, 2024 22:51

zephyrbot assigned galak May 15, 2024

danieldegrasse mentioned this pull request May 15, 2024

drivers: video: video_mcux_smartdma: add SMARTDMA video driver #72827

Merged

danieldegrasse force-pushed the feature/ov7670-driver branch from d5dada1 to 695c7ba Compare May 15, 2024 22:57

loicpoulain reviewed May 20, 2024

View reviewed changes

danieldegrasse force-pushed the feature/ov7670-driver branch from 695c7ba to 0f92f6f Compare May 20, 2024 17:36

loicpoulain previously approved these changes May 31, 2024

View reviewed changes

decsny previously approved these changes May 31, 2024

View reviewed changes

decsny assigned loicpoulain and unassigned galak and loicpoulain May 31, 2024

ngphibang reviewed May 31, 2024

View reviewed changes

danieldegrasse added 2 commits June 1, 2024 13:01

test: drivers: build_all: video: add ov7670 to build test

cfb16b7

Add ov7670 camera driver to build test for video drivers Signed-off-by: Daniel DeGrasse <[email protected]>

danieldegrasse dismissed stale reviews from decsny and loicpoulain via cfb16b7 June 1, 2024 18:01

danieldegrasse force-pushed the feature/ov7670-driver branch from 0f92f6f to cfb16b7 Compare June 1, 2024 18:01

danieldegrasse requested a review from loicpoulain June 3, 2024 16:02

danieldegrasse requested a review from decsny June 3, 2024 16:02

dleach02 approved these changes Jun 3, 2024

View reviewed changes

decsny approved these changes Jun 4, 2024

View reviewed changes

loicpoulain approved these changes Jun 6, 2024

View reviewed changes

josuah approved these changes Jun 6, 2024

View reviewed changes

nashif merged commit 1ef4273 into zephyrproject-rtos:main Jun 7, 2024
22 checks passed

danieldegrasse deleted the feature/ov7670-driver branch June 7, 2024 14:42

josuah mentioned this pull request Jul 27, 2024

drivers: video: image sensor list (from Linux, others) #76378

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: video: ov7670: introduce driver for ov7670 camera #72826

drivers: video: ov7670: introduce driver for ov7670 camera #72826

danieldegrasse commented May 15, 2024

loicpoulain left a comment

danieldegrasse commented May 20, 2024

loicpoulain commented May 20, 2024

danieldegrasse commented May 31, 2024

loicpoulain left a comment

danieldegrasse commented May 31, 2024

danieldegrasse commented May 31, 2024

decsny commented May 31, 2024

ngphibang commented Jun 3, 2024

dleach02 commented Jun 4, 2024

loicpoulain left a comment

josuah left a comment

josuah Jun 6, 2024

danieldegrasse Jun 7, 2024

josuah Jun 7, 2024 •

edited

Loading

josuah Jun 6, 2024

josuah Jun 6, 2024

josuah Jun 6, 2024

josuah Jun 6, 2024

danieldegrasse Jun 7, 2024

josuah Jun 7, 2024

drivers: video: ov7670: introduce driver for ov7670 camera #72826

drivers: video: ov7670: introduce driver for ov7670 camera #72826

Conversation

danieldegrasse commented May 15, 2024

loicpoulain left a comment

Choose a reason for hiding this comment

danieldegrasse commented May 20, 2024

loicpoulain commented May 20, 2024

danieldegrasse commented May 31, 2024

loicpoulain left a comment

Choose a reason for hiding this comment

danieldegrasse commented May 31, 2024

danieldegrasse commented May 31, 2024

decsny commented May 31, 2024

ngphibang commented Jun 3, 2024

dleach02 commented Jun 4, 2024

loicpoulain left a comment

Choose a reason for hiding this comment

josuah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josuah Jun 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josuah Jun 7, 2024 •

edited

Loading