| Message ID | 20230503122035.32026-4-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi all, On Wed, 3 May 2023 at 13:20, Naushir Patuck <naush@raspberrypi.com> wrote: > > The current IPA build files require a flat directory struture for the > IPAs. Modify the build files to remove this restriction and allow a > directory structure such as: > > src/ipa > |- raspberrypi > |- common > |- cam_helpers > |- controller > |- vc4 > |- rkisp1 > |- ipu3 > > where each subdir (e.g. raspberrypi/common, raspberrypi/cam_helper) has > its own meson.build file. Such a directory structure will be introduced > for the Raspberry Pi IPA in a future commit. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > meson.build | 2 +- > src/ipa/ipu3/meson.build | 2 ++ > src/ipa/meson.build | 26 +++++++++++++++++++++----- > src/ipa/raspberrypi/meson.build | 2 ++ > src/ipa/rkisp1/meson.build | 2 ++ > 5 files changed, 28 insertions(+), 6 deletions(-) > > diff --git a/meson.build b/meson.build > index d3289181b7b9..2d99029bf5b7 100644 > --- a/meson.build > +++ b/meson.build > @@ -262,7 +262,7 @@ py_mod.find_installation('python3', modules: py_modules) > ## Summarise Configurations > summary({ > 'Enabled pipelines': pipelines, > - 'Enabled IPA modules': enabled_ipa_modules, > + 'Enabled IPA modules': enabled_ipa_names, > 'Tracing support': tracing_enabled, > 'Android support': android_enabled, > 'GStreamer support': gst_enabled, > diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build > index 658e7c9bc366..66c398432d43 100644 > --- a/src/ipa/ipu3/meson.build > +++ b/src/ipa/ipu3/meson.build > @@ -29,3 +29,5 @@ if ipa_sign_module > install : false, > build_by_default : true) > endif > + > +ipa_names += ipa_name > diff --git a/src/ipa/meson.build b/src/ipa/meson.build > index 76ad5b445601..fac92f32fdb9 100644 > --- a/src/ipa/meson.build > +++ b/src/ipa/meson.build > @@ -36,16 +36,32 @@ if get_option('test') and 'vimc' not in ipa_modules > endif > > enabled_ipa_modules = [] > +enabled_ipa_names = [] > +ipa_names = [] > > # 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. > + > +subdirs = [] > foreach pipeline : pipelines > - if ipa_modules.contains(pipeline) > - subdir(pipeline) > - ipa_names += ipa_install_dir / ipa_name + '.so' > - enabled_ipa_modules += pipeline > + if not ipa_modules.contains(pipeline) > + continue > + endif > + enabled_ipa_names += pipeline > + > + # Allow multi-level directory structuring for the IPAs if needed. > + pipeline = pipeline.split('/')[0] > + if pipeline in subdirs > + continue > endif > + > + subdir(pipeline) > + subdirs += [pipeline] I've just spotted a small bug in this code. The above line should read: subdirs += pipeline to work correctly. Should I send an in-reply-to update to this patch, or can that be fixed when applying (of course assuming there are no other changes required). Regards, Naush > +endforeach > + > +foreach ipa_name : ipa_names > + enabled_ipa_modules += ipa_install_dir / ipa_name + '.so' > endforeach > > if ipa_sign_module > @@ -54,5 +70,5 @@ if ipa_sign_module > # install time, which invalidates the signatures. > meson.add_install_script('ipa-sign-install.sh', > ipa_priv_key.full_path(), > - ipa_names) > + enabled_ipa_modules) > endif > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build > index de78cbd80f9c..95437cbcc962 100644 > --- a/src/ipa/raspberrypi/meson.build > +++ b/src/ipa/raspberrypi/meson.build > @@ -64,3 +64,5 @@ if ipa_sign_module > endif > > subdir('data') > + > +ipa_names += ipa_name > diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build > index ccb84b27525b..e813da53ae9b 100644 > --- a/src/ipa/rkisp1/meson.build > +++ b/src/ipa/rkisp1/meson.build > @@ -29,3 +29,5 @@ if ipa_sign_module > install : false, > build_by_default : true) > endif > + > +ipa_names += ipa_name > -- > 2.34.1 >
Hi Naush On Wed, May 03, 2023 at 01:20:25PM +0100, Naushir Patuck via libcamera-devel wrote: > The current IPA build files require a flat directory struture for the > IPAs. Modify the build files to remove this restriction and allow a > directory structure such as: > > src/ipa > |- raspberrypi > |- common > |- cam_helpers > |- controller > |- vc4 > |- rkisp1 > |- ipu3 > > where each subdir (e.g. raspberrypi/common, raspberrypi/cam_helper) has > its own meson.build file. Such a directory structure will be introduced > for the Raspberry Pi IPA in a future commit. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > meson.build | 2 +- > src/ipa/ipu3/meson.build | 2 ++ > src/ipa/meson.build | 26 +++++++++++++++++++++----- > src/ipa/raspberrypi/meson.build | 2 ++ > src/ipa/rkisp1/meson.build | 2 ++ > 5 files changed, 28 insertions(+), 6 deletions(-) > > diff --git a/meson.build b/meson.build > index d3289181b7b9..2d99029bf5b7 100644 > --- a/meson.build > +++ b/meson.build > @@ -262,7 +262,7 @@ py_mod.find_installation('python3', modules: py_modules) > ## Summarise Configurations > summary({ > 'Enabled pipelines': pipelines, > - 'Enabled IPA modules': enabled_ipa_modules, > + 'Enabled IPA modules': enabled_ipa_names, > 'Tracing support': tracing_enabled, > 'Android support': android_enabled, > 'GStreamer support': gst_enabled, > diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build > index 658e7c9bc366..66c398432d43 100644 > --- a/src/ipa/ipu3/meson.build > +++ b/src/ipa/ipu3/meson.build > @@ -29,3 +29,5 @@ if ipa_sign_module > install : false, > build_by_default : true) > endif > + > +ipa_names += ipa_name Not excited by this one here, as it seems easy to forget. Anyway, I know this version comes after a long discussion, so it's fine With your comment addressed Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > diff --git a/src/ipa/meson.build b/src/ipa/meson.build > index 76ad5b445601..fac92f32fdb9 100644 > --- a/src/ipa/meson.build > +++ b/src/ipa/meson.build > @@ -36,16 +36,32 @@ if get_option('test') and 'vimc' not in ipa_modules > endif > > enabled_ipa_modules = [] > +enabled_ipa_names = [] > +ipa_names = [] > > # 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. > + > +subdirs = [] > foreach pipeline : pipelines > - if ipa_modules.contains(pipeline) > - subdir(pipeline) > - ipa_names += ipa_install_dir / ipa_name + '.so' > - enabled_ipa_modules += pipeline > + if not ipa_modules.contains(pipeline) > + continue > + endif > + enabled_ipa_names += pipeline > + > + # Allow multi-level directory structuring for the IPAs if needed. > + pipeline = pipeline.split('/')[0] > + if pipeline in subdirs > + continue > endif > + > + subdir(pipeline) > + subdirs += [pipeline] > +endforeach > + > +foreach ipa_name : ipa_names > + enabled_ipa_modules += ipa_install_dir / ipa_name + '.so' > endforeach > > if ipa_sign_module > @@ -54,5 +70,5 @@ if ipa_sign_module > # install time, which invalidates the signatures. > meson.add_install_script('ipa-sign-install.sh', > ipa_priv_key.full_path(), > - ipa_names) > + enabled_ipa_modules) > endif > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build > index de78cbd80f9c..95437cbcc962 100644 > --- a/src/ipa/raspberrypi/meson.build > +++ b/src/ipa/raspberrypi/meson.build > @@ -64,3 +64,5 @@ if ipa_sign_module > endif > > subdir('data') > + > +ipa_names += ipa_name > diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build > index ccb84b27525b..e813da53ae9b 100644 > --- a/src/ipa/rkisp1/meson.build > +++ b/src/ipa/rkisp1/meson.build > @@ -29,3 +29,5 @@ if ipa_sign_module > install : false, > build_by_default : true) > endif > + > +ipa_names += ipa_name > -- > 2.34.1 >
Hi Naush, On Thu, May 04, 2023 at 08:55:24AM +0100, Naushir Patuck via libcamera-devel wrote: > On Wed, 3 May 2023 at 13:20, Naushir Patuck wrote: > > > > The current IPA build files require a flat directory struture for the s/struture/structure/ > > IPAs. Modify the build files to remove this restriction and allow a > > directory structure such as: > > > > src/ipa > > |- raspberrypi > > |- common > > |- cam_helpers > > |- controller > > |- vc4 > > |- rkisp1 > > |- ipu3 > > > > where each subdir (e.g. raspberrypi/common, raspberrypi/cam_helper) has > > its own meson.build file. Such a directory structure will be introduced > > for the Raspberry Pi IPA in a future commit. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > --- > > meson.build | 2 +- > > src/ipa/ipu3/meson.build | 2 ++ > > src/ipa/meson.build | 26 +++++++++++++++++++++----- > > src/ipa/raspberrypi/meson.build | 2 ++ > > src/ipa/rkisp1/meson.build | 2 ++ > > 5 files changed, 28 insertions(+), 6 deletions(-) > > > > diff --git a/meson.build b/meson.build > > index d3289181b7b9..2d99029bf5b7 100644 > > --- a/meson.build > > +++ b/meson.build > > @@ -262,7 +262,7 @@ py_mod.find_installation('python3', modules: py_modules) > > ## Summarise Configurations > > summary({ > > 'Enabled pipelines': pipelines, > > - 'Enabled IPA modules': enabled_ipa_modules, > > + 'Enabled IPA modules': enabled_ipa_names, > > 'Tracing support': tracing_enabled, > > 'Android support': android_enabled, > > 'GStreamer support': gst_enabled, > > diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build > > index 658e7c9bc366..66c398432d43 100644 > > --- a/src/ipa/ipu3/meson.build > > +++ b/src/ipa/ipu3/meson.build > > @@ -29,3 +29,5 @@ if ipa_sign_module > > install : false, > > build_by_default : true) > > endif > > + > > +ipa_names += ipa_name > > diff --git a/src/ipa/meson.build b/src/ipa/meson.build > > index 76ad5b445601..fac92f32fdb9 100644 > > --- a/src/ipa/meson.build > > +++ b/src/ipa/meson.build > > @@ -36,16 +36,32 @@ if get_option('test') and 'vimc' not in ipa_modules > > endif > > > > enabled_ipa_modules = [] > > +enabled_ipa_names = [] > > +ipa_names = [] > > > > # The ipa-sign-install.sh script which uses the ipa_names variable will itself ipa_names should be renamed to enabled_ipa_modules. This whole comment should actually move after this foreach loop. > > # prepend MESON_INSTALL_DESTDIR_PREFIX to each ipa module name, therefore we > > # must not include the prefix string here. > > + > > +subdirs = [] > > foreach pipeline : pipelines > > - if ipa_modules.contains(pipeline) > > - subdir(pipeline) > > - ipa_names += ipa_install_dir / ipa_name + '.so' > > - enabled_ipa_modules += pipeline > > + if not ipa_modules.contains(pipeline) > > + continue > > + endif > > + enabled_ipa_names += pipeline > > + > > + # Allow multi-level directory structuring for the IPAs if needed. > > + pipeline = pipeline.split('/')[0] > > + if pipeline in subdirs > > + continue > > endif > > + > > + subdir(pipeline) > > + subdirs += [pipeline] > > I've just spotted a small bug in this code. The above line should read: > > subdirs += pipeline > > to work correctly. Should I send an in-reply-to update to this patch, or can > that be fixed when applying (of course assuming there are no other changes > required). I'll fix it locally. > > +endforeach > > + > > +foreach ipa_name : ipa_names > > + enabled_ipa_modules += ipa_install_dir / ipa_name + '.so' > > endforeach > > > > if ipa_sign_module > > @@ -54,5 +70,5 @@ if ipa_sign_module > > # install time, which invalidates the signatures. > > meson.add_install_script('ipa-sign-install.sh', > > ipa_priv_key.full_path(), > > - ipa_names) > > + enabled_ipa_modules) > > endif > > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build > > index de78cbd80f9c..95437cbcc962 100644 > > --- a/src/ipa/raspberrypi/meson.build > > +++ b/src/ipa/raspberrypi/meson.build > > @@ -64,3 +64,5 @@ if ipa_sign_module > > endif > > > > subdir('data') > > + > > +ipa_names += ipa_name > > diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build > > index ccb84b27525b..e813da53ae9b 100644 > > --- a/src/ipa/rkisp1/meson.build > > +++ b/src/ipa/rkisp1/meson.build > > @@ -29,3 +29,5 @@ if ipa_sign_module > > install : false, > > build_by_default : true) > > endif > > + > > +ipa_names += ipa_name Shouldn't vimc also do this ? With these small issues fixed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I can handle the fixes locally.
Hi Laurent, On Thu, 4 May 2023 at 17:06, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Naush, > > On Thu, May 04, 2023 at 08:55:24AM +0100, Naushir Patuck via libcamera-devel wrote: > > On Wed, 3 May 2023 at 13:20, Naushir Patuck wrote: > > > > > > The current IPA build files require a flat directory struture for the > > s/struture/structure/ > > > > IPAs. Modify the build files to remove this restriction and allow a > > > directory structure such as: > > > > > > src/ipa > > > |- raspberrypi > > > |- common > > > |- cam_helpers > > > |- controller > > > |- vc4 > > > |- rkisp1 > > > |- ipu3 > > > > > > where each subdir (e.g. raspberrypi/common, raspberrypi/cam_helper) has > > > its own meson.build file. Such a directory structure will be introduced > > > for the Raspberry Pi IPA in a future commit. > > > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > > --- > > > meson.build | 2 +- > > > src/ipa/ipu3/meson.build | 2 ++ > > > src/ipa/meson.build | 26 +++++++++++++++++++++----- > > > src/ipa/raspberrypi/meson.build | 2 ++ > > > src/ipa/rkisp1/meson.build | 2 ++ > > > 5 files changed, 28 insertions(+), 6 deletions(-) > > > > > > diff --git a/meson.build b/meson.build > > > index d3289181b7b9..2d99029bf5b7 100644 > > > --- a/meson.build > > > +++ b/meson.build > > > @@ -262,7 +262,7 @@ py_mod.find_installation('python3', modules: py_modules) > > > ## Summarise Configurations > > > summary({ > > > 'Enabled pipelines': pipelines, > > > - 'Enabled IPA modules': enabled_ipa_modules, > > > + 'Enabled IPA modules': enabled_ipa_names, > > > 'Tracing support': tracing_enabled, > > > 'Android support': android_enabled, > > > 'GStreamer support': gst_enabled, > > > diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build > > > index 658e7c9bc366..66c398432d43 100644 > > > --- a/src/ipa/ipu3/meson.build > > > +++ b/src/ipa/ipu3/meson.build > > > @@ -29,3 +29,5 @@ if ipa_sign_module > > > install : false, > > > build_by_default : true) > > > endif > > > + > > > +ipa_names += ipa_name > > > diff --git a/src/ipa/meson.build b/src/ipa/meson.build > > > index 76ad5b445601..fac92f32fdb9 100644 > > > --- a/src/ipa/meson.build > > > +++ b/src/ipa/meson.build > > > @@ -36,16 +36,32 @@ if get_option('test') and 'vimc' not in ipa_modules > > > endif > > > > > > enabled_ipa_modules = [] > > > +enabled_ipa_names = [] > > > +ipa_names = [] > > > > > > # The ipa-sign-install.sh script which uses the ipa_names variable will itself > > ipa_names should be renamed to enabled_ipa_modules. > > This whole comment should actually move after this foreach loop. > > > > # prepend MESON_INSTALL_DESTDIR_PREFIX to each ipa module name, therefore we > > > # must not include the prefix string here. > > > + > > > +subdirs = [] > > > foreach pipeline : pipelines > > > - if ipa_modules.contains(pipeline) > > > - subdir(pipeline) > > > - ipa_names += ipa_install_dir / ipa_name + '.so' > > > - enabled_ipa_modules += pipeline > > > + if not ipa_modules.contains(pipeline) > > > + continue > > > + endif > > > + enabled_ipa_names += pipeline > > > + > > > + # Allow multi-level directory structuring for the IPAs if needed. > > > + pipeline = pipeline.split('/')[0] > > > + if pipeline in subdirs > > > + continue > > > endif > > > + > > > + subdir(pipeline) > > > + subdirs += [pipeline] > > > > I've just spotted a small bug in this code. The above line should read: > > > > subdirs += pipeline > > > > to work correctly. Should I send an in-reply-to update to this patch, or can > > that be fixed when applying (of course assuming there are no other changes > > required). > > I'll fix it locally. > > > > +endforeach > > > + > > > +foreach ipa_name : ipa_names > > > + enabled_ipa_modules += ipa_install_dir / ipa_name + '.so' > > > endforeach > > > > > > if ipa_sign_module > > > @@ -54,5 +70,5 @@ if ipa_sign_module > > > # install time, which invalidates the signatures. > > > meson.add_install_script('ipa-sign-install.sh', > > > ipa_priv_key.full_path(), > > > - ipa_names) > > > + enabled_ipa_modules) > > > endif > > > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build > > > index de78cbd80f9c..95437cbcc962 100644 > > > --- a/src/ipa/raspberrypi/meson.build > > > +++ b/src/ipa/raspberrypi/meson.build > > > @@ -64,3 +64,5 @@ if ipa_sign_module > > > endif > > > > > > subdir('data') > > > + > > > +ipa_names += ipa_name > > > diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build > > > index ccb84b27525b..e813da53ae9b 100644 > > > --- a/src/ipa/rkisp1/meson.build > > > +++ b/src/ipa/rkisp1/meson.build > > > @@ -29,3 +29,5 @@ if ipa_sign_module > > > install : false, > > > build_by_default : true) > > > endif > > > + > > > +ipa_names += ipa_name > > Shouldn't vimc also do this ? Yes, not sure how/why I missed that up. > > With these small issues fixed, > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > I can handle the fixes locally. Thanks for that! Regards, Naush > > -- > Regards, > > Laurent Pinchart
diff --git a/meson.build b/meson.build index d3289181b7b9..2d99029bf5b7 100644 --- a/meson.build +++ b/meson.build @@ -262,7 +262,7 @@ py_mod.find_installation('python3', modules: py_modules) ## Summarise Configurations summary({ 'Enabled pipelines': pipelines, - 'Enabled IPA modules': enabled_ipa_modules, + 'Enabled IPA modules': enabled_ipa_names, 'Tracing support': tracing_enabled, 'Android support': android_enabled, 'GStreamer support': gst_enabled, diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build index 658e7c9bc366..66c398432d43 100644 --- a/src/ipa/ipu3/meson.build +++ b/src/ipa/ipu3/meson.build @@ -29,3 +29,5 @@ if ipa_sign_module install : false, build_by_default : true) endif + +ipa_names += ipa_name diff --git a/src/ipa/meson.build b/src/ipa/meson.build index 76ad5b445601..fac92f32fdb9 100644 --- a/src/ipa/meson.build +++ b/src/ipa/meson.build @@ -36,16 +36,32 @@ if get_option('test') and 'vimc' not in ipa_modules endif enabled_ipa_modules = [] +enabled_ipa_names = [] +ipa_names = [] # 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. + +subdirs = [] foreach pipeline : pipelines - if ipa_modules.contains(pipeline) - subdir(pipeline) - ipa_names += ipa_install_dir / ipa_name + '.so' - enabled_ipa_modules += pipeline + if not ipa_modules.contains(pipeline) + continue + endif + enabled_ipa_names += pipeline + + # Allow multi-level directory structuring for the IPAs if needed. + pipeline = pipeline.split('/')[0] + if pipeline in subdirs + continue endif + + subdir(pipeline) + subdirs += [pipeline] +endforeach + +foreach ipa_name : ipa_names + enabled_ipa_modules += ipa_install_dir / ipa_name + '.so' endforeach if ipa_sign_module @@ -54,5 +70,5 @@ if ipa_sign_module # install time, which invalidates the signatures. meson.add_install_script('ipa-sign-install.sh', ipa_priv_key.full_path(), - ipa_names) + enabled_ipa_modules) endif diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build index de78cbd80f9c..95437cbcc962 100644 --- a/src/ipa/raspberrypi/meson.build +++ b/src/ipa/raspberrypi/meson.build @@ -64,3 +64,5 @@ if ipa_sign_module endif subdir('data') + +ipa_names += ipa_name diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build index ccb84b27525b..e813da53ae9b 100644 --- a/src/ipa/rkisp1/meson.build +++ b/src/ipa/rkisp1/meson.build @@ -29,3 +29,5 @@ if ipa_sign_module install : false, build_by_default : true) endif + +ipa_names += ipa_name
The current IPA build files require a flat directory struture for the IPAs. Modify the build files to remove this restriction and allow a directory structure such as: src/ipa |- raspberrypi |- common |- cam_helpers |- controller |- vc4 |- rkisp1 |- ipu3 where each subdir (e.g. raspberrypi/common, raspberrypi/cam_helper) has its own meson.build file. Such a directory structure will be introduced for the Raspberry Pi IPA in a future commit. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- meson.build | 2 +- src/ipa/ipu3/meson.build | 2 ++ src/ipa/meson.build | 26 +++++++++++++++++++++----- src/ipa/raspberrypi/meson.build | 2 ++ src/ipa/rkisp1/meson.build | 2 ++ 5 files changed, 28 insertions(+), 6 deletions(-)