[libcamera-devel] [PATCH 2/4] libcamera: camera_sensor: Add OV4689 sensor properties

[Jacopo Mondi]

Hi Mikhail

On Thu, Jan 19, 2023 at 08:43:46PM +0300, Mikhail Rudenko via libcamera-devel wrote:
>
> On 2023-01-19 at 18:11 +01, Jacopo Mondi <jacopo.mondi at ideasonboard.com> wrote:
>
> > Hi Mikhail
> >
> > On Thu, Jan 19, 2023 at 06:59:03PM +0300, Mikhail Rudenko via libcamera-devel wrote:
> >> Add an entry to the sensor properties for Omnivision OV4689.
> >>
> >> Kernel supports three more types of color bars patterns, which we do
> >> not expose now.
> >>
> >> Signed-off-by: Mikhail Rudenko <mike.rudenko at gmail.com>
> >> ---
> >>  src/libcamera/camera_sensor_properties.cpp | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
> >> index c3c2cace..ca8ef962 100644
> >> --- a/src/libcamera/camera_sensor_properties.cpp
> >> +++ b/src/libcamera/camera_sensor_properties.cpp
> >> @@ -130,6 +130,13 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> >>  				{ controls::draft::TestPatternModeColorBars, 1},
> >>  			},
> >>  		} },
> >> +		{ "ov4689", {
> >> +			.unitCellSize = { 2000, 2000 },
> >> +			.testPatternModes = {
> >> +				{ controls::draft::TestPatternModeOff, 0 },
> >> +				{ controls::draft::TestPatternModeColorBars, 1}, // TODO: more patterns
> >
> > There's are 3 more patters and they seems easy to add here.
> > Any reason to skip them ?
>
> All the patterns supported by the kernel driver are color bars
> variations. If the following is OK, I'll do it in v2:
>
>     { controls::draft::TestPatternModeOff, 0 },
>     { controls::draft::TestPatternModeColorBars, 1},
>     { controls::draft::TestPatternModeColorBars, 2},
>     { controls::draft::TestPatternModeColorBars, 3},
>     { controls::draft::TestPatternModeColorBars, 4},
>

We're here trying to index test patterns using the modes defined by MIPI CCS
specifications from section 10.1. Looking at the sensor's test pattern
it seems to me the additional "Color bar type 2" corresponds to the
FadeToGray CCS test pattern mode.


OV4689         CCS                     libcamera
ColorBarType1 = 100% Color bars = TestPatternModeColorBars
ColorBarType2 = Fade to gray bars = TestPatternModeColorBarsFadeToGray

I cannot find any other CCS pattern corresponding to ColorBarType2 or
3.

I would then change this to

		{ "ov4689", {
			.unitCellSize = { 2000, 2000 },
			.testPatternModes = {
				{ controls::draft::TestPatternModeOff, 0 },
				{ controls::draft::TestPatternModeColorBars, 1},
				{ controls::draft::TestPatternModeColorBarsFadeToGray, 2},
                                /*
                                 * No corresponding test patterns in
                                 * MIPI CCS specification for sensor's
                                 * colorBarType2 and colorBarType3.
                                 */
                        },
                } },

I can change it when applying with your ack, if no other comments.


> > Thanks
> >   j
> >
> >> +			},
> >> +		} },
> >>  		{ "ov5640", {
> >>  			.unitCellSize = { 1400, 1400 },
> >>  			.testPatternModes = {
> >> --
> >> 2.39.0
> >>
>
> --
> Best regards,
> Mikhail Rudenko