[libcamera-devel,2/5] ipa: rkisp1: Add support of Gamma Sensor Linearization control
diff mbox series

Message ID 20220622151918.451635-3-fsylvestre@baylibre.com
State Accepted
Headers show
Series
  • Add GSL, LSC and DPCC tuning support for rkisp1
Related show

Commit Message

Florian Sylvestre June 22, 2022, 3:19 p.m. UTC 
The GammaSensorLinearization algorithm improves the image dynamic using
a coefficient table in the YAML tuning file.

Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com>
---
 src/ipa/rkisp1/algorithms/gsl.cpp     | 142 ++++++++++++++++++++++++++
 src/ipa/rkisp1/algorithms/gsl.h       |  38 +++++++
 src/ipa/rkisp1/algorithms/meson.build |   1 +
 src/ipa/rkisp1/data/ov5640.yaml       |   6 ++
 src/ipa/rkisp1/rkisp1.cpp             |   1 +
 5 files changed, 188 insertions(+)
 create mode 100644 src/ipa/rkisp1/algorithms/gsl.cpp
 create mode 100644 src/ipa/rkisp1/algorithms/gsl.h

Comments

Nicolas Dufresne via libcamera-devel July 12, 2022, 7:18 a.m. UTC | #1 
Hi Florian,

On Wed, Jun 22, 2022 at 05:19:15PM +0200, Florian Sylvestre via libcamera-devel wrote:
> The GammaSensorLinearization algorithm improves the image dynamic using
> a coefficient table in the YAML tuning file.
> 
> Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com>
> ---
>  src/ipa/rkisp1/algorithms/gsl.cpp     | 142 ++++++++++++++++++++++++++
>  src/ipa/rkisp1/algorithms/gsl.h       |  38 +++++++
>  src/ipa/rkisp1/algorithms/meson.build |   1 +
>  src/ipa/rkisp1/data/ov5640.yaml       |   6 ++
>  src/ipa/rkisp1/rkisp1.cpp             |   1 +
>  5 files changed, 188 insertions(+)
>  create mode 100644 src/ipa/rkisp1/algorithms/gsl.cpp
>  create mode 100644 src/ipa/rkisp1/algorithms/gsl.h
> 
> diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp
> new file mode 100644
> index 00000000..7ff3d360
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/gsl.cpp
> @@ -0,0 +1,142 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021-2022, Ideas On Board
> + *
> + * gsl.cpp - RkISP1 Gamma Sensor Linearization control
> + */
> +
> +#include "gsl.h"
> +
> +#include <libcamera/base/log.h>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +#include "linux/rkisp1-config.h"
> +
> +/**
> + * \file gsl.h
> + */
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +/**
> + * \class GammaSensorLinearization
> + * \brief RkISP1 Gamma Sensor Linearization control
> + *
> + * This algorithm improves the image dynamic using a coefficient table in the
> + * Yaml tuning file, the table contains:
> + *  - 'x-intervals': These values corresponds to GAMMA_DX_n values (from
> + *    GAMMA_DX_0 to GAMMA_DX_16 fields in RKISP1_CIF_ISP_GAMMA_DX_LO and

s/DX_0/DX_1/ ?

> + *    RKISP1_CIF_ISP_GAMMA_DX_HI rkisp1 registers).
> + *    Note: rkisp1 use each value to compute a X interval equal to 2^(value+4)

s/use/uses/

s/a X/an X/

> + *  - 'y' values for each component (red, green , blue) on 12 bits resolution.
> + */
> +
> +LOG_DEFINE_CATEGORY(RkISP1Gsl)
> +
> +#define DEGAMMA_X_INTERVALS 16
> +
> +GammaSensorLinearization::GammaSensorLinearization()
> +	: tuningParameters_(false)

I would call this tuned_ or something along this lines, as this variable
signifies if the tuning parameters have been loaded correctly or not,
and does not actually contain the tuning parameters as the varaible name
suggests.

