Re: [PATCH 1/1] media: Documentation: Improve PIXEL_RATE control documentation

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

Hi Jacopo,

On Wed, Apr 15, 2026 at 03:35:39PM +0200, Jacopo Mondi wrote:
> Hi Sakari
> 
> On Tue, Apr 14, 2026 at 10:00:08PM +0300, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Tue, Apr 14, 2026 at 05:46:51PM +0200, Jacopo Mondi wrote:
> > > Hi Sakari
> > >
> > > On Tue, Apr 14, 2026 at 01:35:59PM +0300, Sakari Ailus wrote:
> > > > Document explicitly that the PIXEL_RATE control reflects the actual
> > > > frequency at which the pixels are read in the pixel array. It is thus
> > > > orthogonal to analogue binning.
> > > >
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > ---
> > > >  .../userspace-api/media/v4l/ext-ctrls-image-process.rst      | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-image-process.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-image-process.rst
> > > > index 6d516f041ca2..8616bcd67270 100644
> > > > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-image-process.rst
> > > > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-image-process.rst
> > > > @@ -41,6 +41,11 @@ Image Process Control IDs
> > > >      The configuration of the frame rate is performed by selecting the desired
> > > >      horizontal and vertical blanking. The unit of this control is Hz.
> > > >
> > > > +    Note that this control isn't affected by analogue binning as the pixels are
> > > > +    still being read at the same frequency as without analogue binning, only
> > > > +    what is being read is different (a single pixel value vs. a binned pixel
> > > > +    value based on the values of two or more pixels).
> > > > +
> > >
> > > This breaks the model implemented to support "special" (aka analogue
> > > ?) binning mode on imx219 where we double the pixel rate to express an
> > > higher frame rate and halve the blanking values before writing them to
> > > registers
> >
> > Note that this patch is a clarification to the existing documentation, not
> > a change as such. It was never expected to be used as it is currently in
> > the imx219 driver.
> >
> > >
> > > I guess we have to keep this mode working not to break existing
> > > userspace, also because it's the only way we can have it working
> > > without the introduction of the FLL and LLP controls.
> >
> > The other option is indeed to leave the HBLANK as-is but based on a
> > discussion with Jai I decided to write a patch to remove the rate
> > multiplier. Overall I'd say it'd be better to change this now rather than
> > leave imx219 using a different interface behaviour than the rest.
> >
> 
> imx219 is PiCamera module v2, it's very popular, I would be careful
> changing it. If I'm not mistaken (please confirm) your series now
> require userspace to program FLL and LLP in units of two lines when
> using binning. What would happen to existing userspace with the new
> version ?

The unit of LLP and FLL are pixels and lines, respectively. Please see my
other reply <aeaVsZQVHsqlOtGP@kekkonen.localdomain>. This patch isn't
directly related to the imx219 driver, the purpose here was to define it in
a way that left no room for interpretation.

The imx219 driver is a different discussion as it does affect an existing
UAPI.

> 
> Again, I would defer it to Dave as the maintainer.
> 
> > >
> > > With the new model and FLL and LLP controls, I presume analog binning
> > > would require userspace to program halved FLL and LLP values ?
> >
> > The userspace would in fact use LLP and FLL as the sensor does, there's no
> > need to divide or multiply these values with another variable.
> >
> > >
> > > Currently in your proposal of V4L2_CID_BINNING_FACTORS it doesn't seem
> > > there is a way to distinguish between  a digital and an analogue
> > > binning mode, they're both 2x2 so I guess it's the driver that has to
> > > realize that if FLL and LLP are smaller the [output width + blank] it
> > > has to activate the analogue binning mode ?
> >
> > The driver is indeed responsible for setting the limits for FLL and LLP (or
> > blanking) controls depending on binning and analogue cropping
> > configuration, a sensor supporting analogue binning is not a special case
> > as such.
> 
> Maybe I wasn't clear
> 
> The imx219 supports 2 different 2x2 binning modes
> 
> #define IMX219_BINNING_X2		0x01
> #define IMX219_BINNING_X2_ANALOG	0x03
> 
> We currently use X2_ANALOG unconditionally, and your patches do not
> change this, I agree.
> 
> My thinking was in general, is there a need to distinguish between a
> 2x2 digital binning and a 2x2 analog binning ? Does the new API allow
> to do so ?

I think we'd need a new control to do that, if we needed to tell them
apart. I'm fairly certain we'll never need the digital option.

-- 
Kind regards,

Sakari Ailus