Add mipi csi2 rx driver #71859

ngphibang · 2024-04-24T09:43:31Z

This PR adds mipi csi2 rx driver. It is splitted from #69810 to ease the review process.

Comments from Daniel Degrasse are addressed (at my best).

drivers/video/video_mcux_mipi_csi2rx.c

dts/bindings/video/nxp,mipi-csi2rx.yaml

ngphibang · 2024-05-06T18:58:43Z

@danieldegrasse , @decsny Thanks. Is it ok for you now ?

danieldegrasse · 2024-05-07T17:58:22Z

Which PR has the devicetree definition for the CSI2RX on the RT11xx after the initial PR was split? At the very least, the SOC devicetree definition can be added here (I know we may need to wait on the board definition until #71854 merges)

ngphibang · 2024-05-07T21:05:14Z

@danieldegrasse : Ok. Added the commit that adds MIPI CSI2Rx node. BTW, it depend on #72420, not #71854

danieldegrasse · 2024-05-07T21:13:55Z

drivers/video/Kconfig.mcux_mipi_csi2rx

+	bool "NXP MIPI CSI-2 Rx driver"
+	default y
+	depends on DT_HAS_NXP_MIPI_CSI2RX_ENABLED
+	select VIDEO_MCUX_CSI


VIDEO_MCUX_CSI currently depends on HAS_MCUX_CSI, which we don't select for the RT11xx. Perhaps the best choice is to remove the dependency on HAS_MCUX_CSI in #72420?

Yes, it is already done in #71860 as a "misc fix".

Otherwise, this does not relate to this PR, right ?

Sorry, I missed that PR- it does relate, because the select VIDEO_MCUX_CSI statement here will trigger a Kconfig error on the RT11xx series when building the CSI2X driver, because VIDEO_MCUX_CSI currently has a dependency on HAS_MCUX_CSI. All this means is that this PR is dependent on #71860, as far as I can tell

But this CSI2Rx driver doesn't build. Both CSI and CSI2Rx will build only when there is a camera sensor connected to them and the build is triggered in the camera sensor overlay (another PR). By default, CSI2Rx and CSI nodes are disabled.

BTW, this PR is dependent on #72420 because of the change from "sensor" to "sdev".

So, there is a mutual dependency here ...

@danieldegrasse : So, is it better to move the commit b226ded to #71860 or make it a standalone PR ? I think the first option is better

I'm fine to move b226dedfbc2c4cbcf37dd1c86e85895a4660afa3 into #71860.

But this CSI2Rx driver doesn't build. Both CSI and CSI2Rx will build only when there is a camera sensor connected to them and the build is triggered in the camera sensor overlay (another PR). By default, CSI2Rx and CSI nodes are disabled.

Maybe I'm not following here- are you saying that since the CSI2Rx driver does not build as part of this PR, there is no dependency on #71860? Generally, we want to be able to build code that is introduced with a PR as part of that PR. Finding out there is a build error later on means we have to fix something in the tree, which is less clean than getting it right the first time around

Okay ...

Added a commit as build test for the mipi csi2rx driver

Split drivers: video: csi: Change sensor dev to source dev #72623 from Modify NXP's CSI driver to support both rt11xx and rt10xx #72420 to avoid mutual dependency

This PR is now dependent on #72623 and #71854 as #71860 is merged.

Is it ok for you now ?

Yes, I think this is ok. To make sure I understand the dependency chain here:

#72623: no dependencies
#71854: no dependencies
#71859: dependent on #72623 and #71854
#72420: dependent on #71859 (and its dependencies)
#72434: dependent on #72420 (and its dependencies)
#72632: dependent on #72434 (and its dependencies)

Is this all accurate? I'm not clear why this PR depends on #71854, but you know this code best

@danieldegrasse It's nearly correct. The dependency chain are:
#72435: no dependencies
#71854: no dependencies
#72623: no dependencies (merged)
#72631: no dependencies (merged)
#71859: dependent on #72623 (merged) and #71854
#72420: dependent on #71859 (and its dependencies)
#72434: dependent on #71859 (and its dependencies)
#72633: dependent on #72434 (and its dependencies)

ngphibang · 2024-05-15T15:08:03Z

Updated "sdev" => "source" according the change in #72623 (which is merged)

loicpoulain

LGTM.

danieldegrasse · 2024-05-20T18:15:52Z

@ngphibang can you rebase this PR now that the dependencies are merged?

mmahadevan108 · 2024-05-20T21:53:31Z

drivers/video/video_mcux_mipi_csi2rx.c

+ * more common and more performant but single lane is also supported.
+ */
+#define DEFAULT_MIPI_CSI_NUM_LANES 2
+#define DEFAULT_CAMERA_FRAME_RATE  30


Is there a value to making this a Kconfig. Write now the user has to edit this driver file to change the default frame rate.

@mmahadevan108 : This has been explained to Daniel. Similar to format, framerate setting needs a runtime accordance and depends on capacity of multiple elements along the camera pipeline. That means which framerate can be supported depends on not only the mipi csi2rx but also other elements (e.g. the camera sensor). It also depends on which format is chosen at runtime.

That's why simply putting this setting in devicetree or Kconfig will not work. I made this PR to add framerate APIs. The ov5640 camera sensor driver will need to be changed as well to support framerate changing. When these things are done and merged, we will be able improve this driver to add support for framerate changing.

ngphibang · 2024-05-21T10:21:03Z

@danieldegrasse yes, rebased now. Could you approve ?

Then, I will need @loicpoulain to approve it again (sorry for this inconvenience... I couldn't rebase it earlier as yesterday is a bank holiday in France).

BTW, the html documentation CI build failure is not related to my change, I think.

Fetching the repository
/usr/bin/git -c protocol.version=2 fetch --prune --no-recurse-submodules origin +refs/heads/:refs/remotes/origin/ +refs/tags/:refs/tags/
Error: error: 5220 bytes of body are still expected
fetch-pack: unexpected disconnect while reading sideband packet
Error: fatal: early EOF
Error: fatal: fetch-pack: invalid index-pack output
The process '/usr/bin/git' failed with exit code 128
Waiting 19 seconds before trying again
/usr/bin/git -c protocol.version=2 fetch --prune --no-recurse-submodules origin +refs/heads/:refs/remotes/origin/ +refs/tags/:refs/tags/
Error: error: 1114 bytes of body are still expected
fetch-pack: unexpected disconnect while reading sideband packet
Error: fatal: early EOF
Error: fatal: fetch-pack: invalid index-pack output
The process '/usr/bin/git' failed with exit code 128
Waiting 13 seconds before trying again
/usr/bin/git -c protocol.version=2 fetch --prune --no-recurse-submodules origin +refs/heads/:refs/remotes/origin/ +refs/tags/:refs/tags/
Error: error: 6365 bytes of body are still expected
fetch-pack: unexpected disconnect while reading sideband packet
Error: fatal: early EOF
Error: fatal: fetch-pack: invalid index-pack output
Error: The process '/usr/bin/git' failed with exit code 128

Add bindings for NXP MIPI CSI-2 Rx which is a MIPI CSI-2 receiver connecting a camera sensor to the NXP CSI. Signed-off-by: Phi Bang Nguyen <[email protected]>

Add driver to support NXP MIPI CSI-2 Rx which is a MIPI CSI-2 receiver connecting a camera sensor to the NXP CSI. This IP is present in the i.MX RT11XX series. Signed-off-by: Phi Bang Nguyen <[email protected]>

Enable power and clocks for MIPI CSI-2 Rx. Configure Video Mux to connect it to CSI. Signed-off-by: Phi Bang Nguyen <[email protected]>

Add a node for MIPI CSI-2 Rx in i.MX RT11xx devicetree and connect it to the CSI node. Signed-off-by: Phi Bang Nguyen <[email protected]>

Add test case for mipi csi2rx driver. As this driver selects the csi driver, the csi node has to be enabled as well. Signed-off-by: Phi Bang Nguyen <[email protected]>

ngphibang requested review from mmahadevan108 and dleach02 as code owners April 24, 2024 09:43

zephyrbot added area: Video Video subsystem platform: NXP NXP area: Devicetree Binding PR modifies or adds a Device Tree binding platform: NXP Drivers NXP Semiconductors, drivers labels Apr 24, 2024

zephyrbot requested review from danieldegrasse, dbaluta, decsny, DerekSnell, EmilioCBen, galak, loicpoulain, manuargue and yvanderv April 24, 2024 09:44

zephyrbot assigned dleach02 Apr 24, 2024

ngphibang mentioned this pull request Apr 24, 2024

Add support for camera on i.MX RT11XX #69810

Closed

danieldegrasse reviewed Apr 25, 2024

View reviewed changes

drivers/video/video_mcux_mipi_csi2rx.c Show resolved Hide resolved

drivers/video/video_mcux_mipi_csi2rx.c Outdated Show resolved Hide resolved

drivers/video/video_mcux_mipi_csi2rx.c Outdated Show resolved Hide resolved

decsny reviewed Apr 25, 2024

View reviewed changes

dts/bindings/video/nxp,mipi-csi2rx.yaml Outdated Show resolved Hide resolved

ngphibang force-pushed the add_mipi_csi2_rx_driver branch from 16dfe24 to d474a98 Compare May 6, 2024 18:45

ngphibang mentioned this pull request May 7, 2024

Modify NXP's CSI driver to support both rt11xx and rt10xx #72420

Merged

ngphibang force-pushed the add_mipi_csi2_rx_driver branch from d474a98 to 098c6ea Compare May 7, 2024 21:00

danieldegrasse reviewed May 7, 2024

View reviewed changes

ngphibang mentioned this pull request May 11, 2024

drivers: video: csi: Change sensor dev to source dev #72623

Merged

ngphibang force-pushed the add_mipi_csi2_rx_driver branch 2 times, most recently from af82217 to 3337b63 Compare May 12, 2024 06:05

ngphibang mentioned this pull request May 12, 2024

boards: shields: Add shield for NXP's ov5640 camera modules #72434

Merged

ngphibang mentioned this pull request May 12, 2024

Update and improve video capture sample #72633

Merged

DerekSnell removed their request for review May 13, 2024 14:07

ngphibang mentioned this pull request May 13, 2024

boards: nxp: mimxrt1170_evk: Enable video feature #72632

Closed

ngphibang force-pushed the add_mipi_csi2_rx_driver branch from 3337b63 to 4b81687 Compare May 15, 2024 15:06

loicpoulain previously approved these changes May 20, 2024

View reviewed changes

mmahadevan108 reviewed May 20, 2024

View reviewed changes

ngphibang dismissed loicpoulain’s stale review via bb8fc9b May 21, 2024 08:59

ngphibang force-pushed the add_mipi_csi2_rx_driver branch from 4b81687 to bb8fc9b Compare May 21, 2024 08:59

ngphibang added 4 commits May 21, 2024 13:38

dts: bindings: video: Add bindings for NXP MIPI CSI-2 Rx

e31e28d

Add bindings for NXP MIPI CSI-2 Rx which is a MIPI CSI-2 receiver connecting a camera sensor to the NXP CSI. Signed-off-by: Phi Bang Nguyen <[email protected]>

drivers: video: Add NXP MIPI CSI-2 Rx driver

0328e92

Add driver to support NXP MIPI CSI-2 Rx which is a MIPI CSI-2 receiver connecting a camera sensor to the NXP CSI. This IP is present in the i.MX RT11XX series. Signed-off-by: Phi Bang Nguyen <[email protected]>

soc: nxp: imxrt11xx: Enable power, clocks and muxing for MIPI CSI-2 Rx

dafbdbd

Enable power and clocks for MIPI CSI-2 Rx. Configure Video Mux to connect it to CSI. Signed-off-by: Phi Bang Nguyen <[email protected]>

dts: arm: nxp: Add devicetree node for MIPI CSI-2 Rx

bcd21fc

Add a node for MIPI CSI-2 Rx in i.MX RT11xx devicetree and connect it to the CSI node. Signed-off-by: Phi Bang Nguyen <[email protected]>

ngphibang force-pushed the add_mipi_csi2_rx_driver branch 2 times, most recently from c210aba to 9446cc4 Compare May 21, 2024 12:58

tests: drivers: build_all: video: Add test for mipi csi2rx

3d13f14

Add test case for mipi csi2rx driver. As this driver selects the csi driver, the csi node has to be enabled as well. Signed-off-by: Phi Bang Nguyen <[email protected]>

ngphibang force-pushed the add_mipi_csi2_rx_driver branch from 9446cc4 to 3d13f14 Compare May 21, 2024 15:24

mmahadevan108 approved these changes May 21, 2024

View reviewed changes

danieldegrasse approved these changes May 21, 2024

View reviewed changes

dleach02 approved these changes May 21, 2024

View reviewed changes

carlescufi merged commit 9e03d5a into zephyrproject-rtos:main May 21, 2024
21 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mipi csi2 rx driver #71859

Add mipi csi2 rx driver #71859

ngphibang commented Apr 24, 2024 •

edited

Loading

ngphibang commented May 6, 2024

danieldegrasse commented May 7, 2024

ngphibang commented May 7, 2024

danieldegrasse May 7, 2024

ngphibang May 7, 2024 •

edited

Loading

ngphibang May 8, 2024

danieldegrasse May 8, 2024

ngphibang May 8, 2024 •

edited

Loading

ngphibang May 8, 2024 •

edited

Loading

danieldegrasse May 9, 2024

ngphibang May 11, 2024 •

edited

Loading

danieldegrasse May 13, 2024 •

edited

Loading

ngphibang May 13, 2024 •

edited

Loading

ngphibang commented May 15, 2024

loicpoulain left a comment

danieldegrasse commented May 20, 2024

mmahadevan108 May 20, 2024

ngphibang May 21, 2024 •

edited

Loading

ngphibang commented May 21, 2024 •

edited

Loading

Add mipi csi2 rx driver #71859

Add mipi csi2 rx driver #71859

Conversation

ngphibang commented Apr 24, 2024 • edited Loading

ngphibang commented May 6, 2024

danieldegrasse commented May 7, 2024

ngphibang commented May 7, 2024

danieldegrasse May 7, 2024

Choose a reason for hiding this comment

ngphibang May 7, 2024 • edited Loading

Choose a reason for hiding this comment

ngphibang May 8, 2024

Choose a reason for hiding this comment

danieldegrasse May 8, 2024

Choose a reason for hiding this comment

ngphibang May 8, 2024 • edited Loading

Choose a reason for hiding this comment

ngphibang May 8, 2024 • edited Loading

Choose a reason for hiding this comment

danieldegrasse May 9, 2024

Choose a reason for hiding this comment

ngphibang May 11, 2024 • edited Loading

Choose a reason for hiding this comment

danieldegrasse May 13, 2024 • edited Loading

Choose a reason for hiding this comment

ngphibang May 13, 2024 • edited Loading

Choose a reason for hiding this comment

ngphibang commented May 15, 2024

loicpoulain left a comment

Choose a reason for hiding this comment

danieldegrasse commented May 20, 2024

mmahadevan108 May 20, 2024

Choose a reason for hiding this comment

ngphibang May 21, 2024 • edited Loading

Choose a reason for hiding this comment

ngphibang commented May 21, 2024 • edited Loading

ngphibang commented Apr 24, 2024 •

edited

Loading

ngphibang May 7, 2024 •

edited

Loading

ngphibang May 8, 2024 •

edited

Loading

ngphibang May 8, 2024 •

edited

Loading

ngphibang May 11, 2024 •

edited

Loading

danieldegrasse May 13, 2024 •

edited

Loading

ngphibang May 13, 2024 •

edited

Loading

ngphibang May 21, 2024 •

edited

Loading

ngphibang commented May 21, 2024 •

edited

Loading