[v4,0/4] ipa: Allow IPA creation by name
mbox series

Message ID 20260408115606.12417-1-johannes.goede@oss.qualcomm.com
Headers show
Series
  • ipa: Allow IPA creation by name
Related show

Message

Hans de Goede April 8, 2026, 11:56 a.m. UTC
Hi All,

This is v4 of Jacopo's v3 "ipa: Allow IPA creation by name" series:
https://patchwork.libcamera.org/project/libcamera/list/?series=5505
With that v3 itself being based on my original attempt to address this:
https://patchwork.libcamera.org/patch/23359/

Since the new camss pipeline handler will use the softISP as a fallback
on platforms where there is no hardware ISP support yet (so most platforms)
it needs to be able to load the "simple" IPA from a pipeline-handler named
"camss" requiring something like this series. This is the first of 3 series
which together introduce the camss pipeline-handler. Here is a branch with
all 3 series:
https://github.com/jwrdegoede/libcamera/commits/camss_pipeline_v1/

I hope to get this prep series merged while work continues on the camss
pipeline handler itself.

Other use-cases for this are using the rkips1 IPA with the rcar-gen4
pipeline handler and using the simple IPA with the atomisp pipeline
handler.

This has been tested by Jacopo on R-Car Gen4 and on Mali-C55 and by me
on Qualcomm Agetti and Hamoa SoCs with both the simple and camss pipeline
handlers.

Changes in v4:
- Rebase, change author of first patch to my new email address

Changes in v3:
- Add "ipa: Allow pipelines to have differently named IPA" patch

Changes in v2:
- Update tags, fix stray blank line

Changes in v1 from Hans' original patch:
- Slightly different approach addressing the review comments on Hans' v1 by
  creating an overload for IPAManager::createIPA() that allows pipelines to
  specify the IPA module name.

Jacopo's original v3 cover-letter:
https://lists.libcamera.org/pipermail/libcamera-devel/2025-October/053821.html

Regards,

Hans


Hans de Goede (2):
  libcamera: ipa_manager: Create IPA by name
  libcamera: software_isp: Explicitly load "simple" IPA

Jacopo Mondi (2):
  ipa: ipa_module: Remove pipelineName
  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 +++++++-------
 src/libcamera/software_isp/software_isp.cpp |  2 +-
 test/ipa/ipa_module_test.cpp                |  3 --
 15 files changed, 84 insertions(+), 47 deletions(-)

Comments

Jacopo Mondi April 9, 2026, 7:53 a.m. UTC | #1
Hi Hans

On Wed, Apr 08, 2026 at 01:56:02PM +0200, Hans de Goede wrote:
> Hi All,
>
> This is v4 of Jacopo's v3 "ipa: Allow IPA creation by name" series:
> https://patchwork.libcamera.org/project/libcamera/list/?series=5505
> With that v3 itself being based on my original attempt to address this:
> https://patchwork.libcamera.org/patch/23359/
>
> Since the new camss pipeline handler will use the softISP as a fallback
> on platforms where there is no hardware ISP support yet (so most platforms)
> it needs to be able to load the "simple" IPA from a pipeline-handler named
> "camss" requiring something like this series. This is the first of 3 series
> which together introduce the camss pipeline-handler. Here is a branch with
> all 3 series:
> https://github.com/jwrdegoede/libcamera/commits/camss_pipeline_v1/
>
> I hope to get this prep series merged while work continues on the camss
> pipeline handler itself.
>
> Other use-cases for this are using the rkips1 IPA with the rcar-gen4
> pipeline handler and using the simple IPA with the atomisp pipeline
> handler.
>
> This has been tested by Jacopo on R-Car Gen4 and on Mali-C55 and by me
> on Qualcomm Agetti and Hamoa SoCs with both the simple and camss pipeline
> handlers.

Thanks for re-proposing this.

I still think this is desirable and doesn't introduce major drawbacks.
If instead, there are compelling reasons why we should do this I would
like them to be clearly listed. I have gone through previous versions
of the series and I didn't find any email clearly stating why this has
been refused.

One note: for R-Car Gen4 we won't probably need it anymore in the
future, it will get a dedicated IPA. Nonethless what appears clear to
me is that we have IPAs that support a specific ISP and pipelines that
are instead dedicated to the that integrates that ISP. Once multiple
platforms integrate the same ISP but change the integration model,
we'll need this patch.

One example: the mali-c55 pipeline is RZ/V2H specific (at least for
its m2m mode). Once another platform integrates a Mali-C55 and will
have a different integration we'll maybe need a new pipeline handler
but the IPA should remain the same.

I think, going forward, this situation will be not that uncommon
(hopefully, as it mean we'll support a lot of new platforms :)

>
> Changes in v4:
> - Rebase, change author of first patch to my new email address
>
> Changes in v3:
> - Add "ipa: Allow pipelines to have differently named IPA" patch
>
> Changes in v2:
> - Update tags, fix stray blank line
>
> Changes in v1 from Hans' original patch:
> - Slightly different approach addressing the review comments on Hans' v1 by
>   creating an overload for IPAManager::createIPA() that allows pipelines to
>   specify the IPA module name.
>
> Jacopo's original v3 cover-letter:
> https://lists.libcamera.org/pipermail/libcamera-devel/2025-October/053821.html
>
> Regards,
>
> Hans
>
>
> Hans de Goede (2):
>   libcamera: ipa_manager: Create IPA by name
>   libcamera: software_isp: Explicitly load "simple" IPA
>
> Jacopo Mondi (2):
>   ipa: ipa_module: Remove pipelineName
>   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 +++++++-------
>  src/libcamera/software_isp/software_isp.cpp |  2 +-
>  test/ipa/ipa_module_test.cpp                |  3 --
>  15 files changed, 84 insertions(+), 47 deletions(-)
>
> --
> 2.53.0
>