Re: [PATCH 15/25] media: i2c: ov01a10: Use native and default for pixel-array size names

[Hans de Goede <hansg@kernel.org>]

Hi Mehdi,

On 28-Oct-25 8:01 PM, Mehdi Djait wrote:
> Hi Hans,
> 
> On Tue, Oct 14, 2025 at 07:40:23PM +0200, Hans de Goede wrote:
>> According to the OV01A10 product-brief PDF the OV01A10 has an active pixel
>> array size of 1296x816. In otherwords the native and active sizes are
>> the same.
> 
> Isn't that an error in the product-brief ?
> 
> I understood the following:
> 
> The native pixel array size is 1296x816
> The active pixel array size is 1280x800 = these are the active pixels that can
> be output.
> 
> 1296x816 is not lised under the supported image sizes, so it
> is not the active pixel array size.

The list of supported image sizes typically is just plain non-sense.

As the later patch adding arbitrary output sizes shows there really
is nothing special about the listed supported image sizes.

Typically the vendor will provide a set of register values
for the supported image sizes, which is why we see various sensor
drivers with a fixed list of modes/output-sizes with then a long
register-list per mode.

That does not mean those are the only modes the sensor can actually
do since most sensors can do cropping at at least a 2x2 pixel precision
making any mode possible (although cropping away a lot will significantly
change the field-of-view).

Sometimes there may be some rows of dark pixels which are covered
with something which does not allow light to pass through for blacklevel
measurement but that does not appear to be the case here.

If you run the libcamera+softISP stack anmd specifically qcam with this
series then you will get 1292x816 as resolution and there are no black
borders or artifacts at the borders so it can really do 1296x816.

Also see the out of tree ov01a1s driver:

https://github.com/intel/ipu6-drivers/blob/master/drivers/media/i2c/ov01a1s.c

which also has an output width of 1296 pixels.

> I think in most sensors the active pixel array size is smaller than the
> native one.

Sometimes it is because there are e.g. a few rows of pixels
for blacklevel measuring. But often the entire array can be used.

Even if there are extra pixels for blacklevel measurement then
the actual active area typically is still bigger then a standard
resolution like e.hg. 1280x800 as many hardware ISPs typically
also need some extra pixels at the border for Bayer pattern
interpolation / demosaicing.

Regards,

Hans



>>
>> Replace the (misspelled) ACTIVE defines for the default resolution of
>> 1280x800 with DEFAULT to avoid giving the impression that the active pixel
>> array size is only 1280x800.
>>
>> And replace PIXEL_ARRAY with NATIVE to make clear this is the native pixel
>> array size / to match the V4L2_SEL_TGT_NATIVE_SIZE naming.
>>
>> Signed-off-by: Hans de Goede <hansg@kernel.org>
>> ---
>>  drivers/media/i2c/ov01a10.c | 32 ++++++++++++++++----------------
>>  1 file changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
>> index d0153e606c9a..f3bcb61c88dd 100644
>> --- a/drivers/media/i2c/ov01a10.c
>> +++ b/drivers/media/i2c/ov01a10.c
>> @@ -34,10 +34,10 @@
>>  #define OV01A10_MODE_STREAMING		0x01
>>  
>>  /* pixel array */
>> -#define OV01A10_PIXEL_ARRAY_WIDTH	1296
>> -#define OV01A10_PIXEL_ARRAY_HEIGHT	816
>> -#define OV01A10_ACITVE_WIDTH		1280
>> -#define OV01A10_ACITVE_HEIGHT		800
>> +#define OV01A10_NATIVE_WIDTH		1296
>> +#define OV01A10_NATIVE_HEIGHT		816
>> +#define OV01A10_DEFAULT_WIDTH		1280
>> +#define OV01A10_DEFAULT_HEIGHT		800
> 
> --
> Kind Regards
> Mehdi Djait