Hi Bingbu,
On Tue, Oct 19, 2021 at 03:58:41PM +0000, Cao, Bingbu wrote:
> > -----Original Message-----
> > From: Cao, Bingbu
> > Sent: Tuesday, October 19, 2021 11:30 PM
> > To: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > Cc: linux-media@xxxxxxxxxxxxxxx; tfiga@xxxxxxxxxxxx;
> > kieran.bingham@xxxxxxxxxxxxxxxx; bingbu.cao@xxxxxxxxxxxxxxx
> > Subject: RE: [PATCH v3] media: imx258: add vblank control to support more
> > frame rate range
> >
> > Sakari,
> >
> > ________________________
> > BRs,
> > Bingbu Cao
> >
> > > -----Original Message-----
> > > From: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > > Sent: Tuesday, October 19, 2021 8:52 PM
> > > To: Cao, Bingbu <bingbu.cao@xxxxxxxxx>
> > > Cc: linux-media@xxxxxxxxxxxxxxx; tfiga@xxxxxxxxxxxx;
> > > kieran.bingham@xxxxxxxxxxxxxxxx; bingbu.cao@xxxxxxxxxxxxxxx
> > > Subject: Re: [PATCH v3] media: imx258: add vblank control to support
> > > more frame rate range
> > >
> > > Hi Bingbu,
> > >
> > > On Mon, Oct 18, 2021 at 11:26:16AM +0800, Bingbu Cao wrote:
> > > > Current imx258 driver enable the automatic frame length tracking
> > > > control by default and did not support VBLANK change, it's always
> > > working at 30fps.
> > > > However, in reality we need a wider frame rate range from 15 to 30.
> > > > This patch disable the automatic frame length tracking control and
> > > > enable the v4l2 VBLANK control to allow user changing frame rate per
> > > requirement.
> > > >
> > > > Signed-off-by: Bingbu Cao <bingbu.cao@xxxxxxxxx>
> > > > ---
> > > > drivers/media/i2c/imx258.c | 23 +++++++++++++++++------
> > > > 1 file changed, 17 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> > > > index 81cdf37216ca..2c787af7074d 100644
> > > > --- a/drivers/media/i2c/imx258.c
> > > > +++ b/drivers/media/i2c/imx258.c
> > > > @@ -29,6 +29,7 @@
> > > > #define IMX258_VTS_MAX 0xffff
> > > >
> > > > /*Frame Length Line*/
> > > > +#define IMX258_REG_FLL 0x0340
> > > > #define IMX258_FLL_MIN 0x08a6
> > > > #define IMX258_FLL_MAX 0xffff
> > > > #define IMX258_FLL_STEP 1
> > > > @@ -241,7 +242,7 @@ static const struct imx258_reg
> > > > mode_4208x3118_regs[]
> > > = {
> > > > { 0x034D, 0x70 },
> > > > { 0x034E, 0x0C },
> > > > { 0x034F, 0x30 },
> > > > - { 0x0350, 0x01 },
> > > > + { 0x0350, 0x00 }, /* no frame length automatic tracking control */
> > > > { 0x0202, 0x0C },
> > > > { 0x0203, 0x46 },
> > > > { 0x0204, 0x00 },
> > > > @@ -360,7 +361,7 @@ static const struct imx258_reg
> > > > mode_2104_1560_regs[]
> > > = {
> > > > { 0x034D, 0x38 },
> > > > { 0x034E, 0x06 },
> > > > { 0x034F, 0x18 },
> > > > - { 0x0350, 0x01 },
> > > > + { 0x0350, 0x00 }, /* no frame length automatic tracking control */
> > > > { 0x0202, 0x06 },
> > > > { 0x0203, 0x2E },
> > > > { 0x0204, 0x00 },
> > > > @@ -479,7 +480,7 @@ static const struct imx258_reg
> > > > mode_1048_780_regs[]
> > > = {
> > > > { 0x034D, 0x18 },
> > > > { 0x034E, 0x03 },
> > > > { 0x034F, 0x0C },
> > > > - { 0x0350, 0x01 },
> > > > + { 0x0350, 0x00 }, /* no frame length automatic tracking control */
> > >
> > > Why is automatic frame length control disabled?
> >
> > My understanding:
> > If automatic frame length control enabled, the frame length is changed
> > automatically when COARSE_INTEGRATE_TIME + 10 > FRAME_LENGTH_LINES, it
> > may not meet the requirement - less integrate time with more frame length.
> > we need control the vertical blank to do that.
> >
>
> If frame length automatic tracking control enabled, the CORSE_INTEGRATE_TIME
> could be larger than FRAME_LENGTH_LINES.
Both are controlled by the driver. The driver is generally responsible for
ensuring the exposure time stays within the limits for a given frame
length.
Unless this sensor does something weird, all you get by disabling this is
undefined behaviour instead of increased frame length when the exposure
time + margin exceeds frame length. This could mean broken frames.
Of course, it takes a driver bug to arrive into this situation.
--
Kind regards,
Sakari Ailus
