[libcamera-devel] libcamera: camera_sensor: Add OV9281 sensor properties
diff mbox series

Message ID 20221111163655.280751-1-kieran.bingham@ideasonboard.com
State New
Headers show
Series
  • [libcamera-devel] libcamera: camera_sensor: Add OV9281 sensor properties
Related show

Commit Message

Kieran Bingham Nov. 11, 2022, 4:36 p.m. UTC
Add an entry to the sensor properties for the OmniVision OV9281. Test
patterns are not enabled yet as the driver is not in an upstream kernel.

Unit cell size obtained from [0].

[0] https://www.ovt.com/wp-content/uploads/2022/01/OV9281-OV9282-PB-v1.3-WEB.pdf

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/camera_sensor_properties.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Dave Stevenson Nov. 11, 2022, 5 p.m. UTC | #1
Hi Kieran

On Fri, 11 Nov 2022 at 16:37, Kieran Bingham via libcamera-devel
<libcamera-devel@lists.libcamera.org> wrote:
>
> Add an entry to the sensor properties for the OmniVision OV9281. Test
> patterns are not enabled yet as the driver is not in an upstream kernel.

It's not going to be merged to an upstream kernel.
OV9282 (which is upstream) is the same as OV9281 except for the CRA in
the optical path.
I've sent patches to make ov9282 work with libcamera ([1] and [2]),
and the Pi kernel is hopefully going to drop our downstream ov9281
driver [3] once the patches are accepted by Sakari.

It does leave the odd subject that Alexander started with regard
adopting the compatible name rather than module name for the sensor
[4], and that seems to have gone a bit quiet.

Can I ask what the fascination is with test patterns? Are they really
that useful for drivers that have been merged to mainline and
therefore really should work?

Cheers
  Dave

[1] https://patchwork.linuxtv.org/project/linux-media/cover/20221005152018.3783890-1-dave.stevenson@raspberrypi.com/
[2] https://patchwork.linuxtv.org/project/linux-media/cover/20221028160902.2696973-1-dave.stevenson@raspberrypi.com/
[3] https://github.com/raspberrypi/linux/pull/5187
[4] https://patchwork.linuxtv.org/project/linux-media/patch/20220728130237.3396663-7-alexander.stein@ew.tq-group.com/

