drivers: video: imagers/cci/imx219: easier image sensor contribution #87004
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The Camera Common Interface part of the MIPI CSI protocol defines methods to configure a camera device over I2C, such as the size of the register address and data. This permits to write wrapper software like it is done on Linux, in order to simplify the image sensor driver development. Signed-off-by: Josuah Demangeon <me@josuah.net>
Fix an API misuse leading to off-by-one and inaccurate results. The API introduced by 0cbaea0 does not expect fie.index to be incremented by the drver. Signed-off-by: Josuah Demangeon <me@josuah.net>
Imagers, also called image sensors, are implemented using the generic video API. This makes image sensor require boilerplate code which can be avoided by providing a common implementation that will fit most image sensors. Signed-off-by: Josuah Demangeon <me@josuah.net>
Add an IMX219 image sensor driver, using the newly introduced "CCI" and "imager" utilities to facilitate image sensor implementation. Signed-off-by: Josuah Demangeon <me@josuah.net>
Replace the ad-hoc register emulation by the dedicated I2C emulator, making it usable with the same APIs as every other image sensors. Signed-off-by: Josuah Demangeon <me@josuah.net>
Now that the emulated I2C is supported, it is possible to use the same APIs as actual image sensors, permitting to cover the image sensor driver logics with unit tests. Signed-off-by: Josuah Demangeon <me@josuah.net>
josuah
added
area: Drivers
area: Video
zephyrbot
requested review from
dbaluta,
decsny,
dleach02,
loicpoulain,
manuargue,
mmahadevan108,
ngphibang and
Raymond0225
|
Case study with the GC2145: a lot of small fix-up PRs:
I believe the original implementation was good, still: this PR uses code from the GC2145 driver. Currently every new driver needs to go through a copy-paste of a driver contributed ~4 years ago [1], then go through a lot of review to update it make it comply with recent guidelines, then fix any bug caused by this conversion. Better fix the original sensors, and the proposal to do that is to put most of the sensor logic in common with every sensor, including the emulated imager that runs on CI. [1]: And I am grateful to have these foundations in place since so long! On top of which we are building today. :) |
josuah
changed the title
This was referenced Apr 1, 2025
|
It is now all dis-aggregated in the PRs above, so closing this one. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, image sensor drivers have a lot of code, while they are essentially a way to pass tables of I2C registers to image sensors.
drivers: video: common: introduce CCI utilities #87935
zephyr,video-emul-imageruses the emulated I2C bus.drivers: video: emul-imager: use the emulated I2C bus #87937
drivers: video: imager: move the boilerplate in a library #88003
drivers: video: add imx219 sensor (RPi Cam v2) #88011
I will heavily split this in several smaller PRs after some polishing.
Is the general approach looking reasonable?
P.S.: The "Imager" terminology instead of "image sensor" is used because it is compact, and IIRC @ngphibang pointed that some "image sensors" were rather "image sensor + other things". Glad to switch to whichever makes most sense.