Re: [PATCH v2 09/12] media: i2c: cap exposure at height + vblank in ov8865

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Daniel,

On Mon, Aug 09, 2021 at 11:58:42PM +0100, Daniel Scally wrote:
> Exposure limits depend on the total height; when vblank is altered (and
> thus the total height is altered), change the exposure limits to reflect
> the new cap.
> 
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> Changes in v2:
> 
> 	- None
> 
>  drivers/media/i2c/ov8865.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
> index db84294b7a03..70747552e32a 100644
> --- a/drivers/media/i2c/ov8865.c
> +++ b/drivers/media/i2c/ov8865.c
> @@ -675,6 +675,7 @@ struct ov8865_ctrls {
>  	struct v4l2_ctrl *pixel_rate;
>  	struct v4l2_ctrl *hblank;
>  	struct v4l2_ctrl *vblank;
> +	struct v4l2_ctrl *exposure;
>  
>  	struct v4l2_ctrl_handler handler;
>  };
> @@ -2461,6 +2462,18 @@ static int ov8865_s_ctrl(struct v4l2_ctrl *ctrl)
>  	unsigned int index;
>  	int ret;
>  
> +	/* If VBLANK is altered we need to update exposure to compensate */
> +	if (ctrl->id == V4L2_CID_VBLANK) {
> +		int exposure_max;
> +
> +		exposure_max = sensor->state.mode->output_size_y + ctrl->val;
> +		__v4l2_ctrl_modify_range(sensor->ctrls.exposure,
> +					 sensor->ctrls.exposure->minimum,
> +					 exposure_max,
> +					 sensor->ctrls.exposure->step,
> +					 min(sensor->ctrls.exposure->val, exposure_max));
> +	}
> +
>  	/* Wait for the sensor to be on before setting controls. */
>  	if (pm_runtime_suspended(sensor->dev))
>  		return 0;
> @@ -2517,8 +2530,8 @@ static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
>  
>  	/* Exposure */
>  
> -	v4l2_ctrl_new_std(handler, ops, V4L2_CID_EXPOSURE, 16, 1048575, 16,
> -			  512);
> +	ctrls->exposure = v4l2_ctrl_new_std(handler, ops, V4L2_CID_EXPOSURE, 16,
> +					    1048575, 16, 512);
>  
>  	/* Gain */
>  
> @@ -2699,6 +2712,7 @@ static int ov8865_set_fmt(struct v4l2_subdev *subdev,
>  	u32 mbus_code = 0;
>  	unsigned int hblank;
>  	unsigned int index;
> +	int exposure_max;
>  	int ret = 0;
>  
>  	mutex_lock(&sensor->mutex);
> @@ -2746,6 +2760,12 @@ static int ov8865_set_fmt(struct v4l2_subdev *subdev,
>  	__v4l2_ctrl_modify_range(sensor->ctrls.hblank, hblank, hblank, 1,
>  				 hblank);
>  
> +	exposure_max = mode->vts;
> +	__v4l2_ctrl_modify_range(sensor->ctrls.exposure,
> +				 sensor->ctrls.exposure->minimum, exposure_max,
> +				 sensor->ctrls.exposure->step,
> +				 min(sensor->ctrls.exposure->val, exposure_max));

Please wrap lines over 80 (unless there's a sound reason not to).

> +
>  complete:
>  	mutex_unlock(&sensor->mutex);
>  

-- 
Sakari Ailus