[PATCH v4 06/11] mali-c55: Plumb the IPA module in

Dan Scally dan.scally at ideasonboard.com
Fri Dec 6 15:19:20 CET 2024


Hi Kieran - thanks for the comments

On 06/12/2024 14:05, Kieran Bingham wrote:
> Quoting Daniel Scally (2024-11-15 12:25:35)
>> From: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>>
>> Plumb the Pipeline-IPA loop in.
>>
>> Load the IPA module at camera creation time and create the loop between
>> the pipeline and the IPA.
>>
>> When a new Request is queued the IPA is asked to prepare the parameters
>> buffer, once ready it notifies the pipeline which queues the parameters
>> to the ISP along with a buffer for statistics and frames,
>>
>> Once statistics are ready they get passed to the IPA which upates its
>> settings for the next frame.
>>
>> Driveby fix an error message in the Pipeline Handler's ::freeBuffers()
>> function which reported a problem with the wrong video device in an
>> error path.
>>
>> Acked-by: Nayden Kanchev  <nayden.kanchev at arm.com>
>> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> Some comments below about TPG handling mostly, but I don't think it
> should block getting this series merged so that development can be on
> top.
>
> This is just first stage plumbing! (And the TPG comments probably apply
> more widely to how we expect to handle TPG devices throughout libcamera
> anyway)


Yeah I think now that we have the CameraSensorFactory we probably want a CameraSensorTPG class to 
handle any differences in a way that meant the pipeline handler / IPA didn't have to care what type 
of CameraSensor they had - looking at that has kinda been on my to-do list for a little while


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


Thanks!


