[3/3] ipa: ipu3: tone_mapping: Make the gamma configurable
diff mbox series

Message ID 20260904062840.46739-4-dmanresa@gmail.com
State New
Headers show
Series
  • ipa: ipu3: black level, gamma and AGC fixes from a Dell Latitude 7275
Related show

Commit Message

D. Manresa Sept. 4, 2026, 6:28 a.m. UTC 
The tone mapping algorithm hard-codes a gamma of 1.1, which produces an
almost linear output and dark mid-tones for a display-referred consumer.
Add an optional 'gamma' tuning parameter, keeping 1.1 as the default.

While at it, document an observation made on a Dell Latitude 7275 with
the IPU3 firmware currently distributed for Linux: in the video pipe the
LUT contents are ignored and a fixed curve is applied whenever the gamma
block is programmed (gammas of 0.5, 1.1 and 3.0 produce identical output),
while leaving the block unprogrammed results in a linear ramp.

Signed-off-by: D. Manresa <dmanresa@gmail.com>
---
 src/ipa/ipu3/algorithms/tone_mapping.cpp | 37 +++++++++++++++++++++++-
 src/ipa/ipu3/algorithms/tone_mapping.h   |  3 ++
 2 files changed, 39 insertions(+), 1 deletion(-)

Comments

Dan Scally Sept. 4, 2026, 6:39 a.m. UTC | #1 
Hello and thanks for the patch

On 04/09/2026 07:28, D. Manresa wrote:
> The tone mapping algorithm hard-codes a gamma of 1.1, which produces an
> almost linear output and dark mid-tones for a display-referred consumer.
> Add an optional 'gamma' tuning parameter, keeping 1.1 as the default.
> 
> While at it, document an observation made on a Dell Latitude 7275 with
> the IPU3 firmware currently distributed for Linux: in the video pipe the
> LUT contents are ignored and a fixed curve is applied whenever the gamma
> block is programmed (gammas of 0.5, 1.1 and 3.0 produce identical output),
> while leaving the block unprogrammed results in a linear ramp.

Huh...I wonder what firmware I've been running, it works fine for me.

The patch I'm afraid has probably been superseded by this series:

https://patchwork.libcamera.org/project/libcamera/list/?series=6162

which adds a centralised Gamma algorithm to libipa and updates the IPU3's ToneMapping algorithm to 
use it - it includes the facility to add a default gamma value to the tuning data.

Thanks
Dan

