drivers: video: add imx219 sensor (RPi Cam v2) #88011

josuah · 2025-04-02T00:59:14Z

Split out of:

drivers: video: imagers/cci/imx219: easier image sensor contribution #87004

Dependency:

This was only tested on hardware that is out-of-tree and at 1920x1080 resolution for now (which will be submitted upstream in maybe April):

https://github.com/tinyvision-ai-inc/tinyvision_zephyr_sdk/blob/main/drivers/video/imx219.c

It works well though.
If you would rather have it submitted before #88003 is merged, let me know, I will refactor it so it can be merged earlier.

P.S.: Thank you Alan Shaju for the original work adapted into this PR!

josuah · 2025-04-02T13:39:10Z

Force-push:

Rebase on main after drivers: video: gc2145: Add Chip ID 0x2145 #87872
Added Co-authored-by: to the commit message

dleach02 · 2025-04-02T20:25:35Z

@josuah please put this PR into draft mode until the dependent PRs merged.

andrewnyland · 2025-05-13T15:17:16Z

Is there any way we can help with this? Looks great so far

josuah · 2025-05-13T21:59:02Z

Is there any way we can help with this? Looks great so far

It might be possible yes! I need to rebase this driver on the latest main (like we did here) and then test again on our own platform https://tinyclunx33.tinyvision.ai/

And then it will be possible to test it on any of the Zephyr platform that supports MIPI/CSI, which for now is mostly the i.MXRT1170 and friends, but more are coming!

I will have more time next week to prepare the way a little bit more...

Thank you for offering your help!

josuah · 2025-06-01T22:10:31Z

Short update on this: I will wait reception of i.MXRT1160 and STM32N6 kits, then remove the library from it, to make it more like the IMX335.

The approach of the "table-only" image sensor driver library was too rigid and does not fit how firmware/driver developers grasp image sensors. Better build tools that fit the community than make the community fit the tools! :)

Following renaming of the raspberry csi connector nexus, rename all places when csi 22pins is mentioned to only keep csi instead. Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

Only VIDEO_CID_ANALOGUE_GAIN and VIDEO_CID_GAIN were defined. Also add the complementary VIDEO_CID_DIGITAL_GAIN. Signed-off-by: Josuah Demangeon <me@josuah.net>

Switch to zephyr-keep-sorted-start/stop to keep checking the new driver imports are always sorted, which reduces chances of git conflict while adding new drivers from separate branches. Signed-off-by: Josuah Demangeon <me@josuah.net>

Add support for the Sony IMX219 CSI sensor. This sensor supports resolution of 3280x2464 in RGGB bayer format either 8 or 10 bits and using 2 or 4 CSI lanes. Only 10 bits on 2 CSI lanes is currently supported, and only in 1920x1080 pixel resolution using cropping. Signed-off-by: Josuah Demangeon <me@josuah.net> Co-authored-by: Alan Shaju <alanshaju@rideltech.com> Co-authored-by: Alain Volmat <alain.volmat@foss.st.com>

Build the IMX219 image sensor driver on the build-all tests. No interconnection with any other device is done, which is not mandatory at the time as no link-related parameter is used. Signed-off-by: Josuah Demangeon <me@josuah.net>

Add a shield for the Raspberry Pi Camera Module 2, featuring the IMX219 sensor. This module is suitable for bot the "normal" and "No IR" (no infra-red) variants and uses the rasapberry pi connector. Signed-off-by: Josuah Demangeon <me@josuah.net> Co-authored-by: Alain Volmat <alain.volmat@foss.st.com>

Add board specific .conf files for the stm32n6570_dk. Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

josuah · 2025-06-24T22:04:04Z

Force-push:

Rebase on top of latest Zephyr
Fix bugs preventing it to be built and run [1]
Set correct board name
Integrate improvement from @avolmat-st branch
Reduce the default exposure

[1]: I had to modify our SDK to support shields so I could test this thing end-to-end.

josuah · 2025-06-24T22:18:31Z

I realize this is now dependent on this PR, so setting as draft in the meantime:

Avoid making difference between 15pins and 22pins raspberry pi CSI connector #91983

sonarqubecloud · 2025-06-24T22:28:52Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

avolmat-st · 2025-06-25T18:10:34Z

drivers/video/imx219.c

+	/* Custom defaults */
+	{IMX219_REG_BINNING_MODE_H, 0x00},	/* No binning */
+	{IMX219_REG_BINNING_MODE_V, 0x00}	/* No binning */,
+	{IMX219_REG_DIGITAL_GAIN, 5000},


Seems that at init time DIGITAL_GAIN is set to 5000 while within the ctrl init, .def is 0x100. Is that on purpose ? Also, maybe a macro containing DIGITAL_GAIN would help here, being used here and within the ctrl_init function ?

This makes sense, I will adjust.

avolmat-st · 2025-06-25T18:10:46Z

drivers/video/imx219.c

+	{IMX219_REG_BINNING_MODE_H, 0x00},	/* No binning */
+	{IMX219_REG_BINNING_MODE_V, 0x00}	/* No binning */,
+	{IMX219_REG_DIGITAL_GAIN, 5000},
+	{IMX219_REG_ANALOG_GAIN, 240},


