[v6,3/5] libcamera: rkisp1: Prepare for additional camera controls

20712 diff mbox series

Currently the rkisp1 pipeline handler only registers controls that are
related to the IPA. This patch prepares the rkisp1 pipeline-handler to
register camera controls which are not related to the IPA.

Hence, introduce a additional ControlInfoMap for IPA controls. These
controls will be merged together with the controls in the pipeline
handler (introduced subsequently) as part of updateControls() and
together will be registered during the registration of the camera.
This is similar to what IPU3 pipeline handler handles its controls.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 28 +++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

Hi Umang,

Thank you for the patch.

On Fri, Jul 26, 2024 at 05:17:13PM +0530, Umang Jain wrote:
> Currently the rkisp1 pipeline handler only registers controls that are
> related to the IPA. This patch prepares the rkisp1 pipeline-handler to
> register camera controls which are not related to the IPA.
> 
> Hence, introduce a additional ControlInfoMap for IPA controls. These

s/a additional/an additional/

> controls will be merged together with the controls in the pipeline
> handler (introduced subsequently) as part of updateControls() and
> together will be registered during the registration of the camera.
> This is similar to what IPU3 pipeline handler handles its controls.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 28 +++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 5f94f422..25f2cc97 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -109,6 +109,8 @@ public:
>  
>  	std::unique_ptr<ipa::rkisp1::IPAProxyRkISP1> ipa_;
>  
> +	ControlInfoMap ipaControls_;
> +
>  private:
>  	void paramFilled(unsigned int frame);
>  	void setSensorControls(unsigned int frame,
> @@ -184,6 +186,8 @@ private:
>  	int allocateBuffers(Camera *camera);
>  	int freeBuffers(Camera *camera);
>  
> +	int updateControls(RkISP1CameraData *data);
> +
>  	MediaDevice *media_;
>  	std::unique_ptr<V4L2Subdevice> isp_;
>  	std::unique_ptr<V4L2VideoDevice> param_;
> @@ -370,7 +374,7 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
>  	}
>  
>  	ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,
> -			 sensorInfo, sensor_->controls(), &controlInfo_);
> +			 sensorInfo, sensor_->controls(), &ipaControls_);
>  	if (ret < 0) {
>  		LOG(RkISP1, Error) << "IPA initialization failure";
>  		return ret;
> @@ -820,12 +824,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  
>  	ipaConfig.sensorControls = data->sensor_->controls();
>  
> -	ret = data->ipa_->configure(ipaConfig, streamConfig, &data->controlInfo_);
> +	ret = data->ipa_->configure(ipaConfig, streamConfig, &data->ipaControls_);
>  	if (ret) {
>  		LOG(RkISP1, Error) << "failed configuring IPA (" << ret << ")";
>  		return ret;
>  	}
> -	return 0;
> +
> +	return updateControls(data);
>  }
>  
>  int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,
> @@ -1101,8 +1106,23 @@ int PipelineHandlerRkISP1::initLinks(Camera *camera,
>  	return 0;
>  }
>  

You may want to copy the documentation from
PipelineHandlerIPU3::updateControls().

> +int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)
> +{
> +	ControlInfoMap::Map rkisp1Controls;

s/rkisp1Controls/controls/

> +
> +	/* Add the IPA registered controls to list of camera controls. */
> +	for (const auto &ipaControl : data->ipaControls_)
> +		rkisp1Controls[ipaControl.first] = ipaControl.second;
> +
> +	data->controlInfo_ = ControlInfoMap(std::move(rkisp1Controls),
> +					    controls::controls);
> +
> +	return 0;
> +}
> +
>  int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>  {
> +	ControlInfoMap::Map rkisp1Controls;

Unused ?

>  	int ret;
>  
>  	std::unique_ptr<RkISP1CameraData> data =
> @@ -1137,6 +1157,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>  	if (ret)
>  		return ret;
>  
> +	updateControls(data.get());
> +
>  	std::set<Stream *> streams{
>  		&data->mainPathStream_,
>  		&data->selfPathStream_,