[PATCH] libcamera: Add open() and close() in CameraLens

[Cheng-Hao Yang]

Hi Kieran and Laurent,

On Mon, Oct 21, 2024 at 12:08 AM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> CC'ing Sakari.
>
> On Sat, Oct 19, 2024 at 11:32:02PM +0100, Kieran Bingham wrote:
> > Quoting Harvey Yang (2024-10-17 08:39:47)
> > > From: Han-Lin Chen <hanlinchen at chromium.org>
> > >
> > > This allows pipeline handlers to save some power when a camera is close.
> >
> > This seems reasonable - but makes me wonder - is there similar
> > limitations with the CameraSensor class that has a very similar design?
>
> I think it's time to bite the bullet and see how to address the issue of
> power management of lens controllers in the kernel. Tying it to open()
> and close() isn't necessarily a great option.

Han-lin told me that CameraLens is a special case of
V4L2Subdevice (at least in mtkisp7), that it's powered on when opened.
Other ones like CameraSensor's `subdev_` might be powered on when
the V4L2VideoDevice that receives buffers are `streamOn`ed. Just FYI.

>
> > > Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> > > Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> > > Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> > > ---
> > >  include/libcamera/internal/camera_lens.h |  4 ++++
> > >  src/libcamera/camera_lens.cpp            | 17 +++++++++++++++++
> > >  2 files changed, 21 insertions(+)
> > >
> > > diff --git a/include/libcamera/internal/camera_lens.h b/include/libcamera/internal/camera_lens.h
> > > index 5a4b993bb..095056791 100644
> > > --- a/include/libcamera/internal/camera_lens.h
> > > +++ b/include/libcamera/internal/camera_lens.h
> > > @@ -26,6 +26,10 @@ public:
> > >         ~CameraLens();
> > >
> > >         int init();
> >
> > Should we be calling close() at the end of init()? Perhaps with some
> > sort of way to make sure if an API call is used on a now closed device
> > it gets opened again first ? (i.e. mostly perhaps V4L2Device::setControls)?

Hmm, in the current API, I don't see function calls that "start" to use
CameraLens.

BR,
Harvey

> >
> > > +
> > > +       int open();
> > > +       void close();
> > > +
> > >         int setFocusPosition(int32_t position);
> > >
> > >         const std::string &model() const { return model_; }
> > > diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp
> > > index ccc2a6a65..039f5ad2a 100644
> > > --- a/src/libcamera/camera_lens.cpp
> > > +++ b/src/libcamera/camera_lens.cpp
> > > @@ -76,6 +76,23 @@ int CameraLens::init()
> > >         return 0;
> > >  }
> > >
> > > +/**
> > > + * \brief Open the subdev
> > > + * \return 0 on success or a negative error code otherwise
> > > + */
> > > +int CameraLens::open()
> > > +{
> > > +       return subdev_->open();
> > > +}
> > > +
> > > +/**
> > > + * \brief Close the subdev
> > > + */
> > > +void CameraLens::close()
> > > +{
> > > +       subdev_->close();
> > > +}
> > > +
> > >  /**
> > >   * \brief This function sets the focal point of the lens to a specific position.
> > >   * \param[in] position The focal point of the lens
>
> --
> Regards,
>
> Laurent Pinchart