> Unit cell size obtained from [0].
>
> [0] https://www.ovt.com/wp-content/uploads/2022/01/OV9281-OV9282-PB-v1.3-WEB.pdf
>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/camera_sensor_properties.cpp | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
> index e5f27f06eb1d..3a6682f37707 100644
> --- a/src/libcamera/camera_sensor_properties.cpp
> +++ b/src/libcamera/camera_sensor_properties.cpp
> @@ -160,6 +160,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                  */
>                         },
>                 } },
> +               { "ov9281", {
> +                       .unitCellSize = { 3000, 3000 },
> +                       .testPatternModes = {
> +                               { controls::draft::TestPatternModeOff, 0 },
> +                       },
> +               } },
>                 { "ov13858", {
>                         .unitCellSize = { 1120, 1120 },
>                         .testPatternModes =  {
> --
> 2.34.1
>
Kieran Bingham Nov. 11, 2022, 11:54 p.m. UTC | #2
Quoting Dave Stevenson (2022-11-11 17:00:25)
> Hi Kieran
> 
> On Fri, 11 Nov 2022 at 16:37, Kieran Bingham via libcamera-devel
> <libcamera-devel@lists.libcamera.org> wrote:
> >
> > Add an entry to the sensor properties for the OmniVision OV9281. Test
> > patterns are not enabled yet as the driver is not in an upstream kernel.
> 
> It's not going to be merged to an upstream kernel.
> OV9282 (which is upstream) is the same as OV9281 except for the CRA in
> the optical path.

Will the sensor differences be exposed to userspace? (i.e. distinct
compatibles / identifiers to userspace?) (Ok, I see that's a yes -
later).

I believe I had this patch locally, because I saw someone complain about
one of the libcamera warnings so I must have looked up the data sheet to
get the basics started.


> I've sent patches to make ov9282 work with libcamera ([1] and [2]),
> and the Pi kernel is hopefully going to drop our downstream ov9281
> driver [3] once the patches are accepted by Sakari.
> 

Ok - interesting, everything related to this so far in libcamera is
ov9281.
libcamera$ find | grep ov928

./install/libcamera-gcc/usr/share/libcamera/ipa/raspberrypi/ov9281.json
./build/package/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
./build/test/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
./build/rpi/bullseye/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
./build/gcc/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
./src/ipa/raspberrypi/data/ov9281_mono.json
./src/ipa/raspberrypi/cam_helper_ov9281.cpp

> It does leave the odd subject that Alexander started with regard
> adopting the compatible name rather than module name for the sensor
> [4], and that seems to have gone a bit quiet.

That sounds correct to me, the compatible string could / should
highlight the distinction between the two sensors IMO. I'll go throw my
hat into that thread to state that I see compatibles as the correct way
to support both of these, even if they are 'almost the same'.

Of course these 'two' sensors are *very* alike, so it does beg, what do
we really want to duplicate in libcamera. Or should we have the ability
to match a single helper to multiple camera identifiers?

I also now wonder if we need to track those differences in CRA in any
helper - or if any applications (or IPA) care about that specific
property ?

 
> Can I ask what the fascination is with test patterns? Are they really
> that useful for drivers that have been merged to mainline and
> therefore really should work?

It's purely to be able to map test patterns to the corresponding V4L2
control id. Because there's no standardised way to express it ... it's a
mapping of an expected pattern in libcamera, to what that should be
represented as in the V4L2 specific control on the sensor. Each sensor
seems to do things differently ... ?

I don't think it's anything special except for a workaround for a poorly
defined implementation of V4L2_CID_TEST_PATTERN and the like...

--
Kieran


> Cheers
>   Dave
> 
> [1] https://patchwork.linuxtv.org/project/linux-media/cover/20221005152018.3783890-1-dave.stevenson@raspberrypi.com/
> [2] https://patchwork.linuxtv.org/project/linux-media/cover/20221028160902.2696973-1-dave.stevenson@raspberrypi.com/
> [3] https://github.com/raspberrypi/linux/pull/5187
> [4] https://patchwork.linuxtv.org/project/linux-media/patch/20220728130237.3396663-7-alexander.stein@ew.tq-group.com/
> 
> > Unit cell size obtained from [0].
> >
> > [0] https://www.ovt.com/wp-content/uploads/2022/01/OV9281-OV9282-PB-v1.3-WEB.pdf
> >
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  src/libcamera/camera_sensor_properties.cpp | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
> > index e5f27f06eb1d..3a6682f37707 100644
> > --- a/src/libcamera/camera_sensor_properties.cpp
> > +++ b/src/libcamera/camera_sensor_properties.cpp
> > @@ -160,6 +160,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> >                                  */
> >                         },
> >                 } },
> > +               { "ov9281", {
> > +                       .unitCellSize = { 3000, 3000 },
> > +                       .testPatternModes = {
> > +                               { controls::draft::TestPatternModeOff, 0 },
> > +                       },
> > +               } },
> >                 { "ov13858", {
> >                         .unitCellSize = { 1120, 1120 },
> >                         .testPatternModes =  {
> > --
> > 2.34.1
> >
Laurent Pinchart Nov. 12, 2022, 8:55 p.m. UTC | #3
Hi Kieran

On Fri, Nov 11, 2022 at 11:54:16PM +0000, Kieran Bingham via libcamera-devel wrote:
> Quoting Dave Stevenson (2022-11-11 17:00:25)
> > On Fri, 11 Nov 2022 at 16:37, Kieran Bingham wrote:
> > >
> > > Add an entry to the sensor properties for the OmniVision OV9281. Test
> > > patterns are not enabled yet as the driver is not in an upstream kernel.
> > 
> > It's not going to be merged to an upstream kernel.
> > OV9282 (which is upstream) is the same as OV9281 except for the CRA in
> > the optical path.
> 
> Will the sensor differences be exposed to userspace? (i.e. distinct
> compatibles / identifiers to userspace?) (Ok, I see that's a yes -
> later).
> 
> I believe I had this patch locally, because I saw someone complain about
> one of the libcamera warnings so I must have looked up the data sheet to
> get the basics started.
> 
> > I've sent patches to make ov9282 work with libcamera ([1] and [2]),
> > and the Pi kernel is hopefully going to drop our downstream ov9281
> > driver [3] once the patches are accepted by Sakari.
> 
> Ok - interesting, everything related to this so far in libcamera is
> ov9281.
> libcamera$ find | grep ov928
> 
> ./install/libcamera-gcc/usr/share/libcamera/ipa/raspberrypi/ov9281.json
> ./build/package/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
> ./build/test/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
> ./build/rpi/bullseye/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
> ./build/gcc/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
> ./src/ipa/raspberrypi/data/ov9281_mono.json
> ./src/ipa/raspberrypi/cam_helper_ov9281.cpp
> 
> > It does leave the odd subject that Alexander started with regard
> > adopting the compatible name rather than module name for the sensor
> > [4], and that seems to have gone a bit quiet.
> 
> That sounds correct to me, the compatible string could / should
> highlight the distinction between the two sensors IMO. I'll go throw my
> hat into that thread to state that I see compatibles as the correct way
> to support both of these, even if they are 'almost the same'.
> 
> Of course these 'two' sensors are *very* alike, so it does beg, what do
> we really want to duplicate in libcamera. Or should we have the ability
> to match a single helper to multiple camera identifiers?

There's not much data to duplicate at the moment, so I don't mind
separate entries, but matching on a set of identifiers is fine too.

> I also now wonder if we need to track those differences in CRA in any
> helper - or if any applications (or IPA) care about that specific
> property ?

I'd expect that to affect the tuning file mostly, not the helpers.

> > Can I ask what the fascination is with test patterns? Are they really
> > that useful for drivers that have been merged to mainline and
> > therefore really should work?
> 
> It's purely to be able to map test patterns to the corresponding V4L2
> control id. Because there's no standardised way to express it ... it's a
> mapping of an expected pattern in libcamera, to what that should be
> represented as in the V4L2 specific control on the sensor. Each sensor
> seems to do things differently ... ?
> 
> I don't think it's anything special except for a workaround for a poorly
> defined implementation of V4L2_CID_TEST_PATTERN and the like...
> 
> > [1] https://patchwork.linuxtv.org/project/linux-media/cover/20221005152018.3783890-1-dave.stevenson@raspberrypi.com/
> > [2] https://patchwork.linuxtv.org/project/linux-media/cover/20221028160902.2696973-1-dave.stevenson@raspberrypi.com/
> > [3] https://github.com/raspberrypi/linux/pull/5187
> > [4] https://patchwork.linuxtv.org/project/linux-media/patch/20220728130237.3396663-7-alexander.stein@ew.tq-group.com/
> > 
> > > Unit cell size obtained from [0].
> > >
> > > [0] https://www.ovt.com/wp-content/uploads/2022/01/OV9281-OV9282-PB-v1.3-WEB.pdf
> > >
> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > ---
> > >  src/libcamera/camera_sensor_properties.cpp | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
> > > index e5f27f06eb1d..3a6682f37707 100644
> > > --- a/src/libcamera/camera_sensor_properties.cpp
> > > +++ b/src/libcamera/camera_sensor_properties.cpp
> > > @@ -160,6 +160,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > >                                  */
> > >                         },
> > >                 } },
> > > +               { "ov9281", {
> > > +                       .unitCellSize = { 3000, 3000 },
> > > +                       .testPatternModes = {
> > > +                               { controls::draft::TestPatternModeOff, 0 },
> > > +                       },
> > > +               } },
> > >                 { "ov13858", {
> > >                         .unitCellSize = { 1120, 1120 },
> > >                         .testPatternModes =  {
Dave Stevenson Nov. 14, 2022, 11:47 a.m. UTC | #4
HI Kieran & Laurent

On Sat, 12 Nov 2022 at 20:55, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Kieran
>
> On Fri, Nov 11, 2022 at 11:54:16PM +0000, Kieran Bingham via libcamera-devel wrote:
> > Quoting Dave Stevenson (2022-11-11 17:00:25)
> > > On Fri, 11 Nov 2022 at 16:37, Kieran Bingham wrote:
> > > >
> > > > Add an entry to the sensor properties for the OmniVision OV9281. Test
> > > > patterns are not enabled yet as the driver is not in an upstream kernel.
> > >
> > > It's not going to be merged to an upstream kernel.
> > > OV9282 (which is upstream) is the same as OV9281 except for the CRA in
> > > the optical path.
> >
> > Will the sensor differences be exposed to userspace? (i.e. distinct
> > compatibles / identifiers to userspace?) (Ok, I see that's a yes -
> > later).
> >
> > I believe I had this patch locally, because I saw someone complain about
> > one of the libcamera warnings so I must have looked up the data sheet to
> > get the basics started.
> >
> > > I've sent patches to make ov9282 work with libcamera ([1] and [2]),
> > > and the Pi kernel is hopefully going to drop our downstream ov9281
> > > driver [3] once the patches are accepted by Sakari.

Sakari's latest pull for 6.2 has just picked the main series, but not
the regulator support (needs a review on the driver changes, but DT
has been Acked-by RobH). The regulator series has been tagged as
"Under Review" in Patchwork for longer than the main series, but
perhaps that means nothing.

At least I can go and fix up my Pi kernel PR with the accepted
patches, and the Pi kernel will be switching over.

> > Ok - interesting, everything related to this so far in libcamera is
> > ov9281.
> > libcamera$ find | grep ov928
> >
> > ./install/libcamera-gcc/usr/share/libcamera/ipa/raspberrypi/ov9281.json
> > ./build/package/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
> > ./build/test/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
> > ./build/rpi/bullseye/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
> > ./build/gcc/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
> > ./src/ipa/raspberrypi/data/ov9281_mono.json
> > ./src/ipa/raspberrypi/cam_helper_ov9281.cpp

> > > It does leave the odd subject that Alexander started with regard
> > > adopting the compatible name rather than module name for the sensor
> > > [4], and that seems to have gone a bit quiet.
> >
> > That sounds correct to me, the compatible string could / should
> > highlight the distinction between the two sensors IMO. I'll go throw my
> > hat into that thread to state that I see compatibles as the correct way
> > to support both of these, even if they are 'almost the same'.

My pending PR picks up patches so that the ov9281 compatible string is
added to the driver and it follows through to the sensor name, so it
makes no difference to us in userspace.
For those using ov9281 on other platforms, they need to make their own
choice on extra patches or userspace name.

> > Of course these 'two' sensors are *very* alike, so it does beg, what do
> > we really want to duplicate in libcamera. Or should we have the ability
> > to match a single helper to multiple camera identifiers?
>
> There's not much data to duplicate at the moment, so I don't mind
> separate entries, but matching on a set of identifiers is fine too.
>
> > I also now wonder if we need to track those differences in CRA in any
> > helper - or if any applications (or IPA) care about that specific
> > property ?
>
> I'd expect that to affect the tuning file mostly, not the helpers.

Most likely lens shading, but that will change based on the lens
attached anyway and we have no way of representing that in the
system.....

> > > Can I ask what the fascination is with test patterns? Are they really
> > > that useful for drivers that have been merged to mainline and
> > > therefore really should work?
> >
> > It's purely to be able to map test patterns to the corresponding V4L2
> > control id. Because there's no standardised way to express it ... it's a
> > mapping of an expected pattern in libcamera, to what that should be
> > represented as in the V4L2 specific control on the sensor. Each sensor
> > seems to do things differently ... ?
> >
> > I don't think it's anything special except for a workaround for a poorly
> > defined implementation of V4L2_CID_TEST_PATTERN and the like...

But where is the desperate desire to be able to control test patterns
within libcamera? It seems a huge amount of effort to support for very
little real world use. Just my opinion.

  Dave

> > > [1] https://patchwork.linuxtv.org/project/linux-media/cover/20221005152018.3783890-1-dave.stevenson@raspberrypi.com/
> > > [2] https://patchwork.linuxtv.org/project/linux-media/cover/20221028160902.2696973-1-dave.stevenson@raspberrypi.com/
> > > [3] https://github.com/raspberrypi/linux/pull/5187
> > > [4] https://patchwork.linuxtv.org/project/linux-media/patch/20220728130237.3396663-7-alexander.stein@ew.tq-group.com/
> > >
> > > > Unit cell size obtained from [0].
> > > >
> > > > [0] https://www.ovt.com/wp-content/uploads/2022/01/OV9281-OV9282-PB-v1.3-WEB.pdf
> > > >
> > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > > ---
> > > >  src/libcamera/camera_sensor_properties.cpp | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
> > > > index e5f27f06eb1d..3a6682f37707 100644
> > > > --- a/src/libcamera/camera_sensor_properties.cpp
> > > > +++ b/src/libcamera/camera_sensor_properties.cpp
> > > > @@ -160,6 +160,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > > >                                  */
> > > >                         },
> > > >                 } },
> > > > +               { "ov9281", {
> > > > +                       .unitCellSize = { 3000, 3000 },
> > > > +                       .testPatternModes = {
> > > > +                               { controls::draft::TestPatternModeOff, 0 },
> > > > +                       },
> > > > +               } },
> > > >                 { "ov13858", {
> > > >                         .unitCellSize = { 1120, 1120 },
> > > >                         .testPatternModes =  {
>
> --
> Regards,
>
> Laurent Pinchart
Kieran Bingham Nov. 14, 2022, 1:10 p.m. UTC | #5
Hi Dave,

Quoting Dave Stevenson (2022-11-14 11:47:17)
> HI Kieran & Laurent
> 
> On Sat, 12 Nov 2022 at 20:55, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > Hi Kieran
> >
> > On Fri, Nov 11, 2022 at 11:54:16PM +0000, Kieran Bingham via libcamera-devel wrote:
> > > Quoting Dave Stevenson (2022-11-11 17:00:25)
> > > > On Fri, 11 Nov 2022 at 16:37, Kieran Bingham wrote:
> > > > >
> > > > > Add an entry to the sensor properties for the OmniVision OV9281. Test
> > > > > patterns are not enabled yet as the driver is not in an upstream kernel.
> > > >
> > > > It's not going to be merged to an upstream kernel.
> > > > OV9282 (which is upstream) is the same as OV9281 except for the CRA in
> > > > the optical path.
> > >
> > > Will the sensor differences be exposed to userspace? (i.e. distinct
> > > compatibles / identifiers to userspace?) (Ok, I see that's a yes -
> > > later).
> > >
> > > I believe I had this patch locally, because I saw someone complain about
> > > one of the libcamera warnings so I must have looked up the data sheet to
> > > get the basics started.
> > >
> > > > I've sent patches to make ov9282 work with libcamera ([1] and [2]),
> > > > and the Pi kernel is hopefully going to drop our downstream ov9281
> > > > driver [3] once the patches are accepted by Sakari.
> 
> Sakari's latest pull for 6.2 has just picked the main series, but not
> the regulator support (needs a review on the driver changes, but DT
> has been Acked-by RobH). The regulator series has been tagged as
> "Under Review" in Patchwork for longer than the main series, but
> perhaps that means nothing.
> 
> At least I can go and fix up my Pi kernel PR with the accepted
> patches, and the Pi kernel will be switching over.
> 
> > > Ok - interesting, everything related to this so far in libcamera is
> > > ov9281.
> > > libcamera$ find | grep ov928
> > >
> > > ./install/libcamera-gcc/usr/share/libcamera/ipa/raspberrypi/ov9281.json
> > > ./build/package/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
> > > ./build/test/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
> > > ./build/rpi/bullseye/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
> > > ./build/gcc/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o
> > > ./src/ipa/raspberrypi/data/ov9281_mono.json
> > > ./src/ipa/raspberrypi/cam_helper_ov9281.cpp
> 
> > > > It does leave the odd subject that Alexander started with regard
> > > > adopting the compatible name rather than module name for the sensor
> > > > [4], and that seems to have gone a bit quiet.
> > >
> > > That sounds correct to me, the compatible string could / should
> > > highlight the distinction between the two sensors IMO. I'll go throw my
> > > hat into that thread to state that I see compatibles as the correct way
> > > to support both of these, even if they are 'almost the same'.
> 
> My pending PR picks up patches so that the ov9281 compatible string is
> added to the driver and it follows through to the sensor name, so it
> makes no difference to us in userspace.
> For those using ov9281 on other platforms, they need to make their own
> choice on extra patches or userspace name.
> 
> > > Of course these 'two' sensors are *very* alike, so it does beg, what do
> > > we really want to duplicate in libcamera. Or should we have the ability
> > > to match a single helper to multiple camera identifiers?
> >
> > There's not much data to duplicate at the moment, so I don't mind
> > separate entries, but matching on a set of identifiers is fine too.
> >
> > > I also now wonder if we need to track those differences in CRA in any
> > > helper - or if any applications (or IPA) care about that specific
> > > property ?
> >
> > I'd expect that to affect the tuning file mostly, not the helpers.
> 
> Most likely lens shading, but that will change based on the lens
> attached anyway and we have no way of representing that in the
> system.....
> 
> > > > Can I ask what the fascination is with test patterns? Are they really
> > > > that useful for drivers that have been merged to mainline and
> > > > therefore really should work?
> > >
> > > It's purely to be able to map test patterns to the corresponding V4L2
> > > control id. Because there's no standardised way to express it ... it's a
> > > mapping of an expected pattern in libcamera, to what that should be
> > > represented as in the V4L2 specific control on the sensor. Each sensor
> > > seems to do things differently ... ?
> > >
> > > I don't think it's anything special except for a workaround for a poorly
> > > defined implementation of V4L2_CID_TEST_PATTERN and the like...
> 
> But where is the desperate desire to be able to control test patterns
> within libcamera? It seems a huge amount of effort to support for very
> little real world use. Just my opinion.

I believe test tools from ChromeOS require being able to set the test
pattern on a sensor. They expect to know what pattern will be generated,
and validate that image is received.

It's nothing more than to change "Give me 'any' test image" into "Give
me an expected / defined test image"...

This is also an optional feature - I don't believe there is a
requirement to add the I would imagine us doing similar test pattern
support. That's why this patch ... hasn't!

--
Kieran



>   Dave
> 
> > > > [1] https://patchwork.linuxtv.org/project/linux-media/cover/20221005152018.3783890-1-dave.stevenson@raspberrypi.com/
> > > > [2] https://patchwork.linuxtv.org/project/linux-media/cover/20221028160902.2696973-1-dave.stevenson@raspberrypi.com/
> > > > [3] https://github.com/raspberrypi/linux/pull/5187
> > > > [4] https://patchwork.linuxtv.org/project/linux-media/patch/20220728130237.3396663-7-alexander.stein@ew.tq-group.com/
> > > >
> > > > > Unit cell size obtained from [0].
> > > > >
> > > > > [0] https://www.ovt.com/wp-content/uploads/2022/01/OV9281-OV9282-PB-v1.3-WEB.pdf
> > > > >
> > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > > > ---
> > > > >  src/libcamera/camera_sensor_properties.cpp | 6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
> > > > > index e5f27f06eb1d..3a6682f37707 100644
> > > > > --- a/src/libcamera/camera_sensor_properties.cpp
> > > > > +++ b/src/libcamera/camera_sensor_properties.cpp
> > > > > @@ -160,6 +160,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > > > >                                  */
> > > > >                         },
> > > > >                 } },
> > > > > +               { "ov9281", {
> > > > > +                       .unitCellSize = { 3000, 3000 },
> > > > > +                       .testPatternModes = {
> > > > > +                               { controls::draft::TestPatternModeOff, 0 },
> > > > > +                       },
> > > > > +               } },
> > > > >                 { "ov13858", {
> > > > >                         .unitCellSize = { 1120, 1120 },
> > > > >                         .testPatternModes =  {
> >
> > --
> > Regards,
> >
> > Laurent Pinchart

Patch
diff mbox series

diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
index e5f27f06eb1d..3a6682f37707 100644
--- a/src/libcamera/camera_sensor_properties.cpp
+++ b/src/libcamera/camera_sensor_properties.cpp
@@ -160,6 +160,12 @@  const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
 				 */
 			},
 		} },
+		{ "ov9281", {
+			.unitCellSize = { 3000, 3000 },
+			.testPatternModes = {
+				{ controls::draft::TestPatternModeOff, 0 },
+			},
+		} },
 		{ "ov13858", {
 			.unitCellSize = { 1120, 1120 },
 			.testPatternModes =  {