Re: [PATCH v3 06/22] media: imx219: Don't update exposure limits while setting format

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

Hi Dave,

On Wed, Mar 25, 2026 at 12:37:54PM +0000, Dave Stevenson wrote:
> Hi Sakari
> 
> On Wed, 25 Mar 2026 at 10:58, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> >
> > Don't update exposure limits explicitly while setting format. This is
> > already done through the s_ctrl() callback.
> 
> done through the s_ctrl() callback for V4L2_CID_VBLANK.
> 
> I believe this change leaves a potential for future failure if any
> modes get added because the ctrl handler framework swallows any s_ctrl
> that doesn't change the value.
> vblank is an offset on top of height. exposure is absolute in the
> number of lines.

That's a fair point. I'll postpone doing this patch for now or squash it
with something else. With the Common Raw Sensor Model, the driver
internally uses frame length in lines and line length in pixels so the
problem goes away by itself.

> 
> As an example, take the current mode of 1920x1080 with fll_def 1763,
> therefore a vblank of 1763-1080 = 683, and max exposure of 1080-4 =
> 1076. Add a new mode of 640x480 with fll_def of 1163, therefore vblank
> is 1163-480 = 683, and max exposure is 480-4 = 476. set_pad_format for
> either mode will set vblank to 683. As it's the same value,
> imx219_set_ctrl will not be called to update vblank, leaving the wrong
> max exposure value.
> 
> You could add V4L2_CTRL_FLAG_EXECUTE_ON_WRITE to V4L2_CID_VBLANK to
> always call s_ctrl, but that would have a larger downside if userspace
> repeatedly set it to the same value.

I agree.

> 
> It doesn't affect any of the current modes, but it's going to be a
> very ugly one for the unwary. imx219 is often pointed to as a
> reference for those writing new drivers, so this would likely
> propagate.

I hope no new modes will be added to this driver. The Common Raw Sensor
Model support makes it freely configurable. I hope to send an update on
that soon.

-- 
Regards,

Sakari Ailus