[PATCH v2 1/6] ipa: libipa: Add black levels to camera sensor helper

[Kieran Bingham]

Quoting Stefan Klug (2024-07-03 11:39:48)
> For a proper tuning process we need to know the sensor black levels. In
> most cases these are fixed and not reported by the kernel driver. Store
> them inside the sensor helpers for later retrieval by the algorithms.
> 
> Add black level value corresponding to the data pedestal for three
> initial sensors as documented in the datasheets. More should be added,
> eventually filling the gaps for all supported sensors.
> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
>  src/ipa/libipa/camera_sensor_helper.cpp | 27 +++++++++++++++++++++++++
>  src/ipa/libipa/camera_sensor_helper.h   |  3 +++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index 782ff9904e81..3d0e756927f0 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -47,6 +47,30 @@ namespace ipa {
>   * function.
>   */
>  
> +/**
> + * \fn CameraSensorHelper::blackLevel()
> + * \brief Fetch the black level of the sensor
> + *
> + * This function returns the black level of the sensor scaled to a 16bit pixel
> + * width. If it is unknown an empty optional is returned.
> + *
> + * Black levels are typically the result of the following phenomena:
> + * - Pedestal added by the sensor to pixel values. They are typically fixed,
> + *   sometimes programmable and should be reported in datasheets (but
> + *   documentation is not always available).
> + * - Dark currents and other physical effects that add charge to pixels in the
> + *   absence of light. Those can depend on the integration time and the sensor
> + *   die temperature, and their contribution to pixel values depend on the
> + *   sensor gains.
> + *
> + * The pedestal is usually the value with the biggest contribution to the
> + * overall black level. In most cases it is either known before or in rare cases
> + * (There is not a single driver with such a control in the linux kernel) can be
> + * queried from the sensor. This function shall provide that fixed, known value.
> + *
> + * \return The black level of the sensor, or std::nullopt if not known
> + */
> +
>  /**
>   * \brief Compute gain code from the analogue gain absolute value
>   * \param[in] gain The real gain to pass
> @@ -396,6 +420,7 @@ class CameraSensorHelperImx219 : public CameraSensorHelper
>  public:
>         CameraSensorHelperImx219()
>         {
> +               blackLevel_ = 4096;
>                 gainType_ = AnalogueGainLinear;
>                 gainConstants_.linear = { 0, 256, -1, 256 };
>         }
> @@ -407,6 +432,7 @@ class CameraSensorHelperImx258 : public CameraSensorHelper
>  public:
>         CameraSensorHelperImx258()
>         {
> +               blackLevel_ = 4096;
>                 gainType_ = AnalogueGainLinear;
>                 gainConstants_.linear = { 0, 512, -1, 512 };
>         }
> @@ -456,6 +482,7 @@ class CameraSensorHelperImx335 : public CameraSensorHelper
>  public:
>         CameraSensorHelperImx335()
>         {
> +               blackLevel_ = 3200;
>                 gainType_ = AnalogueGainExponential;
>                 gainConstants_.exp = { 1.0, expGainDb(0.3) };
>         }
> diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h
> index 0d99073bea82..ac276e27f523 100644
> --- a/src/ipa/libipa/camera_sensor_helper.h
> +++ b/src/ipa/libipa/camera_sensor_helper.h
> @@ -10,6 +10,7 @@
>  #include <stdint.h>
>  
>  #include <memory>
> +#include <optional>
>  #include <string>
>  #include <vector>
>  
> @@ -25,6 +26,7 @@ public:
>         CameraSensorHelper() = default;
>         virtual ~CameraSensorHelper() = default;
>  
> +       std::optional<int16_t> blackLevel() const { return blackLevel_; }
>         virtual uint32_t gainCode(double gain) const;
>         virtual double gain(uint32_t gainCode) const;
>  
> @@ -51,6 +53,7 @@ protected:
>                 AnalogueGainExpConstants exp;
>         };
>  
> +       std::optional<int16_t> blackLevel_;

Shouldn't the black level be a uint16_t ? Otherwise, this would then be
limited to 15 bit max ? Can we expect a blacklevel above 32767?



>         AnalogueGainType gainType_;
>         AnalogueGainConstants gainConstants_;
>  
> -- 
> 2.43.0
>