Re: [PATCH 10/13] media: i2c: os05b10: keep vblank/exposure in sync on mode switch

[Sakari Ailus]

Hi Tarang,

On Fri, Mar 06, 2026 at 06:03:00PM +0530, Tarang Raval wrote:
> When switching sensor modes, V4L2 updates the vblank range/default but
> keeps the previous current value, leaving vertical blanking unchanged.
> Update the vblank and exposure control values to the new mode defaults
> after adjusting the ranges.
> 
> Clamp the exposure default value to the new maximum to prevent -ERANGE
> during format changes.
> 
> Also use pm_runtime_get_if_active() in set_ctrl() to avoid touching the
> sensor when runtime suspended.
> 
> Signed-off-by: Tarang Raval <tarang.raval@xxxxxxxxxxxxxxxxx>
> ---
>  drivers/media/i2c/os05b10.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/i2c/os05b10.c b/drivers/media/i2c/os05b10.c
> index d8d776de5f35..4601e33b7e8f 100644
> --- a/drivers/media/i2c/os05b10.c
> +++ b/drivers/media/i2c/os05b10.c
> @@ -732,16 +732,15 @@ static int os05b10_set_ctrl(struct v4l2_ctrl *ctrl)
>  	if (ctrl->id == V4L2_CID_VBLANK) {
>  		/* Honour the VBLANK limits when setting exposure. */
>  		s64 max = fmt->height + ctrl->val - OS05B10_EXPOSURE_MARGIN;
> -
> +		s64 def = min_t (s64, max, os05b10->exposure->default_value);
>  		ret = __v4l2_ctrl_modify_range(os05b10->exposure,
>  					       os05b10->exposure->minimum, max,
> -					       os05b10->exposure->step,
> -					       os05b10->exposure->default_value);
> +					       os05b10->exposure->step, def);
>  		if (ret)
>  			return ret;
>  	}
>  
> -	if (pm_runtime_get_if_in_use(os05b10->dev) == 0)
> +	if (pm_runtime_get_if_active(os05b10->dev) == 0)

This actually appears to be a bugfix: it was possible to set controls
without them being applied on hardware if the use count was 0 but the
device was still active. I'd consider doing the bugfix separately before
the rest of the patches.

>  		return 0;
>  
>  	switch (ctrl->id) {
> @@ -844,10 +843,18 @@ static int os05b10_set_framing_limits(struct os05b10 *os05b10,
>  	if (ret)
>  		return ret;
>  
> +	ret = __v4l2_ctrl_s_ctrl(os05b10->vblank, vblank);
> +	if (ret)
> +		return ret;
> +
>  	max_exp = mode->vts - OS05B10_EXPOSURE_MARGIN;
> -	return __v4l2_ctrl_modify_range(os05b10->exposure,
> -					OS05B10_EXPOSURE_MIN, max_exp,
> -					OS05B10_EXPOSURE_STEP, mode->exp);
> +	ret = __v4l2_ctrl_modify_range(os05b10->exposure,
> +				       OS05B10_EXPOSURE_MIN, max_exp,
> +				       OS05B10_EXPOSURE_STEP, mode->exp);
> +	if (ret)
> +		return ret;
> +
> +	return __v4l2_ctrl_s_ctrl(os05b10->exposure, mode->exp);
>  }
>  
>  static inline void get_mode_table(unsigned int code,

-- 
Kind regards,

Sakari Ailus