Re: [PATCH v5 06/10] media: imx219: Fix vertical blanking and exposure for analogue binning

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

Hi Jai,

On Mon, Jun 08, 2026 at 04:49:38PM +0530, Jai Luthra wrote:
> Quoting Jai Luthra (2026-06-08 16:01:06)
> > Hi Jacopo, Sakari,
> > ++ Dave, Hans and Laurent,
> > 
> > Quoting Jacopo Mondi (2026-06-08 12:28:46)
> > > Hi Sakari
> > > 
> > > On Mon, Jun 08, 2026 at 12:53:52AM +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
> > > 
> > > Didn't we just discussed two weeks ago in media summit how negative
> > > blankings are a bad idea, and of all drivers one could decide to play
> > > with imx219 is probably the worse due it's large use base and the fact
> > > libcamera doesn't support negative blankings ?
> > > 
> > > Have I missed something ?
> > > 
> > 
> > I think it would be helpful if I write down clearly how this sensor
> > operates (to the best of my knowledge) so we can decide on the correct
> > fix:
> > 
> > --------------------
> > 
> > IMX219 sensor has an active resolution of 3280x2464.
> > 
> > The driver currently programs the sensor VT pixel clock as fixed for a
> > given lane configuration.
> > 
> > In 2-lane mode it reads 182.4 MPixel/second:
> > 
> >     #define IMX219_PIXEL_RATE           182400000
> > 
> > And the framerate is given by:
> > 
> >     PIXEL_RATE / (FRAME_LENGTH * LINE_LENGTH)
> > 
> > where FRAME_LENGTH and LINE_LENGTH are registers that include the active
> > height and width along with blankings.
> > 
> > There are restrictions on the minimum of the LINE_LENGTH register and
> > minimum vertical blanking (32), which cap the framerate for the full resolution
> > mode.
> > 
> >     MIN_LINE_LENGTH: 0xd78 => 3448 pixels
> >     MIN_FRAME_LENGTH: ACTIVE_HEIGHT + MIN_VBLANK (32) = 2464 + 32
> >                            => 2496 lines
> > 
> > The maximum frame rate is
> > 
> >     182400000/(2496*3448) => ~ 21.2 frames/second
> > 
> > --------------------
> > 
> > A user might want to stream a lower resolution with the full field-of-view,
> > let's take 1640x1232 (which is exactly 1/2 of active area) as an example.
> > 
> > The sensor hardware can achieve this using two different binning modes:
> > 
> >     2x2-binning (regval: 0x1)
> >     2x2-analog-(special)-binning (regval: 0x3)
> > 
> > The sensor pipeline looks like:
> > 
> > active pixel array ->
> >     analogue crop (none) ->
> >         2x2 binning and ADC readout ->
> >                 output to CSI-2 bus
> > 
> > The mode names suggest that binning can happen either before or after ADC,
> > but the datasheet is not very clear about the process. We can infer
> > some details though. See below..
> > 
> > --------------------
> > 
> > With the "normal" 2x2-binning mode the sensor allows programming
> > FRAME_LENGTH to a lower value. The driver still uses the min blanking of 32
> > lines, but the height is now 1232, half of the 2464 before.
> > 
> >     MIN_LINE_LENGTH: 0xd78 => 3448 pixels
> >     MIN_FRAME_LENGTH: READOUT_HEIGHT + MIN_VBLANK = 1232 + 32
> >                            => 1264 lines
> > 
> > The maximum frame rate is
> > 
> >     182400000/(1264*3448) => ~ 41.8 frames/second
> > 
> > --------------------
> > 
> > With the "special" 2x2-binning mode, the datasheet notes that FRAME_LENGTH
> > register should be in units of 2 Lines instead of 1 Line. This means
> > cutting it down by half once more:
> > 
> >     MIN_FRAME_LENGTH: (READOUT_HEIGHT + MIN_VBLANK)/2 = (1232 + 32)/2
> >                            => 632 lines
> > 
> > While there is a slightly higher minimum enforced for the line length:
> > 
> >     MIN_LINE_LENGTH: 0xde8 => 3560 pixels
> > 
> > The maximum frame rate is
> > 
> >     182400000/(632*3560) => ~ 81.0 frames/second
> > 
> > ===================
> > 
> > Through the minimum allowed values of the FRAME_LENGTH and LINE_LENGTH
> > registers and maximum possible framerate, I think it is safe to say that:
> > 
> >     2x2-binning => Readout half the pixels (do vertical averaging in the
> >                                             analogue domain, before ADC
> >                                             reads out the voltages)
> > 
> >     2x2-special-binning => Readout a quarter of the pixels (???)
> > 
> > FRAME_LENGTH being 1/4th of normal would seem to suggest that it is a 4x1
> > binning (combining 4 lines instead of blocks of 2x2).. which does not make
> > sense to me.
> > 
> > My best guess is that in 2x2-special-binning mode the sensor does *both
> > horizontal and vertical* averaging in the analogue domain, before the ADC
> > reads out the voltages.
> > 
> > A higher minimum LINE_LENGTH value for this mode is the best "hard"
> > evidence I have for this guess unfortunately, as the datasheet is quite
> > lacking on this topic.
> > 
> 
> Found another evidence for this, see "Table 16 Mode Example" in the
> datasheet.
> 
> It mentions "H Binning = Analog" for x2 binning mode (with ~4x the FPS)
> And "H Binning = Digital" for x4 binning mode
> 
> It doesn't mention a ~2x FPS mode, which I assume is the normal 2x2-binning
> mode, where I again assume that horizontal binning is done digitally.

