[v9,3/8] ipa: Allow pipelines to have differently named IPA
diff mbox series

Message ID 20260818140548.179663-4-johannes.goede@oss.qualcomm.com
State New
Headers show
Series
  • ipa: Allow IPA creation by name
Related show

Commit Message

Hans de Goede Aug. 18, 2026, 2:05 p.m. UTC 
From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Right now the build system assumes a 1-to-1 matching between a pipeline
handler name and an IPA module.

This, as also acknowledged by a \todo comment, is quite a rigid
requirement and only allows a 1-to-1 matching between pipeline and IPA
names. The more platforms libcamera supports, the more it is likely that
a pipeline handler could re-use an IPA module. This is particularly
relevant for the softISP IPA module which could theoretically be plugged
to any pipeline.

When building IPAs, the build system iterates the list of enabled
pipeline handlers and for each of them tries to verify if the 'ipas'
list contains a corresponding entry for it. The 'ipas' meson options is
an array option and, as no default value is specified for it, it
contains by default all its possible choices.

In this way if no value is specified for the 'ipas' option, compiling
the pipeline handlers ['X','Y', 'Z'] will compile the ['X', 'Y', 'Z']
IPAs. If instead the user specifies '-Dipas=X' during the configuration
then only IPA module ['X'] will be built, regardless of which pipeline is
enabled. Building an IPA module will anyway require to build a
corresponding pipeline with the same name.

Relax the 1-to-1 'pipeline'-'IPA' naming requirement by introducing a
dictionary that associates pipelines with IPA modules.

For each enabled pipeline:
1) Make sure an IPA module exists for it
2) Make sure the IPA module is enabled by the 'ipas' option
3) Make sure the IPA is compiled once only

This will require every new pipeline to add an entry to the dictionary
and specify which IPA module they would like to use.

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
Changes in v7:
- Drop R-Car Gen4 mention from commit message

Changes in v3:
- New patch in v3 of this patch-series
---
 src/ipa/meson.build | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

Comments

Barnabás Pőcze Aug. 18, 2026, 2:27 p.m. UTC | #1 
2026. 08. 18. 16:05 keltezéssel, Hans de Goede írta:
> From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> Right now the build system assumes a 1-to-1 matching between a pipeline
> handler name and an IPA module.
> 
> This, as also acknowledged by a \todo comment, is quite a rigid
> requirement and only allows a 1-to-1 matching between pipeline and IPA
> names. The more platforms libcamera supports, the more it is likely that
> a pipeline handler could re-use an IPA module. This is particularly
> relevant for the softISP IPA module which could theoretically be plugged
> to any pipeline.
> 
> When building IPAs, the build system iterates the list of enabled
> pipeline handlers and for each of them tries to verify if the 'ipas'
> list contains a corresponding entry for it. The 'ipas' meson options is
> an array option and, as no default value is specified for it, it
> contains by default all its possible choices.
> 
> In this way if no value is specified for the 'ipas' option, compiling
> the pipeline handlers ['X','Y', 'Z'] will compile the ['X', 'Y', 'Z']
> IPAs. If instead the user specifies '-Dipas=X' during the configuration
> then only IPA module ['X'] will be built, regardless of which pipeline is
> enabled. Building an IPA module will anyway require to build a
> corresponding pipeline with the same name.
> 
> Relax the 1-to-1 'pipeline'-'IPA' naming requirement by introducing a
> dictionary that associates pipelines with IPA modules.
> 
> For each enabled pipeline:
> 1) Make sure an IPA module exists for it
> 2) Make sure the IPA module is enabled by the 'ipas' option
> 3) Make sure the IPA is compiled once only
> 
> This will require every new pipeline to add an entry to the dictionary
> and specify which IPA module they would like to use.
> 
> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> ---
> Changes in v7:
> - Drop R-Car Gen4 mention from commit message
> 
> Changes in v3:
> - New patch in v3 of this patch-series
> ---

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Did you omit it intentionally?