> 
> Signed-off-by: D. Manresa <dmanresa@gmail.com>
> ---
>   src/ipa/ipu3/algorithms/tone_mapping.cpp | 37 +++++++++++++++++++++++-
>   src/ipa/ipu3/algorithms/tone_mapping.h   |  3 ++
>   2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> index 160338c..160b015 100644
> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp
> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> @@ -7,9 +7,14 @@
>   
>   #include "tone_mapping.h"
>   
> +#include <algorithm>
>   #include <cmath>
>   #include <string.h>
>   
> +#include <libcamera/base/log.h>
> +
> +#include "libcamera/internal/value_node.h"
> +
>   /**
>    * \file tone_mapping.h
>    */
> @@ -26,9 +31,39 @@ namespace ipa::ipu3::algorithms {
>    * generated based on a gamma parameter.
>    */
>   
> +LOG_DEFINE_CATEGORY(IPU3ToneMapping)
> +
> +/* Historical default, kept for existing tuning files. */
> +static constexpr double kDefaultGamma = 1.1;
> +
>   ToneMapping::ToneMapping()
> -	: gamma_(1.0)
> +	: gamma_(1.0), tunedGamma_(kDefaultGamma)
> +{
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + *
> + * The optional \a gamma tuning parameter sets the exponent of the encoding
> + * curve programmed in the ImgU gamma correction LUT, output = input^(1/gamma).
> + * The default is 1.1.
> + *
> + * Note that on the IPU3 firmware currently distributed for Linux
> + * (irci_irci_ecr-master_20161208_0213_20170112_1500) the video pipe applies
> + * a fixed curve when the gamma block is programmed and ignores the LUT
> + * contents: values of 0.5, 1.1 and 3.0 produce identical output, while not
> + * programming the block yields a linear ramp. The parameter is still useful
> + * to document the intent and for firmware that honours the LUT.
> + */
> +int ToneMapping::init([[maybe_unused]] IPAContext &context,
> +		      const ValueNode &tuningData)
>   {
> +	tunedGamma_ = std::clamp(tuningData["gamma"].get<double>().value_or(kDefaultGamma),
> +				 0.5, 4.0);
> +
> +	LOG(IPU3ToneMapping, Debug) << "Gamma " << tunedGamma_;
> +
> +	return 0;
>   }
>   
>   /**
> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.h b/src/ipa/ipu3/algorithms/tone_mapping.h
> index b2b3801..6c12867 100644
> --- a/src/ipa/ipu3/algorithms/tone_mapping.h
> +++ b/src/ipa/ipu3/algorithms/tone_mapping.h
> @@ -18,6 +18,8 @@ class ToneMapping : public Algorithm
>   public:
>   	ToneMapping();
>   
> +	int init(IPAContext &context, const ValueNode &tuningData) override;
> +
>   	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
>   	void prepare(IPAContext &context, const uint32_t frame,
>   		     IPAFrameContext &frameContext, ipu3_uapi_params *params) override;
> @@ -28,6 +30,7 @@ public:
>   
>   private:
>   	double gamma_;
> +	double tunedGamma_;
>   };
>   
>   } /* namespace ipa::ipu3::algorithms */
Dan Scally Sept. 4, 2026, 7:25 a.m. UTC | #2 
On 04/09/2026 07:39, Dan Scally wrote:
> Hello and thanks for the patch
> 
> On 04/09/2026 07:28, D. Manresa wrote:
>> The tone mapping algorithm hard-codes a gamma of 1.1, which produces an
>> almost linear output and dark mid-tones for a display-referred consumer.
>> Add an optional 'gamma' tuning parameter, keeping 1.1 as the default.
>>
>> While at it, document an observation made on a Dell Latitude 7275 with
>> the IPU3 firmware currently distributed for Linux: in the video pipe the
>> LUT contents are ignored and a fixed curve is applied whenever the gamma
>> block is programmed (gammas of 0.5, 1.1 and 3.0 produce identical output),
>> while leaving the block unprogrammed results in a linear ramp.
> 
> Huh...I wonder what firmware I've been running, it works fine for me.
> 
> The patch I'm afraid has probably been superseded by this series:
> 
> https://patchwork.libcamera.org/project/libcamera/list/?series=6162
> 
> which adds a centralised Gamma algorithm to libipa and updates the IPU3's ToneMapping algorithm to 
> use it - it includes the facility to add a default gamma value to the tuning data.
> 
> Thanks
> Dan
> 
>>
>> Signed-off-by: D. Manresa <dmanresa@gmail.com>
>> ---
>>   src/ipa/ipu3/algorithms/tone_mapping.cpp | 37 +++++++++++++++++++++++-
>>   src/ipa/ipu3/algorithms/tone_mapping.h   |  3 ++
>>   2 files changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp
>> index 160338c..160b015 100644
>> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp
>> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp
>> @@ -7,9 +7,14 @@
>>   #include "tone_mapping.h"
>> +#include <algorithm>
>>   #include <cmath>
>>   #include <string.h>
>> +#include <libcamera/base/log.h>
>> +
>> +#include "libcamera/internal/value_node.h"
>> +
>>   /**
>>    * \file tone_mapping.h
>>    */
>> @@ -26,9 +31,39 @@ namespace ipa::ipu3::algorithms {
>>    * generated based on a gamma parameter.
>>    */
>> +LOG_DEFINE_CATEGORY(IPU3ToneMapping)
>> +
>> +/* Historical default, kept for existing tuning files. */
>> +static constexpr double kDefaultGamma = 1.1;
>> +
>>   ToneMapping::ToneMapping()
>> -    : gamma_(1.0)
>> +    : gamma_(1.0), tunedGamma_(kDefaultGamma)
>> +{
>> +}
>> +
>> +/**
>> + * \copydoc libcamera::ipa::Algorithm::init
>> + *
>> + * The optional \a gamma tuning parameter sets the exponent of the encoding
>> + * curve programmed in the ImgU gamma correction LUT, output = input^(1/gamma).
>> + * The default is 1.1.
>> + *
>> + * Note that on the IPU3 firmware currently distributed for Linux
>> + * (irci_irci_ecr-master_20161208_0213_20170112_1500) the video pipe applies

Ah, turns out I have this firmware. So perhaps try the series that I linked and see if changing the 
gamma works for you with that?

Thanks
Dan

>> + * a fixed curve when the gamma block is programmed and ignores the LUT
>> + * contents: values of 0.5, 1.1 and 3.0 produce identical output, while not
>> + * programming the block yields a linear ramp. The parameter is still useful
>> + * to document the intent and for firmware that honours the LUT.
>> + */
>> +int ToneMapping::init([[maybe_unused]] IPAContext &context,
>> +              const ValueNode &tuningData)
>>   {
>> +    tunedGamma_ = std::clamp(tuningData["gamma"].get<double>().value_or(kDefaultGamma),
>> +                 0.5, 4.0);
>> +
>> +    LOG(IPU3ToneMapping, Debug) << "Gamma " << tunedGamma_;
>> +
>> +    return 0;
>>   }
>>   /**
>> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.h b/src/ipa/ipu3/algorithms/tone_mapping.h
>> index b2b3801..6c12867 100644
>> --- a/src/ipa/ipu3/algorithms/tone_mapping.h
>> +++ b/src/ipa/ipu3/algorithms/tone_mapping.h
>> @@ -18,6 +18,8 @@ class ToneMapping : public Algorithm
>>   public:
>>       ToneMapping();
>> +    int init(IPAContext &context, const ValueNode &tuningData) override;
>> +
>>       int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
>>       void prepare(IPAContext &context, const uint32_t frame,
>>                IPAFrameContext &frameContext, ipu3_uapi_params *params) override;
>> @@ -28,6 +30,7 @@ public:
>>   private:
>>       double gamma_;
>> +    double tunedGamma_;
>>   };
>>   } /* namespace ipa::ipu3::algorithms */
>

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp
index 160338c..160b015 100644
--- a/src/ipa/ipu3/algorithms/tone_mapping.cpp
+++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp
@@ -7,9 +7,14 @@ 
 
 #include "tone_mapping.h"
 
+#include <algorithm>
 #include <cmath>
 #include <string.h>
 
+#include <libcamera/base/log.h>
+
+#include "libcamera/internal/value_node.h"
+
 /**
  * \file tone_mapping.h
  */
@@ -26,9 +31,39 @@  namespace ipa::ipu3::algorithms {
  * generated based on a gamma parameter.
  */
 
+LOG_DEFINE_CATEGORY(IPU3ToneMapping)
+
+/* Historical default, kept for existing tuning files. */
+static constexpr double kDefaultGamma = 1.1;
+
 ToneMapping::ToneMapping()
-	: gamma_(1.0)
+	: gamma_(1.0), tunedGamma_(kDefaultGamma)
+{
+}
+
+/**
+ * \copydoc libcamera::ipa::Algorithm::init
+ *
+ * The optional \a gamma tuning parameter sets the exponent of the encoding
+ * curve programmed in the ImgU gamma correction LUT, output = input^(1/gamma).
+ * The default is 1.1.
+ *
+ * Note that on the IPU3 firmware currently distributed for Linux
+ * (irci_irci_ecr-master_20161208_0213_20170112_1500) the video pipe applies
+ * a fixed curve when the gamma block is programmed and ignores the LUT
+ * contents: values of 0.5, 1.1 and 3.0 produce identical output, while not
+ * programming the block yields a linear ramp. The parameter is still useful
+ * to document the intent and for firmware that honours the LUT.
+ */
+int ToneMapping::init([[maybe_unused]] IPAContext &context,
+		      const ValueNode &tuningData)
 {
+	tunedGamma_ = std::clamp(tuningData["gamma"].get<double>().value_or(kDefaultGamma),
+				 0.5, 4.0);
+
+	LOG(IPU3ToneMapping, Debug) << "Gamma " << tunedGamma_;
+
+	return 0;
 }
 
 /**
diff --git a/src/ipa/ipu3/algorithms/tone_mapping.h b/src/ipa/ipu3/algorithms/tone_mapping.h
index b2b3801..6c12867 100644
--- a/src/ipa/ipu3/algorithms/tone_mapping.h
+++ b/src/ipa/ipu3/algorithms/tone_mapping.h
@@ -18,6 +18,8 @@  class ToneMapping : public Algorithm
 public:
 	ToneMapping();
 
+	int init(IPAContext &context, const ValueNode &tuningData) override;
+
 	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
 	void prepare(IPAContext &context, const uint32_t frame,
 		     IPAFrameContext &frameContext, ipu3_uapi_params *params) override;
@@ -28,6 +30,7 @@  public:
 
 private:
 	double gamma_;
+	double tunedGamma_;
 };
 
 } /* namespace ipa::ipu3::algorithms */