Re: [PATCH v7 03/14] media: imx219: Account for rate_factor in control steps

On Fri, Aug 07, 2026 at 03:23:58PM +0300, Sakari Ailus wrote:
> The controls that are divided by the rate_factor before writing them to
> the registers have the step of the value of the rate_factor. Take this
> into account when the control's range is modified. The controls are
> created in a configuration where rate_factor is always 1, hence there's no
> need to change the code adding new controls.
> 
> Fixes: f513997119f4 ("media: i2c: imx219: Scale the pixel rate for analog binning")
> Cc: [email protected]
> Signed-off-by: Sakari Ailus <[email protected]>
> Reviewed-by: Dave Stevenson <[email protected]>

Reviewed-by: Laurent Pinchart <[email protected]>

> ---
>  drivers/media/i2c/imx219.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index 5184523de8be..b1f29037af74 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -340,13 +340,13 @@ static const struct imx219_mode supported_modes[] = {
>  		/* 2x2 binned 60fps mode */
>  		.width = 1640,
>  		.height = 1232,
> -		.fll_def = 1707,
> +		.fll_def = 1706,
>  	},
>  	{
>  		/* 640x480 60fps mode */
>  		.width = 640,
>  		.height = 480,
> -		.fll_def = 1707,
> +		.fll_def = 1706,
>  	},
>  };
>  
> @@ -473,8 +473,7 @@ static int imx219_set_ctrl(struct v4l2_ctrl *ctrl)
>  		ret = __v4l2_ctrl_modify_range(imx219->exposure,
>  					       imx219->exposure->minimum,
>  					       exposure_max,
> -					       imx219->exposure->step,
> -					       exposure_def);
> +					       rate_factor, exposure_def);
>  		if (ret)
>  			return ret;
>  
> @@ -902,7 +901,8 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
>  
>  		/* Update limits and set FPS to default */
>  		ret = __v4l2_ctrl_modify_range(imx219->vblank, IMX219_VBLANK_MIN,
> -					       IMX219_FLL_MAX - mode->height, 1,
> +					       IMX219_FLL_MAX - mode->height,
> +					       rate_factor,
>  					       mode->fll_def - mode->height);
>  		if (ret)
>  			return ret;
> @@ -920,8 +920,7 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
>  		ret = __v4l2_ctrl_modify_range(imx219->exposure,
>  					       imx219->exposure->minimum,
>  					       exposure_max,
> -					       imx219->exposure->step,
> -					       exposure_def);
> +					       rate_factor, exposure_def);
>  		if (ret)
>  			return ret;
>  
> @@ -946,8 +945,7 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
>  			return ret;
>  
>  		/* Scale the pixel rate based on the mode specific factor */
> -		pixel_rate = imx219_get_pixel_rate(imx219) *
> -			     imx219_get_rate_factor(state);
> +		pixel_rate = imx219_get_pixel_rate(imx219) * rate_factor;
>  		ret = __v4l2_ctrl_modify_range(imx219->pixel_rate, pixel_rate,
>  					       pixel_rate, 1, pixel_rate);
>  		if (ret)

-- 
Regards,

Laurent Pinchart