[libcamera-devel,v4,0/6] External IPU3 IPA Support

12378 mbox series

Changes in v4:
- Hide constructors from Doxygen in Patch [1/6]
- Other minor cleanups

Changes in v3:
- Drop 7/7 patch - decided to be done on top
- Add a \todo about documenting IPA interfaces in [1/7]
- Few style cleanups

Changes in v2:
- IPA Docs rework patch split (into 3)
- Don't try to make a different 'internal' helper library
  - Drop relevant patch
  - Under discussion for now AND out of scope for this series.
- Drop IPAConfigInfo documentation
  - Needs to happen during a follow up "doc" patch for entire ipu3.mojom
    adapted same as [PATCH 2/7]

Umang Jain (6):
  ipa: Move core IPA interface documentation to a .cpp file
  ipa: mojom: Move CameraSensorInfo struct exclusively to IPA IPC
  ipa: ipc: Rename CameraSensorInfo to IPACameraSensorInfo
  ipa: meson: Install mojom generated headers to include paths
  ipa: ipu3: Introduce IPAConfigInfo in IPC
  meson: Add a configuration option to build IPAs

 Documentation/Doxyfile.in                     |   8 +-
 Documentation/guides/ipa.rst                  |   8 +-
 Documentation/meson.build                     |   1 +
 include/libcamera/internal/camera_sensor.h    |  19 +-
 include/libcamera/ipa/core.mojom              |  74 +------
 include/libcamera/ipa/ipa_interface.h         |   2 -
 include/libcamera/ipa/ipu3.mojom              |  15 +-
 include/libcamera/ipa/meson.build             |   8 +-
 include/libcamera/ipa/raspberrypi.mojom       |   7 +-
 include/libcamera/ipa/rkisp1.mojom            |   7 +-
 include/libcamera/ipa/vimc.mojom              |   5 +
 meson.build                                   |   1 +
 meson_options.txt                             |   5 +
 src/ipa/ipu3/ipu3.cpp                         |  14 +-
 src/ipa/ipu3/ipu3_agc.cpp                     |   2 +-
 src/ipa/meson.build                           |   5 +-
 src/ipa/raspberrypi/raspberrypi.cpp           |   9 +-
 src/ipa/rkisp1/rkisp1.cpp                     |   6 +-
 src/libcamera/camera_sensor.cpp               | 117 +---------
 src/libcamera/ipa/core_ipa_interface.cpp      | 199 ++++++++++++++++++
 src/libcamera/ipa/meson.build                 |   5 +
 src/libcamera/meson.build                     |   1 +
 src/libcamera/pipeline/ipu3/ipu3.cpp          |  14 +-
 .../pipeline/raspberrypi/raspberrypi.cpp      |   4 +-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |   2 +-
 .../definition_functions.tmpl                 |   3 +
 26 files changed, 297 insertions(+), 244 deletions(-)
 create mode 100644 src/libcamera/ipa/core_ipa_interface.cpp
 create mode 100644 src/libcamera/ipa/meson.build

Hi Umang,

Thank you for the patch.

On Mon, May 24, 2021 at 02:50:23PM +0530, Umang Jain wrote:
> There can be multiple IPAs per pipeline-handler or platform.
> They can live in-tree or externally linked. It is upto the vendor

s/upto/up to/

I'll fix when applying.

> whether to use in-tree IPA for a platform or provide it externally.
> In the case of IPAs being external, building in-tree IPAs might be
> futile hence, provide a clear and explicit mechanism to choose whether
> to build the in-tree IPA for the platform or not.
> 
> By default, all in-tree IPAs are built when a matching Pipeline handler
> is also enabled.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  meson.build         | 1 +
>  meson_options.txt   | 5 +++++
>  src/ipa/meson.build | 5 +++--
>  3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index fa2a62cf..35c80c6a 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -167,6 +167,7 @@ py_mod.find_installation('python3', modules: py_modules)
>  ## Summarise Configurations
>  summary({
>              'Enabled pipelines': pipelines,
> +            'Enabled IPA modules': ipa_modules,
>              'Android support': android_enabled,
>              'GStreamer support': gst_enabled,
>              'V4L2 emulation support': v4l2_enabled,
> diff --git a/meson_options.txt b/meson_options.txt
> index 69f11f85..2c80ad8b 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -25,6 +25,11 @@ option('gstreamer',
>          value : 'auto',
>          description : 'Compile libcamera GStreamer plugin')
>  
> +option('ipas',
> +        type : 'array',
> +        choices : ['ipu3', 'raspberrypi', 'rkisp1', 'vimc'],
> +        description : 'Select which IPA modules to build')
> +
>  option('lc-compliance',
>          type : 'feature',
>          value : 'auto',
> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> index 5b5684a1..49245e5e 100644
> --- a/src/ipa/meson.build
> +++ b/src/ipa/meson.build
> @@ -19,14 +19,15 @@ subdir('libipa')
>  
>  ipa_sign = files('ipa-sign.sh')
>  
> -ipas = ['ipu3', 'raspberrypi', 'rkisp1', 'vimc']
>  ipa_names = []
>  
> +ipa_modules = get_option('ipas')
> +
>  # The ipa-sign-install.sh script which uses the ipa_names variable will itself
>  # prepend MESON_INSTALL_DESTDIR_PREFIX to each ipa module name, therefore we
>  # must not include the prefix string here.
>  foreach pipeline : pipelines
> -    if ipas.contains(pipeline)
> +    if ipa_modules.contains(pipeline)
>          subdir(pipeline)
>          ipa_names += ipa_install_dir / ipa_name + '.so'
>      endif