[PATCH v2 1/4] libcamera: libipa: camera_sensor: Provide CameraSensorHelper and CameraSensorProperties for the Sony IMX462 image sensor.

[Jacopo Mondi]

Hi Geoffrey

On Mon, Nov 18, 2024 at 11:53:07PM +0100, Geoffrey Van Landeghem wrote:
> The sensor is largely compatible with the already supported Sony IMX290 so we can reuse the same helpers for the analogue gain conversion functions.

I presume my suggestion to read https://cbea.ms/git-commit/ first
wasn't helpful.

Subject line should be shortened (I would not force it to 50 cols
as the website suggests, but strive for staying in at least 72), and
no full stop at the end.

Same thing for the commit message.

We can adjust it when applying the patch if you don't have to resend
for other reasons.

>
> Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl at gmail.com>
> ---
>  src/ipa/libipa/camera_sensor_helper.cpp           | 5 +++++
>  src/ipa/rpi/cam_helper/cam_helper_imx290.cpp      | 1 +
>  src/libcamera/sensor/camera_sensor_properties.cpp | 4 ++++
>  3 files changed, 10 insertions(+)
>
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index c6169bdc..f870dc28 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -622,6 +622,11 @@ public:
>  };
>  REGISTER_CAMERA_SENSOR_HELPER("imx415", CameraSensorHelperImx415)
>
> +class CameraSensorHelperImx462 : public CameraSensorHelperImx290
> +{
> +};
> +REGISTER_CAMERA_SENSOR_HELPER("imx462", CameraSensorHelperImx462)

One thing we're missing for imx290 is the black level pedestal. The
imx290 datasheet reports a (programmable) black level of
10 bits: 0x3c
12 bits: 0xf0

Could you confirm the imx462 reports the same default values ?

> +
>  class CameraSensorHelperImx477 : public CameraSensorHelper
>  {
>  public:
> diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp
> index e57ab538..0cc24a6d 100644
> --- a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp
> +++ b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp
> @@ -73,3 +73,4 @@ static CamHelper *create()
>  }
>
>  static RegisterCamHelper reg("imx290", &create);
> +static RegisterCamHelper reg462("imx462", &create);
> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> index 6d4136d0..e2305166 100644
> --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> @@ -142,6 +142,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  			.unitCellSize = { 1450, 1450 },
>  			.testPatternModes = {},
>  		} },
> +		{ "imx462", {
> +			.unitCellSize = { 2900, 2900 },
> +			.testPatternModes = {},
> +		} },

I don't have a datasheet but a few online sources confirm a 2.9u pixel
size.

The patch looks good
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j

>  		{ "imx477", {
>  			.unitCellSize = { 1550, 1550 },
>  			.testPatternModes = {},
> --
> 2.43.0
>