Re: [PATCH 05/11] media: i2c: imx290: Support 60fps in 2 lane operation

[Alexander Stein <alexander.stein@ew.tq-group.com>]

Hi Dave,

thanks for the patch.

Am Dienstag, 31. Januar 2023, 20:20:10 CET schrieb Dave Stevenson:
> Commit "97589ad61c73 media: i2c: imx290: Add support for 2 data lanes"
> added support for running in two lane mode (instead of 4), but
> without changing the link frequency that resulted in a max of 30fps.
> 
> Commit "98e0500eadb7 media: i2c: imx290: Add configurable link frequency
> and pixel rate" then doubled the link frequency when in 2 lane mode,
> but didn't undo the correction for running at only 30fps, just extending
> horizontal blanking instead.
> 
> Remove the 30fps limit on 2 lane by correcting the register config
> in accordance with the datasheet for 60fps operation over 2 lanes.
> 
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> ---
>  drivers/media/i2c/imx290.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index bd8729aed43c..6bcfa535872f 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -366,7 +366,7 @@ static const struct imx290_mode imx290_modes_2lanes[] =
> { {
>  		.width = 1920,
>  		.height = 1080,
> -		.hmax = 4400,
> +		.hmax = 2200,
>  		.link_freq_index = FREQ_INDEX_1080P,
>  		.data = imx290_1080p_settings,
>  		.data_size = ARRAY_SIZE(imx290_1080p_settings),
> @@ -374,7 +374,7 @@ static const struct imx290_mode imx290_modes_2lanes[] =
> { {
>  		.width = 1280,
>  		.height = 720,
> -		.hmax = 6600,
> +		.hmax = 3300,
>  		.link_freq_index = FREQ_INDEX_720P,
>  		.data = imx290_720p_settings,
>  		.data_size = ARRAY_SIZE(imx290_720p_settings),
> @@ -518,21 +518,10 @@ static int imx290_set_register_array(struct imx290
> *imx290, static int imx290_set_data_lanes(struct imx290 *imx290)
>  {
>  	int ret = 0;
> -	u32 frsel;
> -
> -	switch (imx290->nlanes) {
> -	case 2:
> -	default:
> -		frsel = 0x02;
> -		break;
> -	case 4:
> -		frsel = 0x01;
> -		break;
> -	}
> 
>  	imx290_write(imx290, IMX290_PHY_LANE_NUM, imx290->nlanes - 1, &ret);
>  	imx290_write(imx290, IMX290_CSI_LANE_MODE, imx290->nlanes - 1, 
&ret);
> -	imx290_write(imx290, IMX290_FR_FDG_SEL, frsel, &ret);
> +	imx290_write(imx290, IMX290_FR_FDG_SEL, 0x01, &ret);
> 
>  	return ret;
>  }

Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>