[libcamera-devel,RFC,v2,3/4] libcamera: pipeline: ipu3: Get and set VCM information
diff mbox series

Message ID 20220421072848.6752-4-hpa@redhat.com
State Superseded
Headers show
Series
  • Enabling AF algorithm to get the VCM attributes from the device driver
Related show

Commit Message

Kate Hsuan April 21, 2022, 7:28 a.m. UTC 
It is an interface of CameraData to get the VCM steps. Also, the VCM step
value is stored in ConfigInfo.

Signed-off-by: Kate Hsuan<hpa@redhat.com>
---
 src/libcamera/pipeline/ipu3/ipu3.cpp | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Jacopo Mondi April 21, 2022, 10:16 a.m. UTC | #1 
Hi Kate

On Thu, Apr 21, 2022 at 03:28:47PM +0800, Kate Hsuan via libcamera-devel wrote:
> It is an interface of CameraData to get the VCM steps. Also, the VCM step
> value is stored in ConfigInfo.
>
> Signed-off-by: Kate Hsuan<hpa@redhat.com>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index fd989e61..d1a1d21c 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -86,6 +86,8 @@ public:
>
>  	ControlInfoMap ipaControls_;
>
> +	int getMaxLensSteps();
> +
>  private:
>  	void metadataReady(unsigned int id, const ControlList &metadata);
>  	void paramsBufferReady(unsigned int id);
> @@ -667,6 +669,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  	CameraLens *lens = data->cio2_.sensor()->focusLens();
>  	if (lens)
>  		configInfo.lensControls = lens->controls();
> +	sensorInfo.maxVcmSteps = data->getMaxLensSteps();

Is a dedicated function necessary ?

Can't it just be

        if (lens) {
  		configInfo.lensControls = lens->controls();
                sensorInfo.maxVcmSteps = lens->getMaxFocusStep();
        }

Thanks
   j

>
>  	configInfo.sensorInfo = sensorInfo;
>  	configInfo.bdsOutputSize = config->imguConfig().bds;
> @@ -1272,6 +1275,18 @@ void IPU3CameraData::setSensorControls([[maybe_unused]] unsigned int id,
>  	focusLens->setFocusPosition(focusValue.get<int32_t>());
>  }
>
> +int IPU3CameraData::getMaxLensSteps()
> +{
> +	int ret = 0;
> +
> +	CameraLens *focusLens = cio2_.sensor()->focusLens();
> +	if (!focusLens)
> +		return 0;
> +
> +	ret = focusLens->getMaxFocusStep();
> +	return ret;
> +}
> +
>  void IPU3CameraData::paramsBufferReady(unsigned int id)
>  {
>  	IPU3Frames::Info *info = frameInfos_.find(id);
> --
> 2.35.1
>
Kate Hsuan April 25, 2022, 1:21 a.m. UTC | #2 
Hi Jacopo,

On Thu, Apr 21, 2022 at 6:16 PM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi Kate
>
> On Thu, Apr 21, 2022 at 03:28:47PM +0800, Kate Hsuan via libcamera-devel wrote:
> > It is an interface of CameraData to get the VCM steps. Also, the VCM step
> > value is stored in ConfigInfo.
> >
> > Signed-off-by: Kate Hsuan<hpa@redhat.com>
> > ---
> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index fd989e61..d1a1d21c 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -86,6 +86,8 @@ public:
> >
> >       ControlInfoMap ipaControls_;
> >
> > +     int getMaxLensSteps();
> > +
> >  private:
> >       void metadataReady(unsigned int id, const ControlList &metadata);
> >       void paramsBufferReady(unsigned int id);
> > @@ -667,6 +669,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> >       CameraLens *lens = data->cio2_.sensor()->focusLens();
> >       if (lens)
> >               configInfo.lensControls = lens->controls();
> > +     sensorInfo.maxVcmSteps = data->getMaxLensSteps();
>
> Is a dedicated function necessary ?
>
> Can't it just be
>
>         if (lens) {
>                 configInfo.lensControls = lens->controls();
>                 sensorInfo.maxVcmSteps = lens->getMaxFocusStep();
>         }
>
> Thanks
>    j
>

Thank you for addressing this.
It should be under "if". I will modify this in my v3.


> >
> >       configInfo.sensorInfo = sensorInfo;
> >       configInfo.bdsOutputSize = config->imguConfig().bds;
> > @@ -1272,6 +1275,18 @@ void IPU3CameraData::setSensorControls([[maybe_unused]] unsigned int id,
> >       focusLens->setFocusPosition(focusValue.get<int32_t>());
> >  }
> >
> > +int IPU3CameraData::getMaxLensSteps()
> > +{
> > +     int ret = 0;
> > +
> > +     CameraLens *focusLens = cio2_.sensor()->focusLens();
> > +     if (!focusLens)
> > +             return 0;
> > +
> > +     ret = focusLens->getMaxFocusStep();
> > +     return ret;
> > +}
> > +
> >  void IPU3CameraData::paramsBufferReady(unsigned int id)
> >  {
> >       IPU3Frames::Info *info = frameInfos_.find(id);
> > --
> > 2.35.1
> >
>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index fd989e61..d1a1d21c 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -86,6 +86,8 @@  public:
 
 	ControlInfoMap ipaControls_;
 
+	int getMaxLensSteps();
+
 private:
 	void metadataReady(unsigned int id, const ControlList &metadata);
 	void paramsBufferReady(unsigned int id);
@@ -667,6 +669,7 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 	CameraLens *lens = data->cio2_.sensor()->focusLens();
 	if (lens)
 		configInfo.lensControls = lens->controls();
+	sensorInfo.maxVcmSteps = data->getMaxLensSteps();
 
 	configInfo.sensorInfo = sensorInfo;
 	configInfo.bdsOutputSize = config->imguConfig().bds;
@@ -1272,6 +1275,18 @@  void IPU3CameraData::setSensorControls([[maybe_unused]] unsigned int id,
 	focusLens->setFocusPosition(focusValue.get<int32_t>());
 }
 
+int IPU3CameraData::getMaxLensSteps()
+{
+	int ret = 0;
+
+	CameraLens *focusLens = cio2_.sensor()->focusLens();
+	if (!focusLens)
+		return 0;
+
+	ret = focusLens->getMaxFocusStep();
+	return ret;
+}
+
 void IPU3CameraData::paramsBufferReady(unsigned int id)
 {
 	IPU3Frames::Info *info = frameInfos_.find(id);