[libcamera-devel] [PATCH v3 3/3] ipu3: ipa: Allow IPA to apply controls to the lens device

[Hanlin Chen]

On Fri, Nov 12, 2021 at 7:58 AM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> On Thu, Nov 11, 2021 at 05:12:52PM +0100, Jean-Michel Hautbois wrote:
> > Hi Han-Lin,
> >
> > On 11/11/2021 11:49, Han-Lin Chen wrote:
> > > Allow IPA to apply controls to the lens device.
> > >
> > > Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> > > ---
> > >   meson.build                          |  6 ++++++
> > >   src/libcamera/pipeline/ipu3/cio2.cpp | 30 ++++++++++++++++++++++++++++
> > >   src/libcamera/pipeline/ipu3/cio2.h   |  3 +++
> > >   src/libcamera/pipeline/ipu3/ipu3.cpp |  9 +++++++--
> > >   4 files changed, 46 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/meson.build b/meson.build
> > > index 7892a9e3..2a4b68a2 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -108,6 +108,12 @@ if cc.has_argument('-Wno-c99-designator')
> > >       ]
> > >   endif
> > >
> > > +if get_option('android_platform') == 'cros'
> > > +    common_arguments += [
> > > +        '-DOS_CHROMEOS',
> > > +    ]
> > > +endif
> >
> > I am not really aware of specifics regarding this, why can't we have
> > something not dependent on the OS ?
>
> There's a mail thread regarding VCM support on MS Surface machines (see
> https://lists.libcamera.org/pipermail/libcamera-devel/2021-November/026526.html).
> Raspberry Pi is also looking into VCM support. It would be best to
> cooperate with all parties involved to define a way to expose the
> relation between camera sensors and lens controllers to userspace, and
> use it here. I'm really not keen on having a CrOS-specific hack.
Looking forward to it :-).
>
> > > +
> > >   c_arguments += common_arguments
> > >   cpp_arguments += common_arguments
> > >
> > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> > > index 59dda56b..233553c2 100644
> > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> > > @@ -16,6 +16,7 @@
> > >   #include <libcamera/geometry.h>
> > >   #include <libcamera/stream.h>
> > >
> > > +#include "libcamera/internal/camera_lens.h"
> > >   #include "libcamera/internal/camera_sensor.h"
> > >   #include "libcamera/internal/framebuffer.h"
> > >   #include "libcamera/internal/media_device.h"
> > > @@ -159,6 +160,35 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
> > >             return -EINVAL;
> > >     }
> > >
> > > +#if defined(OS_CHROMEOS)
> > > +   /*
> > > +    * \todo Read the lens model from the sensor itself or from a device database.
> > > +    * For now use default values taken from ChromeOS.
> > > +    */
> > > +   static std::unordered_map<std::string, std::string> sensorLens = {
> > > +           { "ov13858", "dw9714" },
> > > +           { "imx258", "dw9807" },
> > > +           { "imx355", "ak7375" }
> > > +   };
> >
> > So, we need a list giving the lens model associated to a sensor... How
> > is this dependent on chromeos ? Imagine I want to use the sensor lens on
> > my SGo2, which has a ov8865, what is needed to do that ?
> >
> > > +
> > > +   auto it = sensorLens.find(sensor_->model());
> > > +   if (it != sensorLens.end()) {
> > > +           const std::vector<MediaEntity *> &entities = media->entities();
> > > +           for (auto ent : entities) {
> > > +                   if (ent->function() == MEDIA_ENT_F_LENS) {
> > > +                           lens_ = std::make_unique<CameraLens>(ent);
> > > +                           ret = lens_->init();
> > > +                           if (!ret && lens_->model() == it->second) {
> > > +                                   break;
> > > +                           }
> > > +                           lens_.reset();
> > > +                   }
> > > +                   if (!lens_)
> > > +                           LOG(IPU3, Warning) << "Lens device " << it->second << " not found";
> > > +           }
> > > +   }
> > > +#endif
> > > +
> > >     /*
> > >      * \todo Define when to open and close video device nodes, as they
> > >      * might impact on power consumption.
> > > diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
> > > index ba8f0052..635566c8 100644
> > > --- a/src/libcamera/pipeline/ipu3/cio2.h
> > > +++ b/src/libcamera/pipeline/ipu3/cio2.h
> > > @@ -18,6 +18,7 @@
> > >
> > >   namespace libcamera {
> > >
> > > +class CameraLens;
> > >   class CameraSensor;
> > >   class FrameBuffer;
> > >   class MediaDevice;
> > > @@ -52,6 +53,7 @@ public:
> > >     int stop();
> > >
> > >     CameraSensor *sensor() { return sensor_.get(); }
> > > +   CameraLens *lens() { return lens_.get(); }
> > >     const CameraSensor *sensor() const { return sensor_.get(); }
> > >
> > >     FrameBuffer *queueBuffer(Request *request, FrameBuffer *rawBuffer);
> > > @@ -67,6 +69,7 @@ private:
> > >     void cio2BufferReady(FrameBuffer *buffer);
> > >
> > >     std::unique_ptr<CameraSensor> sensor_;
> > > +   std::unique_ptr<CameraLens> lens_;
> > >     std::unique_ptr<V4L2Subdevice> csi2_;
> > >     std::unique_ptr<V4L2VideoDevice> output_;
> > >
> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > index 97003681..88775f67 100644
> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -24,6 +24,7 @@
> > >   #include <libcamera/stream.h>
> > >
> > >   #include "libcamera/internal/camera.h"
> > > +#include "libcamera/internal/camera_lens.h"
> > >   #include "libcamera/internal/camera_sensor.h"
> > >   #include "libcamera/internal/delayed_controls.h"
> > >   #include "libcamera/internal/device_enumerator.h"
> > > @@ -1255,8 +1256,12 @@ void IPU3CameraData::queueFrameAction(unsigned int id,
> > >   {
> > >     switch (action.op) {
> > >     case ipa::ipu3::ActionSetSensorControls: {
> > > -           const ControlList &controls = action.sensorControls;
> > > -           delayedCtrls_->push(controls);
> > > +           const ControlList &sensorControls = action.sensorControls;
> > > +           delayedCtrls_->push(sensorControls);
> > > +           if (cio2_.lens()) {
> > > +                   ControlList lensControls = action.lensControls;
> > > +                   cio2_.lens()->setControls(&lensControls);
> > > +           }
> > >             break;
> > >     }
> > >     case ipa::ipu3::ActionParamFilled: {
> > >
>
> --
> Regards,
>
> Laurent Pinchart