[PATCH v3 3/6] ipa: rkisp1: blc: Query black levels from camera sensor helper

[Paul Elder]

On Wed, Jul 03, 2024 at 03:49:51PM +0200, Stefan Klug wrote:
> As the camera sensor helper now has the ability to provide the black
> level, use it. Black levels can still be overwritten by the tuning
> file, but the direction is to remove them from the tuning files and move
> them into the sensor helpers.
> 
> Additionally interpret all values based on 16bits. The conversion to the
> scale required by the hardware is done in process(). It ensures all the
> values inside libcamera are the same scale and is in preparation for the
> i.MX8MP where black levels are based on a 20bit scale. Note that this
> breaks existing tuning files. The tuning files distributed with
> libcamera will be fixed in a later patch.
> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>

> ---
>  src/ipa/rkisp1/algorithms/blc.cpp | 54 ++++++++++++++++++++++++++-----
>  1 file changed, 46 insertions(+), 8 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp
> index d2e743541c99..b17b549c55df 100644
> --- a/src/ipa/rkisp1/algorithms/blc.cpp
> +++ b/src/ipa/rkisp1/algorithms/blc.cpp
> @@ -46,10 +46,47 @@ BlackLevelCorrection::BlackLevelCorrection()
>  int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context,
>  			       const YamlObject &tuningData)
>  {
> -	blackLevelRed_ = tuningData["R"].get<int16_t>(256);
> -	blackLevelGreenR_ = tuningData["Gr"].get<int16_t>(256);
> -	blackLevelGreenB_ = tuningData["Gb"].get<int16_t>(256);
> -	blackLevelBlue_ = tuningData["B"].get<int16_t>(256);
> +	std::optional<int16_t> levelRed = tuningData["R"].get<int16_t>();
> +	std::optional<int16_t> levelGreenR = tuningData["Gr"].get<int16_t>();
> +	std::optional<int16_t> levelGreenB = tuningData["Gb"].get<int16_t>();
> +	std::optional<int16_t> levelBlue = tuningData["B"].get<int16_t>();
> +	bool tuningHasLevels = levelRed && levelGreenR && levelGreenB && levelBlue;
> +
> +	auto blackLevel = context.camHelper->blackLevel();
> +	if (!blackLevel) {
> +		/*
> +		 * Not all camera sensor helpers have been updated with black
> +		 * levels. Print a warning and fall back to the levels from the
> +		 * tuning data to preserve backward compatibility. This should
> +		 * be removed once all helpers provide the data.
> +		 */
> +		LOG(RkISP1Blc, Warning)
> +			<< "No black levels provided by camera sensor helper"
> +			<< ", please fix";
> +
> +		blackLevelRed_ = levelRed.value_or(4096);
> +		blackLevelGreenR_ = levelGreenR.value_or(4096);
> +		blackLevelGreenB_ = levelGreenB.value_or(4096);
> +		blackLevelBlue_ = levelBlue.value_or(4096);
> +	} else if (tuningHasLevels) {
> +		/*
> +		 * If black levels are provided in the tuning file, use them to
> +		 * avoid breaking existing camera tuning. This is deprecated and
> +		 * will be removed.
> +		 */
> +		LOG(RkISP1Blc, Warning)
> +			<< "Deprecated: black levels overwritten by tuning file";
> +
> +		blackLevelRed_ = *levelRed;
> +		blackLevelGreenR_ = *levelGreenR;
> +		blackLevelGreenB_ = *levelGreenB;
> +		blackLevelBlue_ = *levelBlue;
> +	} else {
> +		blackLevelRed_ = *blackLevel;
> +		blackLevelGreenR_ = *blackLevel;
> +		blackLevelGreenB_ = *blackLevel;
> +		blackLevelBlue_ = *blackLevel;
> +	}
>  
>  	tuningParameters_ = true;
>  
> @@ -77,10 +114,11 @@ void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context,
>  		return;
>  
>  	params->others.bls_config.enable_auto = 0;
> -	params->others.bls_config.fixed_val.r = blackLevelRed_;
> -	params->others.bls_config.fixed_val.gr = blackLevelGreenR_;
> -	params->others.bls_config.fixed_val.gb = blackLevelGreenB_;
> -	params->others.bls_config.fixed_val.b = blackLevelBlue_;
> +	/* The rkisp1 uses 12bit based black levels. Scale down accordingly. */
> +	params->others.bls_config.fixed_val.r = blackLevelRed_ >> 4;
> +	params->others.bls_config.fixed_val.gr = blackLevelGreenR_ >> 4;
> +	params->others.bls_config.fixed_val.gb = blackLevelGreenB_ >> 4;
> +	params->others.bls_config.fixed_val.b = blackLevelBlue_ >> 4;
>  
>  	params->module_en_update |= RKISP1_CIF_ISP_MODULE_BLS;
>  	params->module_ens |= RKISP1_CIF_ISP_MODULE_BLS;
> -- 
> 2.43.0
>