[libcamera-devel] [PATCH] ipa: rkisp1: Add support for V12 isp blocks

[Kieran Bingham]

Hi Heiko,

On 21/06/2021 15:59, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner at theobroma-systems.com>
> 
> Some values for array sizes differ between v10 and v12, so set them
> in init() and adjust the auto exposure algorithm to the ae value
> from there.

This looks reasonable to me. It looks like some of these are not yet
used, but I don't see that as a problem in this instance, as the
implementation abstracts the platform version differences.

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>



> Signed-off-by: Heiko Stuebner <heiko.stuebner at theobroma-systems.com>
> ---
>  src/ipa/rkisp1/rkisp1.cpp | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index b47ea324..5f529334 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -66,12 +66,31 @@ private:
>  	uint32_t gain_;
>  	uint32_t minGain_;
>  	uint32_t maxGain_;
> +
> +	/* revision-specific data */
> +	int hwAeMeanMax_;
> +	int hwHistBinNMax_;
> +	int hwGammaOutMaxSamples_;
> +	int hwHistogramWeightGridsSize_;
>  };
>  
>  int IPARkISP1::init(unsigned int hwRevision)
>  {
>  	/* \todo Add support for other revisions */
> -	if (hwRevision != RKISP1_V10) {
> +	switch (hwRevision) {
> +	case RKISP1_V10:
> +		hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;
> +		hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;
> +		hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10;
> +		hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10;
> +		break;
> +	case RKISP1_V12:
> +		hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;
> +		hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12;
> +		hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12;
> +		hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12;
> +		break;
> +	default:
>  		LOG(IPARkISP1, Error)
>  			<< "Hardware revision " << hwRevision
>  			<< " is currently not supported";
> @@ -236,7 +255,7 @@ void IPARkISP1::updateStatistics(unsigned int frame,
>  
>  		unsigned int value = 0;
>  		unsigned int num = 0;
> -		for (int i = 0; i < RKISP1_CIF_ISP_AE_MEAN_MAX_V10; i++) {
> +		for (int i = 0; i < hwAeMeanMax_; i++) {
>  			if (ae->exp_mean[i] <= 15)
>  				continue;
>  
>