Same comment as for DIGITAL_GAIN

This makes sense, I will adjust.

avolmat-st · 2025-06-25T18:12:49Z

drivers/video/imx219.c

+	{IMX219_REG_BINNING_MODE_V, 0x00}	/* No binning */,
+	{IMX219_REG_DIGITAL_GAIN, 5000},
+	{IMX219_REG_ANALOG_GAIN, 240},
+	{IMX219_REG_INTEGRATION_TIME, 100},


Here, value matches so that's ok, but here as well, a macro would help to ensure that init value and ctrl def value are same. That's a general comment for stuff that are set via control.

This makes sense, I will adjust.

avolmat-st · 2025-06-25T18:14:09Z

drivers/video/imx219.c

+	{IMX219_REG_ORIENTATION, 0x03},
+};
+
+static const struct video_reg imx219_fmt_raw8_regs[] = {


Optional: in case you want to save few bytes of ROM, this could be a video_reg8

video_reg8 got introduced after this original implementation, I need to upgrade imx219.c to it, yes.

avolmat-st · 2025-06-25T18:14:14Z

drivers/video/imx219.c

+	{IMX219_REG_OPPXCK_DIV, 8},
+};
+
+static const struct video_reg imx219_fmt_raw10_regs[] = {


Optional: in case you want to save few bytes of ROM, this could be a video_reg8

video_reg8 got introduced after this original implementation, I need to upgrade imx219.c to it, yes.

[about this PR]
I kept the documented registers as video_reg (variable size) for now, but instead:

removed IMX219_REG_OPPXCK_DIV which was configured to the same value afterward, only 4 bytes wasted

converted all the undocumented vendor registers to video_reg16 36 bytes spared.

[further-discussion]
I wish to avoid mixing CCI and non-CCI registers both with name IMX219_REG_xxx to avoid confusion. Maybe it needs a naming convention in the future... How about IMX219_CCI_xxx for CCI registers, IMX219_REG_xxx for video_reg8/video_reg16? There is no need to differentiate between 16-bit and 8-bit address inside of a same driver. This also allows to define both a CCI and a non-CCI version in case a register needs to be used in both context.

Advantage of using SENSORNAME_CCI_ and SENSORNAME_REG_ convention is that everything written before CCI integration (i.e. GC2145) is already compatible, and the naming convention can be applied as sensors are converted to CCI (if they need to).

I like this naming convention. Very good idea.

avolmat-st · 2025-06-25T18:18:46Z

drivers/video/imx219.c

+		return ret;
+	}
+
+	fmt.width = imx219_fmts[0].width_min;


Optional: those 3 lines could be set directly on the line:

struct video_format = { .... }; at the beginning of the function

Yes, better get ahead of Coverity.

avolmat-st · 2025-06-25T18:20:39Z

drivers/video/imx219.c

+		return ret;
+	}
+
+	ret = video_write_cci_reg(&cfg->i2c, IMX219_REG_MODE_SELECT, IMX219_MODE_SELECT_STREAMING);


I think the SELECT_STREAMING should only be done at set_stream time, not set_fmt.

This is a good point, better avoid too much "enable/disable" traffic.

avolmat-st · 2025-06-25T18:28:43Z

drivers/video/imx219.c

+		return -EINVAL;
+	}
+
+	ret = video_write_cci_reg(&cfg->i2c, IMX219_REG_MODE_SELECT, IMX219_MODE_SELECT_STANDBY);


I see now, doing this full review and looking at other drivers that there is no "protection" against changing the format while the sensor is already streaming. This is not an issue to this only driver, some other also have this issue.
This could be dangerous since this could lead to issues on the receiver side.
That's something to make keep for later on and not block since driver merge, but something to keep in mind.

I would however avoid on purpose stopping the streaming here and enabling it again later on since I think set_stream (false) should be called first prior to changing a format or a frmival.

This is a good point, better avoid too much "enable/disable" traffic.

avolmat-st · 2025-06-25T18:30:10Z

drivers/video/imx219.c

+
+	video_closest_frmival(dev, &fie);
+
+	ret = video_write_cci_reg(&cfg->i2c, IMX219_REG_MODE_SELECT, IMX219_MODE_SELECT_STANDBY);


same point as for set_fmt, this probably shouldn't change the mode here since normally at that stage the sensor should not be streaming.
Or at least, it should first check the status since with current code it will always start streaming even if set_stream has not been called.

This is a good point, better avoid too much "enable/disable" traffic.

avolmat-st · 2025-06-25T18:36:22Z

tests/drivers/build_all/video/app.overlay

