[PATCH v7 3/4] libcamera: rkisp1: Prepare for additional camera controls

[Kieran Bingham]

Quoting Umang Jain (2024-09-06 07:34:43)
> 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 an 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 at ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 42 ++++++++++++++++++++++--
>  1 file changed, 39 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index c02c7cf3..651258e3 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -108,6 +108,8 @@ public:
>  
>         std::unique_ptr<ipa::rkisp1::IPAProxyRkISP1> ipa_;
>  
> +       ControlInfoMap ipaControls_;
> +
>  private:
>         void paramFilled(unsigned int frame, unsigned int bytesused);
>         void setSensorControls(unsigned int frame,
> @@ -183,6 +185,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_;
> @@ -364,7 +368,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;
> @@ -814,12 +818,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>         ipaConfig.sensorControls = data->sensor_->controls();
>         ipaConfig.paramFormat = paramFormat.fourcc;
>  
> -       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,
> @@ -1086,6 +1091,35 @@ int PipelineHandlerRkISP1::initLinks(Camera *camera,
>         return 0;
>  }
>  
> +/**
> + * \brief Update the camera controls
> + * \param[in] data The camera data
> + *
> + * Compute the camera controls by calculating controls which the pipeline
> + * is reponsible for and merge them with the controls computed by the IPA.
> + *
> + * This function needs data->ipaControls_ to be refreshed when a new
> + * configuration is applied to the camera by the IPA configure() function.
> + *
> + * Always call this function after IPA configure() to make sure to have a
> + * properly refreshed IPA controls list.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)
> +{
> +       ControlInfoMap::Map controls;
> +
> +       /* Add the IPA registered controls to list of camera controls. */
> +       for (const auto &ipaControl : data->ipaControls_)
> +               controls[ipaControl.first] = ipaControl.second;
> +

I guess the merge helpers are on the list - not the info map.

And I'll presume we'll see additions for the RkISP1 Pipeline handler
controls get registered in here somehow, so I'll take a look at the next
patch.

Ok - so we indeed need a hook point.


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



> +       data->controlInfo_ = ControlInfoMap(std::move(controls),
> +                                           controls::controls);
> +
> +       return 0;
> +}
> +
>  int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>  {
>         int ret;
> @@ -1122,6 +1156,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>         if (ret)
>                 return ret;
>  
> +       updateControls(data.get());
> +
>         std::set<Stream *> streams{
>                 &data->mainPathStream_,
>                 &data->selfPathStream_,
> -- 
> 2.45.0
>