[v3,0/4] ipa: Allow IPA creation by name

24686 mbox series

It is getting more and more common for different pipeline handlers to
use the same IPA modules whose name differs from the pipeline's one.

The first case has been the softISP, and now the in-review Renesas R-Car
V4H pipeline handler will re-use the RkISP1 IPA.

Starting from Hans' patch https://patchwork.libcamera.org/patch/23359/
"libcamera: ipa_manager: createIPA: Allow matching by IPA name instead
of by pipeline" I here took a slightly different approach and addressed
the review comments on Hans' v1 by creating an overload for
IPAManager::createIPA() that allows pipelines to specify the IPA module
name. If they don't provide a name, the pipeline handler's name is used.

As the next step, to further decouple IPAs from pipelines, remove the
pipelineName field from IPAModuleInfo that in all cases is identical to
the IPA name itself.

Finaly, in v3, allow to build IPA modules whose name differs from the
pipeline. This is required for R-Car Gen4 which would otherwise require
to build the rkisp1 pipeline in to get the IPA.

I have considered more complex approaches that do not require a map in
src/ipa/meson.build, like defining an 'auto' option for ipas, but it
would have required to

	-Dpipelines=rcar-gen4 -Dipas=rkisp1

while a simple

	-Dpipelines=rcar-gen4

which ropes in rkisp1 IPA I think it's preferrable for users.

Tested on R-Car Gen4 and on Mali-C55.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
Changes in v3:
- Add patches 3 and 4

Changes in v2:
- Changed Hans authorship to different email address
- Removed stray blank line
- Collect tags

---
Hans de Goede (1):
      libcamera: ipa_manager: Create IPA by name

Jacopo Mondi (3):
      ipa: ipa_module: Remove pipelineName
      ipa: meson.build: Remove duplicated variable
      ipa: Allow pipelines to have differently named IPA

 include/libcamera/internal/ipa_manager.h | 13 +++++++++--
 include/libcamera/internal/ipa_module.h  |  4 ++--
 include/libcamera/ipa/ipa_module_info.h  |  1 -
 src/ipa/ipu3/ipu3.cpp                    |  1 -
 src/ipa/mali-c55/mali-c55.cpp            |  1 -
 src/ipa/meson.build                      | 40 +++++++++++++++++++++++---------
 src/ipa/rkisp1/rkisp1.cpp                |  1 -
 src/ipa/rpi/pisp/pisp.cpp                |  1 -
 src/ipa/rpi/vc4/vc4.cpp                  |  1 -
 src/ipa/simple/soft_simple.cpp           |  1 -
 src/ipa/vimc/vimc.cpp                    |  1 -
 src/libcamera/ipa_manager.cpp            | 34 ++++++++++++++++++++++-----
 src/libcamera/ipa_module.cpp             | 27 ++++++++++-----------
 test/ipa/ipa_module_test.cpp             |  3 ---
 14 files changed, 82 insertions(+), 47 deletions(-)
---
base-commit: 05bfebed2657cc1d032c9796efd9041bfbdc881c
change-id: 20251002-ipa-match-by-name-82e6b34a23e9

Best regards,

Hi Jacopo,

Thanks for your series.

On 2025-10-15 17:55:23 +0200, Jacopo Mondi wrote:
> It is getting more and more common for different pipeline handlers to
> use the same IPA modules whose name differs from the pipeline's one.
> 
> The first case has been the softISP, and now the in-review Renesas R-Car
> V4H pipeline handler will re-use the RkISP1 IPA.
> 
> Starting from Hans' patch https://patchwork.libcamera.org/patch/23359/
> "libcamera: ipa_manager: createIPA: Allow matching by IPA name instead
> of by pipeline" I here took a slightly different approach and addressed
> the review comments on Hans' v1 by creating an overload for
> IPAManager::createIPA() that allows pipelines to specify the IPA module
> name. If they don't provide a name, the pipeline handler's name is used.
> 
> As the next step, to further decouple IPAs from pipelines, remove the
> pipelineName field from IPAModuleInfo that in all cases is identical to
> the IPA name itself.
> 
> Finaly, in v3, allow to build IPA modules whose name differs from the
> pipeline. This is required for R-Car Gen4 which would otherwise require
> to build the rkisp1 pipeline in to get the IPA.
> 
> I have considered more complex approaches that do not require a map in
> src/ipa/meson.build, like defining an 'auto' option for ipas, but it
> would have required to
> 
> 	-Dpipelines=rcar-gen4 -Dipas=rkisp1
> 
> while a simple
> 
> 	-Dpipelines=rcar-gen4
> 
> which ropes in rkisp1 IPA I think it's preferrable for users.
> 
> Tested on R-Car Gen4 and on Mali-C55.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

I too tested this on R-Car Gen4 with the latest changes to the 
out-of-tree pipeline handler.

Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> Changes in v3:
> - Add patches 3 and 4
> 
> Changes in v2:
> - Changed Hans authorship to different email address
> - Removed stray blank line
> - Collect tags
> 
> ---
> Hans de Goede (1):
>       libcamera: ipa_manager: Create IPA by name
> 
> Jacopo Mondi (3):
>       ipa: ipa_module: Remove pipelineName
>       ipa: meson.build: Remove duplicated variable
>       ipa: Allow pipelines to have differently named IPA
> 
>  include/libcamera/internal/ipa_manager.h | 13 +++++++++--
>  include/libcamera/internal/ipa_module.h  |  4 ++--
>  include/libcamera/ipa/ipa_module_info.h  |  1 -
>  src/ipa/ipu3/ipu3.cpp                    |  1 -
>  src/ipa/mali-c55/mali-c55.cpp            |  1 -
>  src/ipa/meson.build                      | 40 +++++++++++++++++++++++---------
>  src/ipa/rkisp1/rkisp1.cpp                |  1 -
>  src/ipa/rpi/pisp/pisp.cpp                |  1 -
>  src/ipa/rpi/vc4/vc4.cpp                  |  1 -
>  src/ipa/simple/soft_simple.cpp           |  1 -
>  src/ipa/vimc/vimc.cpp                    |  1 -
>  src/libcamera/ipa_manager.cpp            | 34 ++++++++++++++++++++++-----
>  src/libcamera/ipa_module.cpp             | 27 ++++++++++-----------
>  test/ipa/ipa_module_test.cpp             |  3 ---
>  14 files changed, 82 insertions(+), 47 deletions(-)
> ---
> base-commit: 05bfebed2657cc1d032c9796efd9041bfbdc881c
> change-id: 20251002-ipa-match-by-name-82e6b34a23e9
> 
> Best regards,
> -- 
> Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>