diff mbox series

[41/57] media: atomisp: ov2680: Fix frame_size list

Message ID 20230123125205.622152-42-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show
Series media: atomisp: Big power-management changes + lots of fixes | expand

Commit Message

Hans de Goede Jan. 23, 2023, 12:51 p.m. UTC
3 fixes for the framesize list:

1. Drop modes < 640x480, these are made by significant cropping,
   leading to such a small remainig field-of-view that they are
   not really usable

2. 1616x1082 is presumably intended to be 1600x1080 + 16 pixels
   padding in both dimensions, but the height is wrong.
   Change this to 1616x1096.

3. The 800x600 mode is missing the 16 pixels padding and
   720x592 is missing 16 pixels padding in its width and
   the 720x576 base mode is a mode with non square pixels,
   while the sensor has square pixels.
   Replace both with 768x576 + 16 pixels padding -> 784x592

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

Andy Shevchenko Jan. 24, 2023, 10:46 a.m. UTC | #1
On Mon, Jan 23, 2023 at 01:51:49PM +0100, Hans de Goede wrote:
> 3 fixes for the framesize list:
> 
> 1. Drop modes < 640x480, these are made by significant cropping,
>    leading to such a small remainig field-of-view that they are
>    not really usable

Wondering if we have an algo to actually scale instead of crop.

> 2. 1616x1082 is presumably intended to be 1600x1080 + 16 pixels
>    padding in both dimensions, but the height is wrong.
>    Change this to 1616x1096.
> 
> 3. The 800x600 mode is missing the 16 pixels padding and
>    720x592 is missing 16 pixels padding in its width and
>    the 720x576 base mode is a mode with non square pixels,
>    while the sensor has square pixels.
>    Replace both with 768x576 + 16 pixels padding -> 784x592

Reviewed-by: Andy Shevchenko <andy@kernel.org>

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> index 432539dd274c..81fd36b09090 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> @@ -700,17 +700,13 @@ static int ov2680_enum_frame_size(struct v4l2_subdev *sd,
>  {
>  	static const struct v4l2_frmsize_discrete ov2680_frame_sizes[] = {
>  		{ 1616, 1216 },
> -		{ 1616, 1082 },
> +		{ 1616, 1096 },
>  		{ 1616,  916 },
>  		{ 1456, 1096 },
>  		{ 1296,  976 },
>  		{ 1296,  736 },
> -		{  800,  600 },
> -		{  720,  592 },
> +		{  784,  592 },
>  		{  656,  496 },
> -		{  336,  256 },
> -		{  352,  288 },
> -		{  176,  144 },
>  	};
>  	int index = fse->index;
>  
> -- 
> 2.39.0
>
Hans de Goede Jan. 24, 2023, 11:29 a.m. UTC | #2
Hi,

On 1/24/23 11:46, Andy Shevchenko wrote:
> On Mon, Jan 23, 2023 at 01:51:49PM +0100, Hans de Goede wrote:
>> 3 fixes for the framesize list:
>>
>> 1. Drop modes < 640x480, these are made by significant cropping,
>>    leading to such a small remainig field-of-view that they are
>>    not really usable
> 
> Wondering if we have an algo to actually scale instead of crop.

The sensor can "bin" the pixels, that is use every other pixel,
that already is used. Plain binning gives us 1600x1200 -> 800x600,
640x480 is binning + some cropping.

Regards,

Hans


> 
>> 2. 1616x1082 is presumably intended to be 1600x1080 + 16 pixels
>>    padding in both dimensions, but the height is wrong.
>>    Change this to 1616x1096.
>>
>> 3. The 800x600 mode is missing the 16 pixels padding and
>>    720x592 is missing 16 pixels padding in its width and
>>    the 720x576 base mode is a mode with non square pixels,
>>    while the sensor has square pixels.
>>    Replace both with 768x576 + 16 pixels padding -> 784x592
> 
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
> 
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 8 ++------
>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
>> index 432539dd274c..81fd36b09090 100644
>> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
>> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
>> @@ -700,17 +700,13 @@ static int ov2680_enum_frame_size(struct v4l2_subdev *sd,
>>  {
>>  	static const struct v4l2_frmsize_discrete ov2680_frame_sizes[] = {
>>  		{ 1616, 1216 },
>> -		{ 1616, 1082 },
>> +		{ 1616, 1096 },
>>  		{ 1616,  916 },
>>  		{ 1456, 1096 },
>>  		{ 1296,  976 },
>>  		{ 1296,  736 },
>> -		{  800,  600 },
>> -		{  720,  592 },
>> +		{  784,  592 },
>>  		{  656,  496 },
>> -		{  336,  256 },
>> -		{  352,  288 },
>> -		{  176,  144 },
>>  	};
>>  	int index = fse->index;
>>  
>> -- 
>> 2.39.0
>>
>
diff mbox series

Patch

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
index 432539dd274c..81fd36b09090 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
@@ -700,17 +700,13 @@  static int ov2680_enum_frame_size(struct v4l2_subdev *sd,
 {
 	static const struct v4l2_frmsize_discrete ov2680_frame_sizes[] = {
 		{ 1616, 1216 },
-		{ 1616, 1082 },
+		{ 1616, 1096 },
 		{ 1616,  916 },
 		{ 1456, 1096 },
 		{ 1296,  976 },
 		{ 1296,  736 },
-		{  800,  600 },
-		{  720,  592 },
+		{  784,  592 },
 		{  656,  496 },
-		{  336,  256 },
-		{  352,  288 },
-		{  176,  144 },
 	};
 	int index = fse->index;