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

[Laurent Pinchart]

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.

> > +
> >   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