[libcamera-devel] [PATCH] DNI: Allow nested IPA builds

[Laurent Pinchart]

Hi Naush,

On Fri, Apr 28, 2023 at 02:12:15PM +0100, Naushir Patuck via libcamera-devel wrote:
> ONLY FOR TESTING!
> ---
> 
> Hi Laurent,
> 
> Here is an attempt at allowing the IPAs to be built within nested directories.
> It seems to do the trick, but you might be better suited to do something cleaner
> if desired.
> 
> Regards,
> Naush
> 
>  src/ipa/ipu3/meson.build   |  2 ++
>  src/ipa/meson.build        | 33 +++++++++++++++++++--------------
>  src/ipa/rkisp1/meson.build |  2 ++
>  src/ipa/rpi/meson.build    | 14 ++++++++++++++
>  4 files changed, 37 insertions(+), 14 deletions(-)
>  create mode 100644 src/ipa/rpi/meson.build
> 
> 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 0c622c38a4a0..ca7fc90e21e0 100644
> --- a/src/ipa/meson.build
> +++ b/src/ipa/meson.build
> @@ -36,34 +36,39 @@ if get_option('test') and 'vimc' not in ipa_modules
>  endif
>  
>  enabled_ipa_modules = []
> -
> -# If the Raspberry Pi VC4 IPA is enabled, ensure we include the cam_helper,
> -# common and controller subdirectories in the build.
> -#
> -# This is done here and not within rpi/vc4/meson.build as meson does not
> -# allow the subdir command to traverse up the directory tree.
> -if pipelines.contains('rpi/vc4')
> -    subdir('rpi/cam_helper')
> -    subdir('rpi/common')
> -    subdir('rpi/controller')
> -endif
> +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)

You could do

    if not ipa_modules.contains(pipeline)
        continue
    endif

to lower the indentation level.

> -        subdir(pipeline)
> -        ipa_names += ipa_install_dir / ipa_name + '.so'
>          enabled_ipa_modules += pipeline

As ipa_names now contain the names of the IPA modules, not the module
full path, I'd use enabled_ipa_names here (and update the top-level
meson.build file accordingly).

> +
> +        # Allow multi-level directory structuring for the IPAs if needed.
> +        # An example would be the Raspberry Pi IPA (rpi).
> +        pipeline = pipeline.split('/')[0]
> +        if pipeline in subdirs
> +            continue
> +        endif
> +
> +        subdir(pipeline)
> +        subdirs += [pipeline]
>      endif
>  endforeach
>  
> +foreach ipa_name : ipa_names
> +    enabled_ipa_names += ipa_install_dir / ipa_name + '.so'

And here I'd use enabled_ipa_modules.

> +endforeach
> +
>  if ipa_sign_module
>      # Regenerate the signatures for all IPA modules. We can't simply install the
>      # .sign files, as meson strips the DT_RPATH and DT_RUNPATH from binaries at
>      # install time, which invalidates the signatures.
>      meson.add_install_script('ipa-sign-install.sh',
>                               ipa_priv_key.full_path(),
> -                             ipa_names)
> +                             enabled_ipa_names)

Same here.

Apart from that, I think this looks good.


>  endif
> 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
> diff --git a/src/ipa/rpi/meson.build b/src/ipa/rpi/meson.build
> new file mode 100644
> index 000000000000..4811c76f3f9e
> --- /dev/null
> +++ b/src/ipa/rpi/meson.build
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +subdir('cam_helper')
> +subdir('common')
> +subdir('controller')
> +
> +foreach pipeline : pipelines
> +    pipeline = pipeline.split('/')
> +    if pipeline.length() < 2 or pipeline[0] != 'rpi'
> +        continue
> +    endif
> +
> +    subdir(pipeline[1])
> +endforeach

-- 
Regards,

Laurent Pinchart