Message ID | 20240723141814.76823-1-dse@thaumatec.com |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
Hi Daniel, On 7/23/24 4:17 PM, Daniel Semkowicz wrote: > Add black level value for OV5675 camera. > According to datasheet, default value is 0x10, 10 bits width. > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> Yes.... but I just checked now the init sequence in the Linux kernel driver and it's different from the default :/ The 10b value is stored in two registers: 0x4002 BLC[9:8] as LSBs and 0x4003 for BLC[7:0]. The default value is indeed 0x00 and 0x10, BUT the driver sets 0x4003 to 0x40 so I think this needs to be 0x40? Can you see if this makes things better? Cheers, Quentin
Quoting Daniel Semkowicz (2024-07-23 15:17:58) > Add black level value for OV5675 camera. > According to datasheet, default value is 0x10, 10 bits width. I don't have that datasheet, but given the above statement the below is correct: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> > --- > src/ipa/libipa/camera_sensor_helper.cpp | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp > index a1339c83..79608c00 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 datasheet: 0x10 at 10bits. */ > + blackLevel_ = 1024; > gainType_ = AnalogueGainLinear; > gainConstants_.linear = { 1, 0, 0, 128 }; > } > -- > 2.45.2 > >
True, I missed it... Thanks! I will upload v2 with value from kernel driver. On Tue, Jul 23, 2024 at 4:39 PM Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Quoting Daniel Semkowicz (2024-07-23 15:17:58) > > Add black level value for OV5675 camera. > > According to datasheet, default value is 0x10, 10 bits width. > > I don't have that datasheet, but given the above statement the below is > correct: > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> > > --- > > src/ipa/libipa/camera_sensor_helper.cpp | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp > > index a1339c83..79608c00 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 datasheet: 0x10 at 10bits. */ > > + blackLevel_ = 1024; > > gainType_ = AnalogueGainLinear; > > gainConstants_.linear = { 1, 0, 0, 128 }; > > } > > -- > > 2.45.2 > > > >
Quoting Quentin Schulz (2024-07-23 15:25:34) > Hi Daniel, > > On 7/23/24 4:17 PM, Daniel Semkowicz wrote: > > Add black level value for OV5675 camera. > > According to datasheet, default value is 0x10, 10 bits width. > > > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> > > Yes.... but I just checked now the init sequence in the Linux kernel > driver and it's different from the default :/ > > The 10b value is stored in two registers: 0x4002 BLC[9:8] as LSBs and > 0x4003 for BLC[7:0]. The default value is indeed 0x00 and 0x10, BUT the > driver sets 0x4003 to 0x40 so I think this needs to be 0x40? Can you see > if this makes things better? Yikes, That means we really can't even trust the datasheet values and we should be exposing that value from the kernel through the control framework and reading it (or setting it ?) at runtime. -- Kieran
diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp index a1339c83..79608c00 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 datasheet: 0x10 at 10bits. */ + blackLevel_ = 1024; gainType_ = AnalogueGainLinear; gainConstants_.linear = { 1, 0, 0, 128 }; }
Add black level value for OV5675 camera. According to datasheet, default value is 0x10, 10 bits width. Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> --- src/ipa/libipa/camera_sensor_helper.cpp | 2 ++ 1 file changed, 2 insertions(+)