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.

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

Sony imx296 sensors support #5039

Merged
merged 11 commits into from May 20, 2022
Merged

Conversation

naushir
Copy link
Contributor

@naushir naushir commented May 17, 2022

This driver is not yet fully upstreamed (https://patchwork.linuxtv.org/project/linux-media/list/?series=6989), but I don't anticipate any major changes from this revision.

@pelwell
Copy link
Contributor

pelwell commented May 17, 2022

The overlay is lacking the necessary README and Makefile entries, but otherwise it looks OK (as far as my reading went, which is not very far).

@6by9
Copy link
Contributor

6by9 commented May 17, 2022

I'm reading through it at present.

Presumably the main driver commit is unmodified from that submitted to mainline? I'm just looking at things like exposure being clipped when set, rather than the range being reduced based on VBLANK.

@pelwell
Copy link
Contributor

pelwell commented May 17, 2022

I will need to remember to change the Kconfig entry when forward-porting to 5.18.

@naushir
Copy link
Contributor Author

naushir commented May 17, 2022

Presumably the main driver commit is unmodified from that submitted to mainline? I'm just looking at things like exposure being clipped when set, rather than the range being reduced based on VBLANK.

The main driver commit is unchanged from the mainline patch. Any additions ought to be done on top.

Copy link
Contributor

@6by9 6by9 left a comment

Choose a reason for hiding this comment

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

Looks reasonable.

It'd be nice to fix up the exposure range to follow VBLANK, and to add flips.

reg_frag: fragment@5 {
target = <&cam1_reg>;
cam_reg: __overlay__ {
startup-delay-us = <300000>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you validate that this delay needs to be this high?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it does seem to need 300ms. Not surprising, as it probably has the same reset circuitry as the imx477.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, this does not seem enough - I have had to up it to 500ms for glitch free operation!

target = <&cam1_clk>;
__overlay__ {
status = "okay";
clock-frequency = <37125000>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add an override for the clock frequency?

remote-endpoint = <&csi_ep>;
clock-lanes = <0>;
data-lanes = <1>;
clock-noncontinuous;
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't find reference in the datasheet as to whether the clock lane does drop to LP-11 during blanking or not. Ought to validate it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, need clock-noncontinuous otherwise streaming does not behave.

@6by9
Copy link
Contributor

6by9 commented May 17, 2022

V4L2_CID_LINK_FREQUENCY isn't advertised either, nor is the link frequency validated from DT.

@6by9
Copy link
Contributor

6by9 commented May 17, 2022

We're also missing an update to the defconfigs.

@pelwell
Copy link
Contributor

pelwell commented May 17, 2022

True - and the overlay commit should be called overlays: ..., not dt-bindings: ....

@naushir
Copy link
Contributor Author

naushir commented May 17, 2022

I'll update with all the fixups and post an update soon.

Mani-Sadhasivam and others added 2 commits May 20, 2022 13:33
Add YAML devicetree binding for IMX296 CMOS image sensor. Let's also
add MAINTAINERS entry for the binding and driver.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Rob Herring <robh@kernel.org>
The IMX296LLR is a monochrome 1.60MP CMOS sensor from Sony. The driver
supports cropping and binning (but not both at the same time due to
hardware limitations) and exposure, gain, vertical blanking and test
pattern controls.

Preliminary support is also included for the color IMX296LQR sensor.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
@naushir
Copy link
Contributor Author

naushir commented May 20, 2022

New revision:

  • Fixed the "overlay:" comment
  • Added h/v flip and link frequency ctrls.
  • Switch to 1 frame gain delay.
  • Adjusted delays for reliable operation.

Add an overlay for the Sony IMX296 camera sensor.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
@pelwell
Copy link
Contributor

pelwell commented May 20, 2022

Are you happy your comments have been addressed, @6by9?

arch/arm/configs/bcm2711_defconfig Show resolved Hide resolved
drivers/media/i2c/imx296.c Outdated Show resolved Hide resolved
drivers/media/i2c/imx296.c Show resolved Hide resolved
drivers/media/i2c/imx296.c Show resolved Hide resolved
drivers/media/i2c/imx296.c Outdated Show resolved Hide resolved
drivers/media/i2c/imx296.c Show resolved Hide resolved
drivers/media/i2c/imx296.c Show resolved Hide resolved
drivers/media/i2c/imx296.c Show resolved Hide resolved
@@ -928,6 +928,7 @@ CONFIG_VIDEO_ARDUCAM_PIVARIETY=m
CONFIG_VIDEO_IMX219=m
CONFIG_VIDEO_IMX258=m
CONFIG_VIDEO_IMX290=m
CONFIG_VIDEO_IMX296=m
Copy link
Contributor

Choose a reason for hiding this comment

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

Of course now the commit text is wrong ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Arghhhhh

drivers/media/i2c/imx296.c Show resolved Hide resolved
drivers/media/i2c/imx296.c Show resolved Hide resolved
Build this module by default for the following:
bcm2711_defconfig
bcm2709_defconfig
bcmrpi3_defconfig
bcmrpi_defconfig

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Switch the Bayer ordering advertised by the device driver from BGGR to RGGB.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Add a 1.5ms delay after coming out of standby.
Add a 2ms delay after going into or coming out of streaming state.

All delay values have been deduced from the sensor datasheet.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
The 1/4 resolution derived mode does not stream correctly, so remove it from
the advertised mode list.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Add support for hflip and vflip controls. Adjust the mbus_code value reported
based on the currently programmed flips.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
The exposure lines control limits are adjusted appropriately during any change
to the vblank control. This allows accurate framerate and exposure adjustments
from userland.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
In order to behave in a similar manner to the rolling shutter sensors, set the
gain delay to 1 frame. This simplifies userland control of the gain value.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Add the V4L2_CID_LINK_FREQ menu control for the imx296. Report a single value
of 1188 Mhz for the link frequency.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
@6by9
Copy link
Contributor

6by9 commented May 20, 2022

LGTM. Ship it!

@pelwell pelwell merged commit 0fffce9 into raspberrypi:rpi-5.15.y May 20, 2022
popcornmix added a commit to raspberrypi/firmware that referenced this pull request May 24, 2022
kernel: Build RV8803 RTC driver and add to RTC device tree
See: raspberrypi/linux#5042

kernel: Sony imx296 sensors support
See: raspberrypi/linux#5039

firmware: arm_dt: camera_auto_detect cam0 flag needs to look at Unicam instance, not port

firmware: platform: over-voltage Zero 2 W by two pips
See: #1723
popcornmix added a commit to raspberrypi/rpi-firmware that referenced this pull request May 24, 2022
kernel: Build RV8803 RTC driver and add to RTC device tree
See: raspberrypi/linux#5042

kernel: Sony imx296 sensors support
See: raspberrypi/linux#5039

firmware: arm_dt: camera_auto_detect cam0 flag needs to look at Unicam instance, not port

firmware: platform: over-voltage Zero 2 W by two pips
See: raspberrypi/firmware#1723
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants