| Message ID | 20250313184314.91410-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: i2c: Add Omnivision OV02C10 sensor driver | expand |
to., 13.03.2025 kl. 19.43 +0100, skrev Hans de Goede: > Here is v8 of the patch to upstream the OV02C10 sensor driver > originally writen by Intel which Heimir has been working on > upstreaming. > Many thanks to Heimir and Hans for this excellent work. This makes my workday easier. > (...) > > 1. I don't have hardware to test. I hope that others can test this > soon, > if things don't work the idea is that people can apply my cleanups > 1 by 1 and then we will know which change has broken things. Seems to work fine on my Dell XPS 13 9340. I have not found any glitches so far. Tested with on fedora 41 with qcam, cheese, obs, and firefox - tested with websites gum, jitsi, and webcamtests.com. All these work fine. webcamtests.com was even able to select highest resolution/zoom. Note that chromium does *not* work yet, at least not in Fedora. I now use this as my daily camera, without any problems. I only miss more controls available, for example adjusting colors and white balance. Some basic automatic adjustment seem to work, like when changing rooms or lightning, but when for example you sit by a window with sun on half your face, the image distorts, like getting over- or underexposed. Also, the colors seems a bit washed out compared to my usb cam (and to real life colors) Switching to and fro between v7 and v8 without rebooting gave unstable and strange results, but I presume that is less important, or even expected. Ingvar
Hi Ingvar, On 14-Mar-25 9:52 AM, Ingvar Hagelund wrote: > to., 13.03.2025 kl. 19.43 +0100, skrev Hans de Goede: >> Here is v8 of the patch to upstream the OV02C10 sensor driver >> originally writen by Intel which Heimir has been working on >> upstreaming. >> > > Many thanks to Heimir and Hans for this excellent work. This makes my > workday easier. > >> (...) >> >> 1. I don't have hardware to test. I hope that others can test this >> soon, >> if things don't work the idea is that people can apply my cleanups >> 1 by 1 and then we will know which change has broken things. > > Seems to work fine on my Dell XPS 13 9340. I have not found any > glitches so far. Tested with on fedora 41 with qcam, cheese, obs, and > firefox - tested with websites gum, jitsi, and webcamtests.com. All > these work fine. webcamtests.com was even able to select highest > resolution/zoom. Note that chromium does *not* work yet, at least not > in Fedora. Great thank you for testing. I was afraid that I would have broken something it is good to hear that I did not break anything. Sakari, do you want to take a look at the incremental patches in this v8 to get an idea of what I changed after your last review and maybe give feedback on specific changes, or shall I post a squashed v9 which might be easier for you to review ? > I now use this as my daily camera, without any problems. > > I only miss more controls available, for example adjusting colors and > white balance. Some basic automatic adjustment seem to work, like when > changing rooms or lightning, but when for example you sit by a window > with sun on half your face, the image distorts, like getting over- or > underexposed. Also, the colors seems a bit washed out compared to my > usb cam (and to real life colors) Yes we need to work on improving the image quality and things like the autoexposure algorithm, note this is all work which needs to be done on the libcamera / softisp side not on the kernel side. > Switching to and fro between v7 and v8 without rebooting gave unstable > and strange results, but I presume that is less important, or even > expected. This is expected the sensor and CSI receiver get linked together once both have loaded, rmmod-ing the sensor driver after this linking is done is not really supported. Regards, Hans
Hi Ingvar, On 14-Mar-25 9:52 AM, Ingvar Hagelund wrote: > to., 13.03.2025 kl. 19.43 +0100, skrev Hans de Goede: >> Here is v8 of the patch to upstream the OV02C10 sensor driver >> originally writen by Intel which Heimir has been working on >> upstreaming. >> > > Many thanks to Heimir and Hans for this excellent work. This makes my > workday easier. You're welcome. One more testing request, can you run qcam and then see what it reports for FPS after letting it run for a couple of seconds ? And then report the FPS back here ? Regards, Hans
fr., 14.03.2025 kl. 10.43 +0100, skrev Hans de Goede: > > One more testing request, can you run qcam and > then see what it reports for FPS after letting it run for > a couple of seconds ? > > And then report the FPS back here ? Sure It stabilizes at 30 fps (wandering a bit between 30.00 and 30.02 fps). Also, here is the output of cam -l and modinfo. I observe that cam says Configuration file 'ov02c10.yaml' not found for IPA module 'simple', falling back to 'uncalibrated.yaml' Should this file exist, or will the driver downloaded from the settings from the device, making the configuration file superflous? Best regards, Ingvar $ cam -l [2:16:02.661071222] [25536] INFO Camera camera_manager.cpp:325 libcamera v0.3.2 [2:16:02.677709274] [25539] WARN CameraSensor camera_sensor.cpp:257 'ov02c10 17-0036': Recommended V4L2 control 0x009a0922 not supported [2:16:02.677733285] [25539] ERROR V4L2 v4l2_subdevice.cpp:1085 'ov02c10 17-0036': Unable to get rectangle 2 on pad 0/0: Inappropriate ioctl for device [2:16:02.677742291] [25539] WARN CameraSensor camera_sensor.cpp:304 'ov02c10 17-0036': The PixelArraySize property has been defaulted to 1928x1092 [2:16:02.677747409] [25539] ERROR V4L2 v4l2_subdevice.cpp:1085 'ov02c10 17-0036': Unable to get rectangle 1 on pad 0/0: Inappropriate ioctl for device [2:16:02.677751299] [25539] WARN CameraSensor camera_sensor.cpp:315 'ov02c10 17-0036': The PixelArrayActiveAreas property has been defaulted to (0, 0)/1928x1092 [2:16:02.677757335] [25539] ERROR V4L2 v4l2_subdevice.cpp:1085 'ov02c10 17-0036': Unable to get rectangle 0 on pad 0/0: Inappropriate ioctl for device [2:16:02.677760909] [25539] WARN CameraSensor camera_sensor.cpp:323 'ov02c10 17-0036': Failed to retrieve the sensor crop rectangle [2:16:02.677764402] [25539] WARN CameraSensor camera_sensor.cpp:329 'ov02c10 17-0036': The sensor kernel driver needs to be fixed [2:16:02.677767468] [25539] WARN CameraSensor camera_sensor.cpp:331 'ov02c10 17-0036': See Documentation/sensor_driver_requirements.rst in the libcamera sources for more information [2:16:02.678099594] [25539] WARN CameraSensorProperties camera_sensor_properties.cpp:293 No static properties available for 'ov02c10' [2:16:02.678112388] [25539] WARN CameraSensorProperties camera_sensor_properties.cpp:295 Please consider updating the camera sensor properties database [2:16:02.678116314] [25539] WARN CameraSensor camera_sensor.cpp:477 'ov02c10 17-0036': Failed to retrieve the camera location [2:16:02.678120108] [25539] WARN CameraSensor camera_sensor.cpp:499 'ov02c10 17-0036': Rotation control not available, default to 0 degrees [2:16:02.679507146] [25539] WARN IPAProxy ipa_proxy.cpp:160 Configuration file 'ov02c10.yaml' not found for IPA module 'simple', falling back to 'uncalibrated.yaml' [2:16:02.679530599] [25539] WARN IPASoft soft_simple.cpp:114 Failed to create camera sensor helper for ov02c10 $ modinfo ov02c10 filename: /lib/modules/6.13.6- 200.fc41.x86_64/kernel/drivers/media/i2c/ov02c10.ko.xz license: GPL description: OmniVision OV02C10 sensor driver author: Hao Yao <hao.yao@intel.com> alias: acpi*:OVTI02C1:* depends: videodev,v4l2-cci,v4l2-fwnode,mc,v4l2-async name: ov02c10 retpoline: Y vermagic: 6.13.6-200.fc41.x86_64 SMP preempt mod_unload
Hi, On 14-Mar-25 11:01 AM, Ingvar Hagelund wrote: > fr., 14.03.2025 kl. 10.43 +0100, skrev Hans de Goede: >> >> One more testing request, can you run qcam and >> then see what it reports for FPS after letting it run for >> a couple of seconds ? >> >> And then report the FPS back here ? > > Sure > > It stabilizes at 30 fps (wandering a bit between 30.00 and 30.02 fps). Great, thank you. That means everything is working as it should (I had some doubts about 1 of the timing related register values used by the driver). > Also, here is the output of cam -l and modinfo. > > I observe that cam says > > Configuration file 'ov02c10.yaml' not found for IPA module 'simple', > falling back to 'uncalibrated.yaml' > > Should this file exist, or will the driver downloaded from the settings > from the device, making the configuration file superflous? There is a bunch of low hanging fruit wrt image quality which we need to address in libcamera which should help improve things without needing per sensor calibration. But for the best quality ideally we would have per sensor / camera module calibration profiles for certain things. There still is a long road to go before we will even start looking into that though. Regards, Hans
Hi All, Here is v8 of the patch to upstream the OV02C10 sensor driver originally writen by Intel which Heimir has been working on upstreaming. At Heimir's request I've taken over the upstreaming process. This new version addresses all the review remarks from Sakari, Bryan and Stanislaw, thank you all for the reviews. While working on fixing the review remarks I've also found and fixed / improved a bunch of other things myself. All in all there are quite a few changes, therefor I've chosen to send this as a patch series. I understand this cannot be merged in this form, I'll squash everything back together for v9. There are 2 reasons for sending this v8 as a series: 1. I don't have hardware to test. I hope that others can test this soon, if things don't work the idea is that people can apply my cleanups 1 by 1 and then we will know which change has broken things. 2. There are other sensor drivers from Intel at: https://github.com/intel/ipu6-drivers/tree/master/drivers/media/i2c/ which likely need similar changes. Doing this as an incremental series is also intended to document all the cleanups which likely need to be applied to other Intel drivers too. Note to reviewers there are some suboptimal things in this series wrt adding things and then later removing them again, like e.g. the use of guard(mutex)(&ov02c10->mutex). I did not bother to fix this since this will all get squashed together in v9 anyways. If it is easier for reviewing I can also (at request) post a v9 immediately with everything squashed together. Even then I still believe this admittedly weird v8 is useful for the reasons given above. Regards, Hans Hans de Goede (13): media: ov02c10: merge shared register settings into a shared reg_sequence array media: ov02c10: Fix hts for 2 lane mode media: ov02c10: Fix vts_min for 2 lane mode media: ov02c10: link-freq-index and pixel-rate fixes media: ov02c10: ov02c10_check_hwcfg() improvements media: ov02c10: CCI usage fixes media: ov02c10: Make modes lane-count independent media: ov02c10: Drop handshake pin support media: ov02c10: ov02c10_get_pm_resources() fixes media: ov02c10: Switch to {enable,disable}_streams media: ov02c10: Drop system suspend and resume handlers media: ov02c10: Switch to using the sub-device state lock media: ov02c10: Use v4l2_subdev_get_fmt() as v4l2_subdev_pad_ops.get_fmt() Heimir Thor Sverrisson (1): media: i2c: Add Omnivision OV02C10 sensor driver drivers/media/i2c/Kconfig | 10 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/ov02c10.c | 1012 +++++++++++++++++++++++++++++++++++ 3 files changed, 1023 insertions(+) create mode 100644 drivers/media/i2c/ov02c10.c