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

OV7251 support #189

Closed
szym1998 opened this issue Oct 9, 2024 · 13 comments
Closed

OV7251 support #189

szym1998 opened this issue Oct 9, 2024 · 13 comments

Comments

@szym1998
Copy link

szym1998 commented Oct 9, 2024

Request for OV7251 Sensor Support in Raspberry Pi

Hi,

I've searched extensively for information regarding OV7251 sensor support on Raspberry Pi, but the available resources seem sparse and unclear. I came across the repository by 6by9, which reportedly worked for some users on older Raspberry Pi OS versions, but I haven’t had any success adapting this to the current libcamera versions.

I attempted to modify libcamera to support the OV7251 sensor but encountered issues that I couldn't resolve. I'm wondering if there’s any possibility of adding support for the OV7251 sensor—even without full calibration would be fine at this point.

Are there any current plans for implementing this sensor, or can you provide any guidance or workarounds that may help me get this sensor up and running with libcamera?

Thanks in advance for any insights or suggestions!

Best,
Szymon

@naushir
Copy link
Collaborator

naushir commented Oct 10, 2024

Could you begin by showing us what changes you made to libcamera for OV7251 support? At the very least, you will need to add a basic camera tuning file and camera helper library for basic support. Also, what issues did you encounter?

@6by9
Copy link
Collaborator

6by9 commented Oct 10, 2024

I have patches that get it working. I'll throw them at a PR in a mo.
Both my modules are here with me at home if you were wanting to test though.

I was just looking at the couple of kernel patches we have downstream and whether they can be upstreamed easily. The main one is swapping from V4L2_CID_GAIN to V4L2_CID_ANALOGUE_GAIN which could be counted as a regression if you just drop the old one.

@6by9
Copy link
Collaborator

6by9 commented Oct 10, 2024

PR created #191.
I'm quite happy if someone else wants to throw the patch upstream.

@kbingham
Copy link
Collaborator

I'll post patch 1 now. We can do patch 2 after pisp is merged.

@szym1998
Copy link
Author

Could you begin by showing us what changes you made to libcamera for OV7251 support? At the very least, you will need to add a basic camera tuning file and camera helper library for basic support. Also, what issues did you encounter?

https://github.com/szym1998/libcamera-ov7251.git

Issues where basicly not working not detecting the camera but given my lack of skill in anything that is not python this might be me....

@6by9
Copy link
Collaborator

6by9 commented Oct 10, 2024

I'll post patch 1 now. We can do patch 2 after pisp is merged.

Thanks Kieran - I just don't have things set up for libcamera submissions.

Kernel patches are:
raspberrypi/linux@992c6b9 (fwnode properties)
raspberrypi/linux@d8b299c (make GPIO optional)
raspberrypi/linux@d279825 (change from CID_GAIN to CID_ANALOGUE_GAIN)

I can't work out why I created raspberrypi/linux@51b342e as ov7251_global_init_setting does appear to be sent from ov7251_set_power_on. When creating raspberrypi/linux#4897 I think I may have picked up an early version of "media: i2c: Remove .s_power() from ov7251" which had the issue, but the version in mainline is good.

The driver does look a bit odd in having tables for different fixed frame rates (intervals), but also supporting VBLANK. Time for a quick diff of the tables and possibly some tidying up.

@kbingham
Copy link
Collaborator

Hrm - I see my colleague @djrscally has also been working on OV7251 before. Might be worth roping Dan in here!

@djrscally
Copy link
Contributor

Oh hello. I have the OV7251 working through the IPU3 pipeline handler, but I've never tried it on a raspberry pi (I have the sensor working on a Pi Zero but I think that it was through v4l2 alone rather than libcamera, and not with the upstream driver). So, not totally related work but happy to offer help if needed.

@kbingham

This comment was marked as outdated.

@6by9
Copy link
Collaborator

6by9 commented Oct 10, 2024

Going slightly off topic for this issue, but worth logging.

Looking at the kernel drivers, I think I'm right that raspberrypi/linux@51b342e is redundant. It got fixed in v2 of the patchset.

Diffs between the 30/60/90 fps tables are in registers

  • 0x3016-0x301c - 0xf0 (the default) for all in 30fps, and 0x10 for 0x3016 and 0x00 for the rest in 60/90fps modes. All are SC_CLKRST_x registers.
  • 0x3666 - set to 0x0a (the default) for 30 & 60fps, and not set in 90.
  • 0x3c03 - set to 0x00 (the default) for 30 & 60fps, and not set in 90.
  • 0x3c05 - set to 0x03 (the default) for 30 & 60fps, and 0x01 for 90. These are for low power streaming mode, so irrelevant.
  • 0x380e/f - VTS. 0x6bc for 30fps, 0x35c for 60fps, and 0x23c for 90fps. All redundant as overwritten by the V4L2_CID_VBLANK control value.

So I think all of that can be stripped out.
As with the recent patches for imx214, the fact that the driver advertised frame rates through FRAME_INTERVALS probably needs to be retained, but do very little (possibly update the VBLANK value).

@djrscally View on that?

Also does IPU3 use V4L2_CID_GAIN or V4L2_CID_ANALOGUE_GAIN. We obviously don't want to break existing users of _GAIN, but slaving the two together is a little ugly if not necessary.

I'll see if I can find a few hours to tart those patches up and submit them.

@szym1998
Copy link
Author

Thanks for the updates and your work on OV7251 support. If there's anything I can do to help with testing, please let me know.

Looking forward to the integration.

I'm not very familiar with automatic build systems—once the merge happens, will the support appear in the apt install?

@6by9
Copy link
Collaborator

6by9 commented Oct 14, 2024

The only bit you need is the libcamera update.

Kieran has submitted the patch for me (https://lists.libcamera.org/pipermail/libcamera-devel/2024-October/045749.html), so that should get reviewed in the next few days.
Updates from the main libcamera project flow through to the "next" branch here fairly regularly, but it is a manual process. There is a need to add the Pi5 tuning file as Pi5 support hasn't been upstreamed yet.
Apt gets updated when significant changes are made.

@naushir
Copy link
Collaborator

naushir commented Oct 22, 2024

This has now been merged

@naushir naushir closed this as completed Oct 22, 2024
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

No branches or pull requests

5 participants