>   src/ipa/meson.build | 40 ++++++++++++++++++++++++++++++----------
>   1 file changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> index eb7846e47..c583c7efd 100644
> --- a/src/ipa/meson.build
> +++ b/src/ipa/meson.build
> @@ -24,6 +24,16 @@ subdir('libipa')
>   
>   ipa_sign = files('ipa-sign.sh')
>   
> +supported_ipas = {
> +    'ipu3':       'ipu3',
> +    'mali-c55':   'mali-c55',
> +    'rkisp1':     'rkisp1',
> +    'rpi/pisp':   'rpi/pisp',
> +    'rpi/vc4':    'rpi/vc4',
> +    'simple':     'simple',
> +    'vimc':       'vimc'
> +}
> +
>   ipa_modules = get_option('ipas')
>   
>   # Tests require the vimc IPA, similar to vimc pipline-handler for their
> @@ -39,24 +49,34 @@ ipa_names = []
>   
>   subdirs = []
>   foreach pipeline : pipelines
> -    # The current implementation expects the IPA module name to match the
> -    # pipeline name.
> -    # \todo Make the IPA naming scheme more flexible.
> -    if not ipa_modules.contains(pipeline)
> +    # Make sure an IPA exists for the pipeline
> +    if not supported_ipas.has_key(pipeline)
>           continue
>       endif
> -    enabled_ipa_names += pipeline
> +
> +    ipa = supported_ipas.get(pipeline)
> +
> +    # Only build IPAs specified with '-Dipas'
> +    if not ipa_modules.contains(ipa)
> +        continue
> +    endif
> +
> +    # If enabled already do not add it twice
> +    if enabled_ipa_names.contains(ipa)
> +        continue
> +    endif
> +    enabled_ipa_names += ipa
>   
>       # Allow multi-level directory structuring for the IPAs if needed.
> -    pipeline = pipeline.split('/')[0]
> -    if pipeline in subdirs
> +    ipa = ipa.split('/')[0]
> +    if ipa in subdirs
>           continue
>       endif
>   
> -    subdirs += pipeline
> -    subdir(pipeline)
> +    subdirs += ipa
> +    subdir(ipa)
>   
> -    # Don't reuse the pipeline variable below, the subdirectory may have
> +    # Don't reuse the ipa variable below, the subdirectory may have
>       # overwritten it.
>   endforeach
>
Hans de Goede Aug. 18, 2026, 2:49 p.m. UTC | #2 
Hi,

On 18-Aug-26 16:27, Barnabás Pőcze wrote:
> 2026. 08. 18. 16:05 keltezéssel, Hans de Goede írta:
>> From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>>
>> Right now the build system assumes a 1-to-1 matching between a pipeline
>> handler name and an IPA module.
>>
>> This, as also acknowledged by a \todo comment, is quite a rigid
>> requirement and only allows a 1-to-1 matching between pipeline and IPA
>> names. The more platforms libcamera supports, the more it is likely that
>> a pipeline handler could re-use an IPA module. This is particularly
>> relevant for the softISP IPA module which could theoretically be plugged
>> to any pipeline.
>>
>> When building IPAs, the build system iterates the list of enabled
>> pipeline handlers and for each of them tries to verify if the 'ipas'
>> list contains a corresponding entry for it. The 'ipas' meson options is
>> an array option and, as no default value is specified for it, it
>> contains by default all its possible choices.
>>
>> In this way if no value is specified for the 'ipas' option, compiling
>> the pipeline handlers ['X','Y', 'Z'] will compile the ['X', 'Y', 'Z']
>> IPAs. If instead the user specifies '-Dipas=X' during the configuration
>> then only IPA module ['X'] will be built, regardless of which pipeline is
>> enabled. Building an IPA module will anyway require to build a
>> corresponding pipeline with the same name.
>>
>> Relax the 1-to-1 'pipeline'-'IPA' naming requirement by introducing a
>> dictionary that associates pipelines with IPA modules.
>>
>> For each enabled pipeline:
>> 1) Make sure an IPA module exists for it
>> 2) Make sure the IPA module is enabled by the 'ipas' option
>> 3) Make sure the IPA is compiled once only
>>
>> This will require every new pipeline to add an entry to the dictionary
>> and specify which IPA module they would like to use.
>>
>> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
>> ---
>> Changes in v7:
>> - Drop R-Car Gen4 mention from commit message
>>
>> Changes in v3:
>> - New patch in v3 of this patch-series
>> ---
> 
> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> Did you omit it intentionally?

No I overlooked it, sorry.

Regards,

Hans


