[libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to an IPA and allow it to set sensor controls
Jacopo Mondi
jacopo at jmondi.org
Thu Jan 7 13:40:50 CET 2021
Hi Niklas,
On Tue, Dec 29, 2020 at 05:03:14PM +0100, Niklas Söderlund wrote:
> Attach to the IPA and allow it to push V4L2 controls that applies on the
> camera sensor. The IPA is not fully integrated in the pipeline as
> statistics and parameters buffers are not yet allocated, processed by
> the IPA nor queued to the hardware.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> * Changes since v1
> - Rewrite to not use CameraSensor.
> - Fix mistake where configuration sen to IPA was overwritten.
> - Check that IPA configuration was successful before starting.
> - Update commit message.
> ---
> src/libcamera/pipeline/ipu3/ipu3.cpp | 103 +++++++++++++++++++++++++++
> 1 file changed, 103 insertions(+)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index a87ce8f3378ba2fe..95f1b75dc8be5d40 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -14,11 +14,14 @@
> #include <libcamera/camera.h>
> #include <libcamera/control_ids.h>
> #include <libcamera/formats.h>
> +#include <libcamera/ipa/ipu3.h>
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
>
> #include "libcamera/internal/camera_sensor.h"
> +#include "libcamera/internal/delayed_controls.h"
> #include "libcamera/internal/device_enumerator.h"
> +#include "libcamera/internal/ipa_manager.h"
> #include "libcamera/internal/log.h"
> #include "libcamera/internal/media_device.h"
> #include "libcamera/internal/pipeline_handler.h"
> @@ -53,6 +56,8 @@ public:
> {
> }
>
> + int loadIPA();
> +
> void imguOutputBufferReady(FrameBuffer *buffer);
> void cio2BufferReady(FrameBuffer *buffer);
>
> @@ -62,6 +67,11 @@ public:
> Stream outStream_;
> Stream vfStream_;
> Stream rawStream_;
> +
> + std::unique_ptr<DelayedControls> delayedCtrls_;
> +
> +private:
> + void actOnIpa(unsigned int id, const IPAOperationData &op);
> };
>
> class IPU3CameraConfiguration : public CameraConfiguration
> @@ -590,6 +600,13 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con
> IPU3CameraData *data = cameraData(camera);
> CIO2Device *cio2 = &data->cio2_;
> ImgUDevice *imgu = data->imgu_;
> +
> + CameraSensorInfo sensorInfo = {};
> + std::map<unsigned int, IPAStream> streamConfig;
> + std::map<unsigned int, const ControlInfoMap &> entityControls;
> + IPAOperationData ipaConfig;
> + IPAOperationData result = {};
You could s/= {}/{}
but it's minor
> +
> int ret;
>
> /* Allocate buffers for internal pipeline usage. */
> @@ -597,6 +614,11 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con
> if (ret)
> return ret;
>
> + IPAOperationData ipaData = {};
> + ret = data->ipa_->start(ipaData, nullptr);
> + if (ret)
> + goto error;
> +
> /*
> * Start the ImgU video devices, buffers will be queued to the
> * ImgU output and viewfinder when requests will be queued.
> @@ -612,9 +634,40 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con
> goto error;
> }
>
> + /* Inform IPA of stream configuration and sensor controls. */
> + ret = data->cio2_.sensor()->sensorInfo(&sensorInfo);
> + if (ret) {
> + /* \todo Turn to hard failure once sensors info is mandatory. */
we're rather working in the direction of making SensorInfo never fail.
I think you can drop this todo and hard fail here as if you have a
failure, it's because something bad happened when reading controls or
selection targets.
> + LOG(IPU3, Warning) << "Camera sensor information not available";
> + sensorInfo = {};
> + ret = 0;
> + }
> +
> + streamConfig[0] = {
> + .pixelFormat = data->outStream_.configuration().pixelFormat,
> + .size = data->outStream_.configuration().size,
> + };
> + streamConfig[1] = {
> + .pixelFormat = data->vfStream_.configuration().pixelFormat,
> + .size = data->vfStream_.configuration().size,
> + };
Is this ok if one of the two streams (likely vfStream_) is not
assigned ?
> +
> + entityControls.emplace(0, data->cio2_.sensor()->controls());
> +
> + data->ipa_->configure(sensorInfo, streamConfig, entityControls,
> + ipaConfig, &result);
> +
> + if ((result.operation != IPU3_IPA_STATUS_CONFIGURATION) ||
This seems like it can't happen at the moment, but it's ok
> + (result.data.size() != 1) || (result.data.at(0) != 1)) {
I think the fact that returning 1 in data[0] means success should be
documented in the IPA protocol. Ah wait, we don't have one.
> + LOG(IPU3, Warning) << "Failed to configure IPA";
> + ret = -EINVAL;
> + goto error;
> + }
> +
> return 0;
>
> error:
> + data->ipa_->stop();
> freeBuffers(camera);
> LOG(IPU3, Error) << "Failed to start camera " << camera->id();
>
> @@ -631,6 +684,8 @@ void PipelineHandlerIPU3::stop(Camera *camera)
> if (ret)
> LOG(IPU3, Warning) << "Failed to stop camera " << camera->id();
>
> + data->ipa_->stop();
> +
> freeBuffers(camera);
> }
>
> @@ -762,12 +817,32 @@ int PipelineHandlerIPU3::registerCameras()
> if (ret)
> continue;
>
> + ret = data->loadIPA();
> + if (ret)
> + continue;
> +
> /* Initialize the camera properties. */
> data->properties_ = cio2->sensor()->properties();
>
> /* Initialze the camera controls. */
> data->controlInfo_ = IPU3Controls;
>
> + /*
> + * \todo Read dealy values from the sensor itself or from a
> + * a sensor database. For now use generic values taken from
> + * the Raspberry Pi and listed as generic values.
> + */
> + std::unordered_map<uint32_t, unsigned int> delays = {
> + { V4L2_CID_ANALOGUE_GAIN, 1 },
> + { V4L2_CID_EXPOSURE, 2 },
> + };
> +
> + data->delayedCtrls_ =
> + std::make_unique<DelayedControls>(cio2->sensor()->device(),
> + delays);
sigh
we should get the sensor database in sooner than later and move this
to the camera sensor before the same patter spread in multiple
pipelines
> + data->cio2_.frameStart().connect(data->delayedCtrls_.get(),
> + &DelayedControls::applyControls);
> +
> /**
> * \todo Dynamically assign ImgU and output devices to each
> * stream and camera; as of now, limit support to two cameras
> @@ -811,6 +886,34 @@ int PipelineHandlerIPU3::registerCameras()
> return numCameras ? 0 : -ENODEV;
> }
>
> +int IPU3CameraData::loadIPA()
> +{
> + ipa_ = IPAManager::createIPA(pipe_, 1, 1);
> + if (!ipa_)
> + return -ENOENT;
> +
> + ipa_->queueFrameAction.connect(this, &IPU3CameraData::actOnIpa);
> +
> + ipa_->init(IPASettings{});
> +
> + return 0;
> +}
> +
> +void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id,
> + const IPAOperationData &action)
> +{
> + switch (action.operation) {
> + case IPU3_IPA_ACTION_SET_SENSOR_CONTROLS: {
> + const ControlList &controls = action.controls[0];
> + delayedCtrls_->push(controls);
> + break;
> + }
> + default:
> + LOG(IPU3, Error) << "Unknown action " << action.operation;
> + break;
> + }
> +}
> +
Overall, that's a good starting point
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
> /* -----------------------------------------------------------------------------
> * Buffer Ready slots
> */
> --
> 2.29.2
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list