Re: [PATCH v4 05/29] media: imx219: Fix vertical blanking and exposure for analogue binning

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

Hi Jai,

On Wed, Apr 15, 2026 at 08:08:31PM +0530, Jai Luthra wrote:
> Hi Sakari
> 
> Thank you for the patch, and sorry for the late response. I finally got
> time to test this out, there are some significant issues.
> 
> Did you test the behaviour with an IMX219 sensor before and after this
> series?
> 
> What I see is the following before this series:
> 
>     | Mode | HBLANK | VBLANK | Pixel Rate | Observed FPS | Formula FPS | Error
>     % |
>     |---|---:|---:|---:|---:|---:|---:|
>     | 640x480 | 2920 | 1227 | 364800000 | 60.07 | 60.03 | 0.07 |
>     | 640x480 | 4000 | 1227 | 364800000 | 46.09 | 46.06 | 0.07 |
>     | 640x480 | 6000 | 1227 | 364800000 | 32.20 | 32.18 | 0.05 |
>     | 640x480 | 2920 | 2000 | 364800000 | 41.32 | 41.32 | 0.00 |
>     | 640x480 | 2920 | 3000 | 364800000 | 29.45 | 29.45 | 0.01 |
>     | 1640x1232 | 1920 | 475 | 364800000 | 60.07 | 60.03 | 0.07 |
>     | 1640x1232 | 3000 | 475 | 364800000 | 46.09 | 46.06 | 0.07 |
>     | 1640x1232 | 5000 | 475 | 364800000 | 32.20 | 32.18 | 0.05 |
>     | 1640x1232 | 1920 | 1000 | 364800000 | 45.91 | 45.91 | -0.00 |
>     | 1640x1232 | 1920 | 2000 | 364800000 | 31.71 | 31.71 | 0.01 |
>     | 1920x1080 | 1528 | 683 | 182400000 | 30.01 | 30.01 | 0.01 |
>     | 1920x1080 | 2500 | 683 | 182400000 | 23.43 | 23.41 | 0.10 |
>     | 1920x1080 | 4000 | 683 | 182400000 | 17.48 | 17.48 | 0.02 |
>     | 1920x1080 | 1528 | 1200 | 182400000 | 23.20 | 23.20 | -0.01 |
>     | 1920x1080 | 1528 | 2000 | 182400000 | 17.18 | 17.18 | 0.03 |
> 
> 
> And after this series
> 
>     | Mode | HBLANK | VBLANK | Pixel Rate | Observed FPS | Formula FPS | Error
>     % |
>     |---|---:|---:|---:|---:|---:|---:|
>     | 640x480 | 2920 | 1227 | 182400000 | 30.02 | 30.02 | 0.02 |
>     | 640x480 | 4000 | 1227 | 182400000 | 23.03 | 23.03 | 0.00 |
>     | 640x480 | 6000 | 1227 | 182400000 | 16.09 | 16.09 | -0.02 |
>     | 640x480 | 2920 | 2000 | 182400000 | 20.66 | 20.66 | 0.00 |
>     | 640x480 | 2920 | 3000 | 182400000 | 14.72 | 14.72 | -0.02 |
> 
> So while the "formula" still works, we can't achieve high FPS capture we
> could before.

Could you test again with my metadata-pre branch, please?

> 
> With minimum blankings I could get around ~190 FPS capture before. Now it's
> just ~70 FPS.
> 
> And I couldn't even stream the any other mode than 640x480, because S_FMT
> on the sensor gives an error:
> 
> $ media-ctl -V '"imx219 1-0010":0 [fmt:SRGGB10_1X10/1920x1080 field:none]'
> Unable to setup formats: Numerical result out of range (34)
> 
> This patch sets vblank ranges and default values which don't work out:
>    - 1920x1080: min = 1080, default = 683
>    - 1640x1232: min = 616, default = -379
> 
> Quoting Jacopo Mondi (2026-04-10 14:12:22)
> > Hi Sakari
> > 
> > On Wed, Apr 08, 2026 at 06:39:14PM +0300, Sakari Ailus wrote:
> > > When vertical analogue binning is in use, the minimum frame length in
> > > lines decreases to around half of the normal. In relation to the sensor's
> > > output size this means vertical blanking can be negative but that's not an
> > > issue as control values are signed. Remove the workaround for this
> > > non-issue that doubled the pixel rate, frame length in lines and exposure
> > > time.
> > 
> > I don't think this was a workaround. Doubling the pixel rate and
> > halving the values written to registers for EXPOSURE and VBLANK
> > allowed userspace to maintain a consistent view while the driver
> > accounts for the special binning mode where, at least in my latest
> > understanding, the sensor averages two lines before passing them to
> > the ADC.
> > 
> > I am missing in which case, with the current driver implementation,
> > the vertical blanking can be negative.
> > 
> > >
> > > The resulting change also fixes the minimum, the maximum and the step
> > > values for the control.
> > >
> > > Fixes: f513997119f4 ("media: i2c: imx219: Scale the pixel rate for analog binning")
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > 
> > Jai, and others including Dave and me, have spent quite some time
> > testing and implementing proper support for the special analogue
> > binning mode for this sensor. Of course we might have missed something
> > obvious, but I'm still missing what you're trying to fix here.
> > 
> 
> So, I tend to agree with Jacopo here. There's been a lot of effort to get
> this sensor give correct and high FPS frames.

Yet bugs seem to have remained -- see the beginning of the series. Which I
think indeed proves it's just hard to get these things right. The imx219 is
one of the most scrutinised drivers albeit the configurability it offers
still means there may be corner cases.

The metadata series indeed makes the driver freely configurable. I tested
earlier all configurations that have modes appear to be reachable through
the new APIs but not much more than that.

> 
> As we discussed offline, I don't mind the model where we have negative
> values for blankings, but the sensor should still work the same way, i.e.
> the same FPS range is supported with the new blanking and pixel rate model.
> 
> Otherwise that would count as breaking the userspace, especially for
> probably the most widely used sensor out there :/

I agree. And this is why we test and review code, right? :-)

-- 
Kind regards,

Sakari Ailus