>
>> ---
>> Changes in v4:
>>
>>          - None
>>
>> Changes in v3:
>>
>>          - Corrected the call to ipa_->configurationFile() to include a fallback
>>            file name.
>>          - Used the new CameraSensor::getSensorDelays() to fetch sensor delay
>>            values from CameraSensorProperties.
>> Changes in v2:
>>
>>          - None
>>
>>   src/libcamera/pipeline/mali-c55/mali-c55.cpp | 421 +++++++++++++++++--
>>   1 file changed, 378 insertions(+), 43 deletions(-)
>>
>> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
>> index e451204b..43ef0572 100644
>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
>> @@ -21,17 +21,23 @@
>>   #include <libcamera/camera.h>
>>   #include <libcamera/formats.h>
>>   #include <libcamera/geometry.h>
>> +#include <libcamera/property_ids.h>
>>   #include <libcamera/stream.h>
>>   
>>   #include <libcamera/ipa/core_ipa_interface.h>
>> +#include <libcamera/ipa/mali-c55_ipa_interface.h>
>> +#include <libcamera/ipa/mali-c55_ipa_proxy.h>
>>   
>>   #include "libcamera/internal/bayer_format.h"
>>   #include "libcamera/internal/camera.h"
>>   #include "libcamera/internal/camera_sensor.h"
>> +#include "libcamera/internal/delayed_controls.h"
>>   #include "libcamera/internal/device_enumerator.h"
>>   #include "libcamera/internal/framebuffer.h"
>> +#include "libcamera/internal/ipa_manager.h"
>>   #include "libcamera/internal/media_device.h"
>>   #include "libcamera/internal/pipeline_handler.h"
>> +#include "libcamera/internal/request.h"
>>   #include "libcamera/internal/v4l2_subdevice.h"
>>   #include "libcamera/internal/v4l2_videodevice.h"
>>   
>> @@ -72,6 +78,16 @@ constexpr Size kMaliC55MinSize = { 128, 128 };
>>   constexpr Size kMaliC55MaxSize = { 8192, 8192 };
>>   constexpr unsigned int kMaliC55ISPInternalFormat = MEDIA_BUS_FMT_RGB121212_1X36;
>>   
>> +struct MaliC55FrameInfo {
>> +       Request *request;
>> +
>> +       FrameBuffer *paramBuffer;
>> +       FrameBuffer *statBuffer;
>> +
>> +       bool paramsDone;
>> +       bool statsDone;
>> +};
>> +
>>   class MaliC55CameraData : public Camera::Private
>>   {
>>   public:
>> @@ -81,6 +97,7 @@ public:
>>          }
>>   
>>          int init();
>> +       int loadIPA();
>>   
>>          /* Deflect these functionalities to either TPG or CameraSensor. */
>>          const std::vector<Size> sizes(unsigned int mbusCode) const;
>> @@ -89,7 +106,7 @@ public:
>>          int pixfmtToMbusCode(const PixelFormat &pixFmt) const;
>>          const PixelFormat &bestRawFormat() const;
>>   
>> -       void updateControls();
>> +       void updateControls(const ControlInfoMap &ipaControls);
>>   
>>          PixelFormat adjustRawFormat(const PixelFormat &pixFmt) const;
>>          Size adjustRawSizes(const PixelFormat &pixFmt, const Size &rawSize) const;
>> @@ -102,8 +119,15 @@ public:
>>          Stream frStream_;
>>          Stream dsStream_;
>>   
>> +       std::unique_ptr<ipa::mali_c55::IPAProxyMaliC55> ipa_;
>> +       std::vector<IPABuffer> ipaStatBuffers_;
>> +       std::vector<IPABuffer> ipaParamBuffers_;
>> +
>> +       std::unique_ptr<DelayedControls> delayedCtrls_;
>> +
>>   private:
>>          void initTPGData();
>> +       void setSensorControls(const ControlList &sensorControls);
>>   
>>          std::string id_;
>>          std::vector<unsigned int> tpgCodes_;
>> @@ -167,6 +191,11 @@ void MaliC55CameraData::initTPGData()
>>          tpgResolution_ = tpgSizes_.back();
>>   }
>>   
>> +void MaliC55CameraData::setSensorControls(const ControlList &sensorControls)
>> +{
>> +       delayedCtrls_->push(sensorControls);
>> +}
>> +
>>   const std::vector<Size> MaliC55CameraData::sizes(unsigned int mbusCode) const
>>   {
>>          if (sensor_)
>> @@ -272,7 +301,7 @@ const PixelFormat &MaliC55CameraData::bestRawFormat() const
>>          return invalidPixFmt;
>>   }
>>   
>> -void MaliC55CameraData::updateControls()
>> +void MaliC55CameraData::updateControls(const ControlInfoMap &ipaControls)
>>   {
>>          if (!sensor_)
>>                  return;
>> @@ -290,6 +319,9 @@ void MaliC55CameraData::updateControls()
>>                  ControlInfo(ispMinCrop, sensorInfo.analogCrop,
>>                              sensorInfo.analogCrop);
>>   
>> +       for (auto const &c : ipaControls)
>> +               controls.emplace(c.first, c.second);
>> +
>>          controlInfo_ = ControlInfoMap(std::move(controls), controls::controls);
>>   }
>>   
>> @@ -343,6 +375,45 @@ Size MaliC55CameraData::adjustRawSizes(const PixelFormat &rawFmt, const Size &si
>>          return bestSize;
>>   }
>>   
>> +int MaliC55CameraData::loadIPA()
>> +{
>> +       int ret;
>> +
>> +       /* Do not initialize IPA for TPG. */
>> +       if (!sensor_)
>> +               return 0;
> I'm curious here. Most of the time controls are handled by the IPA. I
> wonder what controls we'll want to handle for A TPG device - but that
> applies to other TPG's too - and I guess it would need some further
> abstraction for the common helpers to directly control a tpg based
> subdev...
>
>> +
>> +       ipa_ = IPAManager::createIPA<ipa::mali_c55::IPAProxyMaliC55>(pipe(), 1, 1);
>> +       if (!ipa_)
>> +               return -ENOENT;
>> +
>> +       ipa_->setSensorControls.connect(this, &MaliC55CameraData::setSensorControls);
>> +
>> +       std::string ipaTuningFile = ipa_->configurationFile(sensor_->model() + ".yaml",
>> +                                                           "uncalibrated.yaml");
>> +
>> +       /* We need to inform the IPA of the sensor configuration */
>> +       ipa::mali_c55::IPAConfigInfo ipaConfig{};
>> +
>> +       ret = sensor_->sensorInfo(&ipaConfig.sensorInfo);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ipaConfig.sensorControls = sensor_->controls();
>> +
>> +       ControlInfoMap ipaControls;
>> +       ret = ipa_->init({ ipaTuningFile, sensor_->model() }, ipaConfig,
>> +                        &ipaControls);
>> +       if (ret) {
>> +               LOG(MaliC55, Error) << "Failed to initialise the Mali-C55 IPA";
>> +               return ret;
>> +       }
>> +
>> +       updateControls(ipaControls);
>> +
>> +       return 0;
>> +}
>> +
>>   class MaliC55CameraConfiguration : public CameraConfiguration
>>   {
>>   public:
>> @@ -352,6 +423,7 @@ public:
>>          }
>>   
>>          Status validate() override;
>> +       const Transform &combinedTransform() { return combinedTransform_; }
>>   
>>          V4L2SubdeviceFormat sensorFormat_;
>>   
>> @@ -359,6 +431,7 @@ private:
>>          static constexpr unsigned int kMaxStreams = 2;
>>   
>>          const MaliC55CameraData *data_;
>> +       Transform combinedTransform_;
>>   };
>>   
>>   CameraConfiguration::Status MaliC55CameraConfiguration::validate()
>> @@ -368,6 +441,19 @@ CameraConfiguration::Status MaliC55CameraConfiguration::validate()
>>          if (config_.empty())
>>                  return Invalid;
>>   
>> +       /*
>> +        * The TPG doesn't support flips, so we only need to calculate a
>> +        * transform if we have a sensor.
>> +        */
>> +       if (data_->sensor_) {
>> +               Orientation requestedOrientation = orientation;
>> +               combinedTransform_ = data_->sensor_->computeTransform(&orientation);
>> +               if (orientation != requestedOrientation)
>> +                       status = Adjusted;
>> +       } else {
>> +               combinedTransform_ = Transform::Rot0;
>> +       }
>> +
> I guess soon we could create a TPG with the 'sensor' interface from the
> CameraSensorFactory - which could clean this up - but we don't have that
> now so ... Lets keep this as is :-)
>
>
>>          /* Only 2 streams available. */
>>          if (config_.size() > kMaxStreams) {
>>                  config_.resize(kMaxStreams);
>> @@ -531,7 +617,10 @@ public:
>>          int queueRequestDevice(Camera *camera, Request *request) override;
>>   
>>          void imageBufferReady(FrameBuffer *buffer);
>> +       void paramsBufferReady(FrameBuffer *buffer);
>>          void statsBufferReady(FrameBuffer *buffer);
>> +       void paramsComputed(unsigned int requestId);
>> +       void statsProcessed(unsigned int requestId, const ControlList &metadata);
>>   
>>          bool match(DeviceEnumerator *enumerator) override;
>>   
>> @@ -576,6 +665,10 @@ private:
>>                          pipe.stream = nullptr;
>>          }
>>   
>> +       MaliC55FrameInfo *findFrameInfo(FrameBuffer *buffer);
>> +       MaliC55FrameInfo *findFrameInfo(Request *request);
>> +       void tryComplete(MaliC55FrameInfo *info);
>> +
>>          int configureRawStream(MaliC55CameraData *data,
>>                                 const StreamConfiguration &config,
>>                                 V4L2SubdeviceFormat &subdevFormat);
>> @@ -585,7 +678,7 @@ private:
>>   
>>          void applyScalerCrop(Camera *camera, const ControlList &controls);
>>   
>> -       void registerMaliCamera(std::unique_ptr<MaliC55CameraData> data,
>> +       bool registerMaliCamera(std::unique_ptr<MaliC55CameraData> data,
>>                                  const std::string &name);
>>          bool registerTPGCamera(MediaLink *link);
>>          bool registerSensorCamera(MediaLink *link);
>> @@ -601,6 +694,8 @@ private:
>>          std::vector<std::unique_ptr<FrameBuffer>> paramsBuffers_;
>>          std::queue<FrameBuffer *> availableParamsBuffers_;
>>   
>> +       std::map<unsigned int, MaliC55FrameInfo> frameInfoMap_;
>> +
>>          std::array<MaliC55Pipe, MaliC55NumPipes> pipes_;
>>   
>>          bool dsFitted_;
>> @@ -849,6 +944,13 @@ int PipelineHandlerMaliC55::configure(Camera *camera,
>>          if (ret)
>>                  return ret;
>>   
>> +       if (data->sensor_) {
>> +               ret = data->sensor_->setFormat(&subdevFormat,
>> +                                              maliConfig->combinedTransform());
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>>          if (data->csi_) {
>>                  ret = data->csi_->setFormat(0, &subdevFormat);
>>                  if (ret)
>> @@ -930,7 +1032,55 @@ int PipelineHandlerMaliC55::configure(Camera *camera,
>>                  pipe->stream = stream;
>>          }
>>   
>> -       data->updateControls();
>> +       if (!data->ipa_)
>> +               return 0;
>> +
>> +       /*
>> +        * Enable the media link between the ISP subdevice and the statistics
>> +        * video device.
>> +        */
>> +       const MediaEntity *ispEntity = isp_->entity();
>> +       ret = ispEntity->getPadByIndex(3)->links()[0]->setEnabled(true);
>> +       if (ret) {
>> +               LOG(MaliC55, Error) << "Couldn't enable statistics link";
>> +               return ret;
>> +       }
>> +
>> +       /*
>> +        * Enable the media link between the ISP subdevice and the parameters
>> +        * video device.
>> +        */
>> +       ret = ispEntity->getPadByIndex(4)->links()[0]->setEnabled(true);
>> +       if (ret) {
>> +               LOG(MaliC55, Error) << "Couldn't enable parameters link";
>> +               return ret;
>> +       }
>> +
>> +       /* We need to inform the IPA of the sensor configuration */
>> +       ipa::mali_c55::IPAConfigInfo ipaConfig{};
>> +
>> +       ret = data->sensor_->sensorInfo(&ipaConfig.sensorInfo);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ipaConfig.sensorControls = data->sensor_->controls();
>> +
>> +       /*
>> +        * And we also need to tell the IPA the bayerOrder of the data (as
>> +        * affected by any flips that we've configured)
>> +        */
>> +       const Transform &combinedTransform = maliConfig->combinedTransform();
>> +       BayerFormat::Order bayerOrder = data->sensor_->bayerOrder(combinedTransform);
>> +
>> +       ControlInfoMap ipaControls;
>> +       ret = data->ipa_->configure(ipaConfig, utils::to_underlying(bayerOrder),
>> +                                   &ipaControls);
>> +       if (ret) {
>> +               LOG(MaliC55, Error) << "Failed to configure IPA";
>> +               return ret;
>> +       }
>> +
>> +       data->updateControls(ipaControls);
>>   
>>          return 0;
>>   }
>> @@ -944,8 +1094,10 @@ int PipelineHandlerMaliC55::exportFrameBuffers(Camera *camera, Stream *stream,
>>          return pipe->cap->exportBuffers(count, buffers);
>>   }
>>   
>> -void PipelineHandlerMaliC55::freeBuffers([[maybe_unused]] Camera *camera)
>> +void PipelineHandlerMaliC55::freeBuffers(Camera *camera)
>>   {
>> +       MaliC55CameraData *data = cameraData(camera);
>> +
>>          while (!availableStatsBuffers_.empty())
>>                  availableStatsBuffers_.pop();
>>          while (!availableParamsBuffers_.empty())
>> @@ -954,11 +1106,18 @@ void PipelineHandlerMaliC55::freeBuffers([[maybe_unused]] Camera *camera)
>>          statsBuffers_.clear();
>>          paramsBuffers_.clear();
>>   
>> +       if (data->ipa_) {
>> +               data->ipa_->unmapBuffers(data->ipaStatBuffers_);
>> +               data->ipa_->unmapBuffers(data->ipaParamBuffers_);
>> +       }
>> +       data->ipaStatBuffers_.clear();
>> +       data->ipaParamBuffers_.clear();
>> +
>>          if (stats_->releaseBuffers())
>>                  LOG(MaliC55, Error) << "Failed to release stats buffers";
>>   
>>          if (params_->releaseBuffers())
>> -               LOG(MaliC55, Error) << "Failed to release stats buffers";
>> +               LOG(MaliC55, Error) << "Failed to release params buffers";
>>   
>>          return;
>>   }
>> @@ -966,6 +1125,7 @@ void PipelineHandlerMaliC55::freeBuffers([[maybe_unused]] Camera *camera)
>>   int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)
>>   {
>>          MaliC55CameraData *data = cameraData(camera);
>> +       unsigned int ipaBufferId = 1;
>>          unsigned int bufferCount;
>>          int ret;
>>   
>> @@ -978,27 +1138,51 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)
>>          if (ret < 0)
>>                  return ret;
>>   
>> -       for (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_)
>> +       for (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) {
>> +               buffer->setCookie(ipaBufferId++);
>> +               data->ipaStatBuffers_.emplace_back(buffer->cookie(),
>> +                                                  buffer->planes());
>>                  availableStatsBuffers_.push(buffer.get());
>> +       }
>>   
>>          ret = params_->allocateBuffers(bufferCount, &paramsBuffers_);
>>          if (ret < 0)
>>                  return ret;
>>   
>> -       for (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_)
>> +       for (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) {
>> +               buffer->setCookie(ipaBufferId++);
>> +               data->ipaParamBuffers_.emplace_back(buffer->cookie(),
>> +                                                   buffer->planes());
>>                  availableParamsBuffers_.push(buffer.get());
>> +       }
>> +
>> +       if (data->ipa_) {
>> +               data->ipa_->mapBuffers(data->ipaStatBuffers_, true);
>> +               data->ipa_->mapBuffers(data->ipaParamBuffers_, false);
>> +       }
>>   
>>          return 0;
>>   }
>>   
>>   int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const ControlList *controls)
>>   {
>> +       MaliC55CameraData *data = cameraData(camera);
>>          int ret;
>>   
>>          ret = allocateBuffers(camera);
>>          if (ret)
>>                  return ret;
>>   
>> +       if (data->ipa_) {
>> +               ret = data->ipa_->start();
>> +               if (ret) {
>> +                       LOG(MaliC55, Error)
>> +                               << "Failed to start IPA" << camera->id();
>> +                       freeBuffers(camera);
>> +                       return ret;
>> +               }
>> +       }
>> +
>>          for (MaliC55Pipe &pipe : pipes_) {
>>                  if (!pipe.stream)
>>                          continue;
>> @@ -1008,6 +1192,8 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control
>>                  ret = pipe.cap->importBuffers(stream->configuration().bufferCount);
>>                  if (ret) {
>>                          LOG(MaliC55, Error) << "Failed to import buffers";
>> +                       if (data->ipa_)
>> +                               data->ipa_->stop();
>>                          freeBuffers(camera);
>>                          return ret;
>>                  }
>> @@ -1015,6 +1201,8 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control
>>                  ret = pipe.cap->streamOn();
>>                  if (ret) {
>>                          LOG(MaliC55, Error) << "Failed to start stream";
>> +                       if (data->ipa_)
>> +                               data->ipa_->stop();
>>                          freeBuffers(camera);
>>                          return ret;
>>                  }
>> @@ -1024,6 +1212,9 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control
>>          if (ret) {
>>                  LOG(MaliC55, Error) << "Failed to start stats stream";
>>   
>> +               if (data->ipa_)
>> +                       data->ipa_->stop();
>> +
>>                  for (MaliC55Pipe &pipe : pipes_) {
>>                          if (pipe.stream)
>>                                  pipe.cap->streamOff();
>> @@ -1038,6 +1229,8 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control
>>                  LOG(MaliC55, Error) << "Failed to start params stream";
>>   
>>                  stats_->streamOff();
>> +               if (data->ipa_)
>> +                       data->ipa_->stop();
>>   
>>                  for (MaliC55Pipe &pipe : pipes_) {
>>                          if (pipe.stream)
>> @@ -1048,11 +1241,19 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control
>>                  return ret;
>>          }
>>   
>> +       ret = isp_->setFrameStartEnabled(true);
>> +       if (ret)
>> +               LOG(MaliC55, Error) << "Failed to enable frame start events";
>> +
>>          return 0;
>>   }
>>   
>> -void PipelineHandlerMaliC55::stopDevice([[maybe_unused]] Camera *camera)
>> +void PipelineHandlerMaliC55::stopDevice(Camera *camera)
>>   {
>> +       MaliC55CameraData *data = cameraData(camera);
>> +
>> +       isp_->setFrameStartEnabled(false);
>> +
>>          for (MaliC55Pipe &pipe : pipes_) {
>>                  if (!pipe.stream)
>>                          continue;
>> @@ -1063,6 +1264,8 @@ void PipelineHandlerMaliC55::stopDevice([[maybe_unused]] Camera *camera)
>>   
>>          stats_->streamOff();
>>          params_->streamOff();
>> +       if (data->ipa_)
>> +               data->ipa_->stop();
>>          freeBuffers(camera);
>>   }
>>   
>> @@ -1164,64 +1367,179 @@ void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,
>>   
>>   int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request)
>>   {
>> -       FrameBuffer *statsBuffer;
>> -       int ret;
>> +       MaliC55CameraData *data = cameraData(camera);
>> +
>> +       /* Do not run the IPA if the TPG is in use. */
>> +       if (!data->ipa_) {
>> +               MaliC55FrameInfo frameInfo;
>> +               frameInfo.request = request;
>> +               frameInfo.statBuffer = nullptr;
>> +               frameInfo.paramBuffer = nullptr;
>> +               frameInfo.paramsDone = true;
>> +               frameInfo.statsDone = true;
>> +
>> +               frameInfoMap_[request->sequence()] = frameInfo;
>> +
>> +               for (auto &[stream, buffer] : request->buffers()) {
>> +                       MaliC55Pipe *pipe = pipeFromStream(data, stream);
>> +
>> +                       pipe->cap->queueBuffer(buffer);
>> +               }
>> +
>> +               return 0;
>> +       }
> Makes me wonder if we should just have a 'TPG-IPA' sometime to handle
> the TPG use cases in the same way as the normal IPA.. (or have the IPA
> support the TPG) But again, out of scope here for now.


I think really this is just to avoid running things that a normal CameraSensor would have but that 
the TPG doesn't - obviously not any actual point running the IPA for a TPG but it'd be better to let 
that happen and abstract things with a CameraSensorTPG class in my opinion.

>
>>   
>>          if (availableStatsBuffers_.empty()) {
>>                  LOG(MaliC55, Error) << "Stats buffer underrun";
>>                  return -ENOENT;
>>          }
>>   
>> -       statsBuffer = availableStatsBuffers_.front();
>> +       if (availableParamsBuffers_.empty()) {
>> +               LOG(MaliC55, Error) << "Params buffer underrun";
>> +               return -ENOENT;
>> +       }
>> +
>> +       MaliC55FrameInfo frameInfo;
>> +       frameInfo.request = request;
>> +
>> +       frameInfo.statBuffer = availableStatsBuffers_.front();
>>          availableStatsBuffers_.pop();
>> +       frameInfo.paramBuffer = availableParamsBuffers_.front();
>> +       availableParamsBuffers_.pop();
>>   
>> -       /*
>> -        * We need to associate the Request to this buffer even though it's a
>> -        * purely internal one because we will need to use request->sequence()
>> -        * later.
>> -        */
>> -       statsBuffer->_d()->setRequest(request);
>> +       frameInfo.paramsDone = false;
>> +       frameInfo.statsDone = false;
>>   
>> -       for (auto &[stream, buffer] : request->buffers()) {
>> -               MaliC55Pipe *pipe = pipeFromStream(cameraData(camera), stream);
>> +       frameInfoMap_[request->sequence()] = frameInfo;
>>   
>> -               ret = pipe->cap->queueBuffer(buffer);
>> -               if (ret)
>> -                       return ret;
>> +       data->ipa_->queueRequest(request->sequence(), request->controls());
>> +       data->ipa_->fillParams(request->sequence(),
>> +                              frameInfo.paramBuffer->cookie());
>> +
>> +       return 0;
>> +}
>> +
>> +MaliC55FrameInfo *PipelineHandlerMaliC55::findFrameInfo(Request *request)
>> +{
>> +       for (auto &[sequence, info] : frameInfoMap_) {
>> +               if (info.request == request)
>> +                       return &info;
>>          }
>>   
>> -       /*
>> -        * Some controls need to be applied immediately, as in example,
>> -        * the ScalerCrop one.
>> -        *
>> -        * \todo Move it buffer queue time (likely after the IPA has filled in
>> -        * the parameters buffer) once we have plumbed the IPA loop in.
>> -        */
>> -       applyScalerCrop(camera, request->controls());
>> +       return nullptr;
>> +}
>>   
>> -       ret = stats_->queueBuffer(statsBuffer);
>> -       if (ret)
>> -               return ret;
>> +MaliC55FrameInfo *PipelineHandlerMaliC55::findFrameInfo(FrameBuffer *buffer)
>> +{
>> +       for (auto &[sequence, info] : frameInfoMap_) {
>> +               if (info.paramBuffer == buffer ||
>> +                   info.statBuffer == buffer)
>> +                       return &info;
>> +       }
>>   
>> -       return 0;
>> +       return nullptr;
>> +}
>> +
>> +void PipelineHandlerMaliC55::tryComplete(MaliC55FrameInfo *info)
>> +{
>> +       if (!info->paramsDone)
>> +               return;
>> +       if (!info->statsDone)
>> +               return;
>> +
>> +       Request *request = info->request;
>> +       if (request->hasPendingBuffers())
>> +               return;
>> +
>> +       if (info->statBuffer)
>> +               availableStatsBuffers_.push(info->statBuffer);
>> +       if (info->paramBuffer)
>> +               availableParamsBuffers_.push(info->paramBuffer);
>> +
>> +       frameInfoMap_.erase(request->sequence());
>> +
>> +       completeRequest(request);
>>   }
>>   
>>   void PipelineHandlerMaliC55::imageBufferReady(FrameBuffer *buffer)
>>   {
>>          Request *request = buffer->request();
>> +       MaliC55FrameInfo *info = findFrameInfo(request);
>> +       ASSERT(info);
>>   
>>          if (completeBuffer(request, buffer))
>> -               completeRequest(request);
>> +               tryComplete(info);
>> +}
>> +
>> +void PipelineHandlerMaliC55::paramsBufferReady(FrameBuffer *buffer)
>> +{
>> +       MaliC55FrameInfo *info = findFrameInfo(buffer);
>> +       ASSERT(info);
>> +
>> +       info->paramsDone = true;
>> +
>> +       tryComplete(info);
>>   }
>>   
>>   void PipelineHandlerMaliC55::statsBufferReady(FrameBuffer *buffer)
>>   {
>> -       availableStatsBuffers_.push(buffer);
>> +       MaliC55FrameInfo *info = findFrameInfo(buffer);
>> +       ASSERT(info);
>> +
>> +       Request *request = info->request;
>> +       MaliC55CameraData *data = cameraData(request->_d()->camera());
>> +
>> +       ControlList sensorControls = data->delayedCtrls_->get(buffer->metadata().sequence);
>> +
>> +       data->ipa_->processStats(request->sequence(), buffer->cookie(),
>> +                                sensorControls);
>>   }
>>   
>> -void PipelineHandlerMaliC55::registerMaliCamera(std::unique_ptr<MaliC55CameraData> data,
>> +void PipelineHandlerMaliC55::paramsComputed(unsigned int requestId)
>> +{
>> +       MaliC55FrameInfo &frameInfo = frameInfoMap_[requestId];
>> +       Request *request = frameInfo.request;
>> +       MaliC55CameraData *data = cameraData(request->_d()->camera());
>> +
>> +       /*
>> +        * Queue buffers for stats and params, then queue buffers to the capture
>> +        * video devices.
>> +        */
>> +
>> +       frameInfo.paramBuffer->_d()->metadata().planes()[0].bytesused =
>> +               sizeof(struct mali_c55_params_buffer);
>> +       params_->queueBuffer(frameInfo.paramBuffer);
>> +       stats_->queueBuffer(frameInfo.statBuffer);
>> +
>> +       for (auto &[stream, buffer] : request->buffers()) {
>> +               MaliC55Pipe *pipe = pipeFromStream(data, stream);
>> +
>> +               pipe->cap->queueBuffer(buffer);
>> +       }
>> +}
>> +
>> +void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId,
>> +                                           const ControlList &metadata)
>> +{
>> +       MaliC55FrameInfo &frameInfo = frameInfoMap_[requestId];
>> +
>> +       frameInfo.statsDone = true;
>> +       frameInfo.request->metadata().merge(metadata);
>> +
>> +       tryComplete(&frameInfo);
>> +}
>> +
>> +bool PipelineHandlerMaliC55::registerMaliCamera(std::unique_ptr<MaliC55CameraData> data,
>>                                                  const std::string &name)
>>   {
>> +       if (data->loadIPA())
>> +               return false;
>> +
>> +       if (data->ipa_) {
>> +               data->ipa_->statsProcessed.connect(this, &PipelineHandlerMaliC55::statsProcessed);
>> +               data->ipa_->paramsComputed.connect(this, &PipelineHandlerMaliC55::paramsComputed);
>> +       }
>> +
>>          std::set<Stream *> streams{ &data->frStream_ };
>>          if (dsFitted_)
>>                  streams.insert(&data->dsStream_);
>> @@ -1229,6 +1547,8 @@ void PipelineHandlerMaliC55::registerMaliCamera(std::unique_ptr<MaliC55CameraDat
>>          std::shared_ptr<Camera> camera = Camera::create(std::move(data),
>>                                                          name, streams);
>>          registerCamera(std::move(camera));
>> +
>> +       return true;
>>   }
>>   
>>   /*
>> @@ -1254,9 +1574,7 @@ bool PipelineHandlerMaliC55::registerTPGCamera(MediaLink *link)
>>          if (data->init())
>>                  return false;
>>   
>> -       registerMaliCamera(std::move(data), name);
>> -
>> -       return true;
>> +       return registerMaliCamera(std::move(data), name);
>>   }
>>   
>>   /*
>> @@ -1283,9 +1601,25 @@ bool PipelineHandlerMaliC55::registerSensorCamera(MediaLink *ispLink)
>>                          return false;
>>   
>>                  data->properties_ = data->sensor_->properties();
>> -               data->updateControls();
>>   
>> -               registerMaliCamera(std::move(data), sensor->name());
>> +               uint8_t exposureDelay, gainDelay, vblankDelay, hblankDelay;
>> +               data->sensor_->getSensorDelays(exposureDelay, gainDelay,
>> +                                              vblankDelay, hblankDelay);
>> +               std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
>> +                       { V4L2_CID_ANALOGUE_GAIN, { gainDelay, false } },
>> +                       { V4L2_CID_EXPOSURE, { exposureDelay, false } },
>> +               };
>> +
>> +               data->delayedCtrls_ =
>> +                       std::make_unique<DelayedControls>(data->sensor_->device(),
>> +                                                         params);
>> +               isp_->frameStart.connect(data->delayedCtrls_.get(),
>> +                                        &DelayedControls::applyControls);
>> +
>> +               /* \todo: Init properties. */
>> +
>> +               if (!registerMaliCamera(std::move(data), sensor->name()))
>> +                       return false;
>>          }
>>   
>>          return true;
>> @@ -1365,6 +1699,7 @@ bool PipelineHandlerMaliC55::match(DeviceEnumerator *enumerator)
>>          }
>>   
>>          stats_->bufferReady.connect(this, &PipelineHandlerMaliC55::statsBufferReady);
>> +       params_->bufferReady.connect(this, &PipelineHandlerMaliC55::paramsBufferReady);
>>   
>>          ispSink = isp_->entity()->getPadByIndex(0);
>>          if (!ispSink || ispSink->links().empty()) {
>> -- 
>> 2.30.2
>>


More information about the libcamera-devel mailing list