linux-kernel - Re: [PATCH 3/6] media: imx335: Update the native pixel array width

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <175558758114.8696.4076428044281728141@freya>
Date: Tue, 19 Aug 2025 12:43:01 +0530
From: Jai Luthra <jai.luthra@...asonboard.com>
To: Kieran Bingham <kieran.bingham@...asonboard.com>, Mauro Carvalho Chehab <mchehab@...nel.org>, Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/6] media: imx335: Update the native pixel array width

Quoting Kieran Bingham (2025-08-13 15:45:59)
> Quoting Jai Luthra (2025-08-13 08:20:34)
> > The sensor datasheet reports actual total number of pixels as 2696x2044.
> 
> Err ...
> 
> Page 2 of the IMX335LQN-D datasheet I have says:
> 
> Total number of pixels: 2704 (H) x 2104 (V) approx 5.69 M pixels
> Number of Effective Pixels: 2616 (H) x 1964 (V) approx 5.14 M pixels
> Number of Active Pixels: 2616 (H) x 1964 (V) approx 5.04 M pixels
> 
> Where does 2696x2044 come from ?
> 
> 
> Argh - then on page 8 - indeed it says
> Total Number of pixels 2696(H) x 2044(V) = 5.51M
> 

Yeah, I don't know which one to use, but the page 8 seems "safer".

> 
> In imx283.c I've moved to defining these windows as a v4l2_rect. I find
> that's a nicer way to convey the rectangles specifically instead of
> through #defines and then they can be directly used to report crop
> rectangles:
> 
> i.e.:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/plain/drivers/media/i2c/imx283.c:
> 
> /* IMX283 native and active pixel array size. */
> static const struct v4l2_rect imx283_native_area = {
>         .top = 0,
>         .left = 0,
>         .width = 5592,
>         .height = 3710,
> };
> 
> static const struct v4l2_rect imx283_active_area = {
>         .top = 40,
>         .left = 108,
>         .width = 5472,
>         .height = 3648,
> };
> 
> Not required - but just an idea (that obviously I like :D)
> 

Ah that is indeed quite nice, will do the same in v2.

> 
> > 
> > This becomes important for supporting 2x2 binning modes that can go
> > beyond the current maximum pixel array width set here.
> > 
> > Signed-off-by: Jai Luthra <jai.luthra@...asonboard.com>
> > ---
> >  drivers/media/i2c/imx335.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c
> > index 6369bdbd2b09ba1f89c143cdf6be061820f2d051..dbf2db4bf9cbfd792ff5865264c6f465eb79a43b 100644
> > --- a/drivers/media/i2c/imx335.c
> > +++ b/drivers/media/i2c/imx335.c
> > @@ -124,10 +124,10 @@
> >  #define IMX335_NUM_DATA_LANES  4
> >  
> >  /* IMX335 native and active pixel array size. */
> > -#define IMX335_NATIVE_WIDTH            2616U
> > -#define IMX335_NATIVE_HEIGHT           1964U
> > -#define IMX335_PIXEL_ARRAY_LEFT                12U
> > -#define IMX335_PIXEL_ARRAY_TOP         12U
> > +#define IMX335_NATIVE_WIDTH            2696U
> 
> The all scan mode on page 56 doesn't show these at all.
> Just 12 + 2592 + 12
> 
> But I think that's the datasheet being inconsistent/restrictive about
> what it says an all scan mode could be.
> 
> It would be interesting to make a 'super resolution' output mode that
> can transmit every pixel possible but not required for this series
> development just for fun tests.
> 

Yeah I think datasheet is being quite restrictive, and even the +12 on both
sides for "color processing" makes it very difficult to parse what the
actual resolution is on the wire, and what they expect to be used post ISP
processing.

The all-pixel scan mode table given in the datasheet is actually different
from what we actually use (we use WINMODE=4 i.e. cropped) for the 2592x1944
mode in driver, as the datasheet settings would give us 2592+24 = 2616
pixels per line.

I didn't want to change the existing mode in this series though. Making the
driver freely-configurable will fix all of this (hopefully).

> I'm torn here - as the datasheet changes it's points of reference to
> make it's "all scan mode" essentially start at 0 which is clearly not
> correct against the 'native' positions.
> 
> So ... I think I'm just going to say I think we don't believe the
> datasheet as we *know* there are more pixels and we are using them so :
> 
> 
> > +#define IMX335_NATIVE_HEIGHT           2044U
> > +#define IMX335_PIXEL_ARRAY_LEFT                52U
> > +#define IMX335_PIXEL_ARRAY_TOP         50U
> 
> I see you have taken the '80' extra pixels on both width and height and
> divided half before and half after centering the window. I have no
> information to say if it's position is otherwise so I think this is all
> we can do:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@...asonboard.com>
> 
> >  #define IMX335_PIXEL_ARRAY_WIDTH       2592U
> >  #define IMX335_PIXEL_ARRAY_HEIGHT      1944U
> >  
> > 
> > -- 
> > 2.50.1
> >

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