Re: [PATCH v2 10/20] media: imx355: Remove redundant fll_min, and implement fixed offset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] The V4L2_CID_EXPOSURE control's maximum limit is not reliably updated when the sensor mode changes, causing incorrect exposure limits to be exposed to userspace.
--

--- Patch [10]: [PATCH v2 10/20] media: imx355: Remove redundant fll_min, and implement fixed offset ---
commit bafcea33e8f344729e8a8a61b9272b5555ee1f02
Author: Dave Stevenson <dave.stevenson@raspberrypi.com>

media: imx355: Remove redundant fll_min, and implement fixed offset

The patch removes the redundant fll_min from imx355_mode configurations.
It sets the vblank control minimum to a fixed 20 lines to improve the
maximum achievable frame rate.

> diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
> index 9d977bac23b6c..354092295b7c3 100644
> --- a/drivers/media/i2c/imx355.c
> +++ b/drivers/media/i2c/imx355.c

[ ... ]

> @@ -888,10 +873,9 @@ imx355_set_pad_format(struct v4l2_subdev *sd,
>  		/* Update limits and set FPS to default */
>  		height = imx355->cur_mode->height;
>  		vblank_def = imx355->cur_mode->fll_def - height;
> -		vblank_min = imx355->cur_mode->fll_min - height;
>  		height = IMX355_FLL_MAX - height;
> -		__v4l2_ctrl_modify_range(imx355->vblank, vblank_min, height, 1,
> -					 vblank_def);
> +		__v4l2_ctrl_modify_range(imx355->vblank, IMX355_VBLANK_MIN,
> +					 height, 1, vblank_def);
>  		__v4l2_ctrl_s_ctrl(imx355->vblank, vblank_def);

[Severity: Medium]
This isn't a bug introduced by this patch, but does this skip updating the 
EXPOSURE control's maximum limit if the new default VBLANK matches the 
current VBLANK value?

If userspace sets VBLANK to a value (e.g., 151 in Mode 3) and then switches 
to a new mode (e.g., Mode 0) where the new default VBLANK exactly matches 
the current VBLANK value, the V4L2 core optimization will skip calling the 
s_ctrl callback.

Since the mode's height changed, the EXPOSURE limits must be recalculated.
Because the s_ctrl callback (imx355_set_ctrl) is skipped, does this cause 
incorrect exposure limits to be exposed to userspace until streaming starts?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-media-imx355-v2-0-1683ec07b897@raspberrypi.com?part=10