[libcamera-devel] [PATCH 1/4] ipa: ipu3: Extend ipu3 ipa interface for sensor and lens controls

[Kieran Bingham]

Quoting Han-Lin Chen (2021-10-28 11:03:16)
> 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 and lens V4L2 controls, and preserve
> the original one for only libcamera Controls to make the content of an event
> more specific.

I like this, it does make the controls clearer.
I wonder if there's much impact on the IPC but when empty, I presume it
will be minimal cost.

> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.com>
> ---
>  include/libcamera/ipa/ipu3.mojom     | 4 ++++
>  src/ipa/ipu3/ipu3.cpp                | 2 +-
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
> index 2f254ed4..cc0d822f 100644
> --- a/include/libcamera/ipa/ipu3.mojom
> +++ b/include/libcamera/ipa/ipu3.mojom
> @@ -23,11 +23,15 @@ struct IPU3Event {
>         int64 frameTimestamp;
>         uint32 bufferId;
>         libcamera.ControlList controls;
> +       libcamera.ControlList sensorControls;
> +       libcamera.ControlList lensControls;

Do events need to send sensor controls and lens controls to the IPA? Or
will these always be unused?

Aha, I see at least the Sensor controls are going to get used in the
next patch... Ok - so keeping them aligned with IPU3Action seems to make
sense.


>  };
>  
>  struct IPU3Action {
>         IPU3Operations op;
>         libcamera.ControlList controls;
> +       libcamera.ControlList sensorControls;
> +       libcamera.ControlList lensControls;
>  };
>  
>  struct IPAConfigInfo {
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 5c51607d..6775570e 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -646,7 +646,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;
>         }
> -- 
> 2.33.1.1089.g2158813163f-goog
>