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

[Mikhail Rudenko]

Hi Jacopo,

On 2023-01-20 at 09:17 +01, Jacopo Mondi <jacopo.mondi at ideasonboard.com> wrote:

> 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.
>                                  */
>                         },
>                 } },

This looks reasonable, thanks! I'll do so in v2 alongside a few other
changes.

Best regards,
Mikhail

> I can change it when applying with your ack, if no other comments.
>
>
>> > Thanks
>> >   j
>> >
>> >> +			},
>> >> +		} },
>> >>  		{ "ov5640", {
>> >>  			.unitCellSize = { 1400, 1400 },
>> >>  			.testPatternModes = {
>> >> --
>> >> 2.39.0
>> >>