| Message ID | 20241209104727.520859-1-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Commit | 229667606e4d30e03d530341baf1ee528e7f2e95 |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush, Thank you for the patch. On Mon, Dec 09, 2024 at 10:47:23AM +0000, Naushir Patuck wrote: > The frameIntegrationDiff value should be 25, otherwise we see invalid > frames when integration times are set to large values. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> I'll trust you on that. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > index a65c8ac08c1c..96f7fff499f5 100644 > --- a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > +++ b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > @@ -25,7 +25,7 @@ private: > * Smallest difference between the frame length and integration time, > * in units of lines. > */ > - static constexpr int frameIntegrationDiff = 4; > + static constexpr int frameIntegrationDiff = 25; > }; > > /*
Thanks Naush On Mon, 9 Dec 2024 at 10:47, Naushir Patuck <naush@raspberrypi.com> wrote: > > The frameIntegrationDiff value should be 25, otherwise we see invalid > frames when integration times are set to large values. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.com> I'm sorting a patch for the kernel driver too, as that has OV9282_EXPOSURE_OFFSET set to 12. Dave > --- > src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > index a65c8ac08c1c..96f7fff499f5 100644 > --- a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > +++ b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > @@ -25,7 +25,7 @@ private: > * Smallest difference between the frame length and integration time, > * in units of lines. > */ > - static constexpr int frameIntegrationDiff = 4; > + static constexpr int frameIntegrationDiff = 25; > }; > > /* > -- > 2.43.0 >
On Mon, Dec 09, 2024 at 10:52:59AM +0000, Dave Stevenson wrote: > On Mon, 9 Dec 2024 at 10:47, Naushir Patuck <naush@raspberrypi.com> wrote: > > > > The frameIntegrationDiff value should be 25, otherwise we see invalid > > frames when integration times are set to large values. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > I'm sorting a patch for the kernel driver too, as that has > OV9282_EXPOSURE_OFFSET set to 12. This makes me wish we had a centralized database for such information. Ideally populated by camera sensor vendors. I know, I'm a dreamer :-) > > --- > > src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > > index a65c8ac08c1c..96f7fff499f5 100644 > > --- a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > > +++ b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > > @@ -25,7 +25,7 @@ private: > > * Smallest difference between the frame length and integration time, > > * in units of lines. > > */ > > - static constexpr int frameIntegrationDiff = 4; > > + static constexpr int frameIntegrationDiff = 25; > > }; > > > > /*
Quoting Naushir Patuck (2024-12-09 10:47:23) > The frameIntegrationDiff value should be 25, otherwise we see invalid > frames when integration times are set to large values. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > index a65c8ac08c1c..96f7fff499f5 100644 > --- a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > +++ b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > @@ -25,7 +25,7 @@ private: > * Smallest difference between the frame length and integration time, > * in units of lines. > */ > - static constexpr int frameIntegrationDiff = 4; > + static constexpr int frameIntegrationDiff = 25; Aha, quite helpfully - Laurent just pointed out to me that this is specified in the datasheet as 0x3502 EXPO 0x00 RW Bit[7:0]: Exposure[7:0] Low 4 bits are fraction bits Minimum exposure time is 1 row period. Maximum exposure time is frame length -25 row periods, where frame length is set by registers {0x380E, 0x380F} Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > }; > > /* > -- > 2.43.0 >
On Mon, 9 Dec 2024 at 12:56, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Quoting Naushir Patuck (2024-12-09 10:47:23) > > The frameIntegrationDiff value should be 25, otherwise we see invalid > > frames when integration times are set to large values. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > --- > > src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > > index a65c8ac08c1c..96f7fff499f5 100644 > > --- a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > > +++ b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp > > @@ -25,7 +25,7 @@ private: > > * Smallest difference between the frame length and integration time, > > * in units of lines. > > */ > > - static constexpr int frameIntegrationDiff = 4; > > + static constexpr int frameIntegrationDiff = 25; > > Aha, quite helpfully - Laurent just pointed out to me that this is > specified in the datasheet as Probably as I'd just sent him a link to the datasheet :-) > 0x3502 EXPO 0x00 RW > > Bit[7:0]: Exposure[7:0] > Low 4 bits are fraction bits > > Minimum exposure time is 1 row period. > Maximum exposure time is frame length -25 row > periods, where frame length is set by registers > {0x380E, 0x380F} > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Matching kernel commit submitted as https://lore.kernel.org/linux-media/20241209-media-ov9282-fix-v1-1-d06bb7546f18@raspberrypi.com/ Dave > > }; > > > > /* > > -- > > 2.43.0 > >
diff --git a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp index a65c8ac08c1c..96f7fff499f5 100644 --- a/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp +++ b/src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp @@ -25,7 +25,7 @@ private: * Smallest difference between the frame length and integration time, * in units of lines. */ - static constexpr int frameIntegrationDiff = 4; + static constexpr int frameIntegrationDiff = 25; }; /*
The frameIntegrationDiff value should be 25, otherwise we see invalid frames when integration times are set to large values. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- src/ipa/rpi/cam_helper/cam_helper_ov9281.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)