| Message ID | 20211123123751.3194696-5-hanlinchen@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Han-lin, (CC'ing Dan) Thank you for the patch. On Tue, Nov 23, 2021 at 08:37:51PM +0800, Han-Lin Chen wrote: > Allow IPA to apply controls to the lens device. > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> > --- > src/libcamera/pipeline/ipu3/cio2.cpp | 29 ++++++++++++++++++++++++++++ > src/libcamera/pipeline/ipu3/cio2.h | 3 +++ > src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++++++++-- > 3 files changed, 42 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp > index 59dda56b..59b2f586 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,34 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index) > return -EINVAL; > } > > + /* > + * \todo Read the lens model from the sensor itself or from a device > + * database. For now use default values taken from ChromeOS database. > + */ > + static std::unordered_map<std::string, std::string> sensorLens = { > + { "ov13858", "dw9714" }, > + { "imx258", "dw9807" }, > + { "imx355", "ak7375" } > + }; Dan, could you share your patches to add an ancillary link between the imaging sensor and the lens controller with Han-lin once they're ready ? This is what we should use here, and Han-lin could help testing them on Chrome OS (which uses different ACPI bindings for the CIO2). > + > + 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"; > + } > + } > + > /* > * \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 c65afdb2..6e04ec8f 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" > @@ -1238,8 +1239,15 @@ 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); > + > + const ControlList lensControls = action.lensControls; > + const ControlValue &focusValue = > + lensControls.get(V4L2_CID_FOCUS_ABSOLUTE); > + if (!focusValue.isNone() && cio2_.lens()) > + cio2_.lens()->setFocusPostion(focusValue.get<int32_t>()); > + The other way around, should we pass the limits of the focus control from the pipeline handler to the IPA ? > break; > } > case ipa::ipu3::ActionParamFilled: {
On Wed, Nov 24, 2021 at 1:57 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Han-lin, > > (CC'ing Dan) > > Thank you for the patch. > > On Tue, Nov 23, 2021 at 08:37:51PM +0800, Han-Lin Chen wrote: > > Allow IPA to apply controls to the lens device. > > > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> > > --- > > src/libcamera/pipeline/ipu3/cio2.cpp | 29 ++++++++++++++++++++++++++++ > > src/libcamera/pipeline/ipu3/cio2.h | 3 +++ > > src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++++++++-- > > 3 files changed, 42 insertions(+), 2 deletions(-) > > > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp > > index 59dda56b..59b2f586 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,34 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index) > > return -EINVAL; > > } > > > > + /* > > + * \todo Read the lens model from the sensor itself or from a device > > + * database. For now use default values taken from ChromeOS database. > > + */ > > + static std::unordered_map<std::string, std::string> sensorLens = { > > + { "ov13858", "dw9714" }, > > + { "imx258", "dw9807" }, > > + { "imx355", "ak7375" } > > + }; > > Dan, could you share your patches to add an ancillary link between the > imaging sensor and the lens controller with Han-lin once they're ready ? > This is what we should use here, and Han-lin could help testing them on > Chrome OS (which uses different ACPI bindings for the CIO2). Loop Tomasz for visibility. Looking forward to it, although it's a little out of my knowledge base ;-). > > > + > > + 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"; > > + } > > + } > > + > > /* > > * \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 c65afdb2..6e04ec8f 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" > > @@ -1238,8 +1239,15 @@ 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); > > + > > + const ControlList lensControls = action.lensControls; > > + const ControlValue &focusValue = > > + lensControls.get(V4L2_CID_FOCUS_ABSOLUTE); > > + if (!focusValue.isNone() && cio2_.lens()) > > + cio2_.lens()->setFocusPostion(focusValue.get<int32_t>()); > > + > > The other way around, should we pass the limits of the focus control > from the pipeline handler to the IPA ? In theory, yes, although Intel seems to hard-code them into its tuning file, and not used for now. > > > break; > > } > > case ipa::ipu3::ActionParamFilled: { > > -- > Regards, > > Laurent Pinchart
Morning On 24/11/2021 05:56, Laurent Pinchart wrote: > Hi Han-lin, > > (CC'ing Dan) > > Thank you for the patch. > > On Tue, Nov 23, 2021 at 08:37:51PM +0800, Han-Lin Chen wrote: >> Allow IPA to apply controls to the lens device. >> >> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> >> --- >> src/libcamera/pipeline/ipu3/cio2.cpp | 29 ++++++++++++++++++++++++++++ >> src/libcamera/pipeline/ipu3/cio2.h | 3 +++ >> src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++++++++-- >> 3 files changed, 42 insertions(+), 2 deletions(-) >> >> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp >> index 59dda56b..59b2f586 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,34 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index) >> return -EINVAL; >> } >> >> + /* >> + * \todo Read the lens model from the sensor itself or from a device >> + * database. For now use default values taken from ChromeOS database. >> + */ >> + static std::unordered_map<std::string, std::string> sensorLens = { >> + { "ov13858", "dw9714" }, >> + { "imx258", "dw9807" }, >> + { "imx355", "ak7375" } >> + }; > Dan, could you share your patches to add an ancillary link between the > imaging sensor and the lens controller with Han-lin once they're ready ? > This is what we should use here, and Han-lin could help testing them on > Chrome OS (which uses different ACPI bindings for the CIO2). Yes, certainly. I was going to share them as a patch to be applied on top of this series, and replacing this section. This part (following the new link and creating the CameraLens) is working now, just need to iron out a few things and I can share something. >> + >> + 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"; >> + } >> + } >> + >> /* >> * \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 c65afdb2..6e04ec8f 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" >> @@ -1238,8 +1239,15 @@ 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); >> + >> + const ControlList lensControls = action.lensControls; >> + const ControlValue &focusValue = >> + lensControls.get(V4L2_CID_FOCUS_ABSOLUTE); >> + if (!focusValue.isNone() && cio2_.lens()) >> + cio2_.lens()->setFocusPostion(focusValue.get<int32_t>()); >> + > The other way around, should we pass the limits of the focus control > from the pipeline handler to the IPA ? > >> break; >> } >> case ipa::ipu3::ActionParamFilled: {
On 24/11/2021 05:56, Laurent Pinchart wrote: > Hi Han-lin, > > (CC'ing Dan) > > Thank you for the patch. > > On Tue, Nov 23, 2021 at 08:37:51PM +0800, Han-Lin Chen wrote: >> Allow IPA to apply controls to the lens device. >> >> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> >> --- >> src/libcamera/pipeline/ipu3/cio2.cpp | 29 ++++++++++++++++++++++++++++ >> src/libcamera/pipeline/ipu3/cio2.h | 3 +++ >> src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++++++++-- >> 3 files changed, 42 insertions(+), 2 deletions(-) >> >> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp >> index 59dda56b..59b2f586 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,34 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index) >> return -EINVAL; >> } >> >> + /* >> + * \todo Read the lens model from the sensor itself or from a device >> + * database. For now use default values taken from ChromeOS database. >> + */ >> + static std::unordered_map<std::string, std::string> sensorLens = { >> + { "ov13858", "dw9714" }, >> + { "imx258", "dw9807" }, >> + { "imx355", "ak7375" } >> + }; > Dan, could you share your patches to add an ancillary link between the > imaging sensor and the lens controller with Han-lin once they're ready ? > This is what we should use here, and Han-lin could help testing them on > Chrome OS (which uses different ACPI bindings for the CIO2). Actually, that spurs a thought; we're getting the VCM to match to the Sensor by using the lens-focus property in the cio2-bridge...but is the ChromeOS ACPI going to be supplying 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"; >> + } >> + } >> + >> /* >> * \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 c65afdb2..6e04ec8f 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" >> @@ -1238,8 +1239,15 @@ 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); >> + >> + const ControlList lensControls = action.lensControls; >> + const ControlValue &focusValue = >> + lensControls.get(V4L2_CID_FOCUS_ABSOLUTE); >> + if (!focusValue.isNone() && cio2_.lens()) >> + cio2_.lens()->setFocusPostion(focusValue.get<int32_t>()); >> + > The other way around, should we pass the limits of the focus control > from the pipeline handler to the IPA ? > >> break; >> } >> case ipa::ipu3::ActionParamFilled: {
On Wed, Nov 24, 2021 at 09:10:10AM +0000, Daniel Scally wrote: > On 24/11/2021 05:56, Laurent Pinchart wrote: > > Hi Han-lin, > > > > (CC'ing Dan) > > > > Thank you for the patch. > > > > On Tue, Nov 23, 2021 at 08:37:51PM +0800, Han-Lin Chen wrote: > >> Allow IPA to apply controls to the lens device. > >> > >> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> > >> --- > >> src/libcamera/pipeline/ipu3/cio2.cpp | 29 ++++++++++++++++++++++++++++ > >> src/libcamera/pipeline/ipu3/cio2.h | 3 +++ > >> src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++++++++-- > >> 3 files changed, 42 insertions(+), 2 deletions(-) > >> > >> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp > >> index 59dda56b..59b2f586 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,34 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index) > >> return -EINVAL; > >> } > >> > >> + /* > >> + * \todo Read the lens model from the sensor itself or from a device > >> + * database. For now use default values taken from ChromeOS database. > >> + */ > >> + static std::unordered_map<std::string, std::string> sensorLens = { > >> + { "ov13858", "dw9714" }, > >> + { "imx258", "dw9807" }, > >> + { "imx355", "ak7375" } > >> + }; > > Dan, could you share your patches to add an ancillary link between the > > imaging sensor and the lens controller with Han-lin once they're ready ? > > This is what we should use here, and Han-lin could help testing them on > > Chrome OS (which uses different ACPI bindings for the CIO2). > > Actually, that spurs a thought; we're getting the VCM to match to the > Sensor by using the lens-focus property in the cio2-bridge...but is the > ChromeOS ACPI going to be supplying that? Likely not out-of-the-box at the moment. This is why I'd like Han-lin to get the patches for testing, to figure out what needs to be done on Chrome OS. > >> + > >> + 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"; > >> + } > >> + } > >> + > >> /* > >> * \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 c65afdb2..6e04ec8f 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" > >> @@ -1238,8 +1239,15 @@ 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); > >> + > >> + const ControlList lensControls = action.lensControls; > >> + const ControlValue &focusValue = > >> + lensControls.get(V4L2_CID_FOCUS_ABSOLUTE); > >> + if (!focusValue.isNone() && cio2_.lens()) > >> + cio2_.lens()->setFocusPostion(focusValue.get<int32_t>()); > >> + > > > > The other way around, should we pass the limits of the focus control > > from the pipeline handler to the IPA ? > > > >> break; > >> } > >> case ipa::ipu3::ActionParamFilled: {
diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index 59dda56b..59b2f586 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,34 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index) return -EINVAL; } + /* + * \todo Read the lens model from the sensor itself or from a device + * database. For now use default values taken from ChromeOS database. + */ + static std::unordered_map<std::string, std::string> sensorLens = { + { "ov13858", "dw9714" }, + { "imx258", "dw9807" }, + { "imx355", "ak7375" } + }; + + 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"; + } + } + /* * \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 c65afdb2..6e04ec8f 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" @@ -1238,8 +1239,15 @@ 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); + + const ControlList lensControls = action.lensControls; + const ControlValue &focusValue = + lensControls.get(V4L2_CID_FOCUS_ABSOLUTE); + if (!focusValue.isNone() && cio2_.lens()) + cio2_.lens()->setFocusPostion(focusValue.get<int32_t>()); + break; } case ipa::ipu3::ActionParamFilled: {
Allow IPA to apply controls to the lens device. Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> --- src/libcamera/pipeline/ipu3/cio2.cpp | 29 ++++++++++++++++++++++++++++ src/libcamera/pipeline/ipu3/cio2.h | 3 +++ src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++++++++-- 3 files changed, 42 insertions(+), 2 deletions(-)