> 
> 
>>   src/ipa/meson.build | 40 ++++++++++++++++++++++++++++++----------
>>   1 file changed, 30 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
>> index eb7846e47..c583c7efd 100644
>> --- a/src/ipa/meson.build
>> +++ b/src/ipa/meson.build
>> @@ -24,6 +24,16 @@ subdir('libipa')
>>     ipa_sign = files('ipa-sign.sh')
>>   +supported_ipas = {
>> +    'ipu3':       'ipu3',
>> +    'mali-c55':   'mali-c55',
>> +    'rkisp1':     'rkisp1',
>> +    'rpi/pisp':   'rpi/pisp',
>> +    'rpi/vc4':    'rpi/vc4',
>> +    'simple':     'simple',
>> +    'vimc':       'vimc'
>> +}
>> +
>>   ipa_modules = get_option('ipas')
>>     # Tests require the vimc IPA, similar to vimc pipline-handler for their
>> @@ -39,24 +49,34 @@ ipa_names = []
>>     subdirs = []
>>   foreach pipeline : pipelines
>> -    # The current implementation expects the IPA module name to match the
>> -    # pipeline name.
>> -    # \todo Make the IPA naming scheme more flexible.
>> -    if not ipa_modules.contains(pipeline)
>> +    # Make sure an IPA exists for the pipeline
>> +    if not supported_ipas.has_key(pipeline)
>>           continue
>>       endif
>> -    enabled_ipa_names += pipeline
>> +
>> +    ipa = supported_ipas.get(pipeline)
>> +
>> +    # Only build IPAs specified with '-Dipas'
>> +    if not ipa_modules.contains(ipa)
>> +        continue
>> +    endif
>> +
>> +    # If enabled already do not add it twice
>> +    if enabled_ipa_names.contains(ipa)
>> +        continue
>> +    endif
>> +    enabled_ipa_names += ipa
>>         # Allow multi-level directory structuring for the IPAs if needed.
>> -    pipeline = pipeline.split('/')[0]
>> -    if pipeline in subdirs
>> +    ipa = ipa.split('/')[0]
>> +    if ipa in subdirs
>>           continue
>>       endif
>>   -    subdirs += pipeline
>> -    subdir(pipeline)
>> +    subdirs += ipa
>> +    subdir(ipa)
>>   -    # Don't reuse the pipeline variable below, the subdirectory may have
>> +    # Don't reuse the ipa variable below, the subdirectory may have
>>       # overwritten it.
>>   endforeach
>>   
>

Patch
diff mbox series 

diff --git a/src/ipa/meson.build b/src/ipa/meson.build
index eb7846e47..c583c7efd 100644
--- a/src/ipa/meson.build
+++ b/src/ipa/meson.build
@@ -24,6 +24,16 @@  subdir('libipa')
 
 ipa_sign = files('ipa-sign.sh')
 
+supported_ipas = {
+    'ipu3':       'ipu3',
+    'mali-c55':   'mali-c55',
+    'rkisp1':     'rkisp1',
+    'rpi/pisp':   'rpi/pisp',
+    'rpi/vc4':    'rpi/vc4',
+    'simple':     'simple',
+    'vimc':       'vimc'
+}
+
 ipa_modules = get_option('ipas')
 
 # Tests require the vimc IPA, similar to vimc pipline-handler for their
@@ -39,24 +49,34 @@  ipa_names = []
 
 subdirs = []
 foreach pipeline : pipelines
-    # The current implementation expects the IPA module name to match the
-    # pipeline name.
-    # \todo Make the IPA naming scheme more flexible.
-    if not ipa_modules.contains(pipeline)
+    # Make sure an IPA exists for the pipeline
+    if not supported_ipas.has_key(pipeline)
         continue
     endif
-    enabled_ipa_names += pipeline
+
+    ipa = supported_ipas.get(pipeline)
+
+    # Only build IPAs specified with '-Dipas'
+    if not ipa_modules.contains(ipa)
+        continue
+    endif
+
+    # If enabled already do not add it twice
+    if enabled_ipa_names.contains(ipa)
+        continue
+    endif
+    enabled_ipa_names += ipa
 
     # Allow multi-level directory structuring for the IPAs if needed.
-    pipeline = pipeline.split('/')[0]
-    if pipeline in subdirs
+    ipa = ipa.split('/')[0]
+    if ipa in subdirs
         continue
     endif
 
-    subdirs += pipeline
-    subdir(pipeline)
+    subdirs += ipa
+    subdir(ipa)
 
-    # Don't reuse the pipeline variable below, the subdirectory may have
+    # Don't reuse the ipa variable below, the subdirectory may have
     # overwritten it.
 endforeach