Message-ID: <5mpch5lokeoxbglc6n3gfugluzluguakv5udt6lflkn5qv23pk@kcllmxh5vqxm>
Date: Thu, 21 Nov 2024 17:10:54 +0530
From: Jai Luthra <jai.luthra@...asonboard.com>
To: Sakari Ailus <sakari.ailus@...ux.intel.com>, 
	Dave Stevenson <dave.stevenson@...pberrypi.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>, 
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] media: i2c: imx219: make HBLANK r/w to allow longer
 exposures

Hi Sakari, Dave,

On Nov 12, 2024 at 10:49:24 +0000, Sakari Ailus wrote:
> Hi Dave,
> 
> On Mon, Nov 11, 2024 at 07:37:56PM +0000, Dave Stevenson wrote:
> > Hi Sakari
> > 
> > On Fri, 8 Nov 2024 at 10:30, Sakari Ailus <sakari.ailus@...ux.intel.com> wrote:
> > >
> > > Hi Dave,
> > >
> > > On Thu, Nov 07, 2024 at 12:43:52PM +0000, Dave Stevenson wrote:
> > > > Hi Sakari
> > > >
> > > > On Fri, 1 Nov 2024 at 08:48, Sakari Ailus <sakari.ailus@...ux.intel.com> wrote:
> > > > >
> > > > > Hi Jai,
> > > > >
> > > > > On Tue, Oct 29, 2024 at 02:27:36PM +0530, Jai Luthra wrote:
> > > > > > From: Dave Stevenson <dave.stevenson@...pberrypi.com>
> > > > > >
> > > > > > The HBLANK control was read-only, and always configured such
> > > > > > that the sensor HTS register was 3448. This limited the maximum
> > > > > > exposure time that could be achieved to around 1.26 secs.
> > > > > >
> > > > > > Make HBLANK read/write so that the line time can be extended,
> > > > > > and thereby allow longer exposures (and slower frame rates).
> > > > > > Retain the overall HTS setting when changing modes rather than
> > > > > > resetting it to a default.
> > > > >
> > > > > It looks like this changes horizontal blanking at least in some cases. Does
> > > > > this also work as expected in binned modes, for instance?
> > > > >
> > > > > Many sensors have image quality related issues on untested albeit
> > > > > functional line length values.
> > > > >
> > > > > So my question is: how has this been validated?
> > > >
> > > > Validated by Sony, or others?
> > > > I've tested a range of values in all modes and not observed any image
> > > > quality issues.
> > >
> > > Somehow at least. :-)
> > >
> > > >
> > > > From previous discussions with Sony, they always provide their big
> > > > spreadsheet of register values for the specific mode and frame rate
> > > > requested. I don't think they even officially state that changing
> > > > VTS/FRM_LENGTH_LINES to change the framerate is permitted.
> > > > There are some Sony datasheets (eg imx258) that state "set to X. Any
> > > > other value please confirm with Sony", but that isn't the case for the
> > > > imx219 datasheet. I take that as it is permitted within the defined
> > > > ranges.
> > >
> > > I'm not that much concerned of vertical blanking, changing that within the
> > > valid range has effects on the image itself very seldom. Horizontal
> > > blanking is different though and this is what the patch makes changeable,
> > > including a change in the default value. Of course there are big
> > > differences between sensors here.
> > 
> > The intention was that the default value shouldn't change, and as the
> > overall PIXELS_PER_LINE value was meant to be retained on a mode
> > change the value used should only change if an application changes
> > V4L2_CID_HBLANK. If I blundered in the implementation of that, then
> > that should be fixed (I know Jacopo made comments, but I haven't had a
> > chance to investigate).
> 
> I guess I misread the patch. It indeed should be the same.
> 
> > 
> > I doubt we'd get validation from Sony beyond the contents of the
> > datasheet. Potentially as the sensor is so old they don't have the
> > information or engineers involved.
> > I'm happy to set up a test system and capture a set of images with
> > HBLANK from min to max at some increment. With the same exposure and
> > gain they should all be identical as long as there isn't any movement
> > (rolling shutter with longer readout times and all that). Would that
> > be satisfactory?
> 
> Sounds good to me. I just thought how it actually had been tested. :-)
> 

While not a thorough test, I manually tested with different values for 
horizontal_blanking for both binned and non-binned modes, and the image 
quality looked okay, with expected behaviour (i.e. increase in exposure 
and decrease in frame rate as total pixels per line increase)

I will send a v2 of this series with all the fixes.

> > 
> > For contrast, the IMX290 datasheet states that VMAX shall be fixed at
> > 0x465 for all-pixel mode / 0x2ee for 720p mode, and HMAX should be
> > changed for frame rate control. As you say, sensors differ.
> 
> -- 
> Kind regards,
> 
> Sakari Ailus

-- 
Thanks,
Jai

GPG Fingerprint: 4DE0 D818 E5D5 75E8 D45A AFC5 43DE 91F9 249A 7145

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists