Re: [PATCH v2 09/13] media: i2c: imx290: VMAX is mode dependent

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Dave,

Thank you for the patch.

On Fri, Feb 03, 2023 at 07:18:07PM +0000, Dave Stevenson wrote:
> The default VMAX for 60fps in 720p mode is 750 according to
> the datasheet, however the driver always left it at 1125
> thereby stopping 60fps being achieved.
> 
> Make VMAX (and therefore V4L2_CID_VBLANK) mode dependent so
> that 720p60 can be achieved.
> 
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/i2c/imx290.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index 403bd7de875e..6235021a8d24 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -110,8 +110,6 @@
>  /* Number of lines by which exposure must be less than VMAX) */
>  #define IMX290_EXPOSURE_OFFSET				2
>  
> -#define IMX290_VMAX_DEFAULT				1125
> -
>  #define IMX290_PIXEL_RATE				148500000
>  
>  /*
> @@ -189,6 +187,7 @@ struct imx290_mode {
>  	u32 width;
>  	u32 height;
>  	u32 hmax_min;
> +	u32 vmax_min;
>  	u8 link_freq_index;
>  
>  	const struct imx290_regval *data;
> @@ -432,6 +431,7 @@ static const struct imx290_mode imx290_modes_2lanes[] = {
>  		.width = 1920,
>  		.height = 1080,
>  		.hmax_min = 2200,
> +		.vmax_min = 1125,
>  		.link_freq_index = FREQ_INDEX_1080P,
>  		.data = imx290_1080p_settings,
>  		.data_size = ARRAY_SIZE(imx290_1080p_settings),
> @@ -440,6 +440,7 @@ static const struct imx290_mode imx290_modes_2lanes[] = {
>  		.width = 1280,
>  		.height = 720,
>  		.hmax_min = 3300,
> +		.vmax_min = 750,
>  		.link_freq_index = FREQ_INDEX_720P,
>  		.data = imx290_720p_settings,
>  		.data_size = ARRAY_SIZE(imx290_720p_settings),
> @@ -451,6 +452,7 @@ static const struct imx290_mode imx290_modes_4lanes[] = {
>  		.width = 1920,
>  		.height = 1080,
>  		.hmax_min = 2200,
> +		.vmax_min = 1125,
>  		.link_freq_index = FREQ_INDEX_1080P,
>  		.data = imx290_1080p_settings,
>  		.data_size = ARRAY_SIZE(imx290_1080p_settings),
> @@ -459,6 +461,7 @@ static const struct imx290_mode imx290_modes_4lanes[] = {
>  		.width = 1280,
>  		.height = 720,
>  		.hmax_min = 3300,
> +		.vmax_min = 750,
>  		.link_freq_index = FREQ_INDEX_720P,
>  		.data = imx290_720p_settings,
>  		.data_size = ARRAY_SIZE(imx290_720p_settings),
> @@ -779,7 +782,7 @@ static void imx290_ctrl_update(struct imx290 *imx290,
>  {
>  	unsigned int hblank_min = mode->hmax_min - mode->width;
>  	unsigned int hblank_max = IMX290_HMAX_MAX - mode->width;
> -	unsigned int vblank_min = IMX290_VMAX_DEFAULT - mode->height;
> +	unsigned int vblank_min = mode->vmax_min - mode->height;
>  	unsigned int vblank_max = IMX290_VMAX_MAX - mode->height;
>  
>  	__v4l2_ctrl_s_ctrl(imx290->link_freq, mode->link_freq_index);

-- 
Regards,

Laurent Pinchart