libcamera: camera_sensor: Add Hynix Hi556 sensor properties and helper
diff mbox series

Message ID 20261018102700.32980-1-tobias.predel@gmail.com
State New
Headers show
Series
  • libcamera: camera_sensor: Add Hynix Hi556 sensor properties and helper
Related show

Commit Message

Tobias Predel Oct. 18, 2026, 10:27 a.m. UTC 
Add camera sensor properties and a camera sensor helper for the Hynix
Hi556, a 5MP sensor used as the user-facing camera on IPU6 laptops such
as the HP Dragonfly 13.5 inch G4.

The unit cell size is 1.12um. The test pattern modes follow the order of
the menu exposed by the kernel driver, the same as for the Hi846.

The black level was measured on raw 10-bit frames as the intercept of a
linear fit of the mean pixel value against exposure time, at analogue
gain codes 0 and 64 (63.95 and 64.20 respectively), giving 64 at 10
bits.

The kernel driver exposes analogue gain codes 0 to 240 in steps of 1.
The gain is 1 + code / 16, giving 1x to 16x. This was verified on raw
frames of a static scene, with a gain code 0 reference frame captured
before and after each step to cancel out light changes: the measured
gain is within 1% of the model for codes 0 to 224, and 1.5% below it at
code 240.

Without the helper, the soft ISP falls back to the uncalibrated defaults
and cannot convert between gain codes and gain values.

Signed-off-by: Tobias Predel <tobias.predel@gmail.com>
Assisted-by: Claude:claude-opus-5-5
---

