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

[Laurent Pinchart]

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 at ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder at 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_,

-- 
Regards,

Laurent Pinchart