[libcamera-devel,28/38] libcamera: pipeline, ipa: raspberrypi: Use new data definition

9732 diff mbox series

Now that we can generate custom functions and data structures with mojo,
switch the raspberrypi pipeline handler and IPA to use the custom data
structures as defined in the mojom data definition file.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
Changes in v2:
- rebased on "libcamera: pipeline: ipa: raspberrypi: Rework drop frame
  signalling"
- use newly customized RPi IPA interface
---
 include/libcamera/ipa/raspberrypi.h           |  37 ++--
 src/ipa/raspberrypi/raspberrypi.cpp           | 152 +++++++---------
 .../pipeline/raspberrypi/raspberrypi.cpp      | 171 +++++++++---------
 3 files changed, 170 insertions(+), 190 deletions(-)

Thanks for the implementation.

On Tue, Sep 22, 2020 at 9:39 PM Paul Elder <paul.elder@ideasonboard.com>
wrote:

> Now that we can generate custom functions and data structures with mojo,
> switch the raspberrypi pipeline handler and IPA to use the custom data
> structures as defined in the mojom data definition file.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
>
> ---
> Changes in v2:
> - rebased on "libcamera: pipeline: ipa: raspberrypi: Rework drop frame
>   signalling"
> - use newly customized RPi IPA interface
> ---
>  include/libcamera/ipa/raspberrypi.h           |  37 ++--
>  src/ipa/raspberrypi/raspberrypi.cpp           | 152 +++++++---------
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 171 +++++++++---------
>  3 files changed, 170 insertions(+), 190 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.h
> b/include/libcamera/ipa/raspberrypi.h
> index 4e38d009..3357bf61 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -24,22 +24,27 @@ enum RPiBufferMask {
>  namespace libcamera {
>
>  /* List of controls handled by the Raspberry Pi IPA */
> -static const ControlInfoMap RPiControls = {
> -       { &controls::AeEnable, ControlInfo(false, true) },
> -       { &controls::ExposureTime, ControlInfo(0, 999999) },
> -       { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> -       { &controls::AeMeteringMode, ControlInfo(0,
> static_cast<int32_t>(controls::MeteringModeMax)) },
> -       { &controls::AeConstraintMode, ControlInfo(0,
> static_cast<int32_t>(controls::ConstraintModeMax)) },
> -       { &controls::AeExposureMode, ControlInfo(0,
> static_cast<int32_t>(controls::ExposureModeMax)) },
> -       { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) },
> -       { &controls::AwbEnable, ControlInfo(false, true) },
> -       { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> -       { &controls::AwbMode, ControlInfo(0,
> static_cast<int32_t>(controls::AwbModeMax)) },
> -       { &controls::Brightness, ControlInfo(-1.0f, 1.0f) },
> -       { &controls::Contrast, ControlInfo(0.0f, 32.0f) },
> -       { &controls::Saturation, ControlInfo(0.0f, 32.0f) },
> -       { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> -       { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
> +static ControlInfoMap RPiControls;
> +
> +inline void initializeRPiControls()
> +{
> +       RPiControls = {
> +               { &controls::AeEnable, ControlInfo(false, true) },
> +               { &controls::ExposureTime, ControlInfo(0, 999999) },
> +               { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> +               { &controls::AeMeteringMode, ControlInfo(0,
> static_cast<int32_t>(controls::MeteringModeMax)) },
> +               { &controls::AeConstraintMode, ControlInfo(0,
> static_cast<int32_t>(controls::ConstraintModeMax)) },
> +               { &controls::AeExposureMode, ControlInfo(0,
> static_cast<int32_t>(controls::ExposureModeMax)) },
> +               { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) },
> +               { &controls::AwbEnable, ControlInfo(false, true) },
> +               { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> +               { &controls::AwbMode, ControlInfo(0,
> static_cast<int32_t>(controls::AwbModeMax)) },
> +               { &controls::Brightness, ControlInfo(-1.0f, 1.0f) },
> +               { &controls::Contrast, ControlInfo(0.0f, 32.0f) },
> +               { &controls::Saturation, ControlInfo(0.0f, 32.0f) },
> +               { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> +               { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f,
> 16.0f) },
> +       };
>  };
>
Just out of curiosity.
Since the RPiControls is not const anymore, and each translation unit has a
copy, the content used in generated ipa_proxy_raspberrypi.cpp would be
empty.
Is it an intended behavior?

>
>  } /* namespace libcamera */
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> index 0555cc4e..805ced33 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -19,6 +19,7 @@
>  #include <libcamera/ipa/ipa_interface.h>
>  #include <libcamera/ipa/ipa_module_info.h>
>  #include <libcamera/ipa/raspberrypi.h>
> +#include <libcamera/ipa/raspberrypi_generated.h>
>  #include <libcamera/request.h>
>  #include <libcamera/span.h>
>
> @@ -60,7 +61,7 @@ namespace libcamera {
>
>  LOG_DEFINE_CATEGORY(IPARPI)
>
> -class IPARPi : public IPAInterface
> +class IPARPi : public IPARPiInterface
>  {
>  public:
>         IPARPi()
> @@ -68,6 +69,7 @@ public:
>                   frame_count_(0), check_count_(0), mistrust_count_(0),
>                   lsTable_(nullptr)
>         {
> +               initializeRPiControls();
>         }
>
>         ~IPARPi()
> @@ -82,12 +84,14 @@ public:
>
>         void configure(const CameraSensorInfo &sensorInfo,
>                        const std::map<unsigned int, IPAStream>
> &streamConfig,
> -                      const std::map<unsigned int, const ControlInfoMap
> &> &entityControls,
> -                      const IPAOperationData &data,
> -                      IPAOperationData *response) override;
> +                      const std::map<unsigned int, ControlInfoMap>
> &entityControls,
> +                      const RPiConfigInput &data,
> +                      RPiConfigOutput *response) override;
>         void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>         void unmapBuffers(const std::vector<unsigned int> &ids) override;
> -       void processEvent(const IPAOperationData &event) override;
> +       void signalStatReady(const uint32_t bufferId) override;
> +       void signalQueueRequest(const ControlList &controls) override;
> +       void signalIspPrepare(const RPiIspPreparePayload &data) override;
>
>  private:
>         void setMode(const CameraSensorInfo &sensorInfo);
> @@ -141,6 +145,11 @@ private:
>         unsigned int mistrust_count_;
>         /* LS table allocation passed in from the pipeline handler. */
>         FileDescriptor lsTableHandle_;
> +       /*
> +        * LS table allocation passed in from the pipeline handler,
> +        * in the context of the pipeline handler.
> +        */
> +       int32_t lsTableHandlePH_;
>         void *lsTable_;
>  };
>
> @@ -190,15 +199,13 @@ void IPARPi::setMode(const CameraSensorInfo
> &sensorInfo)
>
>  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>                        [[maybe_unused]] const std::map<unsigned int,
> IPAStream> &streamConfig,
> -                      const std::map<unsigned int, const ControlInfoMap
> &> &entityControls,
> -                      const IPAOperationData &ipaConfig,
> -                      IPAOperationData *result)
> +                      const std::map<unsigned int, ControlInfoMap>
> &entityControls,
> +                      const RPiConfigInput &ipaConfig,
> +                      RPiConfigOutput *result)
>  {
>         if (entityControls.empty())
>                 return;
>
> -       result->operation = 0;
> -
>         unicam_ctrls_ = entityControls.at(0);
>         isp_ctrls_ = entityControls.at(1);
>         /* Setup a metadata ControlList to output metadata. */
> @@ -220,11 +227,10 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
>                 helper_->GetDelays(exposureDelay, gainDelay);
>                 sensorMetadata = helper_->SensorEmbeddedDataPresent();
>
> -               result->data.push_back(gainDelay);
> -               result->data.push_back(exposureDelay);
> -               result->data.push_back(sensorMetadata);
> -
> -               result->operation |= RPI_IPA_CONFIG_STAGGERED_WRITE;
> +               result->op_ |= RPI_IPA_CONFIG_STAGGERED_WRITE;
> +               result->staggeredWriteResult_.gainDelay_ = gainDelay;
> +               result->staggeredWriteResult_.exposureDelay_ =
> exposureDelay;
> +               result->staggeredWriteResult_.sensorMetadata_ =
> sensorMetadata;
>         }
>
>         /* Re-assemble camera mode using the sensor info. */
> @@ -240,18 +246,15 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
>          */
>         frame_count_ = 0;
>         check_count_ = 0;
> -       unsigned int drop_frame = 0;
> +       result->op_ |= RPI_IPA_CONFIG_DROP_FRAMES;
>         if (controllerInit_) {
> -               drop_frame = helper_->HideFramesModeSwitch();
> +               result->dropFrameCount_ = helper_->HideFramesModeSwitch();
>                 mistrust_count_ = helper_->MistrustFramesModeSwitch();
>         } else {
> -               drop_frame = helper_->HideFramesStartup();
> +               result->dropFrameCount_ = helper_->HideFramesStartup();
>                 mistrust_count_ = helper_->MistrustFramesStartup();
>         }
>
> -       result->data.push_back(drop_frame);
> -       result->operation |= RPI_IPA_CONFIG_DROP_FRAMES;
> -
>         struct AgcStatus agcStatus;
>         /* These zero values mean not program anything (unless
> overwritten). */
>         agcStatus.shutter_time = 0.0;
> @@ -276,15 +279,15 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
>         if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain !=
> 0.0) {
>                 ControlList ctrls(unicam_ctrls_);
>                 applyAGC(&agcStatus, ctrls);
> -               result->controls.push_back(ctrls);
>
> -               result->operation |= RPI_IPA_CONFIG_SENSOR;
> +               result->op_ |= RPI_IPA_CONFIG_SENSOR;
> +               result->controls_ = ctrls;
>         }
>
>         lastMode_ = mode_;
>
>         /* Store the lens shading table pointer and handle if available. */
> -       if (ipaConfig.operation & RPI_IPA_CONFIG_LS_TABLE) {
> +       if (ipaConfig.op_ & RPI_IPA_CONFIG_LS_TABLE) {
>                 /* Remove any previous table, if there was one. */
>                 if (lsTable_) {
>                         munmap(lsTable_, MAX_LS_GRID_SIZE);
> @@ -292,7 +295,8 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
>                 }
>
>                 /* Map the LS table buffer into user space. */
> -               lsTableHandle_ = FileDescriptor(ipaConfig.data[0]);
> +               lsTableHandle_ = FileDescriptor(ipaConfig.lsTableHandle_);
> +               lsTableHandlePH_ = ipaConfig.lsTableHandleStatic_;
>                 if (lsTableHandle_.isValid()) {
>                         lsTable_ = mmap(nullptr, MAX_LS_GRID_SIZE,
> PROT_READ | PROT_WRITE,
>                                         MAP_SHARED, lsTableHandle_.fd(),
> 0);
> @@ -337,56 +341,38 @@ void IPARPi::unmapBuffers(const std::vector<unsigned
> int> &ids)
>         }
>  }
>
> -void IPARPi::processEvent(const IPAOperationData &event)
> +void IPARPi::signalStatReady(const uint32_t bufferId)
>  {
> -       switch (event.operation) {
> -       case RPI_IPA_EVENT_SIGNAL_STAT_READY: {
> -               unsigned int bufferId = event.data[0];
> -
> -               if (++check_count_ != frame_count_) /* assert here? */
> -                       LOG(IPARPI, Error) << "WARNING: Prepare/Process
> mismatch!!!";
> -               if (frame_count_ > mistrust_count_)
> -                       processStats(bufferId);
> -
> -               reportMetadata();
> -
> -               IPAOperationData op;
> -               op.operation = RPI_IPA_ACTION_STATS_METADATA_COMPLETE;
> -               op.data = { bufferId & RPiBufferMask::ID };
> -               op.controls = { libcameraMetadata_ };
> -               queueFrameAction.emit(0, op);
> -               break;
> -       }
> +       if (++check_count_ != frame_count_) /* assert here? */
> +               LOG(IPARPI, Error) << "WARNING: Prepare/Process
> mismatch!!!";
> +       if (frame_count_ > mistrust_count_)
> +               processStats(bufferId);
>
> -       case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE: {
> -               unsigned int embeddedbufferId = event.data[0];
> -               unsigned int bayerbufferId = event.data[1];
> +       reportMetadata();
>
> -               /*
> -                * At start-up, or after a mode-switch, we may want to
> -                * avoid running the control algos for a few frames in case
> -                * they are "unreliable".
> -                */
> -               prepareISP(embeddedbufferId);
> -               frame_count_++;
> -
> -               /* Ready to push the input buffer into the ISP. */
> -               IPAOperationData op;
> -               op.operation = RPI_IPA_ACTION_RUN_ISP;
> -               op.data = { bayerbufferId & RPiBufferMask::ID };
> -               queueFrameAction.emit(0, op);
> -               break;
> -       }
> +       statsMetadataComplete.emit(bufferId & RPiBufferMask::ID,
> libcameraMetadata_);
> +}
>
> -       case RPI_IPA_EVENT_QUEUE_REQUEST: {
> -               queueRequest(event.controls[0]);
> -               break;
> -       }
> +void IPARPi::signalQueueRequest(const ControlList &controls)
> +{
> +       queueRequest(controls);
> +}
>
> -       default:
> -               LOG(IPARPI, Error) << "Unknown event " << event.operation;
> -               break;
> -       }
> +void IPARPi::signalIspPrepare(const RPiIspPreparePayload &data)
> +{
> +       unsigned int embeddedbufferId = data.embeddedbufferId_;
> +       unsigned int bayerbufferId = data.bayerbufferId_;
> +
> +       /*
> +        * At start-up, or after a mode-switch, we may want to
> +        * avoid running the control algos for a few frames in case
> +        * they are "unreliable".
> +        */
> +       prepareISP(embeddedbufferId);
> +       frame_count_++;
> +
> +       /* Ready to push the input buffer into the ISP. */
> +       runIsp.emit(bayerbufferId & RPiBufferMask::ID);
>  }
>
>  void IPARPi::reportMetadata()
> @@ -489,6 +475,8 @@ void IPARPi::queueRequest(const ControlList &controls)
>         /* Clear the return metadata buffer. */
>         libcameraMetadata_.clear();
>
> +       LOG(IPARPI, Info) << "Request ctrl length: " << controls.size();
> +
>         for (auto const &ctrl : controls) {
>                 LOG(IPARPI, Info) << "Request ctrl: "
>                                   << controls::controls.at
> (ctrl.first)->name()
> @@ -698,10 +686,7 @@ void IPARPi::queueRequest(const ControlList &controls)
>
>  void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)
>  {
> -       IPAOperationData op;
> -       op.operation = RPI_IPA_ACTION_EMBEDDED_COMPLETE;
> -       op.data = { bufferId & RPiBufferMask::ID };
> -       queueFrameAction.emit(0, op);
> +       embeddedComplete.emit(bufferId & RPiBufferMask::ID);
>  }
>
>  void IPARPi::prepareISP(unsigned int bufferId)
> @@ -762,12 +747,8 @@ void IPARPi::prepareISP(unsigned int bufferId)
>                 if (dpcStatus)
>                         applyDPC(dpcStatus, ctrls);
>
> -               if (!ctrls.empty()) {
> -                       IPAOperationData op;
> -                       op.operation = RPI_IPA_ACTION_V4L2_SET_ISP;
> -                       op.controls.push_back(ctrls);
> -                       queueFrameAction.emit(0, op);
> -               }
> +               if (!ctrls.empty())
> +                       setIsp.emit(ctrls);
>         }
>  }
>
> @@ -823,10 +804,7 @@ void IPARPi::processStats(unsigned int bufferId)
>                 ControlList ctrls(unicam_ctrls_);
>                 applyAGC(&agcStatus, ctrls);
>
> -               IPAOperationData op;
> -               op.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED;
> -               op.controls.push_back(ctrls);
> -               queueFrameAction.emit(0, op);
> +               setStaggered.emit(ctrls);
>         }
>  }
>
> @@ -1056,7 +1034,7 @@ void IPARPi::applyLS(const struct AlscStatus
> *lsStatus, ControlList &ctrls)
>                 .grid_width = w,
>                 .grid_stride = w,
>                 .grid_height = h,
> -               .dmabuf = lsTableHandle_.fd(),
> +               .dmabuf = lsTableHandlePH_,
>                 .ref_transform = 0,
>                 .corner_sampled = 1,
>                 .gain_format = GAIN_FORMAT_U4P10
> @@ -1136,9 +1114,9 @@ const struct IPAModuleInfo ipaModuleInfo = {
>         "raspberrypi",
>  };
>
> -struct ipa_context *ipaCreate()
> +IPAInterface *ipaCreate()
>  {
> -       return new IPAInterfaceWrapper(std::make_unique<IPARPi>());
> +       return new IPARPi();
>  }
>
>  }; /* extern "C" */
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 50f07182..49e28e0c 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -16,7 +16,9 @@
>  #include <libcamera/control_ids.h>
>  #include <libcamera/file_descriptor.h>
>  #include <libcamera/formats.h>
> +#include <libcamera/ipa/ipa_proxy_raspberrypi.h>
>  #include <libcamera/ipa/raspberrypi.h>
> +#include <libcamera/ipa/raspberrypi_generated.h>
>  #include <libcamera/logging.h>
>  #include <libcamera/property_ids.h>
>  #include <libcamera/request.h>
> @@ -142,7 +144,11 @@ public:
>         int loadIPA();
>         int configureIPA();
>
> -       void queueFrameAction(unsigned int frame, const IPAOperationData
> &action);
> +       void statsMetadataComplete(uint32_t bufferId, const ControlList
> &controls);
> +       void runIsp(uint32_t bufferId);
> +       void embeddedComplete(uint32_t bufferId);
> +       void setIsp(const ControlList &controls);
> +       void setStaggered(const ControlList &controls);
>
>         /* bufferComplete signal handlers. */
>         void unicamBufferDequeue(FrameBuffer *buffer);
> @@ -154,6 +160,8 @@ public:
>         void handleExternalBuffer(FrameBuffer *buffer, RPi::RPiStream
> *stream);
>         void handleState();
>
> +       std::unique_ptr<IPAProxyRPi> ipa_;
> +
>         CameraSensor *sensor_;
>         /* Array of Unicam and ISP device streams and associated
> buffers/streams. */
>         RPi::RPiDevice<Unicam, 2> unicam_;
> @@ -356,6 +364,7 @@ CameraConfiguration::Status
> RPiCameraConfiguration::validate()
>  PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager)
>         : PipelineHandler(manager), unicam_(nullptr), isp_(nullptr)
>  {
> +       initializeRPiControls();
>  }
>
>  CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera
> *camera,
> @@ -974,11 +983,17 @@ void RPiCameraData::frameStarted(uint32_t sequence)
>
>  int RPiCameraData::loadIPA()
>  {
> -       ipa_ = IPAManager::createIPA(pipe_, 1, 1);
> +       std::unique_ptr<IPAProxy> ptr = IPAManager::createIPA(pipe_, 1, 1);
> +       ipa_ =
> std::unique_ptr<IPAProxyRPi>{static_cast<IPAProxyRPi*>(std::move(ptr).release())};
> +
>         if (!ipa_)
>                 return -ENOENT;
>
> -       ipa_->queueFrameAction.connect(this,
> &RPiCameraData::queueFrameAction);
> +       ipa_->statsMetadataComplete.connect(this,
> &RPiCameraData::statsMetadataComplete);
> +       ipa_->runIsp.connect(this, &RPiCameraData::runIsp);
> +       ipa_->embeddedComplete.connect(this,
> &RPiCameraData::embeddedComplete);
> +       ipa_->setIsp.connect(this, &RPiCameraData::setIsp);
> +       ipa_->setStaggered.connect(this, &RPiCameraData::setStaggered);
>
>         IPASettings settings{
>                 .configurationFile =
> ipa_->configurationFile(sensor_->model() + ".json")
> @@ -990,8 +1005,8 @@ int RPiCameraData::loadIPA()
>  int RPiCameraData::configureIPA()
>  {
>         std::map<unsigned int, IPAStream> streamConfig;
> -       std::map<unsigned int, const ControlInfoMap &> entityControls;
> -       IPAOperationData ipaConfig = {};
> +       std::map<unsigned int, ControlInfoMap> entityControls;
> +       RPiConfigInput ipaConfig;
>
>         /* Get the device format to pass to the IPA. */
>         V4L2DeviceFormat sensorFormat;
> @@ -1017,8 +1032,9 @@ int RPiCameraData::configureIPA()
>                         return -ENOMEM;
>
>                 /* Allow the IPA to mmap the LS table via the file
> descriptor. */
> -               ipaConfig.operation = RPI_IPA_CONFIG_LS_TABLE;
> -               ipaConfig.data = { static_cast<unsigned
> int>(lsTable_.fd()) };
> +               ipaConfig.op_ = RPI_IPA_CONFIG_LS_TABLE;
> +               ipaConfig.lsTableHandle_ = lsTable_;
> +               ipaConfig.lsTableHandleStatic_ = lsTable_.fd();
>         }
>
>         CameraSensorInfo sensorInfo = {};
> @@ -1029,105 +1045,92 @@ int RPiCameraData::configureIPA()
>         }
>
>         /* Ready the IPA - it must know about the sensor resolution. */
> -       IPAOperationData result;
> +       RPiConfigOutput result;
>
>         ipa_->configure(sensorInfo, streamConfig, entityControls,
> ipaConfig,
>                         &result);
>
> -       unsigned int resultIdx = 0;
> -       if (result.operation & RPI_IPA_CONFIG_STAGGERED_WRITE) {
> +       if (result.op_ & RPI_IPA_CONFIG_STAGGERED_WRITE) {
> +
>                 /*
>                  * Setup our staggered control writer with the sensor
> default
>                  * gain and exposure delays.
>                  */
>                 if (!staggeredCtrl_) {
>                         staggeredCtrl_.init(unicam_[Unicam::Image].dev(),
> -                                           { { V4L2_CID_ANALOGUE_GAIN,
> result.data[resultIdx++] },
> -                                             { V4L2_CID_EXPOSURE,
> result.data[resultIdx++] } });
> -                       sensorMetadata_ = result.data[resultIdx++];
> +                                       { { V4L2_CID_ANALOGUE_GAIN,
> +
>  result.staggeredWriteResult_.gainDelay_ },
> +                                       { V4L2_CID_EXPOSURE,
> +
>  result.staggeredWriteResult_.exposureDelay_ } });
> +                       sensorMetadata_ =
> result.staggeredWriteResult_.sensorMetadata_;
> +               }
> +
> +               if (result.op_ & RPI_IPA_CONFIG_SENSOR) {
> +                       const ControlList &ctrls = result.controls_;
> +                       if (!staggeredCtrl_.set(ctrls))
> +                               LOG(RPI, Error) << "V4L2 staggered set
> failed";
>                 }
> -       }
>
> -       if (result.operation & RPI_IPA_CONFIG_SENSOR) {
> -               const ControlList &ctrls = result.controls[0];
> -               if (!staggeredCtrl_.set(ctrls))
> -                       LOG(RPI, Error) << "V4L2 staggered set failed";
>         }
>
> -       if (result.operation & RPI_IPA_CONFIG_DROP_FRAMES) {
> +       if (result.op_ & RPI_IPA_CONFIG_DROP_FRAMES) {
>                 /* Configure the number of dropped frames required on
> startup. */
> -               dropFrameCount_ = result.data[resultIdx++];
> +               dropFrameCount_ = result.dropFrameCount_;
>         }
>
>         return 0;
>  }
>
> -void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame,
> -                                    const IPAOperationData &action)
> +void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const
> ControlList &controls)
>  {
> -       /*
> -        * The following actions can be handled when the pipeline handler
> is in
> -        * a stopped state.
> -        */
> -       switch (action.operation) {
> -       case RPI_IPA_ACTION_V4L2_SET_STAGGERED: {
> -               const ControlList &controls = action.controls[0];
> -               if (!staggeredCtrl_.set(controls))
> -                       LOG(RPI, Error) << "V4L2 staggered set failed";
> -               goto done;
> -       }
> +       if (state_ == State::Stopped)
> +               handleState();
>
> -       case RPI_IPA_ACTION_V4L2_SET_ISP: {
> -               ControlList controls = action.controls[0];
> -               isp_[Isp::Input].dev()->setControls(&controls);
> -               goto done;
> -       }
> -       }
> +       FrameBuffer *buffer = isp_[Isp::Stats].getBuffers().at(bufferId);
> +
> +       handleStreamBuffer(buffer, &isp_[Isp::Stats]);
> +       /* Fill the Request metadata buffer with what the IPA has provided
> */
> +       requestQueue_.front()->metadata() = std::move(controls);
> +       state_ = State::IpaComplete;
> +       handleState();
> +}
>
> +void RPiCameraData::runIsp(uint32_t bufferId)
> +{
>         if (state_ == State::Stopped)
> -               goto done;
> +               handleState();
>
> -       /*
> -        * The following actions must not be handled when the pipeline
> handler
> -        * is in a stopped state.
> -        */
> -       switch (action.operation) {
> -       case RPI_IPA_ACTION_STATS_METADATA_COMPLETE: {
> -               unsigned int bufferId = action.data[0];
> -               FrameBuffer *buffer =
> isp_[Isp::Stats].getBuffers().at(bufferId);
> -
> -               handleStreamBuffer(buffer, &isp_[Isp::Stats]);
> -               /* Fill the Request metadata buffer with what the IPA has
> provided */
> -               requestQueue_.front()->metadata() =
> std::move(action.controls[0]);
> -               state_ = State::IpaComplete;
> -               break;
> -       }
> +       FrameBuffer *buffer =
> unicam_[Unicam::Image].getBuffers().at(bufferId);
>
> -       case RPI_IPA_ACTION_EMBEDDED_COMPLETE: {
> -               unsigned int bufferId = action.data[0];
> -               FrameBuffer *buffer =
> unicam_[Unicam::Embedded].getBuffers().at(bufferId);
> -               handleStreamBuffer(buffer, &unicam_[Unicam::Embedded]);
> -               break;
> -       }
> +       LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " << bufferId
> +                       << ", timestamp: " << buffer->metadata().timestamp;
>
> -       case RPI_IPA_ACTION_RUN_ISP: {
> -               unsigned int bufferId = action.data[0];
> -               FrameBuffer *buffer =
> unicam_[Unicam::Image].getBuffers().at(bufferId);
> +       isp_[Isp::Input].queueBuffer(buffer);
> +       ispOutputCount_ = 0;
> +       handleState();
> +}
>
> -               LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " <<
> bufferId
> -                               << ", timestamp: " <<
> buffer->metadata().timestamp;
> +void RPiCameraData::embeddedComplete(uint32_t bufferId)
> +{
> +       if (state_ == State::Stopped)
> +               handleState();
>
> -               isp_[Isp::Input].queueBuffer(buffer);
> -               ispOutputCount_ = 0;
> -               break;
> -       }
> +       FrameBuffer *buffer =
> unicam_[Unicam::Embedded].getBuffers().at(bufferId);
> +       handleStreamBuffer(buffer, &unicam_[Unicam::Embedded]);
> +       handleState();
> +}
>
> -       default:
> -               LOG(RPI, Error) << "Unknown action " << action.operation;
> -               break;
> -       }
> +void RPiCameraData::setIsp(const ControlList &controls)
> +{
> +       ControlList ctrls = controls;
> +       isp_[Isp::Input].dev()->setControls(&ctrls);
> +       handleState();
> +}
>
> -done:
> +void RPiCameraData::setStaggered(const ControlList &controls)
> +{
> +       if (!staggeredCtrl_.set(controls))
> +               LOG(RPI, Error) << "V4L2 staggered set failed";
>         handleState();
>  }
>
> @@ -1227,10 +1230,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer
> *buffer)
>          * application until after the IPA signals so.
>          */
>         if (stream == &isp_[Isp::Stats]) {
> -               IPAOperationData op;
> -               op.operation = RPI_IPA_EVENT_SIGNAL_STAT_READY;
> -               op.data = { RPiBufferMask::STATS | static_cast<unsigned
> int>(index) };
> -               ipa_->processEvent(op);
> +               ipa_->signalStatReady(RPiBufferMask::STATS |
> static_cast<unsigned int>(index));
>         } else {
>                 /* Any other ISP output can be handed back to the
> application now. */
>                 handleStreamBuffer(buffer, stream);
> @@ -1403,7 +1403,6 @@ void RPiCameraData::checkRequestCompleted()
>  void RPiCameraData::tryRunPipeline()
>  {
>         FrameBuffer *bayerBuffer, *embeddedBuffer;
> -       IPAOperationData op;
>
>         /* If any of our request or buffer queues are empty, we cannot
> proceed. */
>         if (state_ != State::Idle || requestQueue_.empty() ||
> @@ -1454,9 +1453,7 @@ void RPiCameraData::tryRunPipeline()
>          * queue the ISP output buffer listed in the request to start the
> HW
>          * pipeline.
>          */
> -       op.operation = RPI_IPA_EVENT_QUEUE_REQUEST;
> -       op.controls = { request->controls() };
> -       ipa_->processEvent(op);
> +       ipa_->signalQueueRequest(request->controls());
>
>         /* Ready to use the buffers, pop them off the queue. */
>         bayerQueue_.pop();
> @@ -1472,10 +1469,10 @@ void RPiCameraData::tryRunPipeline()
>                         << " Bayer buffer id: " << bayerId
>                         << " Embedded buffer id: " << embeddedId;
>
> -       op.operation = RPI_IPA_EVENT_SIGNAL_ISP_PREPARE;
> -       op.data = { RPiBufferMask::EMBEDDED_DATA | embeddedId,
> -                   RPiBufferMask::BAYER_DATA | bayerId };
> -       ipa_->processEvent(op);
> +       RPiIspPreparePayload ispPrepare;
> +       ispPrepare.embeddedbufferId_ = RPiBufferMask::EMBEDDED_DATA |
> embeddedId;
> +       ispPrepare.bayerbufferId_ = RPiBufferMask::BAYER_DATA | bayerId;
> +       ipa_->signalIspPrepare(ispPrepare);
>  }
>
>  void RPiCameraData::tryFlushQueues()
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>

Hi Han-lin,

On Wed, Sep 30, 2020 at 08:24:37PM +0800, Han-lin Chen wrote:
> Thanks for the implementation.
> 
> On Tue, Sep 22, 2020 at 9:39 PM Paul Elder wrote:
> 
> > Now that we can generate custom functions and data structures with mojo,
> > switch the raspberrypi pipeline handler and IPA to use the custom data
> > structures as defined in the mojom data definition file.
> >
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> >
> > ---
> > Changes in v2:
> > - rebased on "libcamera: pipeline: ipa: raspberrypi: Rework drop frame
> >   signalling"
> > - use newly customized RPi IPA interface
> > ---
> >  include/libcamera/ipa/raspberrypi.h           |  37 ++--
> >  src/ipa/raspberrypi/raspberrypi.cpp           | 152 +++++++---------
> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 171 +++++++++---------
> >  3 files changed, 170 insertions(+), 190 deletions(-)
> >
> > diff --git a/include/libcamera/ipa/raspberrypi.h
> > b/include/libcamera/ipa/raspberrypi.h
> > index 4e38d009..3357bf61 100644
> > --- a/include/libcamera/ipa/raspberrypi.h
> > +++ b/include/libcamera/ipa/raspberrypi.h
> > @@ -24,22 +24,27 @@ enum RPiBufferMask {
> >  namespace libcamera {
> >
> >  /* List of controls handled by the Raspberry Pi IPA */
> > -static const ControlInfoMap RPiControls = {
> > -       { &controls::AeEnable, ControlInfo(false, true) },
> > -       { &controls::ExposureTime, ControlInfo(0, 999999) },
> > -       { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> > -       { &controls::AeMeteringMode, ControlInfo(0, static_cast<int32_t>(controls::MeteringModeMax)) },
> > -       { &controls::AeConstraintMode, ControlInfo(0, static_cast<int32_t>(controls::ConstraintModeMax)) },
> > -       { &controls::AeExposureMode, ControlInfo(0, static_cast<int32_t>(controls::ExposureModeMax)) },
> > -       { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) },
> > -       { &controls::AwbEnable, ControlInfo(false, true) },
> > -       { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> > -       { &controls::AwbMode, ControlInfo(0, static_cast<int32_t>(controls::AwbModeMax)) },
> > -       { &controls::Brightness, ControlInfo(-1.0f, 1.0f) },
> > -       { &controls::Contrast, ControlInfo(0.0f, 32.0f) },
> > -       { &controls::Saturation, ControlInfo(0.0f, 32.0f) },
> > -       { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> > -       { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
> > +static ControlInfoMap RPiControls;
> > +
> > +inline void initializeRPiControls()
> > +{
> > +       RPiControls = {
> > +               { &controls::AeEnable, ControlInfo(false, true) },
> > +               { &controls::ExposureTime, ControlInfo(0, 999999) },
> > +               { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> > +               { &controls::AeMeteringMode, ControlInfo(0, static_cast<int32_t>(controls::MeteringModeMax)) },
> > +               { &controls::AeConstraintMode, ControlInfo(0, static_cast<int32_t>(controls::ConstraintModeMax)) },
> > +               { &controls::AeExposureMode, ControlInfo(0, static_cast<int32_t>(controls::ExposureModeMax)) },
> > +               { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) },
> > +               { &controls::AwbEnable, ControlInfo(false, true) },
> > +               { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> > +               { &controls::AwbMode, ControlInfo(0, static_cast<int32_t>(controls::AwbModeMax)) },
> > +               { &controls::Brightness, ControlInfo(-1.0f, 1.0f) },
> > +               { &controls::Contrast, ControlInfo(0.0f, 32.0f) },
> > +               { &controls::Saturation, ControlInfo(0.0f, 32.0f) },
> > +               { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> > +               { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
> > +       };
> >  };
> >
> Just out of curiosity.
> Since the RPiControls is not const anymore, and each translation unit has a
> copy, the content used in generated ipa_proxy_raspberrypi.cpp would be
> empty.
> Is it an intended behavior?

I've wondered the same, and replied in a review of the next version that
I think this should either move to the pipeline handler or to the IPA,
and be passed to the other side at runtime. We'll rework this code.

> >  } /* namespace libcamera */
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> > b/src/ipa/raspberrypi/raspberrypi.cpp
> > index 0555cc4e..805ced33 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -19,6 +19,7 @@
> >  #include <libcamera/ipa/ipa_interface.h>
> >  #include <libcamera/ipa/ipa_module_info.h>
> >  #include <libcamera/ipa/raspberrypi.h>
> > +#include <libcamera/ipa/raspberrypi_generated.h>
> >  #include <libcamera/request.h>
> >  #include <libcamera/span.h>
> >
> > @@ -60,7 +61,7 @@ namespace libcamera {
> >
> >  LOG_DEFINE_CATEGORY(IPARPI)
> >
> > -class IPARPi : public IPAInterface
> > +class IPARPi : public IPARPiInterface
> >  {
> >  public:
> >         IPARPi()
> > @@ -68,6 +69,7 @@ public:
> >                   frame_count_(0), check_count_(0), mistrust_count_(0),
> >                   lsTable_(nullptr)
> >         {
> > +               initializeRPiControls();
> >         }
> >
> >         ~IPARPi()
> > @@ -82,12 +84,14 @@ public:
> >
> >         void configure(const CameraSensorInfo &sensorInfo,
> >                        const std::map<unsigned int, IPAStream> &streamConfig,
> > -                      const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> > -                      const IPAOperationData &data,
> > -                      IPAOperationData *response) override;
> > +                      const std::map<unsigned int, ControlInfoMap> &entityControls,
> > +                      const RPiConfigInput &data,
> > +                      RPiConfigOutput *response) override;
> >         void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> >         void unmapBuffers(const std::vector<unsigned int> &ids) override;
> > -       void processEvent(const IPAOperationData &event) override;
> > +       void signalStatReady(const uint32_t bufferId) override;
> > +       void signalQueueRequest(const ControlList &controls) override;
> > +       void signalIspPrepare(const RPiIspPreparePayload &data) override;
> >
> >  private:
> >         void setMode(const CameraSensorInfo &sensorInfo);
> > @@ -141,6 +145,11 @@ private:
> >         unsigned int mistrust_count_;
> >         /* LS table allocation passed in from the pipeline handler. */
> >         FileDescriptor lsTableHandle_;
> > +       /*
> > +        * LS table allocation passed in from the pipeline handler,
> > +        * in the context of the pipeline handler.
> > +        */
> > +       int32_t lsTableHandlePH_;
> >         void *lsTable_;
> >  };
> >
> > @@ -190,15 +199,13 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
> >
> >  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> >                        [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
> > -                      const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> > -                      const IPAOperationData &ipaConfig,
> > -                      IPAOperationData *result)
> > +                      const std::map<unsigned int, ControlInfoMap> &entityControls,
> > +                      const RPiConfigInput &ipaConfig,
> > +                      RPiConfigOutput *result)
> >  {
> >         if (entityControls.empty())
> >                 return;
> >
> > -       result->operation = 0;
> > -
> >         unicam_ctrls_ = entityControls.at(0);
> >         isp_ctrls_ = entityControls.at(1);
> >         /* Setup a metadata ControlList to output metadata. */
> > @@ -220,11 +227,10 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> >                 helper_->GetDelays(exposureDelay, gainDelay);
> >                 sensorMetadata = helper_->SensorEmbeddedDataPresent();
> >
> > -               result->data.push_back(gainDelay);
> > -               result->data.push_back(exposureDelay);
> > -               result->data.push_back(sensorMetadata);
> > -
> > -               result->operation |= RPI_IPA_CONFIG_STAGGERED_WRITE;
> > +               result->op_ |= RPI_IPA_CONFIG_STAGGERED_WRITE;
> > +               result->staggeredWriteResult_.gainDelay_ = gainDelay;
> > +               result->staggeredWriteResult_.exposureDelay_ = exposureDelay;
> > +               result->staggeredWriteResult_.sensorMetadata_ = sensorMetadata;
> >         }
> >
> >         /* Re-assemble camera mode using the sensor info. */
> > @@ -240,18 +246,15 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> >          */
> >         frame_count_ = 0;
> >         check_count_ = 0;
> > -       unsigned int drop_frame = 0;
> > +       result->op_ |= RPI_IPA_CONFIG_DROP_FRAMES;
> >         if (controllerInit_) {
> > -               drop_frame = helper_->HideFramesModeSwitch();
> > +               result->dropFrameCount_ = helper_->HideFramesModeSwitch();
> >                 mistrust_count_ = helper_->MistrustFramesModeSwitch();
> >         } else {
> > -               drop_frame = helper_->HideFramesStartup();
> > +               result->dropFrameCount_ = helper_->HideFramesStartup();
> >                 mistrust_count_ = helper_->MistrustFramesStartup();
> >         }
> >
> > -       result->data.push_back(drop_frame);
> > -       result->operation |= RPI_IPA_CONFIG_DROP_FRAMES;
> > -
> >         struct AgcStatus agcStatus;
> >         /* These zero values mean not program anything (unless overwritten). */
> >         agcStatus.shutter_time = 0.0;
> > @@ -276,15 +279,15 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> >         if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) {
> >                 ControlList ctrls(unicam_ctrls_);
> >                 applyAGC(&agcStatus, ctrls);
> > -               result->controls.push_back(ctrls);
> >
> > -               result->operation |= RPI_IPA_CONFIG_SENSOR;
> > +               result->op_ |= RPI_IPA_CONFIG_SENSOR;
> > +               result->controls_ = ctrls;
> >         }
> >
> >         lastMode_ = mode_;
> >
> >         /* Store the lens shading table pointer and handle if available. */
> > -       if (ipaConfig.operation & RPI_IPA_CONFIG_LS_TABLE) {
> > +       if (ipaConfig.op_ & RPI_IPA_CONFIG_LS_TABLE) {
> >                 /* Remove any previous table, if there was one. */
> >                 if (lsTable_) {
> >                         munmap(lsTable_, MAX_LS_GRID_SIZE);
> > @@ -292,7 +295,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> >                 }
> >
> >                 /* Map the LS table buffer into user space. */
> > -               lsTableHandle_ = FileDescriptor(ipaConfig.data[0]);
> > +               lsTableHandle_ = FileDescriptor(ipaConfig.lsTableHandle_);
> > +               lsTableHandlePH_ = ipaConfig.lsTableHandleStatic_;
> >                 if (lsTableHandle_.isValid()) {
> >                         lsTable_ = mmap(nullptr, MAX_LS_GRID_SIZE, PROT_READ | PROT_WRITE,
> >                                         MAP_SHARED, lsTableHandle_.fd(), 0);
> > @@ -337,56 +341,38 @@ void IPARPi::unmapBuffers(const std::vector<unsigned int> &ids)
> >         }
> >  }
> >
> > -void IPARPi::processEvent(const IPAOperationData &event)
> > +void IPARPi::signalStatReady(const uint32_t bufferId)
> >  {
> > -       switch (event.operation) {
> > -       case RPI_IPA_EVENT_SIGNAL_STAT_READY: {
> > -               unsigned int bufferId = event.data[0];
> > -
> > -               if (++check_count_ != frame_count_) /* assert here? */
> > -                       LOG(IPARPI, Error) << "WARNING: Prepare/Process mismatch!!!";
> > -               if (frame_count_ > mistrust_count_)
> > -                       processStats(bufferId);
> > -
> > -               reportMetadata();
> > -
> > -               IPAOperationData op;
> > -               op.operation = RPI_IPA_ACTION_STATS_METADATA_COMPLETE;
> > -               op.data = { bufferId & RPiBufferMask::ID };
> > -               op.controls = { libcameraMetadata_ };
> > -               queueFrameAction.emit(0, op);
> > -               break;
> > -       }
> > +       if (++check_count_ != frame_count_) /* assert here? */
> > +               LOG(IPARPI, Error) << "WARNING: Prepare/Process mismatch!!!";
> > +       if (frame_count_ > mistrust_count_)
> > +               processStats(bufferId);
> >
> > -       case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE: {
> > -               unsigned int embeddedbufferId = event.data[0];
> > -               unsigned int bayerbufferId = event.data[1];
> > +       reportMetadata();
> >
> > -               /*
> > -                * At start-up, or after a mode-switch, we may want to
> > -                * avoid running the control algos for a few frames in case
> > -                * they are "unreliable".
> > -                */
> > -               prepareISP(embeddedbufferId);
> > -               frame_count_++;
> > -
> > -               /* Ready to push the input buffer into the ISP. */
> > -               IPAOperationData op;
> > -               op.operation = RPI_IPA_ACTION_RUN_ISP;
> > -               op.data = { bayerbufferId & RPiBufferMask::ID };
> > -               queueFrameAction.emit(0, op);
> > -               break;
> > -       }
> > +       statsMetadataComplete.emit(bufferId & RPiBufferMask::ID, libcameraMetadata_);
> > +}
> >
> > -       case RPI_IPA_EVENT_QUEUE_REQUEST: {
> > -               queueRequest(event.controls[0]);
> > -               break;
> > -       }
> > +void IPARPi::signalQueueRequest(const ControlList &controls)
> > +{
> > +       queueRequest(controls);
> > +}
> >
> > -       default:
> > -               LOG(IPARPI, Error) << "Unknown event " << event.operation;
> > -               break;
> > -       }
> > +void IPARPi::signalIspPrepare(const RPiIspPreparePayload &data)
> > +{
> > +       unsigned int embeddedbufferId = data.embeddedbufferId_;
> > +       unsigned int bayerbufferId = data.bayerbufferId_;
> > +
> > +       /*
> > +        * At start-up, or after a mode-switch, we may want to
> > +        * avoid running the control algos for a few frames in case
> > +        * they are "unreliable".
> > +        */
> > +       prepareISP(embeddedbufferId);
> > +       frame_count_++;
> > +
> > +       /* Ready to push the input buffer into the ISP. */
> > +       runIsp.emit(bayerbufferId & RPiBufferMask::ID);
> >  }
> >
> >  void IPARPi::reportMetadata()
> > @@ -489,6 +475,8 @@ void IPARPi::queueRequest(const ControlList &controls)
> >         /* Clear the return metadata buffer. */
> >         libcameraMetadata_.clear();
> >
> > +       LOG(IPARPI, Info) << "Request ctrl length: " << controls.size();
> > +
> >         for (auto const &ctrl : controls) {
> >                 LOG(IPARPI, Info) << "Request ctrl: "
> >                                   << controls::controls.at (ctrl.first)->name()
> > @@ -698,10 +686,7 @@ void IPARPi::queueRequest(const ControlList &controls)
> >
> >  void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)
> >  {
> > -       IPAOperationData op;
> > -       op.operation = RPI_IPA_ACTION_EMBEDDED_COMPLETE;
> > -       op.data = { bufferId & RPiBufferMask::ID };
> > -       queueFrameAction.emit(0, op);
> > +       embeddedComplete.emit(bufferId & RPiBufferMask::ID);
> >  }
> >
> >  void IPARPi::prepareISP(unsigned int bufferId)
> > @@ -762,12 +747,8 @@ void IPARPi::prepareISP(unsigned int bufferId)
> >                 if (dpcStatus)
> >                         applyDPC(dpcStatus, ctrls);
> >
> > -               if (!ctrls.empty()) {
> > -                       IPAOperationData op;
> > -                       op.operation = RPI_IPA_ACTION_V4L2_SET_ISP;
> > -                       op.controls.push_back(ctrls);
> > -                       queueFrameAction.emit(0, op);
> > -               }
> > +               if (!ctrls.empty())
> > +                       setIsp.emit(ctrls);
> >         }
> >  }
> >
> > @@ -823,10 +804,7 @@ void IPARPi::processStats(unsigned int bufferId)
> >                 ControlList ctrls(unicam_ctrls_);
> >                 applyAGC(&agcStatus, ctrls);
> >
> > -               IPAOperationData op;
> > -               op.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED;
> > -               op.controls.push_back(ctrls);
> > -               queueFrameAction.emit(0, op);
> > +               setStaggered.emit(ctrls);
> >         }
> >  }
> >
> > @@ -1056,7 +1034,7 @@ void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls)
> >                 .grid_width = w,
> >                 .grid_stride = w,
> >                 .grid_height = h,
> > -               .dmabuf = lsTableHandle_.fd(),
> > +               .dmabuf = lsTableHandlePH_,
> >                 .ref_transform = 0,
> >                 .corner_sampled = 1,
> >                 .gain_format = GAIN_FORMAT_U4P10
> > @@ -1136,9 +1114,9 @@ const struct IPAModuleInfo ipaModuleInfo = {
> >         "raspberrypi",
> >  };
> >
> > -struct ipa_context *ipaCreate()
> > +IPAInterface *ipaCreate()
> >  {
> > -       return new IPAInterfaceWrapper(std::make_unique<IPARPi>());
> > +       return new IPARPi();
> >  }
> >
> >  }; /* extern "C" */
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 50f07182..49e28e0c 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -16,7 +16,9 @@
> >  #include <libcamera/control_ids.h>
> >  #include <libcamera/file_descriptor.h>
> >  #include <libcamera/formats.h>
> > +#include <libcamera/ipa/ipa_proxy_raspberrypi.h>
> >  #include <libcamera/ipa/raspberrypi.h>
> > +#include <libcamera/ipa/raspberrypi_generated.h>
> >  #include <libcamera/logging.h>
> >  #include <libcamera/property_ids.h>
> >  #include <libcamera/request.h>
> > @@ -142,7 +144,11 @@ public:
> >         int loadIPA();
> >         int configureIPA();
> >
> > -       void queueFrameAction(unsigned int frame, const IPAOperationData &action);
> > +       void statsMetadataComplete(uint32_t bufferId, const ControlList &controls);
> > +       void runIsp(uint32_t bufferId);
> > +       void embeddedComplete(uint32_t bufferId);
> > +       void setIsp(const ControlList &controls);
> > +       void setStaggered(const ControlList &controls);
> >
> >         /* bufferComplete signal handlers. */
> >         void unicamBufferDequeue(FrameBuffer *buffer);
> > @@ -154,6 +160,8 @@ public:
> >         void handleExternalBuffer(FrameBuffer *buffer, RPi::RPiStream *stream);
> >         void handleState();
> >
> > +       std::unique_ptr<IPAProxyRPi> ipa_;
> > +
> >         CameraSensor *sensor_;
> >         /* Array of Unicam and ISP device streams and associated buffers/streams. */
> >         RPi::RPiDevice<Unicam, 2> unicam_;
> > @@ -356,6 +364,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> >  PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager)
> >         : PipelineHandler(manager), unicam_(nullptr), isp_(nullptr)
> >  {
> > +       initializeRPiControls();
> >  }
> >
> >  CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> > @@ -974,11 +983,17 @@ void RPiCameraData::frameStarted(uint32_t sequence)
> >
> >  int RPiCameraData::loadIPA()
> >  {
> > -       ipa_ = IPAManager::createIPA(pipe_, 1, 1);
> > +       std::unique_ptr<IPAProxy> ptr = IPAManager::createIPA(pipe_, 1, 1);
> > +       ipa_ = std::unique_ptr<IPAProxyRPi>{static_cast<IPAProxyRPi*>(std::move(ptr).release())};
> > +
> >         if (!ipa_)
> >                 return -ENOENT;
> >
> > -       ipa_->queueFrameAction.connect(this, &RPiCameraData::queueFrameAction);
> > +       ipa_->statsMetadataComplete.connect(this, &RPiCameraData::statsMetadataComplete);
> > +       ipa_->runIsp.connect(this, &RPiCameraData::runIsp);
> > +       ipa_->embeddedComplete.connect(this, &RPiCameraData::embeddedComplete);
> > +       ipa_->setIsp.connect(this, &RPiCameraData::setIsp);
> > +       ipa_->setStaggered.connect(this, &RPiCameraData::setStaggered);
> >
> >         IPASettings settings{
> >                 .configurationFile = ipa_->configurationFile(sensor_->model() + ".json")
> > @@ -990,8 +1005,8 @@ int RPiCameraData::loadIPA()
> >  int RPiCameraData::configureIPA()
> >  {
> >         std::map<unsigned int, IPAStream> streamConfig;
> > -       std::map<unsigned int, const ControlInfoMap &> entityControls;
> > -       IPAOperationData ipaConfig = {};
> > +       std::map<unsigned int, ControlInfoMap> entityControls;
> > +       RPiConfigInput ipaConfig;
> >
> >         /* Get the device format to pass to the IPA. */
> >         V4L2DeviceFormat sensorFormat;
> > @@ -1017,8 +1032,9 @@ int RPiCameraData::configureIPA()
> >                         return -ENOMEM;
> >
> >                 /* Allow the IPA to mmap the LS table via the file descriptor. */
> > -               ipaConfig.operation = RPI_IPA_CONFIG_LS_TABLE;
> > -               ipaConfig.data = { static_cast<unsigned int>(lsTable_.fd()) };
> > +               ipaConfig.op_ = RPI_IPA_CONFIG_LS_TABLE;
> > +               ipaConfig.lsTableHandle_ = lsTable_;
> > +               ipaConfig.lsTableHandleStatic_ = lsTable_.fd();
> >         }
> >
> >         CameraSensorInfo sensorInfo = {};
> > @@ -1029,105 +1045,92 @@ int RPiCameraData::configureIPA()
> >         }
> >
> >         /* Ready the IPA - it must know about the sensor resolution. */
> > -       IPAOperationData result;
> > +       RPiConfigOutput result;
> >
> >         ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,
> >                         &result);
> >
> > -       unsigned int resultIdx = 0;
> > -       if (result.operation & RPI_IPA_CONFIG_STAGGERED_WRITE) {
> > +       if (result.op_ & RPI_IPA_CONFIG_STAGGERED_WRITE) {
> > +
> >                 /*
> >                  * Setup our staggered control writer with the sensor default
> >                  * gain and exposure delays.
> >                  */
> >                 if (!staggeredCtrl_) {
> >                         staggeredCtrl_.init(unicam_[Unicam::Image].dev(),
> > -                                           { { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },
> > -                                             { V4L2_CID_EXPOSURE, result.data[resultIdx++] } });
> > -                       sensorMetadata_ = result.data[resultIdx++];
> > +                                       { { V4L2_CID_ANALOGUE_GAIN,
> > +
> >  result.staggeredWriteResult_.gainDelay_ },
> > +                                       { V4L2_CID_EXPOSURE,
> > +
> >  result.staggeredWriteResult_.exposureDelay_ } });
> > +                       sensorMetadata_ = result.staggeredWriteResult_.sensorMetadata_;
> > +               }
> > +
> > +               if (result.op_ & RPI_IPA_CONFIG_SENSOR) {
> > +                       const ControlList &ctrls = result.controls_;
> > +                       if (!staggeredCtrl_.set(ctrls))
> > +                               LOG(RPI, Error) << "V4L2 staggered set failed";
> >                 }
> > -       }
> >
> > -       if (result.operation & RPI_IPA_CONFIG_SENSOR) {
> > -               const ControlList &ctrls = result.controls[0];
> > -               if (!staggeredCtrl_.set(ctrls))
> > -                       LOG(RPI, Error) << "V4L2 staggered set failed";
> >         }
> >
> > -       if (result.operation & RPI_IPA_CONFIG_DROP_FRAMES) {
> > +       if (result.op_ & RPI_IPA_CONFIG_DROP_FRAMES) {
> >                 /* Configure the number of dropped frames required on startup. */
> > -               dropFrameCount_ = result.data[resultIdx++];
> > +               dropFrameCount_ = result.dropFrameCount_;
> >         }
> >
> >         return 0;
> >  }
> >
> > -void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame,
> > -                                    const IPAOperationData &action)
> > +void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &controls)
> >  {
> > -       /*
> > -        * The following actions can be handled when the pipeline handler is in
> > -        * a stopped state.
> > -        */
> > -       switch (action.operation) {
> > -       case RPI_IPA_ACTION_V4L2_SET_STAGGERED: {
> > -               const ControlList &controls = action.controls[0];
> > -               if (!staggeredCtrl_.set(controls))
> > -                       LOG(RPI, Error) << "V4L2 staggered set failed";
> > -               goto done;
> > -       }
> > +       if (state_ == State::Stopped)
> > +               handleState();
> >
> > -       case RPI_IPA_ACTION_V4L2_SET_ISP: {
> > -               ControlList controls = action.controls[0];
> > -               isp_[Isp::Input].dev()->setControls(&controls);
> > -               goto done;
> > -       }
> > -       }
> > +       FrameBuffer *buffer = isp_[Isp::Stats].getBuffers().at(bufferId);
> > +
> > +       handleStreamBuffer(buffer, &isp_[Isp::Stats]);
> > +       /* Fill the Request metadata buffer with what the IPA has provided */
> > +       requestQueue_.front()->metadata() = std::move(controls);
> > +       state_ = State::IpaComplete;
> > +       handleState();
> > +}
> >
> > +void RPiCameraData::runIsp(uint32_t bufferId)
> > +{
> >         if (state_ == State::Stopped)
> > -               goto done;
> > +               handleState();
> >
> > -       /*
> > -        * The following actions must not be handled when the pipeline handler
> > -        * is in a stopped state.
> > -        */
> > -       switch (action.operation) {
> > -       case RPI_IPA_ACTION_STATS_METADATA_COMPLETE: {
> > -               unsigned int bufferId = action.data[0];
> > -               FrameBuffer *buffer = isp_[Isp::Stats].getBuffers().at(bufferId);
> > -
> > -               handleStreamBuffer(buffer, &isp_[Isp::Stats]);
> > -               /* Fill the Request metadata buffer with what the IPA has provided */
> > -               requestQueue_.front()->metadata() = std::move(action.controls[0]);
> > -               state_ = State::IpaComplete;
> > -               break;
> > -       }
> > +       FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers().at(bufferId);
> >
> > -       case RPI_IPA_ACTION_EMBEDDED_COMPLETE: {
> > -               unsigned int bufferId = action.data[0];
> > -               FrameBuffer *buffer = unicam_[Unicam::Embedded].getBuffers().at(bufferId);
> > -               handleStreamBuffer(buffer, &unicam_[Unicam::Embedded]);
> > -               break;
> > -       }
> > +       LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " << bufferId
> > +                       << ", timestamp: " << buffer->metadata().timestamp;
> >
> > -       case RPI_IPA_ACTION_RUN_ISP: {
> > -               unsigned int bufferId = action.data[0];
> > -               FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers().at(bufferId);
> > +       isp_[Isp::Input].queueBuffer(buffer);
> > +       ispOutputCount_ = 0;
> > +       handleState();
> > +}
> >
> > -               LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " << bufferId
> > -                               << ", timestamp: " << buffer->metadata().timestamp;
> > +void RPiCameraData::embeddedComplete(uint32_t bufferId)
> > +{
> > +       if (state_ == State::Stopped)
> > +               handleState();
> >
> > -               isp_[Isp::Input].queueBuffer(buffer);
> > -               ispOutputCount_ = 0;
> > -               break;
> > -       }
> > +       FrameBuffer *buffer = unicam_[Unicam::Embedded].getBuffers().at(bufferId);
> > +       handleStreamBuffer(buffer, &unicam_[Unicam::Embedded]);
> > +       handleState();
> > +}
> >
> > -       default:
> > -               LOG(RPI, Error) << "Unknown action " << action.operation;
> > -               break;
> > -       }
> > +void RPiCameraData::setIsp(const ControlList &controls)
> > +{
> > +       ControlList ctrls = controls;
> > +       isp_[Isp::Input].dev()->setControls(&ctrls);
> > +       handleState();
> > +}
> >
> > -done:
> > +void RPiCameraData::setStaggered(const ControlList &controls)
> > +{
> > +       if (!staggeredCtrl_.set(controls))
> > +               LOG(RPI, Error) << "V4L2 staggered set failed";
> >         handleState();
> >  }
> >
> > @@ -1227,10 +1230,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)
> >          * application until after the IPA signals so.
> >          */
> >         if (stream == &isp_[Isp::Stats]) {
> > -               IPAOperationData op;
> > -               op.operation = RPI_IPA_EVENT_SIGNAL_STAT_READY;
> > -               op.data = { RPiBufferMask::STATS | static_cast<unsigned int>(index) };
> > -               ipa_->processEvent(op);
> > +               ipa_->signalStatReady(RPiBufferMask::STATS | static_cast<unsigned int>(index));
> >         } else {
> >                 /* Any other ISP output can be handed back to the application now. */
> >                 handleStreamBuffer(buffer, stream);
> > @@ -1403,7 +1403,6 @@ void RPiCameraData::checkRequestCompleted()
> >  void RPiCameraData::tryRunPipeline()
> >  {
> >         FrameBuffer *bayerBuffer, *embeddedBuffer;
> > -       IPAOperationData op;
> >
> >         /* If any of our request or buffer queues are empty, we cannot proceed. */
> >         if (state_ != State::Idle || requestQueue_.empty() ||
> > @@ -1454,9 +1453,7 @@ void RPiCameraData::tryRunPipeline()
> >          * queue the ISP output buffer listed in the request to start the HW
> >          * pipeline.
> >          */
> > -       op.operation = RPI_IPA_EVENT_QUEUE_REQUEST;
> > -       op.controls = { request->controls() };
> > -       ipa_->processEvent(op);
> > +       ipa_->signalQueueRequest(request->controls());
> >
> >         /* Ready to use the buffers, pop them off the queue. */
> >         bayerQueue_.pop();
> > @@ -1472,10 +1469,10 @@ void RPiCameraData::tryRunPipeline()
> >                         << " Bayer buffer id: " << bayerId
> >                         << " Embedded buffer id: " << embeddedId;
> >
> > -       op.operation = RPI_IPA_EVENT_SIGNAL_ISP_PREPARE;
> > -       op.data = { RPiBufferMask::EMBEDDED_DATA | embeddedId,
> > -                   RPiBufferMask::BAYER_DATA | bayerId };
> > -       ipa_->processEvent(op);
> > +       RPiIspPreparePayload ispPrepare;
> > +       ispPrepare.embeddedbufferId_ = RPiBufferMask::EMBEDDED_DATA | embeddedId;
> > +       ispPrepare.bayerbufferId_ = RPiBufferMask::BAYER_DATA | bayerId;
> > +       ipa_->signalIspPrepare(ispPrepare);
> >  }
> >
> >  void RPiCameraData::tryFlushQueues()

On Mon, Oct 5, 2020 at 5:50 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Han-lin,
>
> On Wed, Sep 30, 2020 at 08:24:37PM +0800, Han-lin Chen wrote:
> > Thanks for the implementation.
> >
> > On Tue, Sep 22, 2020 at 9:39 PM Paul Elder wrote:
> >
> > > Now that we can generate custom functions and data structures with mojo,
> > > switch the raspberrypi pipeline handler and IPA to use the custom data
> > > structures as defined in the mojom data definition file.
> > >
> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > >
> > > ---
> > > Changes in v2:
> > > - rebased on "libcamera: pipeline: ipa: raspberrypi: Rework drop frame
> > >   signalling"
> > > - use newly customized RPi IPA interface
> > > ---
> > >  include/libcamera/ipa/raspberrypi.h           |  37 ++--
> > >  src/ipa/raspberrypi/raspberrypi.cpp           | 152 +++++++---------
> > >  .../pipeline/raspberrypi/raspberrypi.cpp      | 171 +++++++++---------
> > >  3 files changed, 170 insertions(+), 190 deletions(-)
> > >
> > > diff --git a/include/libcamera/ipa/raspberrypi.h
> > > b/include/libcamera/ipa/raspberrypi.h
> > > index 4e38d009..3357bf61 100644
> > > --- a/include/libcamera/ipa/raspberrypi.h
> > > +++ b/include/libcamera/ipa/raspberrypi.h
> > > @@ -24,22 +24,27 @@ enum RPiBufferMask {
> > >  namespace libcamera {
> > >
> > >  /* List of controls handled by the Raspberry Pi IPA */
> > > -static const ControlInfoMap RPiControls = {
> > > -       { &controls::AeEnable, ControlInfo(false, true) },
> > > -       { &controls::ExposureTime, ControlInfo(0, 999999) },
> > > -       { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> > > -       { &controls::AeMeteringMode, ControlInfo(0, static_cast<int32_t>(controls::MeteringModeMax)) },
> > > -       { &controls::AeConstraintMode, ControlInfo(0, static_cast<int32_t>(controls::ConstraintModeMax)) },
> > > -       { &controls::AeExposureMode, ControlInfo(0, static_cast<int32_t>(controls::ExposureModeMax)) },
> > > -       { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) },
> > > -       { &controls::AwbEnable, ControlInfo(false, true) },
> > > -       { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> > > -       { &controls::AwbMode, ControlInfo(0, static_cast<int32_t>(controls::AwbModeMax)) },
> > > -       { &controls::Brightness, ControlInfo(-1.0f, 1.0f) },
> > > -       { &controls::Contrast, ControlInfo(0.0f, 32.0f) },
> > > -       { &controls::Saturation, ControlInfo(0.0f, 32.0f) },
> > > -       { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> > > -       { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
> > > +static ControlInfoMap RPiControls;
> > > +
> > > +inline void initializeRPiControls()
> > > +{
> > > +       RPiControls = {
> > > +               { &controls::AeEnable, ControlInfo(false, true) },
> > > +               { &controls::ExposureTime, ControlInfo(0, 999999) },
> > > +               { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> > > +               { &controls::AeMeteringMode, ControlInfo(0, static_cast<int32_t>(controls::MeteringModeMax)) },
> > > +               { &controls::AeConstraintMode, ControlInfo(0, static_cast<int32_t>(controls::ConstraintModeMax)) },
> > > +               { &controls::AeExposureMode, ControlInfo(0, static_cast<int32_t>(controls::ExposureModeMax)) },
> > > +               { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) },
> > > +               { &controls::AwbEnable, ControlInfo(false, true) },
> > > +               { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> > > +               { &controls::AwbMode, ControlInfo(0, static_cast<int32_t>(controls::AwbModeMax)) },
> > > +               { &controls::Brightness, ControlInfo(-1.0f, 1.0f) },
> > > +               { &controls::Contrast, ControlInfo(0.0f, 32.0f) },
> > > +               { &controls::Saturation, ControlInfo(0.0f, 32.0f) },
> > > +               { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> > > +               { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
> > > +       };
> > >  };
> > >
> > Just out of curiosity.
> > Since the RPiControls is not const anymore, and each translation unit has a
> > copy, the content used in generated ipa_proxy_raspberrypi.cpp would be
> > empty.
> > Is it an intended behavior?
>
> I've wondered the same, and replied in a review of the next version that
> I think this should either move to the pipeline handler or to the IPA,
> and be passed to the other side at runtime. We'll rework this code.
Thanks! Will follow up on the new thread.
>
> > >  } /* namespace libcamera */
> > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> > > b/src/ipa/raspberrypi/raspberrypi.cpp
> > > index 0555cc4e..805ced33 100644
> > > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > > @@ -19,6 +19,7 @@
> > >  #include <libcamera/ipa/ipa_interface.h>
> > >  #include <libcamera/ipa/ipa_module_info.h>
> > >  #include <libcamera/ipa/raspberrypi.h>
> > > +#include <libcamera/ipa/raspberrypi_generated.h>
> > >  #include <libcamera/request.h>
> > >  #include <libcamera/span.h>
> > >
> > > @@ -60,7 +61,7 @@ namespace libcamera {
> > >
> > >  LOG_DEFINE_CATEGORY(IPARPI)
> > >
> > > -class IPARPi : public IPAInterface
> > > +class IPARPi : public IPARPiInterface
> > >  {
> > >  public:
> > >         IPARPi()
> > > @@ -68,6 +69,7 @@ public:
> > >                   frame_count_(0), check_count_(0), mistrust_count_(0),
> > >                   lsTable_(nullptr)
> > >         {
> > > +               initializeRPiControls();
> > >         }
> > >
> > >         ~IPARPi()
> > > @@ -82,12 +84,14 @@ public:
> > >
> > >         void configure(const CameraSensorInfo &sensorInfo,
> > >                        const std::map<unsigned int, IPAStream> &streamConfig,
> > > -                      const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> > > -                      const IPAOperationData &data,
> > > -                      IPAOperationData *response) override;
> > > +                      const std::map<unsigned int, ControlInfoMap> &entityControls,
> > > +                      const RPiConfigInput &data,
> > > +                      RPiConfigOutput *response) override;
> > >         void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> > >         void unmapBuffers(const std::vector<unsigned int> &ids) override;
> > > -       void processEvent(const IPAOperationData &event) override;
> > > +       void signalStatReady(const uint32_t bufferId) override;
> > > +       void signalQueueRequest(const ControlList &controls) override;
> > > +       void signalIspPrepare(const RPiIspPreparePayload &data) override;
> > >
> > >  private:
> > >         void setMode(const CameraSensorInfo &sensorInfo);
> > > @@ -141,6 +145,11 @@ private:
> > >         unsigned int mistrust_count_;
> > >         /* LS table allocation passed in from the pipeline handler. */
> > >         FileDescriptor lsTableHandle_;
> > > +       /*
> > > +        * LS table allocation passed in from the pipeline handler,
> > > +        * in the context of the pipeline handler.
> > > +        */
> > > +       int32_t lsTableHandlePH_;
> > >         void *lsTable_;
> > >  };
> > >
> > > @@ -190,15 +199,13 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
> > >
> > >  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> > >                        [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
> > > -                      const std::map<unsigned int, const ControlInfoMap &> &entityControls,
> > > -                      const IPAOperationData &ipaConfig,
> > > -                      IPAOperationData *result)
> > > +                      const std::map<unsigned int, ControlInfoMap> &entityControls,
> > > +                      const RPiConfigInput &ipaConfig,
> > > +                      RPiConfigOutput *result)
> > >  {
> > >         if (entityControls.empty())
> > >                 return;
> > >
> > > -       result->operation = 0;
> > > -
> > >         unicam_ctrls_ = entityControls.at(0);
> > >         isp_ctrls_ = entityControls.at(1);
> > >         /* Setup a metadata ControlList to output metadata. */
> > > @@ -220,11 +227,10 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> > >                 helper_->GetDelays(exposureDelay, gainDelay);
> > >                 sensorMetadata = helper_->SensorEmbeddedDataPresent();
> > >
> > > -               result->data.push_back(gainDelay);
> > > -               result->data.push_back(exposureDelay);
> > > -               result->data.push_back(sensorMetadata);
> > > -
> > > -               result->operation |= RPI_IPA_CONFIG_STAGGERED_WRITE;
> > > +               result->op_ |= RPI_IPA_CONFIG_STAGGERED_WRITE;
> > > +               result->staggeredWriteResult_.gainDelay_ = gainDelay;
> > > +               result->staggeredWriteResult_.exposureDelay_ = exposureDelay;
> > > +               result->staggeredWriteResult_.sensorMetadata_ = sensorMetadata;
> > >         }
> > >
> > >         /* Re-assemble camera mode using the sensor info. */
> > > @@ -240,18 +246,15 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> > >          */
> > >         frame_count_ = 0;
> > >         check_count_ = 0;
> > > -       unsigned int drop_frame = 0;
> > > +       result->op_ |= RPI_IPA_CONFIG_DROP_FRAMES;
> > >         if (controllerInit_) {
> > > -               drop_frame = helper_->HideFramesModeSwitch();
> > > +               result->dropFrameCount_ = helper_->HideFramesModeSwitch();
> > >                 mistrust_count_ = helper_->MistrustFramesModeSwitch();
> > >         } else {
> > > -               drop_frame = helper_->HideFramesStartup();
> > > +               result->dropFrameCount_ = helper_->HideFramesStartup();
> > >                 mistrust_count_ = helper_->MistrustFramesStartup();
> > >         }
> > >
> > > -       result->data.push_back(drop_frame);
> > > -       result->operation |= RPI_IPA_CONFIG_DROP_FRAMES;
> > > -
> > >         struct AgcStatus agcStatus;
> > >         /* These zero values mean not program anything (unless overwritten). */
> > >         agcStatus.shutter_time = 0.0;
> > > @@ -276,15 +279,15 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> > >         if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) {
> > >                 ControlList ctrls(unicam_ctrls_);
> > >                 applyAGC(&agcStatus, ctrls);
> > > -               result->controls.push_back(ctrls);
> > >
> > > -               result->operation |= RPI_IPA_CONFIG_SENSOR;
> > > +               result->op_ |= RPI_IPA_CONFIG_SENSOR;
> > > +               result->controls_ = ctrls;
> > >         }
> > >
> > >         lastMode_ = mode_;
> > >
> > >         /* Store the lens shading table pointer and handle if available. */
> > > -       if (ipaConfig.operation & RPI_IPA_CONFIG_LS_TABLE) {
> > > +       if (ipaConfig.op_ & RPI_IPA_CONFIG_LS_TABLE) {
> > >                 /* Remove any previous table, if there was one. */
> > >                 if (lsTable_) {
> > >                         munmap(lsTable_, MAX_LS_GRID_SIZE);
> > > @@ -292,7 +295,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> > >                 }
> > >
> > >                 /* Map the LS table buffer into user space. */
> > > -               lsTableHandle_ = FileDescriptor(ipaConfig.data[0]);
> > > +               lsTableHandle_ = FileDescriptor(ipaConfig.lsTableHandle_);
> > > +               lsTableHandlePH_ = ipaConfig.lsTableHandleStatic_;
> > >                 if (lsTableHandle_.isValid()) {
> > >                         lsTable_ = mmap(nullptr, MAX_LS_GRID_SIZE, PROT_READ | PROT_WRITE,
> > >                                         MAP_SHARED, lsTableHandle_.fd(), 0);
> > > @@ -337,56 +341,38 @@ void IPARPi::unmapBuffers(const std::vector<unsigned int> &ids)
> > >         }
> > >  }
> > >
> > > -void IPARPi::processEvent(const IPAOperationData &event)
> > > +void IPARPi::signalStatReady(const uint32_t bufferId)
> > >  {
> > > -       switch (event.operation) {
> > > -       case RPI_IPA_EVENT_SIGNAL_STAT_READY: {
> > > -               unsigned int bufferId = event.data[0];
> > > -
> > > -               if (++check_count_ != frame_count_) /* assert here? */
> > > -                       LOG(IPARPI, Error) << "WARNING: Prepare/Process mismatch!!!";
> > > -               if (frame_count_ > mistrust_count_)
> > > -                       processStats(bufferId);
> > > -
> > > -               reportMetadata();
> > > -
> > > -               IPAOperationData op;
> > > -               op.operation = RPI_IPA_ACTION_STATS_METADATA_COMPLETE;
> > > -               op.data = { bufferId & RPiBufferMask::ID };
> > > -               op.controls = { libcameraMetadata_ };
> > > -               queueFrameAction.emit(0, op);
> > > -               break;
> > > -       }
> > > +       if (++check_count_ != frame_count_) /* assert here? */
> > > +               LOG(IPARPI, Error) << "WARNING: Prepare/Process mismatch!!!";
> > > +       if (frame_count_ > mistrust_count_)
> > > +               processStats(bufferId);
> > >
> > > -       case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE: {
> > > -               unsigned int embeddedbufferId = event.data[0];
> > > -               unsigned int bayerbufferId = event.data[1];
> > > +       reportMetadata();
> > >
> > > -               /*
> > > -                * At start-up, or after a mode-switch, we may want to
> > > -                * avoid running the control algos for a few frames in case
> > > -                * they are "unreliable".
> > > -                */
> > > -               prepareISP(embeddedbufferId);
> > > -               frame_count_++;
> > > -
> > > -               /* Ready to push the input buffer into the ISP. */
> > > -               IPAOperationData op;
> > > -               op.operation = RPI_IPA_ACTION_RUN_ISP;
> > > -               op.data = { bayerbufferId & RPiBufferMask::ID };
> > > -               queueFrameAction.emit(0, op);
> > > -               break;
> > > -       }
> > > +       statsMetadataComplete.emit(bufferId & RPiBufferMask::ID, libcameraMetadata_);
> > > +}
> > >
> > > -       case RPI_IPA_EVENT_QUEUE_REQUEST: {
> > > -               queueRequest(event.controls[0]);
> > > -               break;
> > > -       }
> > > +void IPARPi::signalQueueRequest(const ControlList &controls)
> > > +{
> > > +       queueRequest(controls);
> > > +}
> > >
> > > -       default:
> > > -               LOG(IPARPI, Error) << "Unknown event " << event.operation;
> > > -               break;
> > > -       }
> > > +void IPARPi::signalIspPrepare(const RPiIspPreparePayload &data)
> > > +{
> > > +       unsigned int embeddedbufferId = data.embeddedbufferId_;
> > > +       unsigned int bayerbufferId = data.bayerbufferId_;
> > > +
> > > +       /*
> > > +        * At start-up, or after a mode-switch, we may want to
> > > +        * avoid running the control algos for a few frames in case
> > > +        * they are "unreliable".
> > > +        */
> > > +       prepareISP(embeddedbufferId);
> > > +       frame_count_++;
> > > +
> > > +       /* Ready to push the input buffer into the ISP. */
> > > +       runIsp.emit(bayerbufferId & RPiBufferMask::ID);
> > >  }
> > >
> > >  void IPARPi::reportMetadata()
> > > @@ -489,6 +475,8 @@ void IPARPi::queueRequest(const ControlList &controls)
> > >         /* Clear the return metadata buffer. */
> > >         libcameraMetadata_.clear();
> > >
> > > +       LOG(IPARPI, Info) << "Request ctrl length: " << controls.size();
> > > +
> > >         for (auto const &ctrl : controls) {
> > >                 LOG(IPARPI, Info) << "Request ctrl: "
> > >                                   << controls::controls.at (ctrl.first)->name()
> > > @@ -698,10 +686,7 @@ void IPARPi::queueRequest(const ControlList &controls)
> > >
> > >  void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)
> > >  {
> > > -       IPAOperationData op;
> > > -       op.operation = RPI_IPA_ACTION_EMBEDDED_COMPLETE;
> > > -       op.data = { bufferId & RPiBufferMask::ID };
> > > -       queueFrameAction.emit(0, op);
> > > +       embeddedComplete.emit(bufferId & RPiBufferMask::ID);
> > >  }
> > >
> > >  void IPARPi::prepareISP(unsigned int bufferId)
> > > @@ -762,12 +747,8 @@ void IPARPi::prepareISP(unsigned int bufferId)
> > >                 if (dpcStatus)
> > >                         applyDPC(dpcStatus, ctrls);
> > >
> > > -               if (!ctrls.empty()) {
> > > -                       IPAOperationData op;
> > > -                       op.operation = RPI_IPA_ACTION_V4L2_SET_ISP;
> > > -                       op.controls.push_back(ctrls);
> > > -                       queueFrameAction.emit(0, op);
> > > -               }
> > > +               if (!ctrls.empty())
> > > +                       setIsp.emit(ctrls);
> > >         }
> > >  }
> > >
> > > @@ -823,10 +804,7 @@ void IPARPi::processStats(unsigned int bufferId)
> > >                 ControlList ctrls(unicam_ctrls_);
> > >                 applyAGC(&agcStatus, ctrls);
> > >
> > > -               IPAOperationData op;
> > > -               op.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED;
> > > -               op.controls.push_back(ctrls);
> > > -               queueFrameAction.emit(0, op);
> > > +               setStaggered.emit(ctrls);
> > >         }
> > >  }
> > >
> > > @@ -1056,7 +1034,7 @@ void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls)
> > >                 .grid_width = w,
> > >                 .grid_stride = w,
> > >                 .grid_height = h,
> > > -               .dmabuf = lsTableHandle_.fd(),
> > > +               .dmabuf = lsTableHandlePH_,
> > >                 .ref_transform = 0,
> > >                 .corner_sampled = 1,
> > >                 .gain_format = GAIN_FORMAT_U4P10
> > > @@ -1136,9 +1114,9 @@ const struct IPAModuleInfo ipaModuleInfo = {
> > >         "raspberrypi",
> > >  };
> > >
> > > -struct ipa_context *ipaCreate()
> > > +IPAInterface *ipaCreate()
> > >  {
> > > -       return new IPAInterfaceWrapper(std::make_unique<IPARPi>());
> > > +       return new IPARPi();
> > >  }
> > >
> > >  }; /* extern "C" */
> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > index 50f07182..49e28e0c 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > @@ -16,7 +16,9 @@
> > >  #include <libcamera/control_ids.h>
> > >  #include <libcamera/file_descriptor.h>
> > >  #include <libcamera/formats.h>
> > > +#include <libcamera/ipa/ipa_proxy_raspberrypi.h>
> > >  #include <libcamera/ipa/raspberrypi.h>
> > > +#include <libcamera/ipa/raspberrypi_generated.h>
> > >  #include <libcamera/logging.h>
> > >  #include <libcamera/property_ids.h>
> > >  #include <libcamera/request.h>
> > > @@ -142,7 +144,11 @@ public:
> > >         int loadIPA();
> > >         int configureIPA();
> > >
> > > -       void queueFrameAction(unsigned int frame, const IPAOperationData &action);
> > > +       void statsMetadataComplete(uint32_t bufferId, const ControlList &controls);
> > > +       void runIsp(uint32_t bufferId);
> > > +       void embeddedComplete(uint32_t bufferId);
> > > +       void setIsp(const ControlList &controls);
> > > +       void setStaggered(const ControlList &controls);
> > >
> > >         /* bufferComplete signal handlers. */
> > >         void unicamBufferDequeue(FrameBuffer *buffer);
> > > @@ -154,6 +160,8 @@ public:
> > >         void handleExternalBuffer(FrameBuffer *buffer, RPi::RPiStream *stream);
> > >         void handleState();
> > >
> > > +       std::unique_ptr<IPAProxyRPi> ipa_;
> > > +
> > >         CameraSensor *sensor_;
> > >         /* Array of Unicam and ISP device streams and associated buffers/streams. */
> > >         RPi::RPiDevice<Unicam, 2> unicam_;
> > > @@ -356,6 +364,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> > >  PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager)
> > >         : PipelineHandler(manager), unicam_(nullptr), isp_(nullptr)
> > >  {
> > > +       initializeRPiControls();
> > >  }
> > >
> > >  CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> > > @@ -974,11 +983,17 @@ void RPiCameraData::frameStarted(uint32_t sequence)
> > >
> > >  int RPiCameraData::loadIPA()
> > >  {
> > > -       ipa_ = IPAManager::createIPA(pipe_, 1, 1);
> > > +       std::unique_ptr<IPAProxy> ptr = IPAManager::createIPA(pipe_, 1, 1);
> > > +       ipa_ = std::unique_ptr<IPAProxyRPi>{static_cast<IPAProxyRPi*>(std::move(ptr).release())};
> > > +
> > >         if (!ipa_)
> > >                 return -ENOENT;
> > >
> > > -       ipa_->queueFrameAction.connect(this, &RPiCameraData::queueFrameAction);
> > > +       ipa_->statsMetadataComplete.connect(this, &RPiCameraData::statsMetadataComplete);
> > > +       ipa_->runIsp.connect(this, &RPiCameraData::runIsp);
> > > +       ipa_->embeddedComplete.connect(this, &RPiCameraData::embeddedComplete);
> > > +       ipa_->setIsp.connect(this, &RPiCameraData::setIsp);
> > > +       ipa_->setStaggered.connect(this, &RPiCameraData::setStaggered);
> > >
> > >         IPASettings settings{
> > >                 .configurationFile = ipa_->configurationFile(sensor_->model() + ".json")
> > > @@ -990,8 +1005,8 @@ int RPiCameraData::loadIPA()
> > >  int RPiCameraData::configureIPA()
> > >  {
> > >         std::map<unsigned int, IPAStream> streamConfig;
> > > -       std::map<unsigned int, const ControlInfoMap &> entityControls;
> > > -       IPAOperationData ipaConfig = {};
> > > +       std::map<unsigned int, ControlInfoMap> entityControls;
> > > +       RPiConfigInput ipaConfig;
> > >
> > >         /* Get the device format to pass to the IPA. */
> > >         V4L2DeviceFormat sensorFormat;
> > > @@ -1017,8 +1032,9 @@ int RPiCameraData::configureIPA()
> > >                         return -ENOMEM;
> > >
> > >                 /* Allow the IPA to mmap the LS table via the file descriptor. */
> > > -               ipaConfig.operation = RPI_IPA_CONFIG_LS_TABLE;
> > > -               ipaConfig.data = { static_cast<unsigned int>(lsTable_.fd()) };
> > > +               ipaConfig.op_ = RPI_IPA_CONFIG_LS_TABLE;
> > > +               ipaConfig.lsTableHandle_ = lsTable_;
> > > +               ipaConfig.lsTableHandleStatic_ = lsTable_.fd();
> > >         }
> > >
> > >         CameraSensorInfo sensorInfo = {};
> > > @@ -1029,105 +1045,92 @@ int RPiCameraData::configureIPA()
> > >         }
> > >
> > >         /* Ready the IPA - it must know about the sensor resolution. */
> > > -       IPAOperationData result;
> > > +       RPiConfigOutput result;
> > >
> > >         ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,
> > >                         &result);
> > >
> > > -       unsigned int resultIdx = 0;
> > > -       if (result.operation & RPI_IPA_CONFIG_STAGGERED_WRITE) {
> > > +       if (result.op_ & RPI_IPA_CONFIG_STAGGERED_WRITE) {
> > > +
> > >                 /*
> > >                  * Setup our staggered control writer with the sensor default
> > >                  * gain and exposure delays.
> > >                  */
> > >                 if (!staggeredCtrl_) {
> > >                         staggeredCtrl_.init(unicam_[Unicam::Image].dev(),
> > > -                                           { { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },
> > > -                                             { V4L2_CID_EXPOSURE, result.data[resultIdx++] } });
> > > -                       sensorMetadata_ = result.data[resultIdx++];
> > > +                                       { { V4L2_CID_ANALOGUE_GAIN,
> > > +
> > >  result.staggeredWriteResult_.gainDelay_ },
> > > +                                       { V4L2_CID_EXPOSURE,
> > > +
> > >  result.staggeredWriteResult_.exposureDelay_ } });
> > > +                       sensorMetadata_ = result.staggeredWriteResult_.sensorMetadata_;
> > > +               }
> > > +
> > > +               if (result.op_ & RPI_IPA_CONFIG_SENSOR) {
> > > +                       const ControlList &ctrls = result.controls_;
> > > +                       if (!staggeredCtrl_.set(ctrls))
> > > +                               LOG(RPI, Error) << "V4L2 staggered set failed";
> > >                 }
> > > -       }
> > >
> > > -       if (result.operation & RPI_IPA_CONFIG_SENSOR) {
> > > -               const ControlList &ctrls = result.controls[0];
> > > -               if (!staggeredCtrl_.set(ctrls))
> > > -                       LOG(RPI, Error) << "V4L2 staggered set failed";
> > >         }
> > >
> > > -       if (result.operation & RPI_IPA_CONFIG_DROP_FRAMES) {
> > > +       if (result.op_ & RPI_IPA_CONFIG_DROP_FRAMES) {
> > >                 /* Configure the number of dropped frames required on startup. */
> > > -               dropFrameCount_ = result.data[resultIdx++];
> > > +               dropFrameCount_ = result.dropFrameCount_;
> > >         }
> > >
> > >         return 0;
> > >  }
> > >
> > > -void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame,
> > > -                                    const IPAOperationData &action)
> > > +void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &controls)
> > >  {
> > > -       /*
> > > -        * The following actions can be handled when the pipeline handler is in
> > > -        * a stopped state.
> > > -        */
> > > -       switch (action.operation) {
> > > -       case RPI_IPA_ACTION_V4L2_SET_STAGGERED: {
> > > -               const ControlList &controls = action.controls[0];
> > > -               if (!staggeredCtrl_.set(controls))
> > > -                       LOG(RPI, Error) << "V4L2 staggered set failed";
> > > -               goto done;
> > > -       }
> > > +       if (state_ == State::Stopped)
> > > +               handleState();
> > >
> > > -       case RPI_IPA_ACTION_V4L2_SET_ISP: {
> > > -               ControlList controls = action.controls[0];
> > > -               isp_[Isp::Input].dev()->setControls(&controls);
> > > -               goto done;
> > > -       }
> > > -       }
> > > +       FrameBuffer *buffer = isp_[Isp::Stats].getBuffers().at(bufferId);
> > > +
> > > +       handleStreamBuffer(buffer, &isp_[Isp::Stats]);
> > > +       /* Fill the Request metadata buffer with what the IPA has provided */
> > > +       requestQueue_.front()->metadata() = std::move(controls);
> > > +       state_ = State::IpaComplete;
> > > +       handleState();
> > > +}
> > >
> > > +void RPiCameraData::runIsp(uint32_t bufferId)
> > > +{
> > >         if (state_ == State::Stopped)
> > > -               goto done;
> > > +               handleState();
> > >
> > > -       /*
> > > -        * The following actions must not be handled when the pipeline handler
> > > -        * is in a stopped state.
> > > -        */
> > > -       switch (action.operation) {
> > > -       case RPI_IPA_ACTION_STATS_METADATA_COMPLETE: {
> > > -               unsigned int bufferId = action.data[0];
> > > -               FrameBuffer *buffer = isp_[Isp::Stats].getBuffers().at(bufferId);
> > > -
> > > -               handleStreamBuffer(buffer, &isp_[Isp::Stats]);
> > > -               /* Fill the Request metadata buffer with what the IPA has provided */
> > > -               requestQueue_.front()->metadata() = std::move(action.controls[0]);
> > > -               state_ = State::IpaComplete;
> > > -               break;
> > > -       }
> > > +       FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers().at(bufferId);
> > >
> > > -       case RPI_IPA_ACTION_EMBEDDED_COMPLETE: {
> > > -               unsigned int bufferId = action.data[0];
> > > -               FrameBuffer *buffer = unicam_[Unicam::Embedded].getBuffers().at(bufferId);
> > > -               handleStreamBuffer(buffer, &unicam_[Unicam::Embedded]);
> > > -               break;
> > > -       }
> > > +       LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " << bufferId
> > > +                       << ", timestamp: " << buffer->metadata().timestamp;
> > >
> > > -       case RPI_IPA_ACTION_RUN_ISP: {
> > > -               unsigned int bufferId = action.data[0];
> > > -               FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers().at(bufferId);
> > > +       isp_[Isp::Input].queueBuffer(buffer);
> > > +       ispOutputCount_ = 0;
> > > +       handleState();
> > > +}
> > >
> > > -               LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " << bufferId
> > > -                               << ", timestamp: " << buffer->metadata().timestamp;
> > > +void RPiCameraData::embeddedComplete(uint32_t bufferId)
> > > +{
> > > +       if (state_ == State::Stopped)
> > > +               handleState();
> > >
> > > -               isp_[Isp::Input].queueBuffer(buffer);
> > > -               ispOutputCount_ = 0;
> > > -               break;
> > > -       }
> > > +       FrameBuffer *buffer = unicam_[Unicam::Embedded].getBuffers().at(bufferId);
> > > +       handleStreamBuffer(buffer, &unicam_[Unicam::Embedded]);
> > > +       handleState();
> > > +}
> > >
> > > -       default:
> > > -               LOG(RPI, Error) << "Unknown action " << action.operation;
> > > -               break;
> > > -       }
> > > +void RPiCameraData::setIsp(const ControlList &controls)
> > > +{
> > > +       ControlList ctrls = controls;
> > > +       isp_[Isp::Input].dev()->setControls(&ctrls);
> > > +       handleState();
> > > +}
> > >
> > > -done:
> > > +void RPiCameraData::setStaggered(const ControlList &controls)
> > > +{
> > > +       if (!staggeredCtrl_.set(controls))
> > > +               LOG(RPI, Error) << "V4L2 staggered set failed";
> > >         handleState();
> > >  }
> > >
> > > @@ -1227,10 +1230,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)
> > >          * application until after the IPA signals so.
> > >          */
> > >         if (stream == &isp_[Isp::Stats]) {
> > > -               IPAOperationData op;
> > > -               op.operation = RPI_IPA_EVENT_SIGNAL_STAT_READY;
> > > -               op.data = { RPiBufferMask::STATS | static_cast<unsigned int>(index) };
> > > -               ipa_->processEvent(op);
> > > +               ipa_->signalStatReady(RPiBufferMask::STATS | static_cast<unsigned int>(index));
> > >         } else {
> > >                 /* Any other ISP output can be handed back to the application now. */
> > >                 handleStreamBuffer(buffer, stream);
> > > @@ -1403,7 +1403,6 @@ void RPiCameraData::checkRequestCompleted()
> > >  void RPiCameraData::tryRunPipeline()
> > >  {
> > >         FrameBuffer *bayerBuffer, *embeddedBuffer;
> > > -       IPAOperationData op;
> > >
> > >         /* If any of our request or buffer queues are empty, we cannot proceed. */
> > >         if (state_ != State::Idle || requestQueue_.empty() ||
> > > @@ -1454,9 +1453,7 @@ void RPiCameraData::tryRunPipeline()
> > >          * queue the ISP output buffer listed in the request to start the HW
> > >          * pipeline.
> > >          */
> > > -       op.operation = RPI_IPA_EVENT_QUEUE_REQUEST;
> > > -       op.controls = { request->controls() };
> > > -       ipa_->processEvent(op);
> > > +       ipa_->signalQueueRequest(request->controls());
> > >
> > >         /* Ready to use the buffers, pop them off the queue. */
> > >         bayerQueue_.pop();
> > > @@ -1472,10 +1469,10 @@ void RPiCameraData::tryRunPipeline()
> > >                         << " Bayer buffer id: " << bayerId
> > >                         << " Embedded buffer id: " << embeddedId;
> > >
> > > -       op.operation = RPI_IPA_EVENT_SIGNAL_ISP_PREPARE;
> > > -       op.data = { RPiBufferMask::EMBEDDED_DATA | embeddedId,
> > > -                   RPiBufferMask::BAYER_DATA | bayerId };
> > > -       ipa_->processEvent(op);
> > > +       RPiIspPreparePayload ispPrepare;
> > > +       ispPrepare.embeddedbufferId_ = RPiBufferMask::EMBEDDED_DATA | embeddedId;
> > > +       ispPrepare.bayerbufferId_ = RPiBufferMask::BAYER_DATA | bayerId;
> > > +       ipa_->signalIspPrepare(ispPrepare);
> > >  }
> > >
> > >  void RPiCameraData::tryFlushQueues()
>
> --
> Regards,
>
> Laurent Pinchart