[libcamera-devel,3/3] ipa: rkisp1: update the uapi to the latest kernel version
diff mbox series

Message ID 20210216122624.27940-4-dafna.hirschfeld@collabora.com
State Changes Requested
Headers show
Series
  • rkisp1: add an initial support to different hw revision
Related show

Commit Message

Dafna Hirschfeld Feb. 16, 2021, 12:26 p.m. UTC 
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.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@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(-)

Comments

Paul Elder Feb. 17, 2021, 8:49 a.m. UTC | #1 
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@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@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 <<
> +		<< " 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
>
Dafna Hirschfeld Feb. 17, 2021, 6:13 p.m. UTC | #2 
On 17.02.21 09:49, paul.elder@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@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@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
>>

Patch
diff mbox series 

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 */
+	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 <<
+		<< " 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;