[libcamera-devel,v5,01/14] ipa: ipu3: Extend ipu3 ipa interface for sensor controls

14591 diff mbox series

From: Han-Lin Chen <hanlinchen@chromium.org>

IPU3Event and IPU3Action use single ControlList for both libcamera and
V4L2 controls, and it's content could be either one based on the
context.  Extend IPU3Event and IPU3Action for sensor V4L2 controls, and
preserve the original one for only libcamera Controls to make the
content of an event more specific.

Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
[Jean-Michel: remove lensControls from the original patch]
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
---
 include/libcamera/ipa/ipu3.mojom     | 2 ++
 src/ipa/ipu3/ipu3.cpp                | 2 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

Hi Jean-Michel,

On Sat, Nov 13, 2021 at 09:49:34AM +0100, Jean-Michel Hautbois wrote:
> From: Han-Lin Chen <hanlinchen@chromium.org>
> 
> IPU3Event and IPU3Action use single ControlList for both libcamera and
> V4L2 controls, and it's content could be either one based on the
> context.  Extend IPU3Event and IPU3Action for sensor V4L2 controls, and
> preserve the original one for only libcamera Controls to make the
> content of an event more specific.
> 
> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> [Jean-Michel: remove lensControls from the original patch]
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  include/libcamera/ipa/ipu3.mojom     | 2 ++
>  src/ipa/ipu3/ipu3.cpp                | 2 +-
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
> index 2f254ed4..16e3462e 100644
> --- a/include/libcamera/ipa/ipu3.mojom
> +++ b/include/libcamera/ipa/ipu3.mojom
> @@ -23,11 +23,13 @@ struct IPU3Event {
>  	int64 frameTimestamp;
>  	uint32 bufferId;
>  	libcamera.ControlList controls;
> +	libcamera.ControlList sensorControls;
>  };
>  
>  struct IPU3Action {
>  	IPU3Operations op;
>  	libcamera.ControlList controls;
> +	libcamera.ControlList sensorControls;
>  };

Could you refactor the IPU3 IPA API to move towards ad-hoc methods and
data structures ? The IPU3Event and IPU3Action come from the time when
we had a C-based API with a single method in each direction
(queueFrameAction() and processEvent()), and we're not limited by that
anymore.

>  
>  struct IPAConfigInfo {
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 93b700bd..bcc3863b 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -653,7 +653,7 @@ void IPAIPU3::setControls(unsigned int frame)
>  	ControlList ctrls(ctrls_);
>  	ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
>  	ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
> -	op.controls = ctrls;
> +	op.sensorControls = ctrls;
>  
>  	queueFrameAction.emit(frame, op);
>  }
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index eb714aa6..8816efc5 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -1248,7 +1248,7 @@ void IPU3CameraData::queueFrameAction(unsigned int id,
>  {
>  	switch (action.op) {
>  	case ipa::ipu3::ActionSetSensorControls: {
> -		const ControlList &controls = action.controls;
> +		const ControlList &controls = action.sensorControls;
>  		delayedCtrls_->push(controls);
>  		break;
>  	}