+
+			test_i2c_imx219: imx219@10 {
+				compatible = "sony,imx219";
+				reg = <0x7>;


reg = 0x10 ?

The build_all test does not actually run anywhere, and I2C addresses are just incrementing numbers just to avoid address clash.

The @10 better be corrected to @7 though, that is true!

actually, test_i2c_mipid02 should be @8, test_i2c_ov9655 @9, and test_i2c_imx219 @A

github-actions bot added area: Video Video subsystem platform: NXP Drivers NXP Semiconductors, drivers labels Apr 2, 2025

github-actions bot requested review from dbaluta, decsny, dleach02, loicpoulain, manuargue, mmahadevan108 and Raymond0225 April 2, 2025 01:00

github-actions bot assigned dleach02 and mmahadevan108 Apr 2, 2025

josuah force-pushed the pr-imx219 branch from 4b6f87c to 9bf5b08 Compare April 2, 2025 01:11

josuah mentioned this pull request Apr 2, 2025

drivers: video: imagers/cci/imx219: easier image sensor contribution #87004

Closed

josuah force-pushed the pr-imx219 branch 2 times, most recently from e2dfd8d to 9d1bda0 Compare April 2, 2025 13:27

decsny requested review from ngphibang and JiafeiPan April 2, 2025 15:51

josuah marked this pull request as draft April 2, 2025 20:36

This was referenced Apr 3, 2025

drivers: video: imager: move the boilerplate in a library #88003

Closed

drivers: video: formats: add most of the raw formats #88138

Merged

josuah mentioned this pull request Apr 10, 2025

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

Open

josuah mentioned this pull request Apr 20, 2025

Video: addition of the imx335 driver #88825

Merged

josuah closed this May 13, 2025

josuah reopened this May 13, 2025

josuah mentioned this pull request May 18, 2025

drivers: video: link frequencies, blanking, flip-aware formats #90099

Open

Alain Volmat added 2 commits June 22, 2025 14:17

boards: stm32: stm32n6570_dk: rename 22pins CSI into CSI

3709f5a

Following renaming of the raspberry csi connector nexus, rename all places when csi 22pins is mentioned to only keep csi instead. Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

shields: st_b_cams_imx_mb1854: rename 22pins CSI into CSI

c08d628

Following renaming of the raspberry csi connector nexus, rename all places when csi 22pins is mentioned to only keep csi instead. Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

avolmat-st mentioned this pull request Jun 22, 2025

Avoid making difference between 15pins and 22pins raspberry pi CSI connector #91983

Merged

josuah added 2 commits June 24, 2025 15:56

driver: video: ctrls: add VIDEO_CID_DIGITAL_GAIN

13b9c08

Only VIDEO_CID_ANALOGUE_GAIN and VIDEO_CID_GAIN were defined. Also add the complementary VIDEO_CID_DIGITAL_GAIN. Signed-off-by: Josuah Demangeon <me@josuah.net>

drivers: video: sort CMakeLists.txt/Kconfig imports

32f22bd

Switch to zephyr-keep-sorted-start/stop to keep checking the new driver imports are always sorted, which reduces chances of git conflict while adding new drivers from separate branches. Signed-off-by: Josuah Demangeon <me@josuah.net>

josuah force-pushed the pr-imx219 branch from 75d03a6 to 4b4119a Compare June 24, 2025 21:27

josuah and others added 4 commits June 24, 2025 22:03

tests: drivers: build_all: video: add IMX219

42adcbf

Build the IMX219 image sensor driver on the build-all tests. No interconnection with any other device is done, which is not mandatory at the time as no link-related parameter is used. Signed-off-by: Josuah Demangeon <me@josuah.net>

shields: raspberry_pi_camera_module_2: add stm32n6570_dk configs files

ea05073

Add board specific .conf files for the stm32n6570_dk. Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>

josuah force-pushed the pr-imx219 branch from 4b4119a to ea05073 Compare June 24, 2025 22:03

josuah marked this pull request as ready for review June 24, 2025 22:04

github-actions bot added platform: NXP Drivers NXP Semiconductors, drivers platform: STM32 ST Micro STM32 labels Jun 24, 2025

github-actions bot requested review from avisconti, dbaluta, decsny, dleach02, erwango, henrikbrixandersen, jfischer-no, manuargue, mmahadevan108 and mnkp June 24, 2025 22:05

josuah removed the request for review from dleach02 June 24, 2025 22:18

josuah marked this pull request as draft June 24, 2025 22:18

avolmat-st requested changes Jun 25, 2025

View reviewed changes


		video_closest_frmival(dev, &fie);

		ret = video_write_cci_reg(&cfg->i2c, IMX219_REG_MODE_SELECT, IMX219_MODE_SELECT_STANDBY);

drivers: video: add imx219 sensor (RPi Cam v2) #88011

Are you sure you want to change the base?

drivers: video: add imx219 sensor (RPi Cam v2) #88011

Conversation

josuah commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josuah commented Apr 2, 2025

Uh oh!

dleach02 commented Apr 2, 2025

Uh oh!

andrewnyland commented May 13, 2025

Uh oh!

josuah commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josuah commented Jun 1, 2025

Uh oh!

josuah commented Jun 24, 2025

Uh oh!

josuah commented Jun 24, 2025

Uh oh!

sonarqubecloud bot commented Jun 24, 2025

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josuah Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

josuah commented Apr 2, 2025 •

edited

Loading

josuah commented May 13, 2025 •

edited

Loading

josuah Jun 26, 2025 •

edited

Loading