Add libipa camera sensor helper support for Sony IMX208 sensor
diff mbox series

Message ID 20250406191549.13225-1-peter.lishov@gmail.com
State New
Headers show
Series
  • Add libipa camera sensor helper support for Sony IMX208 sensor
Related show

Commit Message

Petar Lishov April 6, 2025, 7:15 p.m. UTC
Added imx208 camera sensor helper to libipa and tested on a
Google Pixelbook Go (Atlas) running barebones Fedora 41 with
stock kernel (6.11.4) which has an Intel IPU3 and Sony IMX208.

Camera sensor parameters were taken from Sony IMX214 parameters
in the same file, so they are not backed by a sensor datasheet
and may be slightly inaccurate.
---
 src/ipa/libipa/camera_sensor_helper.cpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Laurent Pinchart April 7, 2025, 7:02 a.m. UTC | #1
Hi Petar,

Thank you for the patch.

On Sun, Apr 06, 2025 at 08:15:49PM +0100, Petar Lishov wrote:
> Added imx208 camera sensor helper to libipa and tested on a
> Google Pixelbook Go (Atlas) running barebones Fedora 41 with
> stock kernel (6.11.4) which has an Intel IPU3 and Sony IMX208.
> 
> Camera sensor parameters were taken from Sony IMX214 parameters
> in the same file, so they are not backed by a sensor datasheet
> and may be slightly inaccurate.

The patch is missing a Signed-off-by line. Please see
https://libcamera.org/contributing.html#submitting-patches. This issue
should have been caught by the checkstyle.py script. You can run it
manually before submitting patches, but I recommend automating the
process with a git hook. You can find more information at
https://libcamera.org/coding-style.html#tools.

> ---
>  src/ipa/libipa/camera_sensor_helper.cpp | 12 ++++++++++++

Please also add an entry to
src/libcamera/sensor/camera_sensor_properties.cpp.

>  1 file changed, 12 insertions(+)
> 
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index 7c66cd57..5580a530 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -497,6 +497,18 @@ public:
>  };
>  REGISTER_CAMERA_SENSOR_HELPER("gc08a3", CameraSensorHelperGc08a3)
>  
> +class CameraSensorHelperImx208 : public CameraSensorHelper
> +{
> +public:
> +	CameraSensorHelperImx208()
> +	{
> +		// These values are pure guesses, datasheet yet to be found

Even if we're limited by the lack of datasheet, the black level can at
least be estimated by capturing raw images with the sensor completely
occluded, and checking the minimal pixel values.

> +		blackLevel_ = 4096;
> +		gain_ = AnalogueGainLinear{ 0, 512, -1, 512 };

Looking at the driver, this doesn't seem right at all. The analog gain
control is mapped to a single register, with a range of [0, 224] and a
default value of 0. Your linear gain model would mean that the default
gain is 0.0, which would make images completely black. I'm pretty sure
that's not the case.

> +	}
> +};
> +REGISTER_CAMERA_SENSOR_HELPER("imx208", CameraSensorHelperImx208)
> +
>  class CameraSensorHelperImx214 : public CameraSensorHelper
>  {
>  public:

Patch
diff mbox series

diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
index 7c66cd57..5580a530 100644
--- a/src/ipa/libipa/camera_sensor_helper.cpp
+++ b/src/ipa/libipa/camera_sensor_helper.cpp
@@ -497,6 +497,18 @@  public:
 };
 REGISTER_CAMERA_SENSOR_HELPER("gc08a3", CameraSensorHelperGc08a3)
 
+class CameraSensorHelperImx208 : public CameraSensorHelper
+{
+public:
+	CameraSensorHelperImx208()
+	{
+		// These values are pure guesses, datasheet yet to be found
+		blackLevel_ = 4096;
+		gain_ = AnalogueGainLinear{ 0, 512, -1, 512 };
+	}
+};
+REGISTER_CAMERA_SENSOR_HELPER("imx208", CameraSensorHelperImx208)
+
 class CameraSensorHelperImx214 : public CameraSensorHelper
 {
 public: