Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 2 commits into from
May 23, 2024
Merged

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

merged 2 commits into from
May 23, 2024

Conversation

ngphibang
Copy link
Contributor

@ngphibang ngphibang commented May 7, 2024 •
edited
Loading

This PR modify the NXP's CSI driver to support both rt11xx and rt10xx. This depends on this PR
It is split from #69810 as required by @loicpoulain to ease the review process.
It already got reviewed by @danieldegrasse.

The PR depends on #71859

@zephyrbot zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding area: Video Video subsystem platform: NXP Drivers NXP Semiconductors, drivers platform: NXP NXP labels May 7, 2024
@ngphibang
Copy link
Contributor Author

@loicpoulain : Please ... :-). We need this feature and it has been there for quite a while.
Thank you !

@ngphibang
Copy link
Contributor Author

The CI is failed because of the dependency on this PR

danieldegrasse
danieldegrasse reviewed May 7, 2024
View reviewed changes
drivers/video/video_mcux_csi.c
return -ENOTSUP;
}
video_pix_fmt_convert(&format, false);
data->csi_config.dataBus = kCSI_DataBus24Bit;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe instead of making this dependent on Kconfig we should move the dataBus configuration to devicetree? Not sure if all cameras are 8bit bus width, or if some would use more pins

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't make this configurable in devicetree. The databus is either 8 bits or 24 bits depending on the path of image data. If it passes by mipi csi2rx, data are implicitly converted to 32-bit format and 24 bits need to be set for it to work.

include/zephyr/drivers/video.h Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the change to this generic file into a separate commit from the one adding support for the CSI2RX to the CSI driver

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Done

@ngphibang ngphibang mentioned this pull request May 7, 2024
Merged
@DerekSnell DerekSnell removed their request for review May 8, 2024 12:18
@ngphibang ngphibang mentioned this pull request May 11, 2024
Merged
@ngphibang ngphibang force-pushed the csi_support_rt11xx branch 2 times, most recently from 67c1a19 to 1ebeaa9 Compare May 11, 2024 19:13
@danieldegrasse
Copy link
Collaborator

@ngphibang please take a look at CI failures

@ngphibang
Copy link
Contributor Author

@danieldegrasse Yes, CI failures are just because of dependencies on other PRs. We can wait for them to be merged first then I will update this.

@ngphibang ngphibang mentioned this pull request May 13, 2024
Closed
loicpoulain
loicpoulain previously approved these changes May 13, 2024
View reviewed changes
@ngphibang
Copy link
Contributor Author

Rebased because #72623 is now merged.

@dleach02
Copy link
Member

You have a Kconfig compliance check error to address

@ngphibang
Copy link
Contributor Author

@dleach02 As being said in my previous comment to Daniel, the undefined Kconfig error CONFIG_VIDEO_MCUX_MIPI_CSI2RX is just because of the dependency on #71859. Once this one is merged, it will be ok.

@ngphibang
include: video: Add XYUV32 pixel format
de2be86 
Add XYUV32 pixel format

Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
@ngphibang
drivers: video: csi: Add support for i.MX RT11XX
668b5c6 
On i.MX RT11XX which has MIPI CSI-2 Rx, image data from the camera sensor
after passing through the camera pipeline (MIPI CSI-2 Rx --> Video Mux -->
CSI) will be implicitly converted to a 32-bits pixel formats. For example,
an input in RGB565 / YUYV (2-bytes format) will become an XRGB32 / XYUV32
(4-bytes format), respectively. Make changes to support this.

Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
@ngphibang
Copy link
Contributor Author

ngphibang commented May 22, 2024 •
edited
Loading

#71859 dependency is now merged.
@loicpoulain @danieldegrasse : Could you approve this again ?

The CI failures are not due to my changes, I think.

HEAD is now at 75d0880 tests: Update mps2 board name
ERROR: update failed for projects: psa-arch-tests, sof, tflite-micro, thrift, hal_espressif, hal_nxp
Error: Error: failed to run script step: command terminated with non-zero exit code: Error executing in Docker Container: 1
Error: Process completed with exit code 1.
Error: Executing the custom container implementation failed. Please contact your self hosted runner administrator.


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: RPC failed; curl 92 HTTP/2 stream 0 was not closed cleanly: CANCEL (err 8)
  Error: error: 7507 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 15 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: RPC failed; curl 92 HTTP/2 stream 0 was not closed cleanly: CANCEL (err 8)
  Error: error: 574 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 12 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: 6868 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

danieldegrasse
danieldegrasse approved these changes May 22, 2024
View reviewed changes
Copy link
Collaborator

@danieldegrasse danieldegrasse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will rekick CI once things are working again, the failures do not seem to be related to this PR

@nashif nashif merged commit ea02b93 into zephyrproject-rtos:main May 23, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmahadevan108 mmahadevan108 mmahadevan108 approved these changes

@dleach02 dleach02 dleach02 approved these changes

@danieldegrasse danieldegrasse danieldegrasse approved these changes

@loicpoulain loicpoulain loicpoulain left review comments

@dbaluta dbaluta Awaiting requested review from dbaluta

@decsny decsny Awaiting requested review from decsny

@EmilioCBen EmilioCBen Awaiting requested review from EmilioCBen

@galak galak Awaiting requested review from galak

@manuargue manuargue Awaiting requested review from manuargue

@yvanderv yvanderv Awaiting requested review from yvanderv

Assignees

@dleach02 dleach02

Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: Video Video subsystem platform: NXP Drivers NXP Semiconductors, drivers platform: NXP NXP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@ngphibang @danieldegrasse @dleach02 @mmahadevan108 @loicpoulain @nashif @zephyrbot