linux-kernel - Re: [PATCH v6 4/5] media: i2c: imx219: Increase minimum LLP to fix blocky artefacts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPY8ntC1-S6zKtDvmc6EgyxP+j6rTShuG8Dr8PKb9XQr2PeS_w@mail.gmail.com>
Date: Mon, 10 Feb 2025 19:05:52 +0000
From: Dave Stevenson <dave.stevenson@...pberrypi.com>
To: Jai Luthra <jai.luthra@...asonboard.com>
Cc: Sakari Ailus <sakari.ailus@...ux.intel.com>, 
	Mauro Carvalho Chehab <mchehab@...nel.org>, linux-media@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Jacopo Mondi <jacopo.mondi@...asonboard.com>, 
	Laurent Pinchart <laurent.pinchart@...asonboard.com>
Subject: Re: [PATCH v6 4/5] media: i2c: imx219: Increase minimum LLP to fix
 blocky artefacts

Hi Jai

Thanks for the patch

On Tue, 4 Feb 2025 at 07:05, Jai Luthra <jai.luthra@...asonboard.com> wrote:
>
> The sensor's internal ADC supports a minimum line length of 3448 pixels,
> which may be too small to use with analog binning, where ADC operates on
> two lines together. Switch to a higher minimum line length of 3560
> pixels to fix the blocky artefacts seen with analog binning [1].
>
> To keep the same default framerate as before for all the modes, lower
> the default fll value to compensate for the increase in llp.

Extending the line length as you've done reduces the maximum frame
rate of all modes by ~3%.
Why do we need to reduce the max frame rate for non-binned modes which
have no artefacts?

Default frame rate is a relatively useless setting - any sensible
application should be setting the blanking controls to configure the
frame rate that it wants.

The datasheet does list line_length_pix default value as 3448 (0xd78),
and 3448 is mentioned numerous times throughout the datasheet for
calculations. Deviating from it for all modes is a brave move IMHO.

  Dave

> [1]: https://github.com/raspberrypi/rpicam-apps/issues/281#issuecomment-1082894118
>
> Signed-off-by: Jai Luthra <jai.luthra@...asonboard.com>
> ---
>  drivers/media/i2c/imx219.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index 8239e7ea8ec03849b339c1f314485266d4c4d8bd..e4aa6e66b673bb7a8942bf8daf27267c2884ec95 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -74,7 +74,7 @@
>  #define IMX219_FLL_MAX                 0xffff
>  #define IMX219_VBLANK_MIN              32
>  #define IMX219_REG_LINE_LENGTH_A       CCI_REG16(0x0162)
> -#define IMX219_LLP_MIN                 0x0d78
> +#define IMX219_LLP_MIN                 0x0de8
>  #define IMX219_LLP_MAX                 0x7ff0
>
>  #define IMX219_REG_X_ADD_STA_A         CCI_REG16(0x0164)
> @@ -286,25 +286,25 @@ static const struct imx219_mode supported_modes[] = {
>                 /* 8MPix 15fps mode */
>                 .width = 3280,
>                 .height = 2464,
> -               .fll_def = 3526,
> +               .fll_def = 3415,
>         },
>         {
>                 /* 1080P 30fps cropped */
>                 .width = 1920,
>                 .height = 1080,
> -               .fll_def = 1763,
> +               .fll_def = 1707,
>         },
>         {
>                 /* 2x2 binned 30fps mode */
>                 .width = 1640,
>                 .height = 1232,
> -               .fll_def = 1763,
> +               .fll_def = 1707,
>         },
>         {
>                 /* 640x480 30fps mode */
>                 .width = 640,
>                 .height = 480,
> -               .fll_def = 1763,
> +               .fll_def = 1707,
>         },
>  };
>
>
> --
> 2.48.1
>