[libcamera-devel] [PATCH 3/3] ipa: rkisp1: update the uapi to the latest kernel version

Dafna Hirschfeld dafna.hirschfeld at collabora.com
Wed Feb 17 19:13:28 CET 2021



On 17.02.21 09:49, paul.elder at ideasonboard.com wrote:
> Hi Dafna,
> 
> On Tue, Feb 16, 2021 at 01:26:24PM +0100, Dafna Hirschfeld wrote:
>> In kernel 5.11 the rkisp1 uapi had changed to support
>> different hardware versions. This patch updates the
>> uapi header and adds support to hardware version 10 only.
>> In the future, support to other version will be added.
> 
> s/to/for
> 
>>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld at collabora.com>
>> ---
>>   include/libcamera/ipa/rkisp1.mojom       |  2 +-
>>   include/linux/rkisp1-config.h            | 88 +++++++++++++++++++++---
>>   src/ipa/rkisp1/rkisp1.cpp                | 16 +++--
>>   src/libcamera/pipeline/rkisp1/rkisp1.cpp |  3 +-
>>   4 files changed, 92 insertions(+), 17 deletions(-)
>>
>> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
>> index 95fa0d93..b097fc8f 100644
>> --- a/include/libcamera/ipa/rkisp1.mojom
>> +++ b/include/libcamera/ipa/rkisp1.mojom
>> @@ -29,7 +29,7 @@ interface IPARkISP1Interface {
>>   	start() => (int32 ret);
>>   	stop();
>>   
>> -	configure(CameraSensorInfo sensorInfo,
>> +	configure(uint32 hwRevision, CameraSensorInfo sensorInfo,
>>   		  map<uint32, IPAStream> streamConfig,
>>   		  map<uint32, ControlInfoMap> entityControls) => (int32 ret);
>>   
>> diff --git a/include/linux/rkisp1-config.h b/include/linux/rkisp1-config.h
>> index 9c24867d..3bccccbc 100644
>> --- a/include/linux/rkisp1-config.h
>> +++ b/include/linux/rkisp1-config.h
>> @@ -1,4 +1,4 @@
>> -/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/* SPDX-License-Identifier: ((GPL-2.0+ WITH Linux-syscall-note) OR MIT) */
>>   /*
>>    * Rockchip ISP1 userspace API
>>    * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
>> @@ -53,8 +53,14 @@
>>   #define RKISP1_CIF_ISP_CTK_COEFF_MAX            0x100
>>   #define RKISP1_CIF_ISP_CTK_OFFSET_MAX           0x800
>>   
>> -#define RKISP1_CIF_ISP_AE_MEAN_MAX              25
>> -#define RKISP1_CIF_ISP_HIST_BIN_N_MAX           16
>> +#define RKISP1_CIF_ISP_AE_MEAN_MAX_V10		25
>> +#define RKISP1_CIF_ISP_AE_MEAN_MAX_V12		81
>> +#define RKISP1_CIF_ISP_AE_MEAN_MAX		RKISP1_CIF_ISP_AE_MEAN_MAX_V12
>> +
>> +#define RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10	16
>> +#define RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12	32
>> +#define RKISP1_CIF_ISP_HIST_BIN_N_MAX		RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12
>> +
>>   #define RKISP1_CIF_ISP_AFM_MAX_WINDOWS          3
>>   #define RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE       17
>>   
>> @@ -90,7 +96,9 @@
>>    * Gamma out
>>    */
>>   /* Maximum number of color samples supported */
>> -#define RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES       17
>> +#define RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10   17
>> +#define RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12   34
>> +#define RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES       RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12
>>   
>>   /*
>>    * Lens shade correction
>> @@ -106,8 +114,9 @@
>>   /*
>>    * Histogram calculation
>>    */
>> -/* Last 3 values unused. */
>> -#define RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE 28
>> +#define RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10 25
>> +#define RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12 81
>> +#define RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE     RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12
>>   
>>   /*
>>    * Defect Pixel Cluster Correction
>> @@ -128,6 +137,21 @@
>>   #define RKISP1_CIF_ISP_STAT_AFM           (1U << 2)
>>   #define RKISP1_CIF_ISP_STAT_HIST          (1U << 3)
>>   
>> +/**
>> + * enum rkisp1_cif_isp_version - ISP variants
>> + *
>> + * @RKISP1_V10: used at least in rk3288 and rk3399
>> + * @RKISP1_V11: declared in the original vendor code, but not used
>> + * @RKISP1_V12: used at least in rk3326 and px30
>> + * @RKISP1_V13: used at least in rk1808
>> + */
>> +enum rkisp1_cif_isp_version {
>> +	RKISP1_V10 = 10,
>> +	RKISP1_V11,
>> +	RKISP1_V12,
>> +	RKISP1_V13,
>> +};
>> +
>>   enum rkisp1_cif_isp_histogram_mode {
>>   	RKISP1_CIF_ISP_HISTOGRAM_MODE_DISABLE,
>>   	RKISP1_CIF_ISP_HISTOGRAM_MODE_RGB_COMBINED,
>> @@ -514,6 +538,15 @@ enum rkisp1_cif_isp_goc_mode {
>>    *
>>    * @mode: goc mode (from enum rkisp1_cif_isp_goc_mode)
>>    * @gamma_y: gamma out curve y-axis for all color components
>> + *
>> + * The number of entries of @gamma_y depends on the hardware revision
>> + * as is reported by the hw_revision field of the struct media_device_info
>> + * that is returned by ioctl MEDIA_IOC_DEVICE_INFO.
>> + *
>> + * Versions <= V11 have RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10
>> + * entries, versions >= V12 have RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12
>> + * entries. RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES is equal to the maximum
>> + * of the two.
>>    */
>>   struct rkisp1_cif_isp_goc_config {
>>   	__u32 mode;
>> @@ -528,6 +561,15 @@ struct rkisp1_cif_isp_goc_config {
>>    *			  skipped
>>    * @meas_window: coordinates of the measure window
>>    * @hist_weight: weighting factor for sub-windows
>> + *
>> + * The number of entries of @hist_weight depends on the hardware revision
>> + * as is reported by the hw_revision field of the struct media_device_info
>> + * that is returned by ioctl MEDIA_IOC_DEVICE_INFO.
>> + *
>> + * Versions <= V11 have RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10
>> + * entries, versions >= V12 have RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12
>> + * entries. RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE is equal to the maximum
>> + * of the two.
>>    */
>>   struct rkisp1_cif_isp_hst_config {
>>   	__u32 mode;
>> @@ -815,7 +857,15 @@ struct rkisp1_cif_isp_bls_meas_val {
>>    * @exp_mean: Mean luminance value of block xx
>>    * @bls_val:  BLS measured values
>>    *
>> - * Image is divided into 5x5 blocks.
>> + * The number of entries of @exp_mean depends on the hardware revision
>> + * as is reported by the hw_revision field of the struct media_device_info
>> + * that is returned by ioctl MEDIA_IOC_DEVICE_INFO.
>> + *
>> + * Versions <= V11 have RKISP1_CIF_ISP_AE_MEAN_MAX_V10 entries,
>> + * versions >= V12 have RKISP1_CIF_ISP_AE_MEAN_MAX_V12 entries.
>> + * RKISP1_CIF_ISP_AE_MEAN_MAX is equal to the maximum of the two.
>> + *
>> + * Image is divided into 5x5 blocks on V10 and 9x9 blocks on V12.
>>    */
>>   struct rkisp1_cif_isp_ae_stat {
>>   	__u8 exp_mean[RKISP1_CIF_ISP_AE_MEAN_MAX];
>> @@ -848,13 +898,29 @@ struct rkisp1_cif_isp_af_stat {
>>   /**
>>    * struct rkisp1_cif_isp_hist_stat - statistics histogram data
>>    *
>> - * @hist_bins: measured bin counters
>> + * @hist_bins: measured bin counters. Each bin is a 20 bits unsigned fixed point
>> + *	       type. Bits 0-4 are the fractional part and bits 5-19 are the
>> + *	       integer part.
>> + *
>> + * The window of the measurements area is divided to 5x5 sub-windows for
>> + * V10/V11 and to 9x9 sub-windows for V12. The histogram is then computed for
>> + * each sub-window independently and the final result is a weighted average of
>> + * the histogram measurements on all sub-windows. The window of the
>> + * measurements area and the weight of each sub-window are configurable using
>> + * struct @rkisp1_cif_isp_hst_config.
>> + *
>> + * The histogram contains 16 bins in V10/V11 and 32 bins in V12/V13.
>> + *
>> + * The number of entries of @hist_bins depends on the hardware revision
>> + * as is reported by the hw_revision field of the struct media_device_info
>> + * that is returned by ioctl MEDIA_IOC_DEVICE_INFO.
>>    *
>> - * Measurement window divided into 25 sub-windows, set
>> - * with ISP_HIST_XXX
>> + * Versions <= V11 have RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10 entries,
>> + * versions >= V12 have RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12 entries.
>> + * RKISP1_CIF_ISP_HIST_BIN_N_MAX is equal to the maximum of the two.
>>    */
>>   struct rkisp1_cif_isp_hist_stat {
>> -	__u16 hist_bins[RKISP1_CIF_ISP_HIST_BIN_N_MAX];
>> +	__u32 hist_bins[RKISP1_CIF_ISP_HIST_BIN_N_MAX];
>>   };
>>   
>>   /**
>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
>> index b0641faf..a3ba3dcd 100644
>> --- a/src/ipa/rkisp1/rkisp1.cpp
>> +++ b/src/ipa/rkisp1/rkisp1.cpp
>> @@ -38,7 +38,7 @@ public:
>>   	int start() override { return 0; }
>>   	void stop() override {}
>>   
>> -	int configure(const CameraSensorInfo &info,
>> +	int configure(uint32_t hwRevision, const CameraSensorInfo &info,
>>   		       const std::map<uint32_t, IPAStream> &streamConfig,
>>   		       const std::map<uint32_t, ControlInfoMap> &entityControls) override;
>>   	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>> @@ -75,10 +75,17 @@ private:
>>    * assemble one. Make sure the reported sensor information are relevant
>>    * before accessing them.
>>    */
>> -int IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,
>> +int IPARkISP1::configure(uint32_t hwRevision, [[maybe_unused]] const CameraSensorInfo &info,
>>   			  [[maybe_unused]] const std::map<uint32_t, IPAStream> &streamConfig,
>>   			  const std::map<uint32_t, ControlInfoMap> &entityControls)
>>   {
>> +	/* \todo add support to other revisions */
> 
> s/to/for
> 
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> 
>> +	if (hwRevision != RKISP1_V10) {
>> +		LOG(IPARkISP1, Error) << "Hardware version " << hwRevision << " is currently"
>> +				      << " not supported";
>> +		return -1;
>> +	}
>> +
>>   	if (entityControls.empty())
>>   		return -1;
>>   
>> @@ -107,7 +114,8 @@ int IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,
>>   	gain_ = minGain_;
>>   
>>   	LOG(IPARkISP1, Info)
>> -		<< "Exposure: " << minExposure_ << "-" << maxExposure_
>> +		<< "Hw Revision: " hwRevision <<

oops, this does not compile (missing a '<<'),  it should be replaced with:
		<< "Hw Revision: " << hwRevision <<

Thanks,
Dafna


>> +		<< " Exposure: " << minExposure_ << "-" << maxExposure_
>>   		<< " Gain: " << minGain_ << "-" << maxGain_;
>>   
>>   	setControls(0);
>> @@ -217,7 +225,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; i++) {
>> +		for (int i = 0; i < RKISP1_CIF_ISP_AE_MEAN_MAX_V10; i++) {
>>   			if (ae->exp_mean[i] <= 15)
>>   				continue;
>>   
>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> index dad8e5d4..32158519 100644
>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> @@ -653,7 +653,8 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>>   	std::map<uint32_t, ControlInfoMap> entityControls;
>>   	entityControls.emplace(0, data->sensor_->controls());
>>   
>> -	ret = data->ipa_->configure(sensorInfo, streamConfig, entityControls);
>> +	ret = data->ipa_->configure(media_->hwRevision(), sensorInfo,
>> +				    streamConfig, entityControls);
>>   	if (ret) {
>>   		LOG(RkISP1, Error) << "failed configuring IPA (" << ret << ")";
>>   		return ret;
>> -- 
>> 2.17.1
>>


More information about the libcamera-devel mailing list