Most likely 4x vertical binning uses a combination of analogue and digital
binning. The type of the binning in general doesn't matter much, apart from
affecting the minimum line length in pixels and frame length in lines
values.

> 
> 
> > ====================
> > 
> > The APIs before Sakari's series expose HBLANK and VBLANK controls instead
> > of the actual FRAME_LENGTH/LINE_LENGTH registers to the userspace.
> > 
> > The minimum value of FRAME_LENGTH is 632 when we are streaming 1640x1232,
> > so the sensor registers would suggest that we have a *negative vertical
> > blanking*. Which as Jacopo and Laurent both point out, does not make any
> > conceptual sense whatsoever.
> > 
> > What the driver does today to avoid these negative values is to double the
> > PIXEL_RATE control value to 364800000 when using 2x2-special-binning mode
> > (and thus userspace has no idea the FRAME_LENGTH/VBLANK is in units of
> >  2xLines)
> > 
> > As Sakari pointed out, that does not make any sense either. The sensor PLL
> > values are completely unchanged, so the pixel readout must still be
> > happening at the same rate. Moreover, the new raw sensor model will expose
> > FRAME_LENGTH and LINE_LENGTH directly to userspace, and this hack of
> > doubling the PIXEL_RATE breaks those calculations.
> > 
> > ===================
> > 
> > If we want to support the new raw sensor model (that mandates the new
> > FRAME_LENGTH and LINE_LENGTH controls) for this sensor we have to
> > fix the PIXEL_RATE for sure. I see two options going forward:
> > 
> > OPTION 1 (as proposed by Sakari):
> > 
> >     Fix PIXEL_RATE to 182400000 and allow **negative values** for HBLANK
> >     and VBLANK controls when using 2x2-special-binning mode.
> > 
> >     This will break any userspace tools, many libcamera pipelines included,
> >     that never expected those control values to be negative (even though
> >     the API has always permitted those)

That's a clear bug, but it only becomes apparent when the sign bit is set.

> > 
> > OPTION 2 (something that struck me today morning discussing with Jacopo):
> >     
> >     Fix PIXEL_RATE to 182400000 but **adjust the HBLANK values** to go
> >     lower to compensate, which will diverge from the sensor registers which
> >     keep MIN_LINE_LENGTH fixed across both binning modes.
> > 
> >     This will make the driver quite more complicated, but userspace
> >     expectations of non-negative blankings will be met. And it's likely
> >     that the sensor is internally doing pre-ADC averaging horizontally as
> >     well, or so my best guess is.
> > 
> 
> Which makes me lean more on OPTION 2 now.
> 
> In our case above with 2x2-special-binning mode for 1640x1232, the register
> values are:
> 
>     LINE_LENGTH = 3560 pixels
>     FRAME_LENGTH = 632 lines
> 
> 
> So OPTION 1 would give us:
> 
>     HBLANK = 1920, VBLANK = -600
> 
> When actually the datasheet's "H Binning = Analog" would suggest to me that
> reality looks more like:
> 
>     LINE_LENGTH = 1780 pixels
>     FRAME_LENGTH = 1264 lines
> 
> So OPTION 2 would give us:
> 
>     HBLANK = 140, VBLANK = 32

This approach has the downsides that 1) it doesn't reflect what the sensor
apparently does and 2) you lose one bit of granularity on line length in
pixels.

The advantage still is that it works around the sign bit issue. The new
controls still have their proper values but conversion between the two
becomes rather complicated. See
<URL:https://git.linuxtv.org/sailus/media_tree.git/commit/?h=metadata&id=366cb25c7d944ef2935668a07471e4576e7088ed>
for instance how it looks like without that.

> 
> > Of course, there are other options to just leave this highly used sensor
> > alone, or support embedded data and internal pads without mandating the new
> > FRAME_LENGTH and LINE_LENGTH controls. But I personally would leave that as
> > a last resort.

I agree. Especially because of its wide user base, it'd be really nice to
convert the imx219 to the Common Raw Sensor Model.

-- 
Regards,

Sakari Ailus