Re: [PATCH 03/21] media: i2c: imx258: Disable digital cropping on binned modes

From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
To: Dave Stevenson <dave.stevenson@raspberrypi.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>,
	linux-media@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 03/21] media: i2c: imx258: Disable digital cropping on binned modes
Date: Wed, 31 May 2023 17:02:49 +0200	[thread overview]
Message-ID: <j7rh46mdcdyttxkygdstyed4ilcw4hljw6zxzkrmo5joaqlnzf@xmmk24qd4skd> (raw)
In-Reply-To: <20230530173000.3060865-4-dave.stevenson@raspberrypi.com>

Hi Dave

On Tue, May 30, 2023 at 06:29:42PM +0100, Dave Stevenson wrote:
> The binned modes set DIG_CROP_X_OFFSET and DIG_CROP_IMAGE_WIDTH
> to less than the full image, even though the image being captured
> is meant to be a scaled version of the full array size.
>
> Reduce X_OFFSET to 0, and increase IMAGE_WIDTH to the full array.

Do I read it wrong that:

Digital crop is processed after image readout operation or in order of
[Analog crop -> Binning -> Digital crop]

Hence the digital crop output should be set to the frame output size
and not to the analog crop sizes ? In facts the DIG_CROP_IMAGE_HEIGHT
values match the frame output height.

>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> ---
>  drivers/media/i2c/imx258.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> index 946b1a12971d..aabd5c3e8af9 100644
> --- a/drivers/media/i2c/imx258.c
> +++ b/drivers/media/i2c/imx258.c
> @@ -340,11 +340,11 @@ static const struct imx258_reg mode_2104_1560_regs[] = {
>  	{ 0x0404, 0x00 },
>  	{ 0x0405, 0x20 },
>  	{ 0x0408, 0x00 },
> -	{ 0x0409, 0x02 },
> +	{ 0x0409, 0x00 },
>  	{ 0x040A, 0x00 },
>  	{ 0x040B, 0x00 },
>  	{ 0x040C, 0x10 },
> -	{ 0x040D, 0x6A },
> +	{ 0x040D, 0x70 },

DIG_CROP_IMAGE_WIDTH = 0x1070 = 4208

>  	{ 0x040E, 0x06 },
>  	{ 0x040F, 0x18 },

DIG_CROP_IMAGE_HEIGHT = 0x618 = 1560

>  	{ 0x3038, 0x00 },
> @@ -459,11 +459,11 @@ static const struct imx258_reg mode_1048_780_regs[] = {
>  	{ 0x0404, 0x00 },
>  	{ 0x0405, 0x40 },
>  	{ 0x0408, 0x00 },
> -	{ 0x0409, 0x06 },
> +	{ 0x0409, 0x00 },
>  	{ 0x040A, 0x00 },
>  	{ 0x040B, 0x00 },
>  	{ 0x040C, 0x10 },
> -	{ 0x040D, 0x64 },
> +	{ 0x040D, 0x70 },

DIG_CROP_IMAGE_WIDTH = 0x1070 = 4208

>  	{ 0x040E, 0x03 },
>  	{ 0x040F, 0x0C },

DIG_CROP_IMAGE_HEIGHT = 0x30c = 780

Should DIG_CROP_IMAGE_WIDTH be set to 2104 and 1048 respectively ?

>  	{ 0x3038, 0x00 },
> --
> 2.25.1
>