Also, what happens if no tuning file is provided? Does the algorithm
just... not work (as in it doesn't do anything)? Is that better than
running on some default tuning values? (I feel like I saw discussion on
this topic elsewhere but I'm just asking it again :S)

> +{
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + */
> +int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context,
> +				   const YamlObject &tuningData)
> +{
> +	std::vector<uint16_t> xIntervals_ = tuningData["x-intervals"].getList<uint16_t>();
> +	if (xIntervals_.size() != DEGAMMA_X_INTERVALS) {
> +		LOG(RkISP1Gsl, Error)
> +			<< "Issue while parsing 'x'"
> +			<< "in tuning file (list size:"
> +			<< xIntervals_.size() << "/"
> +			<< DEGAMMA_X_INTERVALS << ")";

I would prefer slightly more verbosity, like printing which/where the
tuning file is, and saying explicitly "expected". Also I think you're
missing spaces in the string concatenation.

Same for the other error messages below.

> +		return -EINVAL;
> +	}
> +
> +	/* Compute gammaDx_ intervals from xIntervals_ values */
> +	gammaDx_[0] = 0;
> +	gammaDx_[1] = 0;
> +	for (int i = 0; i < DEGAMMA_X_INTERVALS; ++i) {
> +		gammaDx_[i / 8] |= (xIntervals_[i] & 0x07) << ((i % 8) * 4);
> +	}

I don't think you need the braces.

Neat math trick :)


Paul

> +
> +	const YamlObject &yObject = tuningData["y"];
> +
> +	if (!yObject.isDictionary()) {
> +		LOG(RkISP1Gsl, Error)
> +			<< "Issue while parsing 'y' in tuning file: "
> +			<< "entry must be a dictionary";
> +		return -EINVAL;
> +	}
> +
> +	curveYr_ = yObject["red"].getList<uint16_t>();
> +	if (curveYr_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
> +		LOG(RkISP1Gsl, Error)
> +			<< "Issue while parsing 'y:red'"
> +			<< "in tuning file (list size: "
> +			<< curveYr_.size() << "/"
> +			<< RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE << ")";
> +		return -EINVAL;
> +	}
> +
> +	curveYg_ = yObject["green"].getList<uint16_t>();
> +	if (curveYg_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
> +		LOG(RkISP1Gsl, Error)
> +			<< "Issue while parsing 'y:green'"
> +			<< "in tuning file (list size: "
> +			<< curveYg_.size() << "/"
> +			<< RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE << ")";
> +		return -EINVAL;
> +	}
> +
> +	curveYb_ = yObject["blue"].getList<uint16_t>();
> +	if (curveYb_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
> +		LOG(RkISP1Gsl, Error)
> +			<< "Issue while parsing 'y:blue'"
> +			<< "in tuning file (list size: "
> +			<< curveYb_.size() << "/"
> +			<< RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE << ")";
> +		return -EINVAL;
> +	}
> +
> +	tuningParameters_ = true;
> +	return 0;
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::prepare
> + */
> +void GammaSensorLinearization::prepare(IPAContext &context,
> +				       rkisp1_params_cfg *params)
> +{
> +	if (context.frameContext.frameCount > 0)
> +		return;
> +
> +	if (!tuningParameters_)
> +		return;
> +
> +	params->others.sdg_config.xa_pnts.gamma_dx0 = gammaDx_[0];
> +	params->others.sdg_config.xa_pnts.gamma_dx1 = gammaDx_[1];
> +
> +	std::copy(curveYr_.begin(), curveYr_.end(),
> +		  params->others.sdg_config.curve_r.gamma_y);
> +	std::copy(curveYg_.begin(), curveYg_.end(),
> +		  params->others.sdg_config.curve_g.gamma_y);
> +	std::copy(curveYb_.begin(), curveYb_.end(),
> +		  params->others.sdg_config.curve_b.gamma_y);
> +
> +	params->module_en_update |= RKISP1_CIF_ISP_MODULE_SDG;
> +	params->module_ens |= RKISP1_CIF_ISP_MODULE_SDG;
> +	params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_SDG;
> +}
> +
> +REGISTER_IPA_ALGORITHM(GammaSensorLinearization)
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/gsl.h b/src/ipa/rkisp1/algorithms/gsl.h
> new file mode 100644
> index 00000000..df16a89b
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/gsl.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021-2022, Ideas On Board
> + *
> + * gsl.h - RkISP1 Gamma Sensor Linearization control
> + */
> +
> +#pragma once
> +
> +#include <linux/rkisp1-config.h>
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +struct IPACameraSensorInfo;
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +class GammaSensorLinearization : public Algorithm
> +{
> +public:
> +	GammaSensorLinearization();
> +	~GammaSensorLinearization() = default;
> +
> +	int init(IPAContext &context, const YamlObject &tuningData) override;
> +	void prepare(IPAContext &context, rkisp1_params_cfg *params) override;
> +
> +private:
> +	bool tuningParameters_;
> +	uint32_t gammaDx_[2];
> +	std::vector<uint16_t> curveYr_;
> +	std::vector<uint16_t> curveYg_;
> +	std::vector<uint16_t> curveYb_;
> +};
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> index 7ec53d89..0597c353 100644
> --- a/src/ipa/rkisp1/algorithms/meson.build
> +++ b/src/ipa/rkisp1/algorithms/meson.build
> @@ -4,4 +4,5 @@ rkisp1_ipa_algorithms = files([
>      'agc.cpp',
>      'awb.cpp',
>      'blc.cpp',
> +    'gsl.cpp',
>  ])
> diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml
> index 232d8ae8..6cb84ed9 100644
> --- a/src/ipa/rkisp1/data/ov5640.yaml
> +++ b/src/ipa/rkisp1/data/ov5640.yaml
> @@ -10,4 +10,10 @@ algorithms:
>        Gr: 256
>        Gb: 256
>        B:  256
> +  - GammaSensorLinearization:
> +      x-intervals: [ 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2 ]
> +      y:
> +        red:   [ 0, 256, 512, 768, 1024, 1280, 1536, 1792, 2048, 2304, 2560, 2816, 3072, 3328, 3584, 3840, 4096 ]
> +        green: [ 0, 256, 512, 768, 1024, 1280, 1536, 1792, 2048, 2304, 2560, 2816, 3072, 3328, 3584, 3840, 4096 ]
> +        blue:  [ 0, 256, 512, 768, 1024, 1280, 1536, 1792, 2048, 2304, 2560, 2816, 3072, 3328, 3584, 3840, 4096 ]
>  ...
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index d052954e..622a0d61 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -31,6 +31,7 @@
>  #include "algorithms/algorithm.h"
>  #include "algorithms/awb.h"
>  #include "algorithms/blc.h"
> +#include "algorithms/gsl.h"
>  #include "libipa/camera_sensor_helper.h"
>  
>  #include "ipa_context.h"
> -- 
> 2.34.1
>
Laurent Pinchart July 12, 2022, 11:59 p.m. UTC | #2 
Hello,

On Tue, Jul 12, 2022 at 04:18:01PM +0900, Paul Elder via libcamera-devel wrote:
> On Wed, Jun 22, 2022 at 05:19:15PM +0200, Florian Sylvestre via libcamera-devel wrote:
> > The GammaSensorLinearization algorithm improves the image dynamic using

It's not really about improving the image dynamic, but about
compensating the non-linearities of the camera sensor to obtain a linear
image for further processing.

> > a coefficient table in the YAML tuning file.
> > 
> > Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com>
> > ---
> >  src/ipa/rkisp1/algorithms/gsl.cpp     | 142 ++++++++++++++++++++++++++
> >  src/ipa/rkisp1/algorithms/gsl.h       |  38 +++++++
> >  src/ipa/rkisp1/algorithms/meson.build |   1 +
> >  src/ipa/rkisp1/data/ov5640.yaml       |   6 ++
> >  src/ipa/rkisp1/rkisp1.cpp             |   1 +
> >  5 files changed, 188 insertions(+)
> >  create mode 100644 src/ipa/rkisp1/algorithms/gsl.cpp
> >  create mode 100644 src/ipa/rkisp1/algorithms/gsl.h
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp
> > new file mode 100644
> > index 00000000..7ff3d360
> > --- /dev/null
> > +++ b/src/ipa/rkisp1/algorithms/gsl.cpp
> > @@ -0,0 +1,142 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021-2022, Ideas On Board
> > + *
> > + * gsl.cpp - RkISP1 Gamma Sensor Linearization control
> > + */
> > +
> > +#include "gsl.h"
> > +
> > +#include <libcamera/base/log.h>
> > +
> > +#include "libcamera/internal/yaml_parser.h"

Please add a blank line here.

> > +#include "linux/rkisp1-config.h"
> > +
> > +/**
> > + * \file gsl.h
> > + */
> > +
> > +namespace libcamera {
> > +
> > +namespace ipa::rkisp1::algorithms {
> > +
> > +/**
> > + * \class GammaSensorLinearization
> > + * \brief RkISP1 Gamma Sensor Linearization control
> > + *
> > + * This algorithm improves the image dynamic using a coefficient table in the
> > + * Yaml tuning file, the table contains:

s/Yaml/YAML/
s/, the table/. The table/

> > + *  - 'x-intervals': These values corresponds to GAMMA_DX_n values (from
> > + *    GAMMA_DX_0 to GAMMA_DX_16 fields in RKISP1_CIF_ISP_GAMMA_DX_LO and
> 
> s/DX_0/DX_1/ ?
> 
> > + *    RKISP1_CIF_ISP_GAMMA_DX_HI rkisp1 registers).
> > + *    Note: rkisp1 use each value to compute a X interval equal to 2^(value+4)
> 
> s/use/uses/
> 
> s/a X/an X/
> 
> > + *  - 'y' values for each component (red, green , blue) on 12 bits resolution.

I'd make it explicit that the curves are specified as piecewise linear
functions.

 * This algorithm linearizes the sensor output to compensate the sensor
 * non-linearities by applying piecewise linear curves to the red, green and
 * blue channels.
 *
 * The curves are specified in the tuning data and defined using 17 points.
 *
 * - The X coordinates are expressed using 16 intervals, with the first point at
 *   X coordinate 0. Each interval is expressed as a 2-bit value DX (from
 *   GAMMA_DX_1 to GAMMA_DX_16), stored in the RKISP1_CIF_ISP_GAMMA_DX_LO and
 *   RKISP1_CIF_ISP_GAMMA_DX_HI registers. The real interval is equal to
 *   \f$2^{dx+4}\f$. X coordinates are shared between the red, green and blue
 *   curves.
 * 
 * - The Y coordinates are specified as 17 values separately for the
 *   red, green and blue channels, with a 12-bit resolution. Each value must be
 *   in the [-2048, 2047] range compared to the previous value.

> > + */
> > +
> > +LOG_DEFINE_CATEGORY(RkISP1Gsl)
> > +
> > +#define DEGAMMA_X_INTERVALS 16

static constexpr unsigned int kDegammaXIntervals = 16;

> > +
> > +GammaSensorLinearization::GammaSensorLinearization()
> > +	: tuningParameters_(false)
> 
> I would call this tuned_ or something along this lines, as this variable
> signifies if the tuning parameters have been loaded correctly or not,
> and does not actually contain the tuning parameters as the varaible name
> suggests.

Or maybe just "valid_" or "loaded_" ?

> Also, what happens if no tuning file is provided? Does the algorithm
> just... not work (as in it doesn't do anything)? Is that better than
> running on some default tuning values? (I feel like I saw discussion on
> this topic elsewhere but I'm just asking it again :S)

Algorithms are instantiated based on the tuning file, so if the tuning
file doesn't list a gsl algorithm, this class won't be instantiaed. See
Module::createAlgorithm() in src/ipa/libipa/module.h.

We have an uncalibrated.yaml file that could list a gsl algorithm with
default values, but that will just be a linear curve. Not enabling the
algorithm will disable the corresponding hardware block, which I think
is better.

> > +{
> > +}
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::init
> > + */
> > +int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context,
> > +				   const YamlObject &tuningData)
> > +{
> > +	std::vector<uint16_t> xIntervals_ = tuningData["x-intervals"].getList<uint16_t>();

This is not a class member variable, it should be named without the
trailing _.

> > +	if (xIntervals_.size() != DEGAMMA_X_INTERVALS) {
> > +		LOG(RkISP1Gsl, Error)
> > +			<< "Issue while parsing 'x'"
> > +			<< "in tuning file (list size:"
> > +			<< xIntervals_.size() << "/"
> > +			<< DEGAMMA_X_INTERVALS << ")";
> 
> I would prefer slightly more verbosity, like printing which/where the
> tuning file is,

I wouldn't add the file name here as we don't have access to the
information, and it would be a bit painful to pass the file name around
everywhere. We could however print the file name in IPARkISP1::init() in
case of errors.

> and saying explicitly "expected". Also I think you're
> missing spaces in the string concatenation.

Maybe as follows ?

		LOG(RkISP1Gsl, Error)
			<< "Invalid 'x' coordinates: expected "
			<< kDegammaXIntervals << " elements, got "
			<< xIntervals.size();


> Same for the other error messages below.
> 
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Compute gammaDx_ intervals from xIntervals_ values */
> > +	gammaDx_[0] = 0;
> > +	gammaDx_[1] = 0;
> > +	for (int i = 0; i < DEGAMMA_X_INTERVALS; ++i) {

unsigned int i

> > +		gammaDx_[i / 8] |= (xIntervals_[i] & 0x07) << ((i % 8) * 4);
> > +	}
> 
> I don't think you need the braces.

Indeed.

> Neat math trick :)
> 
> > +
> > +	const YamlObject &yObject = tuningData["y"];
> > +
> > +	if (!yObject.isDictionary()) {
> > +		LOG(RkISP1Gsl, Error)
> > +			<< "Issue while parsing 'y' in tuning file: "
> > +			<< "entry must be a dictionary";
> > +		return -EINVAL;
> > +	}
> > +
> > +	curveYr_ = yObject["red"].getList<uint16_t>();
> > +	if (curveYr_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
> > +		LOG(RkISP1Gsl, Error)
> > +			<< "Issue while parsing 'y:red'"
> > +			<< "in tuning file (list size: "
> > +			<< curveYr_.size() << "/"
> > +			<< RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE << ")";
> > +		return -EINVAL;
> > +	}
> > +
> > +	curveYg_ = yObject["green"].getList<uint16_t>();
> > +	if (curveYg_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
> > +		LOG(RkISP1Gsl, Error)
> > +			<< "Issue while parsing 'y:green'"
> > +			<< "in tuning file (list size: "
> > +			<< curveYg_.size() << "/"
> > +			<< RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE << ")";
> > +		return -EINVAL;
> > +	}
> > +
> > +	curveYb_ = yObject["blue"].getList<uint16_t>();
> > +	if (curveYb_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
> > +		LOG(RkISP1Gsl, Error)
> > +			<< "Issue while parsing 'y:blue'"
> > +			<< "in tuning file (list size: "
> > +			<< curveYb_.size() << "/"
> > +			<< RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE << ")";
> > +		return -EINVAL;
> > +	}
> > +
> > +	tuningParameters_ = true;
> > +	return 0;
> > +}
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::prepare
> > + */
> > +void GammaSensorLinearization::prepare(IPAContext &context,
> > +				       rkisp1_params_cfg *params)
> > +{
> > +	if (context.frameContext.frameCount > 0)
> > +		return;
> > +
> > +	if (!tuningParameters_)
> > +		return;
> > +
> > +	params->others.sdg_config.xa_pnts.gamma_dx0 = gammaDx_[0];
> > +	params->others.sdg_config.xa_pnts.gamma_dx1 = gammaDx_[1];
> > +
> > +	std::copy(curveYr_.begin(), curveYr_.end(),
> > +		  params->others.sdg_config.curve_r.gamma_y);
> > +	std::copy(curveYg_.begin(), curveYg_.end(),
> > +		  params->others.sdg_config.curve_g.gamma_y);
> > +	std::copy(curveYb_.begin(), curveYb_.end(),
> > +		  params->others.sdg_config.curve_b.gamma_y);
> > +
> > +	params->module_en_update |= RKISP1_CIF_ISP_MODULE_SDG;
> > +	params->module_ens |= RKISP1_CIF_ISP_MODULE_SDG;
> > +	params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_SDG;
> > +}
> > +
> > +REGISTER_IPA_ALGORITHM(GammaSensorLinearization)
> > +
> > +} /* namespace ipa::rkisp1::algorithms */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/rkisp1/algorithms/gsl.h b/src/ipa/rkisp1/algorithms/gsl.h
> > new file mode 100644
> > index 00000000..df16a89b
> > --- /dev/null
> > +++ b/src/ipa/rkisp1/algorithms/gsl.h
> > @@ -0,0 +1,38 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021-2022, Ideas On Board
> > + *
> > + * gsl.h - RkISP1 Gamma Sensor Linearization control
> > + */
> > +
> > +#pragma once
> > +
> > +#include <linux/rkisp1-config.h>
> > +
> > +#include "algorithm.h"
> > +
> > +namespace libcamera {
> > +
> > +struct IPACameraSensorInfo;

I think you can drop this, it's not used in this header.
linux/rkisp1-config.h can also be dropped, its only usage is for the
parameters of the functions defined by the Algorithm base class, so
there's a guarantee the right headers will be included by including
algorithm.h.

> > +
> > +namespace ipa::rkisp1::algorithms {
> > +
> > +class GammaSensorLinearization : public Algorithm
> > +{
> > +public:
> > +	GammaSensorLinearization();
> > +	~GammaSensorLinearization() = default;
> > +
> > +	int init(IPAContext &context, const YamlObject &tuningData) override;
> > +	void prepare(IPAContext &context, rkisp1_params_cfg *params) override;
> > +
> > +private:
> > +	bool tuningParameters_;
> > +	uint32_t gammaDx_[2];
> > +	std::vector<uint16_t> curveYr_;
> > +	std::vector<uint16_t> curveYg_;
> > +	std::vector<uint16_t> curveYb_;
> > +};
> > +
> > +} /* namespace ipa::rkisp1::algorithms */
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> > index 7ec53d89..0597c353 100644
> > --- a/src/ipa/rkisp1/algorithms/meson.build
> > +++ b/src/ipa/rkisp1/algorithms/meson.build
> > @@ -4,4 +4,5 @@ rkisp1_ipa_algorithms = files([
> >      'agc.cpp',
> >      'awb.cpp',
> >      'blc.cpp',
> > +    'gsl.cpp',
> >  ])
> > diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml
> > index 232d8ae8..6cb84ed9 100644
> > --- a/src/ipa/rkisp1/data/ov5640.yaml
> > +++ b/src/ipa/rkisp1/data/ov5640.yaml
> > @@ -10,4 +10,10 @@ algorithms:
> >        Gr: 256
> >        Gb: 256
> >        B:  256
> > +  - GammaSensorLinearization:
> > +      x-intervals: [ 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2 ]
> > +      y:
> > +        red:   [ 0, 256, 512, 768, 1024, 1280, 1536, 1792, 2048, 2304, 2560, 2816, 3072, 3328, 3584, 3840, 4096 ]
> > +        green: [ 0, 256, 512, 768, 1024, 1280, 1536, 1792, 2048, 2304, 2560, 2816, 3072, 3328, 3584, 3840, 4096 ]
> > +        blue:  [ 0, 256, 512, 768, 1024, 1280, 1536, 1792, 2048, 2304, 2560, 2816, 3072, 3328, 3584, 3840, 4096 ]

The last value should be 4095, 4096 doesn't fit in 12 bits.

> >  ...
> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > index d052954e..622a0d61 100644
> > --- a/src/ipa/rkisp1/rkisp1.cpp
> > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > @@ -31,6 +31,7 @@
> >  #include "algorithms/algorithm.h"
> >  #include "algorithms/awb.h"
> >  #include "algorithms/blc.h"
> > +#include "algorithms/gsl.h"
> >  #include "libipa/camera_sensor_helper.h"
> >  
> >  #include "ipa_context.h"

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp
new file mode 100644
index 00000000..7ff3d360
--- /dev/null
+++ b/src/ipa/rkisp1/algorithms/gsl.cpp
@@ -0,0 +1,142 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2021-2022, Ideas On Board
+ *
+ * gsl.cpp - RkISP1 Gamma Sensor Linearization control
+ */
+
+#include "gsl.h"
+
+#include <libcamera/base/log.h>
+
+#include "libcamera/internal/yaml_parser.h"
+#include "linux/rkisp1-config.h"
+
+/**
+ * \file gsl.h
+ */
+
+namespace libcamera {
+
+namespace ipa::rkisp1::algorithms {
+
+/**
+ * \class GammaSensorLinearization
+ * \brief RkISP1 Gamma Sensor Linearization control
+ *
+ * This algorithm improves the image dynamic using a coefficient table in the
+ * Yaml tuning file, the table contains:
+ *  - 'x-intervals': These values corresponds to GAMMA_DX_n values (from
+ *    GAMMA_DX_0 to GAMMA_DX_16 fields in RKISP1_CIF_ISP_GAMMA_DX_LO and
+ *    RKISP1_CIF_ISP_GAMMA_DX_HI rkisp1 registers).
+ *    Note: rkisp1 use each value to compute a X interval equal to 2^(value+4)
+ *  - 'y' values for each component (red, green , blue) on 12 bits resolution.
+ */
+
+LOG_DEFINE_CATEGORY(RkISP1Gsl)
+
+#define DEGAMMA_X_INTERVALS 16
+
+GammaSensorLinearization::GammaSensorLinearization()
+	: tuningParameters_(false)
+{
+}
+
+/**
+ * \copydoc libcamera::ipa::Algorithm::init
+ */
+int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context,
+				   const YamlObject &tuningData)
+{
+	std::vector<uint16_t> xIntervals_ = tuningData["x-intervals"].getList<uint16_t>();
+	if (xIntervals_.size() != DEGAMMA_X_INTERVALS) {
+		LOG(RkISP1Gsl, Error)
+			<< "Issue while parsing 'x'"
+			<< "in tuning file (list size:"
+			<< xIntervals_.size() << "/"
+			<< DEGAMMA_X_INTERVALS << ")";
+		return -EINVAL;
+	}
+
+	/* Compute gammaDx_ intervals from xIntervals_ values */
+	gammaDx_[0] = 0;
+	gammaDx_[1] = 0;
+	for (int i = 0; i < DEGAMMA_X_INTERVALS; ++i) {
+		gammaDx_[i / 8] |= (xIntervals_[i] & 0x07) << ((i % 8) * 4);
+	}
+
+	const YamlObject &yObject = tuningData["y"];
+
+	if (!yObject.isDictionary()) {
+		LOG(RkISP1Gsl, Error)
+			<< "Issue while parsing 'y' in tuning file: "
+			<< "entry must be a dictionary";
+		return -EINVAL;
+	}
+
+	curveYr_ = yObject["red"].getList<uint16_t>();
+	if (curveYr_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
+		LOG(RkISP1Gsl, Error)
+			<< "Issue while parsing 'y:red'"
+			<< "in tuning file (list size: "
+			<< curveYr_.size() << "/"
+			<< RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE << ")";
+		return -EINVAL;
+	}
+
+	curveYg_ = yObject["green"].getList<uint16_t>();
+	if (curveYg_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
+		LOG(RkISP1Gsl, Error)
+			<< "Issue while parsing 'y:green'"
+			<< "in tuning file (list size: "
+			<< curveYg_.size() << "/"
+			<< RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE << ")";
+		return -EINVAL;
+	}
+
+	curveYb_ = yObject["blue"].getList<uint16_t>();
+	if (curveYb_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
+		LOG(RkISP1Gsl, Error)
+			<< "Issue while parsing 'y:blue'"
+			<< "in tuning file (list size: "
+			<< curveYb_.size() << "/"
+			<< RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE << ")";
+		return -EINVAL;
+	}
+
+	tuningParameters_ = true;
+	return 0;
+}
+
+/**
+ * \copydoc libcamera::ipa::Algorithm::prepare
+ */
+void GammaSensorLinearization::prepare(IPAContext &context,
+				       rkisp1_params_cfg *params)
+{
+	if (context.frameContext.frameCount > 0)
+		return;
+
+	if (!tuningParameters_)
+		return;
+
+	params->others.sdg_config.xa_pnts.gamma_dx0 = gammaDx_[0];
+	params->others.sdg_config.xa_pnts.gamma_dx1 = gammaDx_[1];
+
+	std::copy(curveYr_.begin(), curveYr_.end(),
+		  params->others.sdg_config.curve_r.gamma_y);
+	std::copy(curveYg_.begin(), curveYg_.end(),
+		  params->others.sdg_config.curve_g.gamma_y);
+	std::copy(curveYb_.begin(), curveYb_.end(),
+		  params->others.sdg_config.curve_b.gamma_y);
+
+	params->module_en_update |= RKISP1_CIF_ISP_MODULE_SDG;
+	params->module_ens |= RKISP1_CIF_ISP_MODULE_SDG;
+	params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_SDG;
+}
+
+REGISTER_IPA_ALGORITHM(GammaSensorLinearization)
+
+} /* namespace ipa::rkisp1::algorithms */
+
+} /* namespace libcamera */
diff --git a/src/ipa/rkisp1/algorithms/gsl.h b/src/ipa/rkisp1/algorithms/gsl.h
new file mode 100644
index 00000000..df16a89b
--- /dev/null
+++ b/src/ipa/rkisp1/algorithms/gsl.h
@@ -0,0 +1,38 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2021-2022, Ideas On Board
+ *
+ * gsl.h - RkISP1 Gamma Sensor Linearization control
+ */
+
+#pragma once
+
+#include <linux/rkisp1-config.h>
+
+#include "algorithm.h"
+
+namespace libcamera {
+
+struct IPACameraSensorInfo;
+
+namespace ipa::rkisp1::algorithms {
+
+class GammaSensorLinearization : public Algorithm
+{
+public:
+	GammaSensorLinearization();
+	~GammaSensorLinearization() = default;
+
+	int init(IPAContext &context, const YamlObject &tuningData) override;
+	void prepare(IPAContext &context, rkisp1_params_cfg *params) override;
+
+private:
+	bool tuningParameters_;
+	uint32_t gammaDx_[2];
+	std::vector<uint16_t> curveYr_;
+	std::vector<uint16_t> curveYg_;
+	std::vector<uint16_t> curveYb_;
+};
+
+} /* namespace ipa::rkisp1::algorithms */
+} /* namespace libcamera */
diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
index 7ec53d89..0597c353 100644
--- a/src/ipa/rkisp1/algorithms/meson.build
+++ b/src/ipa/rkisp1/algorithms/meson.build
@@ -4,4 +4,5 @@  rkisp1_ipa_algorithms = files([
     'agc.cpp',
     'awb.cpp',
     'blc.cpp',
+    'gsl.cpp',
 ])
diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml
index 232d8ae8..6cb84ed9 100644
--- a/src/ipa/rkisp1/data/ov5640.yaml
+++ b/src/ipa/rkisp1/data/ov5640.yaml
@@ -10,4 +10,10 @@  algorithms:
       Gr: 256
       Gb: 256
       B:  256
+  - GammaSensorLinearization:
+      x-intervals: [ 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2 ]
+      y:
+        red:   [ 0, 256, 512, 768, 1024, 1280, 1536, 1792, 2048, 2304, 2560, 2816, 3072, 3328, 3584, 3840, 4096 ]
+        green: [ 0, 256, 512, 768, 1024, 1280, 1536, 1792, 2048, 2304, 2560, 2816, 3072, 3328, 3584, 3840, 4096 ]
+        blue:  [ 0, 256, 512, 768, 1024, 1280, 1536, 1792, 2048, 2304, 2560, 2816, 3072, 3328, 3584, 3840, 4096 ]
 ...
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index d052954e..622a0d61 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -31,6 +31,7 @@ 
 #include "algorithms/algorithm.h"
 #include "algorithms/awb.h"
 #include "algorithms/blc.h"
+#include "algorithms/gsl.h"
 #include "libipa/camera_sensor_helper.h"
 
 #include "ipa_context.h"