[{"id":27013,"web_url":"https://patchwork.libcamera.org/comment/27013/","msgid":"<20230502160257.GD22691@pendragon.ideasonboard.com>","date":"2023-05-02T16:02:57","subject":"Re: [libcamera-devel] [PATCH] DNI: Allow nested IPA builds","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Fri, Apr 28, 2023 at 02:12:15PM +0100, Naushir Patuck via libcamera-devel wrote:\n> ONLY FOR TESTING!\n> ---\n> \n> Hi Laurent,\n> \n> Here is an attempt at allowing the IPAs to be built within nested directories.\n> It seems to do the trick, but you might be better suited to do something cleaner\n> if desired.\n> \n> Regards,\n> Naush\n> \n> src/ipa/ipu3/meson.build | 2 ++\n> src/ipa/meson.build | 33 +++++++++++++++++++--------------\n> src/ipa/rkisp1/meson.build | 2 ++\n> src/ipa/rpi/meson.build | 14 ++++++++++++++\n> 4 files changed, 37 insertions(+), 14 deletions(-)\n> create mode 100644 src/ipa/rpi/meson.build\n> \n> diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build\n> index 658e7c9bc366..66c398432d43 100644\n> --- a/src/ipa/ipu3/meson.build\n> +++ b/src/ipa/ipu3/meson.build\n> @@ -29,3 +29,5 @@ if ipa_sign_module\n> install : false,\n> build_by_default : true)\n> endif\n> +\n> +ipa_names += ipa_name\n> diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> index 0c622c38a4a0..ca7fc90e21e0 100644\n> --- a/src/ipa/meson.build\n> +++ b/src/ipa/meson.build\n> @@ -36,34 +36,39 @@ if get_option('test') and 'vimc' not in ipa_modules\n> endif\n> \n> enabled_ipa_modules = []\n> -\n> -# If the Raspberry Pi VC4 IPA is enabled, ensure we include the cam_helper,\n> -# common and controller subdirectories in the build.\n> -#\n> -# This is done here and not within rpi/vc4/meson.build as meson does not\n> -# allow the subdir command to traverse up the directory tree.\n> -if pipelines.contains('rpi/vc4')\n> - subdir('rpi/cam_helper')\n> - subdir('rpi/common')\n> - subdir('rpi/controller')\n> -endif\n> +enabled_ipa_names = []\n> +ipa_names = []\n> \n> # The ipa-sign-install.sh script which uses the ipa_names variable will itself\n> # prepend MESON_INSTALL_DESTDIR_PREFIX to each ipa module name, therefore we\n> # must not include the prefix string here.\n> +\n> +subdirs = []\n> foreach pipeline : pipelines\n> if ipa_modules.contains(pipeline)\n\nYou could do\n\n if not ipa_modules.contains(pipeline)\n continue\n endif\n\nto lower the indentation level.\n\n> - subdir(pipeline)\n> - ipa_names += ipa_install_dir / ipa_name + '.so'\n> enabled_ipa_modules += pipeline\n\nAs ipa_names now contain the names of the IPA modules, not the module\nfull path, I'd use enabled_ipa_names here (and update the top-level\nmeson.build file accordingly).\n\n> +\n> + # Allow multi-level directory structuring for the IPAs if needed.\n> + # An example would be the Raspberry Pi IPA (rpi).\n> + pipeline = pipeline.split('/')[0]\n> + if pipeline in subdirs\n> + continue\n> + endif\n> +\n> + subdir(pipeline)\n> + subdirs += [pipeline]\n> endif\n> endforeach\n> \n> +foreach ipa_name : ipa_names\n> + enabled_ipa_names += ipa_install_dir / ipa_name + '.so'\n\nAnd here I'd use enabled_ipa_modules.\n\n> +endforeach\n> +\n> if ipa_sign_module\n> # Regenerate the signatures for all IPA modules. We can't simply install the\n> # .sign files, as meson strips the DT_RPATH and DT_RUNPATH from binaries at\n> # install time, which invalidates the signatures.\n> meson.add_install_script('ipa-sign-install.sh',\n> ipa_priv_key.full_path(),\n> - ipa_names)\n> + enabled_ipa_names)\n\nSame here.\n\nApart from that, I think this looks good.\n\n\n> endif\n> diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build\n> index ccb84b27525b..e813da53ae9b 100644\n> --- a/src/ipa/rkisp1/meson.build\n> +++ b/src/ipa/rkisp1/meson.build\n> @@ -29,3 +29,5 @@ if ipa_sign_module\n> install : false,\n> build_by_default : true)\n> endif\n> +\n> +ipa_names += ipa_name\n> diff --git a/src/ipa/rpi/meson.build b/src/ipa/rpi/meson.build\n> new file mode 100644\n> index 000000000000..4811c76f3f9e\n> --- /dev/null\n> +++ b/src/ipa/rpi/meson.build\n> @@ -0,0 +1,14 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +subdir('cam_helper')\n> +subdir('common')\n> +subdir('controller')\n> +\n> +foreach pipeline : pipelines\n> + pipeline = pipeline.split('/')\n> + if pipeline.length() < 2 or pipeline[0] != 'rpi'\n> + continue\n> + endif\n> +\n> + subdir(pipeline[1])\n> +endforeach","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7F331BD16B\n\tfor ;\n\tTue, 2 May 2023 16:02:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E5576633B4;\n\tTue, 2 May 2023 18:02:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9FCE960538\n\tfor ;\n\tTue, 2 May 2023 18:02:45 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(133-32-181-51.west.xps.vectant.ne.jp [133.32.181.51])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0795E800;\n\tTue, 2 May 2023 18:02:42 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683043366;\n\tbh=Nz1flu/ccbalBmArfHR+6kdG97xVH5Qn0pDOmFsnZa4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=KdHLF4KA5JGCYO+UiKZmHFnDtctRJ3D3sIfazULBBT1sfAq0bKXjBr0sLuuqeHSmq\n\tdaAbQy25vbP6PRXtqBrZD7M5/xvViNvD6A4GEujPQtYtCmf7/vyI6CHg398vZfcJ+P\n\tkG4558JdxJajBbhjmFARkxMZZlYNdI6kywRYG0GviMbbhyGB6x73bdEfiIaUjopyd+\n\tteC2k0dWFHxErTNjO3p3VqPpvcXk3y0uZ24jqQS5xDgo5mxJBsask+yPZgPMX6O3h/\n\tLyJWXWDphLWFYsGgp335Dp3RgIqnr+DLDMAeVhFYH+uO2cy2ChQMTJdg8Aoh7dGMTE\n\tJ3hfqUcvlRCUA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1683043363;\n\tbh=Nz1flu/ccbalBmArfHR+6kdG97xVH5Qn0pDOmFsnZa4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UdCl6gJdK1eE0cdrMlrJDuhwzLwjxHj9YfvWpJoE5stU+XTUmFdzrMi99cXtpcMvQ\n\t6ze23EvzmM2pk1T2zgZgybOn8lh3Y34QWt3RXBSTxTGPmRiV0C2F2E3au03egMrrZo\n\tkKZu0w4iPdm8Y0d3KF8ckx7vQF8evymdSyjWGXz0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"UdCl6gJd\"; dkim-atps=neutral","Date":"Tue, 2 May 2023 19:02:57 +0300","To":"Naushir Patuck ","Message-ID":"<20230502160257.GD22691@pendragon.ideasonboard.com>","References":"<20230428131215.16070-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230428131215.16070-1-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH] DNI: Allow nested IPA builds","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]