Re: [PATCH] media: i2c: imx415: Use target vblank to calculate vmax in s_ctrl

[Iron Berrington]

Hi Michael, hi Dave,

Thank you both for reviewing the patch and for the Reviewed-by tags.

Best regards,
Wenqiang


On Mon, 19 Oct 2026 22:04:36 +0200, Michael Riesch
<michael.riesch@xxxxxxxxxxxxx> wrote:
> Hi Wenqiang,
>
> Thanks for the patch!
>
> On 10/18/26 14:54, Wenqiang Zhao via B4 Relay wrote:
> > From: Wenqiang Zhao <berringtoni36@xxxxxxxxx>
> >
> > When setting V4L2_CID_VBLANK, imx415_s_ctrl() writes the new frame length
> > (format->height + ctrl->val) to the IMX415_VMAX register, and then falls
> > through to case V4L2_CID_EXPOSURE to update the electronic shutter register
> > (IMX415_SHR0 = vmax - exposure) so that exposure is preserved across frame
> > rate changes.
> >
> > However, in case V4L2_CID_EXPOSURE, vmax is calculated as:
> >     vmax = format->height + sensor->vblank->cur.val;
> >
> > Within the V4L2 control framework, sensor->vblank->cur.val holds the
> > committed old vertical blanking value, which is not updated to the new
> > value until s_ctrl() returns 0. Consequently, when VBLANK changes, vmax
> > is calculated from the stale vblank value:
> > - If VBLANK increases, SHR0 is computed using the smaller old VMAX, causing
> >   the actual hardware exposure (VMAX - SHR0) to jump by the blanking delta.
> > - If VBLANK decreases, SHR0 is computed using the larger old VMAX, causing
> >   the actual hardware exposure to drop by the blanking delta.
> > - Additionally, ctrl->val is clamped against the stale vmax limit.
> >
> > Fix this by using sensor->vblank->val instead of sensor->vblank->cur.val,
> > which holds the target vertical blanking value currently being applied.
> > When setting V4L2_CID_EXPOSURE directly, sensor->vblank->val equals the
> > current vblank, keeping behavior unchanged.
> >
> > Fixes: 3bcae55ab96a ("media: i2c: imx415: Add read/write control of VBLANK")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Wenqiang Zhao <berringtoni36@xxxxxxxxx>
>
> LGTM!
>
> Reviewed-by: Michael Riesch <michael.riesch@xxxxxxxxxxxxx>
>
> Best regards,
> Michael
>
> > ---
> >  drivers/media/i2c/imx415.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/i2c/imx415.c b/drivers/media/i2c/imx415.c
> > index c3b22b238..e5aa741fc 100644
> > --- a/drivers/media/i2c/imx415.c
> > +++ b/drivers/media/i2c/imx415.c
> > @@ -729,7 +729,7 @@ static int imx415_s_ctrl(struct v4l2_ctrl *ctrl)
> >  		fallthrough;
> >  	case V4L2_CID_EXPOSURE:
> >  		/* clamp the exposure value to VMAX. */
> > -		vmax = format->height + sensor->vblank->cur.val;
> > +		vmax = format->height + sensor->vblank->val;
> >  		ctrl->val = min_t(int, ctrl->val, vmax);
> >  		ret = cci_write(sensor->regmap, IMX415_SHR0,
> >  				vmax - ctrl->val, NULL);
> >
> > ---
> > base-commit: a5ebb76233b79db01e82e061173cb73c6d2b5c6b
> > change-id: 20261018-fix-imx415-vblank-exposure-sync-20261018-967b008cf3fb
> >
> > Best regards,
> > --
> > Wenqiang Zhao <berringtoni36@xxxxxxxxx>
> >
> >