imx290 HCG and analogue gain updates #5859
Conversation
|
Hit the wrong button and created the PR before I'd described it :-( For testing on https://forums.raspberrypi.com/viewtopic.php?t=363696 |
popcornmix
force-pushed
the
rpi-6.6.y
branch
2 times, most recently
from
6036a96 to
aa41065
Compare
6by9
force-pushed
the
rpi-6.6.y-cams
branch
2 times, most recently
from
1cbf915 to
c890df2
Compare
|
Hi 6by9, I'm wondering if you could also add IMX462 to the driver:
|
|
IIRC then there's little point in doing so as it is identical to one of the others (I'd need to check which one) When there was the belief that imx462 could do 12bit 120fps readout then it would have been worth it, but that has been confirmed by Sony not to be the case and was an error on the flyer. |
|
OK, well, I was suggesting it just to have the binding explicitly defined. Anyway, now that there is a small difference in max analog gain wouldn't we also need to modify the device three overlays for IMX462 so that it binds more correctly? |
|
IMX462 has a max gain of 29.4dB, same as IMX327. Now 2d41947 did add differences in the register initialisation between 327 and 290, and I haven't checked through which option is correct for 462. If we had that, then it would be worth adding in 462 as well. |
|
Would it be OK enough to check if the IMX327 binding works for IMX462? Because that's something I could do. |
|
I know the 462 produces frames with the 290 or 327 configuration. It's very minor changes to some image quality that are different between the sensor models. |
6by9
force-pushed
the
rpi-6.6.y-cams
branch
2 times, most recently
from
db78f99 to
4b2e42f
Compare
|
I got distracted, so did the comparison of the registers. There's only one register different from imx290, however max analog gain is only 29.4dB vs 30.0dB. PR updated to include adding a new compatible for imx462 (both mono and colour variants) |
|
Looks to be working. Will you further upstream this one to mainline? |
|
Sent upstream - https://lore.kernel.org/linux-media/[email protected]/ I noted one minor change needed whilst reviewing it, so I'll update this again and then it should be good to merge. |
|
All seems reasonable to me. I do wonder if this HCG enable should be automatic after a certain gain value maybe? Are there any downsides of using it unconditionally at high gains? |
This was discussed in following topic: https://forums.raspberrypi.com/viewtopic.php?p=2182756&hilit=exposure#p2182756 |
|
https://forums.raspberrypi.com/viewtopic.php?p=2186273#p2186273 confirmed that enabling HCG adds x5.8 of gain, so trying to enable it programmatically would cause instability in AE. I'll hold fire on merging this just now, as there are a couple of comments from upstreaming the imx462 support patches. |
Commit ec75fd952b0b5cdab7b606cdacba237c57c1fdda upstream. The imx327 only supports up to 29.4dB of analogue gain, vs the imx290 going up to 30dB. Both are in 0.3dB steps. As we now have model specific config, fix this mismatch, and delete the comment referencing it. Signed-off-by: Dave Stevenson <[email protected]> Reviewed-by: Laurent Pinchart <[email protected]> Reviewed-by: Alexander Stein <[email protected]> Signed-off-by: Sakari Ailus <[email protected]> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Commit f2055c1d62d6dfd25a31d1d1923883f21305aea5 upstream. Reviewing the datasheets, register 0x3011 is meant to be 0x02 on imx327 and 0x00 on imx290. Move it out of the common registers, and set it appropriately in the sensor specific sections. (Included for imx290 to be explicit, rather than relying on the default value). Fixes: 2d41947 ("media: i2c: imx290: Add support for imx327 variant") Signed-off-by: Dave Stevenson <[email protected]> Reviewed-by: Laurent Pinchart <[email protected]> Signed-off-by: Sakari Ailus <[email protected]> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Commit e4faac99d5bb4b6c80f2495c40fcd71a67c40b27 upstream. IMX462 is the successor to IMX290, which is supportable by the existing IMX290 driver via a new compatible string. Signed-off-by: Dave Stevenson <[email protected]> Acked-by: Conor Dooley <[email protected]> Reviewed-by: Laurent Pinchart <[email protected]> Signed-off-by: Sakari Ailus <[email protected]> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Commit c699b6c7c857baba1375a1ed090bf71f695e2971 upstream. IMX462 is the successor to IMX290, and wants very minor changes to the register setup. Add the relevant configuration to support it. Signed-off-by: Dave Stevenson <[email protected]> Reviewed-by: Laurent Pinchart <[email protected]> Signed-off-by: Sakari Ailus <[email protected]> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
The sensor has Low Conversion Gain (HCG) and High Conversion Gain (HCG) modes, with the supposedly the HCG mode having better noise performance at high gains. As this parameter changes the gain range of the sensor, it isn't possible to make this an automatic property, and there is no suitable V4L2 control to set it, so just add it as a module parameter. Signed-off-by: Dave Stevenson <[email protected]>
Now that imx462 has a separate compatible string, make use of it. Signed-off-by: Dave Stevenson <[email protected]>
|
I updated this branch last week with the accepted patches from upstream, but forgot to comment on it. I'm happy with this PR now. |
|
LGTM |
|
Would it not be cruel to merge before this PR's birthday? |
Do I have to buy the cake?! |
No description provided.