mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] media: i2c: imx415: Use target vblank to calculate vmax in s_ctrl
@ 2026-10-18 12:54 Wenqiang Zhao via B4 Relay
  2026-10-19 14:09 ` Dave Stevenson
  2026-10-19 20:04 ` Michael Riesch
  0 siblings, 2 replies; 4+ messages in thread
From: Wenqiang Zhao via B4 Relay @ 2026-10-18 12:54 UTC (permalink / raw)
  To: Michael Riesch
  Cc: linux-kernel, Sakari Ailus, Dave Stevenson, linux-media,
	Mauro Carvalho Chehab, stable, Wenqiang Zhao

From: Wenqiang Zhao <berringtoni36@gmail.com>

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@vger.kernel.org
Signed-off-by: Wenqiang Zhao <berringtoni36@gmail.com>
---
 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@gmail.com>



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] media: i2c: imx415: Use target vblank to calculate vmax in s_ctrl
  2026-10-18 12:54 [PATCH] media: i2c: imx415: Use target vblank to calculate vmax in s_ctrl Wenqiang Zhao via B4 Relay
@ 2026-10-19 14:09 ` Dave Stevenson
  2026-10-19 20:04 ` Michael Riesch
  1 sibling, 0 replies; 4+ messages in thread
From: Dave Stevenson @ 2026-10-19 14:09 UTC (permalink / raw)
  To: berringtoni36
  Cc: Michael Riesch, linux-kernel, Sakari Ailus, linux-media,
	Mauro Carvalho Chehab, stable

On Sun, 18 Oct 2026 at 13:54, Wenqiang Zhao via B4 Relay
<devnull+berringtoni36.gmail.com@kernel.org> wrote:
>
> From: Wenqiang Zhao <berringtoni36@gmail.com>
>
> 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@vger.kernel.org
> Signed-off-by: Wenqiang Zhao <berringtoni36@gmail.com>

Thanks for the patch.

Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

> ---
>  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@gmail.com>
>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] media: i2c: imx415: Use target vblank to calculate vmax in s_ctrl
  2026-10-18 12:54 [PATCH] media: i2c: imx415: Use target vblank to calculate vmax in s_ctrl Wenqiang Zhao via B4 Relay
  2026-10-19 14:09 ` Dave Stevenson
@ 2026-10-19 20:04 ` Michael Riesch
  2026-10-20  3:53   ` Iron Berrington
  1 sibling, 1 reply; 4+ messages in thread
From: Michael Riesch @ 2026-10-19 20:04 UTC (permalink / raw)
  To: berringtoni36
  Cc: linux-kernel, Sakari Ailus, Dave Stevenson, linux-media,
	Mauro Carvalho Chehab, stable

Hi Wenqiang,

Thanks for the patch!

On 10/18/26 14:54, Wenqiang Zhao via B4 Relay wrote:
> From: Wenqiang Zhao <berringtoni36@gmail.com>
> 
> 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@vger.kernel.org
> Signed-off-by: Wenqiang Zhao <berringtoni36@gmail.com>

LGTM!

Reviewed-by: Michael Riesch <michael.riesch@collabora.com>

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@gmail.com>
> 
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] media: i2c: imx415: Use target vblank to calculate vmax in s_ctrl
  2026-10-19 20:04 ` Michael Riesch
@ 2026-10-20  3:53   ` Iron Berrington
  0 siblings, 0 replies; 4+ messages in thread
From: Iron Berrington @ 2026-10-20  3:53 UTC (permalink / raw)
  To: Michael Riesch
  Cc: Dave Stevenson, Sakari Ailus, Mauro Carvalho Chehab, linux-media,
	linux-kernel, stable

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@collabora.com> wrote:
> Hi Wenqiang,
>
> Thanks for the patch!
>
> On 10/18/26 14:54, Wenqiang Zhao via B4 Relay wrote:
> > From: Wenqiang Zhao <berringtoni36@gmail.com>
> >
> > 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@vger.kernel.org
> > Signed-off-by: Wenqiang Zhao <berringtoni36@gmail.com>
>
> LGTM!
>
> Reviewed-by: Michael Riesch <michael.riesch@collabora.com>
>
> 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@gmail.com>
> >
> >

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-20  3:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-18 12:54 [PATCH] media: i2c: imx415: Use target vblank to calculate vmax in s_ctrl Wenqiang Zhao via B4 Relay
2026-10-19 14:09 ` Dave Stevenson
2026-10-19 20:04 ` Michael Riesch
2026-10-20  3:53   ` Iron Berrington

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®