Notes:
    Tested on an HP Dragonfly 13.5 inch G4 (Hi556 on IPU6 ISYS, simple pipeline
    handler with the soft ISP); the runtime log confirms that the helper and the
    sensor properties are picked up.
    
    This patch was written with the help of an AI coding assistant (hence the
    Assisted-by trailer, following the Linux kernel's coding-assistants
    guidelines; checkstyle doesn't know that trailer key yet). I reviewed the
    code and ran the measurements on the hardware myself.

 src/ipa/libipa/camera_sensor_helper.cpp        | 13 +++++++++++++
 .../sensor/camera_sensor_properties.cpp        | 18 ++++++++++++++++++
 2 files changed, 31 insertions(+)

Comments

Jacopo Mondi Oct. 19, 2026, 3:20 p.m. UTC | #1 
On Sun, Oct 18, 2026 at 12:27:00PM +0200, Tobias Predel wrote:
> Add camera sensor properties and a camera sensor helper for the Hynix
> Hi556, a 5MP sensor used as the user-facing camera on IPU6 laptops such
> as the HP Dragonfly 13.5 inch G4.
>
> The unit cell size is 1.12um. The test pattern modes follow the order of
> the menu exposed by the kernel driver, the same as for the Hi846.
>
> The black level was measured on raw 10-bit frames as the intercept of a
> linear fit of the mean pixel value against exposure time, at analogue
> gain codes 0 and 64 (63.95 and 64.20 respectively), giving 64 at 10
> bits.

I would be interested in knowing how you have performed measures,
especially why you seems to suggest you have varied both the exposure
time and the analogue gain (if I understand it right).

>
> The kernel driver exposes analogue gain codes 0 to 240 in steps of 1.
> The gain is 1 + code / 16, giving 1x to 16x. This was verified on raw
> frames of a static scene, with a gain code 0 reference frame captured
> before and after each step to cancel out light changes: the measured

Was that done with continuous capture ? Or are you changing the gain and
taking 1 picture ? What does it mean capturing a reference frame with
0 gain before a single capture ?

> gain is within 1% of the model for codes 0 to 224, and 1.5% below it at
> code 240.
>
> Without the helper, the soft ISP falls back to the uncalibrated defaults
> and cannot convert between gain codes and gain values.
>
> Signed-off-by: Tobias Predel <tobias.predel@gmail.com>
> Assisted-by: Claude:claude-opus-5-5

libcamera doesn't accept LLM generated patches.

I should have read the before reviewing, maybe.

In this case the code change is minimal and you seem to have done most
the work doing the measurement etc, so I'm not sure if this patch has
to be considered or not.

> ---
>
> Notes:
>     Tested on an HP Dragonfly 13.5 inch G4 (Hi556 on IPU6 ISYS, simple pipeline
>     handler with the soft ISP); the runtime log confirms that the helper and the
>     sensor properties are picked up.
>
>     This patch was written with the help of an AI coding assistant (hence the
>     Assisted-by trailer, following the Linux kernel's coding-assistants
>     guidelines; checkstyle doesn't know that trailer key yet). I reviewed the
>     code and ran the measurements on the hardware myself.
>
>  src/ipa/libipa/camera_sensor_helper.cpp        | 13 +++++++++++++
>  .../sensor/camera_sensor_properties.cpp        | 18 ++++++++++++++++++
>  2 files changed, 31 insertions(+)
>
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index 359040ec7..87d72afa0 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -519,6 +519,19 @@ public:
>  };
>  REGISTER_CAMERA_SENSOR_HELPER("gc08a3", CameraSensorHelperGc08a3)
>
> +class CameraSensorHelperHi556 : public CameraSensorHelper
> +{
> +public:
> +	CameraSensorHelperHi556()
> +	{
> +		/* Measured: 64 at 10bits. */
> +		blackLevel_ = 4096;
> +		/* Analogue gain is 1 + code / 16, codes 0 to 240 (1x to 16x). */
> +		gain_ = AnalogueGainLinear{ 1, 16, 0, 16 };
> +	}
> +};
> +REGISTER_CAMERA_SENSOR_HELPER("hi556", CameraSensorHelperHi556)
> +
>  class CameraSensorHelperHm1246 : public CameraSensorHelper
>  {
>  public:
> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> index 20cfd0bc1..203ffb382 100644
> --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> @@ -132,6 +132,24 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				.hblankDelay = 2
>  			},
>  		} },
> +		{ "hi556", {
> +			.unitCellSize = { 1120, 1120 },
> +			.testPatternModes = {
> +				{ controls::draft::TestPatternModeOff, 0 },
> +				{ controls::draft::TestPatternModeSolidColor, 1 },
> +				{ controls::draft::TestPatternModeColorBars, 2 },
> +				{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
> +				{ controls::draft::TestPatternModePn9, 4 },
> +				/*
> +				 * No corresponding test pattern mode for:
> +				 * 5: "Gradient Horizontal"
> +				 * 6: "Gradient Vertical"
> +				 * 7: "Check Board"
> +				 * 8: "Slant Pattern"
> +				 */
> +			},
> +			.sensorDelays = { },
> +		} },
>  		{ "hi846", {
>  			.unitCellSize = { 1120, 1120 },
>  			.testPatternModes = {
> --
> 2.56.0
>
Tobias Predel Oct. 19, 2026, 5:22 p.m. UTC | #2 
Hi Jacopo,

Thanks for the review.

On Mon, Oct 19, 2026 at 05:20:14PM +0200, Jacopo Mondi wrote:
> On Sun, Oct 18, 2026 at 12:27:00PM +0200, Tobias Predel wrote:
> > The black level was measured on raw 10-bit frames as the intercept of a
> > linear fit of the mean pixel value against exposure time, at analogue
> > gain codes 0 and 64 (63.95 and 64.20 respectively), giving 64 at 10
> > bits.
>
> I would be interested in knowing how you have performed measures,
> especially why you seems to suggest you have varied both the exposure
> time and the analogue gain (if I understand it right).

Yes, both were varied, but for different purposes. For each of the
two gain codes I captured raw BA10 frames at several exposure times
and fitted the mean pixel value against the exposure time. The
intercept at zero exposure is the black level.

The second gain code was only a check of where the pedestal sits. If
the offset were added before the analogue gain, the intercept at code
64 (5x) would be about 5 * 64 = 320; it stays at 64 (63.95 at 1x,
64.20 at 5x), so the pedestal is added after the gain and a single
black level is correct for all gains.

> > The kernel driver exposes analogue gain codes 0 to 240 in steps of 1.
> > The gain is 1 + code / 16, giving 1x to 16x. This was verified on raw
> > frames of a static scene, with a gain code 0 reference frame captured
> > before and after each step to cancel out light changes: the measured
>
> Was that done with continuous capture ? Or are you changing the gain and
> taking 1 picture ? What does it mean capturing a reference frame with
> 0 gain before a single capture ?

Not continuous. The script sets the controls on the sensor subdev with
v4l2-ctl and then starts a separate short stream for each capture,
skipping the first 4 frames and averaging the next 3, at 2592x1944
BA10. Exposure (2000 lines), vertical blanking (30000) and digital
gain (1x) are fixed for the whole run; only the analogue gain changes.

The scene was a wall in daylight, and daylight drifts by a few percent
over the minute the sweep takes. So the sequence is

  code 0, code 16, code 0, code 32, code 0, ..., code 240, code 0

and the gain at each step is

  (mean(code N) - 64) / ((mean(code 0 before) + mean(code 0 after)) / 2 - 64)

i.e. each gain capture is compared to the average of the 1x captures
taken just before and after it, which cancels a slow, roughly linear
change in light. The means only use pixels that are below 900 in the
code 240 capture, so clipped pixels don't bias the higher gains.

> > Signed-off-by: Tobias Predel <tobias.predel@gmail.com>
> > Assisted-by: Claude:claude-opus-5-5
>
> libcamera doesn't accept LLM generated patches.
>
> I should have read the before reviewing, maybe.
>
> In this case the code change is minimal and you seem to have done most
> the work doing the measurement etc, so I'm not sure if this patch has
> to be considered or not.

Sorry, I wasn't aware of this policy; I looked in
Documentation/contributing.rst and didn't find it there, so I added the
trailer to be transparent about it.

Claude Code was driving the whole testing as described above to get
the correct values, meaning it drafted and ran the scripts by itself
and applied/tested the patches.

If it helps, I can rewrite the commit message myself and resend a v2
without the trailer, or drop the patch if the policy rules it out. I'll
go with whatever you and the other maintainers prefer.

Regards,
Tobias
Barnabás Pőcze Oct. 22, 2026, 8:27 a.m. UTC | #3 
Hi

2026. 10. 18. 12:27 keltezéssel, Tobias Predel írta:
> Add camera sensor properties and a camera sensor helper for the Hynix
> Hi556, a 5MP sensor used as the user-facing camera on IPU6 laptops such
> as the HP Dragonfly 13.5 inch G4.
> 
> The unit cell size is 1.12um. The test pattern modes follow the order of
> the menu exposed by the kernel driver, the same as for the Hi846.
> 
> The black level was measured on raw 10-bit frames as the intercept of a
> linear fit of the mean pixel value against exposure time, at analogue
> gain codes 0 and 64 (63.95 and 64.20 respectively), giving 64 at 10
> bits.
> 
> The kernel driver exposes analogue gain codes 0 to 240 in steps of 1.
> The gain is 1 + code / 16, giving 1x to 16x. This was verified on raw
> frames of a static scene, with a gain code 0 reference frame captured
> before and after each step to cancel out light changes: the measured
> gain is within 1% of the model for codes 0 to 224, and 1.5% below it at
> code 240.
> 
> Without the helper, the soft ISP falls back to the uncalibrated defaults
> and cannot convert between gain codes and gain values.
> 
> Signed-off-by: Tobias Predel <tobias.predel@gmail.com>
> Assisted-by: Claude:claude-opus-5-5
> ---
> 
> Notes:
>      Tested on an HP Dragonfly 13.5 inch G4 (Hi556 on IPU6 ISYS, simple pipeline
>      handler with the soft ISP); the runtime log confirms that the helper and the
>      sensor properties are picked up.
>      
>      This patch was written with the help of an AI coding assistant (hence the
>      Assisted-by trailer, following the Linux kernel's coding-assistants
>      guidelines; checkstyle doesn't know that trailer key yet). I reviewed the
>      code and ran the measurements on the hardware myself.
> 
>   src/ipa/libipa/camera_sensor_helper.cpp        | 13 +++++++++++++
>   .../sensor/camera_sensor_properties.cpp        | 18 ++++++++++++++++++
>   2 files changed, 31 insertions(+)
> 
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index 359040ec7..87d72afa0 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -519,6 +519,19 @@ public:
>   };
>   REGISTER_CAMERA_SENSOR_HELPER("gc08a3", CameraSensorHelperGc08a3)
>   
> +class CameraSensorHelperHi556 : public CameraSensorHelper
> +{
> +public:
> +	CameraSensorHelperHi556()
> +	{
> +		/* Measured: 64 at 10bits. */
> +		blackLevel_ = 4096;
> +		/* Analogue gain is 1 + code / 16, codes 0 to 240 (1x to 16x). */
> +		gain_ = AnalogueGainLinear{ 1, 16, 0, 16 };

The unit cell size and analogue gain are both confirmed by data sheets:

   * https://www.scribd.com/document/658971145/Hi-556W
   * https://cdn.specpick.com/images/photonics/products/AAA0556NXX_YACF5D0C9SHC__637812903077918008.pdf

It also mentions "black level calibration":

   Black level is caused from pixel characteristics and analog channel offset. It makes poor image quality in dark
   condition and misleads color balance. To reduce these phenomenon, sensor automatically calibrates the black
   level every frame. The masked pixels in pixel array are used to calculate the black level.

but I couldn't find any indication that some kind of target could be set / retrieved.


> +	}
> +};
> +REGISTER_CAMERA_SENSOR_HELPER("hi556", CameraSensorHelperHi556)
> +
>   class CameraSensorHelperHm1246 : public CameraSensorHelper
>   {
>   public:
> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> index 20cfd0bc1..203ffb382 100644
> --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> @@ -132,6 +132,24 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>   				.hblankDelay = 2
>   			},
>   		} },
> +		{ "hi556", {
> +			.unitCellSize = { 1120, 1120 },
> +			.testPatternModes = {
> +				{ controls::draft::TestPatternModeOff, 0 },
> +				{ controls::draft::TestPatternModeSolidColor, 1 },
> +				{ controls::draft::TestPatternModeColorBars, 2 },
> +				{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
> +				{ controls::draft::TestPatternModePn9, 4 },

Did you verify these?


> +				/*
> +				 * No corresponding test pattern mode for:
> +				 * 5: "Gradient Horizontal"
> +				 * 6: "Gradient Vertical"
> +				 * 7: "Check Board"
> +				 * 8: "Slant Pattern"
> +				 */
> +			},
> +			.sensorDelays = { },
> +		} },
>   		{ "hi846", {
>   			.unitCellSize = { 1120, 1120 },
>   			.testPatternModes = {

Patch
diff mbox series 

diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
index 359040ec7..87d72afa0 100644
--- a/src/ipa/libipa/camera_sensor_helper.cpp
+++ b/src/ipa/libipa/camera_sensor_helper.cpp
@@ -519,6 +519,19 @@  public:
 };
 REGISTER_CAMERA_SENSOR_HELPER("gc08a3", CameraSensorHelperGc08a3)
 
+class CameraSensorHelperHi556 : public CameraSensorHelper
+{
+public:
+	CameraSensorHelperHi556()
+	{
+		/* Measured: 64 at 10bits. */
+		blackLevel_ = 4096;
+		/* Analogue gain is 1 + code / 16, codes 0 to 240 (1x to 16x). */
+		gain_ = AnalogueGainLinear{ 1, 16, 0, 16 };
+	}
+};
+REGISTER_CAMERA_SENSOR_HELPER("hi556", CameraSensorHelperHi556)
+
 class CameraSensorHelperHm1246 : public CameraSensorHelper
 {
 public:
diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
index 20cfd0bc1..203ffb382 100644
--- a/src/libcamera/sensor/camera_sensor_properties.cpp
+++ b/src/libcamera/sensor/camera_sensor_properties.cpp
@@ -132,6 +132,24 @@  const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
 				.hblankDelay = 2
 			},
 		} },
+		{ "hi556", {
+			.unitCellSize = { 1120, 1120 },
+			.testPatternModes = {
+				{ controls::draft::TestPatternModeOff, 0 },
+				{ controls::draft::TestPatternModeSolidColor, 1 },
+				{ controls::draft::TestPatternModeColorBars, 2 },
+				{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
+				{ controls::draft::TestPatternModePn9, 4 },
+				/*
+				 * No corresponding test pattern mode for:
+				 * 5: "Gradient Horizontal"
+				 * 6: "Gradient Vertical"
+				 * 7: "Check Board"
+				 * 8: "Slant Pattern"
+				 */
+			},
+			.sensorDelays = { },
+		} },
 		{ "hi846", {
 			.unitCellSize = { 1120, 1120 },
 			.testPatternModes = {