| Message ID | 20210521132823.322076-1-umang.jain@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Fri, May 21, 2021 at 06:58:23PM +0530, Umang Jain wrote: > There can be multiple IPAs per pipeline-handler or platform. > They can live in-tree or externally linked. To support the externally > linked IPA use-case, provide a mechanism to choose whether or not > to build the IPAs in tree, with the help of a meson configuration > option. We can still build an out-of-tree IPA module if the in-tree module is built. I don't dispute the fact that it's useful to select which in-tree IPA modules to build, but maybe a more accurate explanation could be useful in the commit message ? Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > 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> > --- > 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
Hi Umang, Thank you for the series. You're nearly there, I've only had minor comments. The corresponding changes are however a bit too large for me to be comfortable with fixing them when applying, so I'm afraid a v4 is needed. I'm quite sure it will be the last one. On Fri, May 21, 2021 at 06:58:17PM +0530, Umang Jain wrote: > 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 | 237 ++++++++++++++++++ > 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 +- > 25 files changed, 332 insertions(+), 244 deletions(-) > create mode 100644 src/libcamera/ipa/core_ipa_interface.cpp > create mode 100644 src/libcamera/ipa/meson.build >
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 | 237 ++++++++++++++++++ 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 +- 25 files changed, 332 insertions(+), 244 deletions(-) create mode 100644 src/libcamera/ipa/core_ipa_interface.cpp create mode 100644 src/libcamera/ipa/meson.build