| Message ID | 20240723145736.81241-1-dse@thaumatec.com |
|---|---|
| State | Accepted |
| Commit | 7ee9ef451dfe6c07747b206a390455d4e8df1638 |
| Headers | show |
| Series |
|
| Related | show |
Hi Daniel, On 7/23/24 4:56 PM, Daniel Semkowicz wrote: > Add black level value for OV5675 camera sensor. > According to datasheet, default value is 0x10, 10 bits width. > However, Linux kernel driver initializes black level target value > to 0x40. Set the value to the same as in kernel driver, but scaled > to 16 bits. > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de> Considering that this value can change over time, c.f. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99cd12425dfe9a400b672f112512a3678dfd1eb6 for ov5670, we should probably think of having the kernel expose this value as Kieran suggested in v1. I don't think this should be a blocker for this patch though (especially since the upstream kernel has had this value since the introducing commit). Thanks! Quentin
Quoting Quentin Schulz (2024-07-23 16:04:39) > Hi Daniel, > > On 7/23/24 4:56 PM, Daniel Semkowicz wrote: > > Add black level value for OV5675 camera sensor. > > According to datasheet, default value is 0x10, 10 bits width. > > However, Linux kernel driver initializes black level target value > > to 0x40. Set the value to the same as in kernel driver, but scaled > > to 16 bits. > > > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> > > Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de> > > Considering that this value can change over time, c.f. > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99cd12425dfe9a400b672f112512a3678dfd1eb6 > for ov5670, we should probably think of having the kernel expose this > value as Kieran suggested in v1. I don't think this should be a blocker > for this patch though (especially since the upstream kernel has had this > value since the introducing commit). Agreed, This value matches what upstream kernels will configure the device so it's appropriate now. But we need to resolve this in the future - ideally for all camera sensors in a consistent manner. That's far more tricky :D but for this v2 patch ... Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Thanks! > Quentin
diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp index a1339c83..29a00d7f 100644 --- a/src/ipa/libipa/camera_sensor_helper.cpp +++ b/src/ipa/libipa/camera_sensor_helper.cpp @@ -601,6 +601,8 @@ class CameraSensorHelperOv5675 : public CameraSensorHelper public: CameraSensorHelperOv5675() { + /* From Linux kernel driver: 0x40 at 10bits. */ + blackLevel_ = 4096; gainType_ = AnalogueGainLinear; gainConstants_.linear = { 1, 0, 0, 128 }; }
Add black level value for OV5675 camera sensor. According to datasheet, default value is 0x10, 10 bits width. However, Linux kernel driver initializes black level target value to 0x40. Set the value to the same as in kernel driver, but scaled to 16 bits. Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> --- src/ipa/libipa/camera_sensor_helper.cpp | 2 ++ 1 file changed, 2 insertions(+)