[libcamera-devel,03/13] pipeline: raspberrypi: Refactor and move pipeline handler code
diff mbox series

Message ID 20230426131057.21550-4-naush@raspberrypi.com
State Superseded
Headers show
Series
  • Raspberry Pi: Code refactoring
Related show

Commit Message

Naushir Patuck April 26, 2023, 1:10 p.m. UTC
Split the Raspberry Pi pipeline handler code into common and VC4/BCM2835
specific file structures.

The common code files now live in src/libcamera/pipeline/rpi/common/
and the vc4 specific files in src/libcamera/pipeline/rpi/vc4/.

To build the pipeline handler, the meson configuration option to select
the Raspberry Pi pipeline handler has now changed from "raspberrypi" to
"rpi/vc4":

meson setup build -Dpipelines=rpi/vc4

There are no functional changes in the pipeline handler code itself.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 Documentation/environment_variables.rst                  | 2 +-
 Documentation/guides/introduction.rst                    | 2 +-
 Documentation/guides/ipa.rst                             | 2 +-
 Documentation/guides/pipeline-handler.rst                | 2 +-
 include/libcamera/ipa/meson.build                        | 2 +-
 meson.build                                              | 2 +-
 meson_options.txt                                        | 2 +-
 src/libcamera/pipeline/meson.build                       | 9 +++++++++
 .../{raspberrypi => rpi/common}/delayed_controls.cpp     | 0
 .../{raspberrypi => rpi/common}/delayed_controls.h       | 0
 src/libcamera/pipeline/rpi/common/meson.build            | 8 ++++++++
 .../pipeline/{raspberrypi => rpi/common}/rpi_stream.cpp  | 0
 .../pipeline/{raspberrypi => rpi/common}/rpi_stream.h    | 0
 .../pipeline/{raspberrypi => rpi/vc4}/data/example.yaml  | 0
 .../pipeline/{raspberrypi => rpi/vc4}/data/meson.build   | 2 +-
 .../pipeline/{raspberrypi => rpi/vc4}/dma_heaps.cpp      | 0
 .../pipeline/{raspberrypi => rpi/vc4}/dma_heaps.h        | 0
 .../pipeline/{raspberrypi => rpi/vc4}/meson.build        | 2 --
 .../pipeline/{raspberrypi => rpi/vc4}/raspberrypi.cpp    | 2 +-
 19 files changed, 26 insertions(+), 11 deletions(-)
 rename src/libcamera/pipeline/{raspberrypi => rpi/common}/delayed_controls.cpp (100%)
 rename src/libcamera/pipeline/{raspberrypi => rpi/common}/delayed_controls.h (100%)
 create mode 100644 src/libcamera/pipeline/rpi/common/meson.build
 rename src/libcamera/pipeline/{raspberrypi => rpi/common}/rpi_stream.cpp (100%)
 rename src/libcamera/pipeline/{raspberrypi => rpi/common}/rpi_stream.h (100%)
 rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/data/example.yaml (100%)
 rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/data/meson.build (63%)
 rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/dma_heaps.cpp (100%)
 rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/dma_heaps.h (100%)
 rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/meson.build (71%)
 rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/raspberrypi.cpp (99%)

Comments

Jacopo Mondi April 27, 2023, 8:18 a.m. UTC | #1
Hi Naush,

On Wed, Apr 26, 2023 at 02:10:47PM +0100, Naushir Patuck via libcamera-devel wrote:
> Split the Raspberry Pi pipeline handler code into common and VC4/BCM2835
> specific file structures.
>
> The common code files now live in src/libcamera/pipeline/rpi/common/
> and the vc4 specific files in src/libcamera/pipeline/rpi/vc4/.
>
> To build the pipeline handler, the meson configuration option to select
> the Raspberry Pi pipeline handler has now changed from "raspberrypi" to
> "rpi/vc4":
>
> meson setup build -Dpipelines=rpi/vc4
>
> There are no functional changes in the pipeline handler code itself.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>

Looks good!
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> ---
>  Documentation/environment_variables.rst                  | 2 +-
>  Documentation/guides/introduction.rst                    | 2 +-
>  Documentation/guides/ipa.rst                             | 2 +-
>  Documentation/guides/pipeline-handler.rst                | 2 +-
>  include/libcamera/ipa/meson.build                        | 2 +-
>  meson.build                                              | 2 +-
>  meson_options.txt                                        | 2 +-
>  src/libcamera/pipeline/meson.build                       | 9 +++++++++
>  .../{raspberrypi => rpi/common}/delayed_controls.cpp     | 0
>  .../{raspberrypi => rpi/common}/delayed_controls.h       | 0
>  src/libcamera/pipeline/rpi/common/meson.build            | 8 ++++++++
>  .../pipeline/{raspberrypi => rpi/common}/rpi_stream.cpp  | 0
>  .../pipeline/{raspberrypi => rpi/common}/rpi_stream.h    | 0
>  .../pipeline/{raspberrypi => rpi/vc4}/data/example.yaml  | 0
>  .../pipeline/{raspberrypi => rpi/vc4}/data/meson.build   | 2 +-
>  .../pipeline/{raspberrypi => rpi/vc4}/dma_heaps.cpp      | 0
>  .../pipeline/{raspberrypi => rpi/vc4}/dma_heaps.h        | 0
>  .../pipeline/{raspberrypi => rpi/vc4}/meson.build        | 2 --
>  .../pipeline/{raspberrypi => rpi/vc4}/raspberrypi.cpp    | 2 +-
>  19 files changed, 26 insertions(+), 11 deletions(-)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/delayed_controls.cpp (100%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/delayed_controls.h (100%)
>  create mode 100644 src/libcamera/pipeline/rpi/common/meson.build
>  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/rpi_stream.cpp (100%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/rpi_stream.h (100%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/data/example.yaml (100%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/data/meson.build (63%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/dma_heaps.cpp (100%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/dma_heaps.h (100%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/meson.build (71%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/raspberrypi.cpp (99%)
>
> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst
> index ceeb251a2ac0..4bf38b877897 100644
> --- a/Documentation/environment_variables.rst
> +++ b/Documentation/environment_variables.rst
> @@ -40,7 +40,7 @@ LIBCAMERA_IPA_MODULE_PATH
>  LIBCAMERA_RPI_CONFIG_FILE
>     Define a custom configuration file to use in the Raspberry Pi pipeline handler.
>
> -   Example value: ``/usr/local/share/libcamera/pipeline/raspberrypi/minimal_mem.yaml``
> +   Example value: ``/usr/local/share/libcamera/pipeline/rpi/vc4/minimal_mem.yaml``
>
>  Further details
>  ---------------
> diff --git a/Documentation/guides/introduction.rst b/Documentation/guides/introduction.rst
> index 2d1760c1866b..700ec2d33c30 100644
> --- a/Documentation/guides/introduction.rst
> +++ b/Documentation/guides/introduction.rst
> @@ -288,7 +288,7 @@ with dedicated pipeline handlers:
>
>     -  Intel IPU3 (ipu3)
>     -  Rockchip RK3399 (rkisp1)
> -   -  RaspberryPi 3 and 4 (raspberrypi)
> +   -  RaspberryPi 3 and 4 (rpi/vc4)
>
>  Furthermore, generic platform support is provided for the following:
>
> diff --git a/Documentation/guides/ipa.rst b/Documentation/guides/ipa.rst
> index 89839408672a..10301d89fc80 100644
> --- a/Documentation/guides/ipa.rst
> +++ b/Documentation/guides/ipa.rst
> @@ -279,7 +279,7 @@ For example, adding the raspberrypi.mojom file to meson:
>  .. code-block:: none
>
>          pipeline_ipa_mojom_mapping = [
> -            'raspberrypi': 'raspberrypi.mojom',
> +            'rpi/vc4': 'raspberrypi.mojom',
>          ]
>
>  This will cause the mojo data definition file to be compiled. Specifically, it
> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
> index 4d38fa23fbcd..7d143b0eaecb 100644
> --- a/Documentation/guides/pipeline-handler.rst
> +++ b/Documentation/guides/pipeline-handler.rst
> @@ -183,7 +183,7 @@ to the libcamera build options in the top level ``meson_options.txt``.
>
>     option('pipelines',
>             type : 'array',
> -           choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc', 'vivid'],
> +           choices : ['ipu3', 'rpi/vc4', 'rkisp1', 'simple', 'uvcvideo', 'vimc', 'vivid'],
>             description : 'Select which pipeline handlers to include')
>
>
> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
> index 67c31cb04ccf..bae15d64d7d8 100644
> --- a/include/libcamera/ipa/meson.build
> +++ b/include/libcamera/ipa/meson.build
> @@ -64,7 +64,7 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',
>  pipeline_ipa_mojom_mapping = {
>      'ipu3': 'ipu3.mojom',
>      'rkisp1': 'rkisp1.mojom',
> -    'raspberrypi': 'raspberrypi.mojom',
> +    'rpi/vc4': 'raspberrypi.mojom',
>      'vimc': 'vimc.mojom',
>  }
>
> diff --git a/meson.build b/meson.build
> index 8628e6acebee..f60da3e17719 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -179,7 +179,7 @@ arch_x86 = ['x86', 'x86_64']
>  pipelines_support = {
>      'imx8-isi':     arch_arm,
>      'ipu3':         arch_x86,
> -    'raspberrypi':  arch_arm,
> +    'rpi/vc4':      arch_arm,
>      'rkisp1':       arch_arm,
>      'simple':       arch_arm,
>      'uvcvideo':     ['any'],
> diff --git a/meson_options.txt b/meson_options.txt
> index 78a78b7214d5..e1f4c205aa94 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -43,7 +43,7 @@ option('pipelines',
>              'auto',
>              'imx8-isi',
>              'ipu3',
> -            'raspberrypi',
> +            'rpi/vc4',
>              'rkisp1',
>              'simple',
>              'uvcvideo',
> diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build
> index f14869f3a3c0..4f55611013db 100644
> --- a/src/libcamera/pipeline/meson.build
> +++ b/src/libcamera/pipeline/meson.build
> @@ -3,6 +3,15 @@
>  # Location of pipeline specific configuration files
>  pipeline_data_dir = libcamera_datadir / 'pipeline'
>
> +# If the Raspberry Pi VC4 pipeline handler is enabled, ensure we include the
> +# rpi/common subdirectory 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/common')
> +endif
> +
>  foreach pipeline : pipelines
>      subdir(pipeline)
>  endforeach
> diff --git a/src/libcamera/pipeline/raspberrypi/delayed_controls.cpp b/src/libcamera/pipeline/rpi/common/delayed_controls.cpp
> similarity index 100%
> rename from src/libcamera/pipeline/raspberrypi/delayed_controls.cpp
> rename to src/libcamera/pipeline/rpi/common/delayed_controls.cpp
> diff --git a/src/libcamera/pipeline/raspberrypi/delayed_controls.h b/src/libcamera/pipeline/rpi/common/delayed_controls.h
> similarity index 100%
> rename from src/libcamera/pipeline/raspberrypi/delayed_controls.h
> rename to src/libcamera/pipeline/rpi/common/delayed_controls.h
> diff --git a/src/libcamera/pipeline/rpi/common/meson.build b/src/libcamera/pipeline/rpi/common/meson.build
> new file mode 100644
> index 000000000000..1dec6d3d028b
> --- /dev/null
> +++ b/src/libcamera/pipeline/rpi/common/meson.build
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +libcamera_sources += files([
> +    'delayed_controls.cpp',
> +    'rpi_stream.cpp',
> +])
> +
> +includes += include_directories('.')
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> similarity index 100%
> rename from src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> rename to src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> similarity index 100%
> rename from src/libcamera/pipeline/raspberrypi/rpi_stream.h
> rename to src/libcamera/pipeline/rpi/common/rpi_stream.h
> diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/rpi/vc4/data/example.yaml
> similarity index 100%
> rename from src/libcamera/pipeline/raspberrypi/data/example.yaml
> rename to src/libcamera/pipeline/rpi/vc4/data/example.yaml
> diff --git a/src/libcamera/pipeline/raspberrypi/data/meson.build b/src/libcamera/pipeline/rpi/vc4/data/meson.build
> similarity index 63%
> rename from src/libcamera/pipeline/raspberrypi/data/meson.build
> rename to src/libcamera/pipeline/rpi/vc4/data/meson.build
> index 1c70433bbcbc..a7dfa02320e5 100644
> --- a/src/libcamera/pipeline/raspberrypi/data/meson.build
> +++ b/src/libcamera/pipeline/rpi/vc4/data/meson.build
> @@ -5,4 +5,4 @@ conf_files = files([
>  ])
>
>  install_data(conf_files,
> -             install_dir : pipeline_data_dir / 'raspberrypi')
> +             install_dir : pipeline_data_dir / 'rpi/vc4')
> diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp b/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
> similarity index 100%
> rename from src/libcamera/pipeline/raspberrypi/dma_heaps.cpp
> rename to src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
> diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.h b/src/libcamera/pipeline/rpi/vc4/dma_heaps.h
> similarity index 100%
> rename from src/libcamera/pipeline/raspberrypi/dma_heaps.h
> rename to src/libcamera/pipeline/rpi/vc4/dma_heaps.h
> diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/rpi/vc4/meson.build
> similarity index 71%
> rename from src/libcamera/pipeline/raspberrypi/meson.build
> rename to src/libcamera/pipeline/rpi/vc4/meson.build
> index 59e8fb18c555..228823f30922 100644
> --- a/src/libcamera/pipeline/raspberrypi/meson.build
> +++ b/src/libcamera/pipeline/rpi/vc4/meson.build
> @@ -1,10 +1,8 @@
>  # SPDX-License-Identifier: CC0-1.0
>
>  libcamera_sources += files([
> -    'delayed_controls.cpp',
>      'dma_heaps.cpp',
>      'raspberrypi.cpp',
> -    'rpi_stream.cpp',
>  ])
>
>  subdir('data')
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
> similarity index 99%
> rename from src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> rename to src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
> index a4fff28bf198..4595773d2517 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
> @@ -2,7 +2,7 @@
>  /*
>   * Copyright (C) 2019-2021, Raspberry Pi Ltd
>   *
> - * raspberrypi.cpp - Pipeline handler for Raspberry Pi devices
> + * raspberrypi.cpp - Pipeline handler for VC4 based Raspberry Pi devices
>   */
>  #include <algorithm>
>  #include <assert.h>
> --
> 2.34.1
>
Laurent Pinchart April 27, 2023, 1:14 p.m. UTC | #2
Hi Naush,

Thank you for the patch.

On Wed, Apr 26, 2023 at 02:10:47PM +0100, Naushir Patuck via libcamera-devel wrote:
> Split the Raspberry Pi pipeline handler code into common and VC4/BCM2835
> specific file structures.
> 
> The common code files now live in src/libcamera/pipeline/rpi/common/
> and the vc4 specific files in src/libcamera/pipeline/rpi/vc4/.
> 
> To build the pipeline handler, the meson configuration option to select
> the Raspberry Pi pipeline handler has now changed from "raspberrypi" to
> "rpi/vc4":
> 
> meson setup build -Dpipelines=rpi/vc4

This breaks the selection of IPA modules in src/ipa/meson.build:

foreach pipeline : pipelines
    if ipa_modules.contains(pipeline)
        subdir(pipeline)
        ipa_names += ipa_install_dir / ipa_name + '.so'
        enabled_ipa_modules += pipeline
    endif
endforeach

> There are no functional changes in the pipeline handler code itself.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  Documentation/environment_variables.rst                  | 2 +-
>  Documentation/guides/introduction.rst                    | 2 +-
>  Documentation/guides/ipa.rst                             | 2 +-
>  Documentation/guides/pipeline-handler.rst                | 2 +-
>  include/libcamera/ipa/meson.build                        | 2 +-
>  meson.build                                              | 2 +-
>  meson_options.txt                                        | 2 +-
>  src/libcamera/pipeline/meson.build                       | 9 +++++++++
>  .../{raspberrypi => rpi/common}/delayed_controls.cpp     | 0
>  .../{raspberrypi => rpi/common}/delayed_controls.h       | 0
>  src/libcamera/pipeline/rpi/common/meson.build            | 8 ++++++++
>  .../pipeline/{raspberrypi => rpi/common}/rpi_stream.cpp  | 0
>  .../pipeline/{raspberrypi => rpi/common}/rpi_stream.h    | 0
>  .../pipeline/{raspberrypi => rpi/vc4}/data/example.yaml  | 0
>  .../pipeline/{raspberrypi => rpi/vc4}/data/meson.build   | 2 +-
>  .../pipeline/{raspberrypi => rpi/vc4}/dma_heaps.cpp      | 0
>  .../pipeline/{raspberrypi => rpi/vc4}/dma_heaps.h        | 0
>  .../pipeline/{raspberrypi => rpi/vc4}/meson.build        | 2 --
>  .../pipeline/{raspberrypi => rpi/vc4}/raspberrypi.cpp    | 2 +-
>  19 files changed, 26 insertions(+), 11 deletions(-)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/delayed_controls.cpp (100%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/delayed_controls.h (100%)
>  create mode 100644 src/libcamera/pipeline/rpi/common/meson.build
>  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/rpi_stream.cpp (100%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/rpi_stream.h (100%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/data/example.yaml (100%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/data/meson.build (63%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/dma_heaps.cpp (100%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/dma_heaps.h (100%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/meson.build (71%)
>  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/raspberrypi.cpp (99%)
> 
> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst
> index ceeb251a2ac0..4bf38b877897 100644
> --- a/Documentation/environment_variables.rst
> +++ b/Documentation/environment_variables.rst
> @@ -40,7 +40,7 @@ LIBCAMERA_IPA_MODULE_PATH
>  LIBCAMERA_RPI_CONFIG_FILE
>     Define a custom configuration file to use in the Raspberry Pi pipeline handler.
>  
> -   Example value: ``/usr/local/share/libcamera/pipeline/raspberrypi/minimal_mem.yaml``
> +   Example value: ``/usr/local/share/libcamera/pipeline/rpi/vc4/minimal_mem.yaml``
>  
>  Further details
>  ---------------
> diff --git a/Documentation/guides/introduction.rst b/Documentation/guides/introduction.rst
> index 2d1760c1866b..700ec2d33c30 100644
> --- a/Documentation/guides/introduction.rst
> +++ b/Documentation/guides/introduction.rst
> @@ -288,7 +288,7 @@ with dedicated pipeline handlers:
>  
>     -  Intel IPU3 (ipu3)
>     -  Rockchip RK3399 (rkisp1)
> -   -  RaspberryPi 3 and 4 (raspberrypi)
> +   -  RaspberryPi 3 and 4 (rpi/vc4)
>  
>  Furthermore, generic platform support is provided for the following:
>  
> diff --git a/Documentation/guides/ipa.rst b/Documentation/guides/ipa.rst
> index 89839408672a..10301d89fc80 100644
> --- a/Documentation/guides/ipa.rst
> +++ b/Documentation/guides/ipa.rst
> @@ -279,7 +279,7 @@ For example, adding the raspberrypi.mojom file to meson:
>  .. code-block:: none
>  
>          pipeline_ipa_mojom_mapping = [
> -            'raspberrypi': 'raspberrypi.mojom',
> +            'rpi/vc4': 'raspberrypi.mojom',
>          ]
>  
>  This will cause the mojo data definition file to be compiled. Specifically, it
> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
> index 4d38fa23fbcd..7d143b0eaecb 100644
> --- a/Documentation/guides/pipeline-handler.rst
> +++ b/Documentation/guides/pipeline-handler.rst
> @@ -183,7 +183,7 @@ to the libcamera build options in the top level ``meson_options.txt``.
>  
>     option('pipelines',
>             type : 'array',
> -           choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc', 'vivid'],
> +           choices : ['ipu3', 'rpi/vc4', 'rkisp1', 'simple', 'uvcvideo', 'vimc', 'vivid'],

'rpi/vc4' should now go after 'rkisp1' in alphabetical order.

>             description : 'Select which pipeline handlers to include')
>  
>  
> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
> index 67c31cb04ccf..bae15d64d7d8 100644
> --- a/include/libcamera/ipa/meson.build
> +++ b/include/libcamera/ipa/meson.build
> @@ -64,7 +64,7 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',
>  pipeline_ipa_mojom_mapping = {
>      'ipu3': 'ipu3.mojom',
>      'rkisp1': 'rkisp1.mojom',
> -    'raspberrypi': 'raspberrypi.mojom',
> +    'rpi/vc4': 'raspberrypi.mojom',
>      'vimc': 'vimc.mojom',
>  }
>  
> diff --git a/meson.build b/meson.build
> index 8628e6acebee..f60da3e17719 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -179,7 +179,7 @@ arch_x86 = ['x86', 'x86_64']
>  pipelines_support = {
>      'imx8-isi':     arch_arm,
>      'ipu3':         arch_x86,
> -    'raspberrypi':  arch_arm,
> +    'rpi/vc4':      arch_arm,

This should also go after 'rkisp1'. Same below, and possibly in other
patches in this series.

>      'rkisp1':       arch_arm,
>      'simple':       arch_arm,
>      'uvcvideo':     ['any'],
> diff --git a/meson_options.txt b/meson_options.txt
> index 78a78b7214d5..e1f4c205aa94 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -43,7 +43,7 @@ option('pipelines',
>              'auto',
>              'imx8-isi',
>              'ipu3',
> -            'raspberrypi',
> +            'rpi/vc4',
>              'rkisp1',
>              'simple',
>              'uvcvideo',
> diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build
> index f14869f3a3c0..4f55611013db 100644
> --- a/src/libcamera/pipeline/meson.build
> +++ b/src/libcamera/pipeline/meson.build
> @@ -3,6 +3,15 @@
>  # Location of pipeline specific configuration files
>  pipeline_data_dir = libcamera_datadir / 'pipeline'
>  
> +# If the Raspberry Pi VC4 pipeline handler is enabled, ensure we include the
> +# rpi/common subdirectory 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/common')
> +endif

I don't like how this becomes a special case :-S If we want to support
multi-level directory trees instead of a flat hierarchy, this should be
automatic.

We could do something like this:

----
subdirs = []

foreach pipeline : pipelines
    pipeline = pipeline.split('/')[0]
    if pipeline in subdirs
        continue
    endif

    subdir(pipeline)
    subdirs += [pipeline]
endforeach
----

The logic in src/libcamera/pipeline/rpi/meson.build would be similar,
iterating over all pipelines and selecting the ones that start with
'rpi' only. Something like the following should hopefully work:

----
# SPDX-License-Identifier: CC0-1.0

subdir('common')

foreach pipeline : pipelines
    pipeline = pipeline.split('/')
    if pipeline.length() < 2 or pipeline[0] != 'rpi'
        continue
    endif

    subdir(pipeline[1])
endforeach
----

> +
>  foreach pipeline : pipelines
>      subdir(pipeline)
>  endforeach
> diff --git a/src/libcamera/pipeline/raspberrypi/delayed_controls.cpp b/src/libcamera/pipeline/rpi/common/delayed_controls.cpp
> similarity index 100%
> rename from src/libcamera/pipeline/raspberrypi/delayed_controls.cpp
> rename to src/libcamera/pipeline/rpi/common/delayed_controls.cpp
> diff --git a/src/libcamera/pipeline/raspberrypi/delayed_controls.h b/src/libcamera/pipeline/rpi/common/delayed_controls.h
> similarity index 100%
> rename from src/libcamera/pipeline/raspberrypi/delayed_controls.h
> rename to src/libcamera/pipeline/rpi/common/delayed_controls.h
> diff --git a/src/libcamera/pipeline/rpi/common/meson.build b/src/libcamera/pipeline/rpi/common/meson.build
> new file mode 100644
> index 000000000000..1dec6d3d028b
> --- /dev/null
> +++ b/src/libcamera/pipeline/rpi/common/meson.build
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +libcamera_sources += files([
> +    'delayed_controls.cpp',
> +    'rpi_stream.cpp',
> +])
> +
> +includes += include_directories('.')
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> similarity index 100%
> rename from src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> rename to src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> similarity index 100%
> rename from src/libcamera/pipeline/raspberrypi/rpi_stream.h
> rename to src/libcamera/pipeline/rpi/common/rpi_stream.h
> diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/rpi/vc4/data/example.yaml
> similarity index 100%
> rename from src/libcamera/pipeline/raspberrypi/data/example.yaml
> rename to src/libcamera/pipeline/rpi/vc4/data/example.yaml
> diff --git a/src/libcamera/pipeline/raspberrypi/data/meson.build b/src/libcamera/pipeline/rpi/vc4/data/meson.build
> similarity index 63%
> rename from src/libcamera/pipeline/raspberrypi/data/meson.build
> rename to src/libcamera/pipeline/rpi/vc4/data/meson.build
> index 1c70433bbcbc..a7dfa02320e5 100644
> --- a/src/libcamera/pipeline/raspberrypi/data/meson.build
> +++ b/src/libcamera/pipeline/rpi/vc4/data/meson.build
> @@ -5,4 +5,4 @@ conf_files = files([
>  ])
>  
>  install_data(conf_files,
> -             install_dir : pipeline_data_dir / 'raspberrypi')
> +             install_dir : pipeline_data_dir / 'rpi/vc4')

             install_dir : pipeline_data_dir / 'rpi' / 'vc4')

> diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp b/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
> similarity index 100%
> rename from src/libcamera/pipeline/raspberrypi/dma_heaps.cpp
> rename to src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
> diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.h b/src/libcamera/pipeline/rpi/vc4/dma_heaps.h
> similarity index 100%
> rename from src/libcamera/pipeline/raspberrypi/dma_heaps.h
> rename to src/libcamera/pipeline/rpi/vc4/dma_heaps.h
> diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/rpi/vc4/meson.build
> similarity index 71%
> rename from src/libcamera/pipeline/raspberrypi/meson.build
> rename to src/libcamera/pipeline/rpi/vc4/meson.build
> index 59e8fb18c555..228823f30922 100644
> --- a/src/libcamera/pipeline/raspberrypi/meson.build
> +++ b/src/libcamera/pipeline/rpi/vc4/meson.build
> @@ -1,10 +1,8 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  libcamera_sources += files([
> -    'delayed_controls.cpp',
>      'dma_heaps.cpp',

I wondered if this could be a good candidate for the common directory,
but I think we should turn it into a core libcamera helper if it gets
shared by multiple pipeline handlers, so I'm happy to leave it here.

>      'raspberrypi.cpp',
> -    'rpi_stream.cpp',
>  ])
>  
>  subdir('data')
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
> similarity index 99%
> rename from src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> rename to src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
> index a4fff28bf198..4595773d2517 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
> @@ -2,7 +2,7 @@
>  /*
>   * Copyright (C) 2019-2021, Raspberry Pi Ltd
>   *
> - * raspberrypi.cpp - Pipeline handler for Raspberry Pi devices
> + * raspberrypi.cpp - Pipeline handler for VC4 based Raspberry Pi devices

s/VC4 based/VC4-based/

>   */
>  #include <algorithm>
>  #include <assert.h>
Naushir Patuck April 27, 2023, 2:02 p.m. UTC | #3
Hi Laurent,

Thank you for your feedback.

On Thu, 27 Apr 2023 at 14:14, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Naush,
>
> Thank you for the patch.
>
> On Wed, Apr 26, 2023 at 02:10:47PM +0100, Naushir Patuck via libcamera-devel wrote:
> > Split the Raspberry Pi pipeline handler code into common and VC4/BCM2835
> > specific file structures.
> >
> > The common code files now live in src/libcamera/pipeline/rpi/common/
> > and the vc4 specific files in src/libcamera/pipeline/rpi/vc4/.
> >
> > To build the pipeline handler, the meson configuration option to select
> > the Raspberry Pi pipeline handler has now changed from "raspberrypi" to
> > "rpi/vc4":
> >
> > meson setup build -Dpipelines=rpi/vc4
>
> This breaks the selection of IPA modules in src/ipa/meson.build:
>
> foreach pipeline : pipelines
>     if ipa_modules.contains(pipeline)
>         subdir(pipeline)
>         ipa_names += ipa_install_dir / ipa_name + '.so'
>         enabled_ipa_modules += pipeline
>     endif
> endforeach

Sorry, I forgot to note in my cover letter that patches 2 and 3 want to be
squashed before merging for this very reason. I've keep them separate for
easier review.
>
> > There are no functional changes in the pipeline handler code itself.
> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > ---
> >  Documentation/environment_variables.rst                  | 2 +-
> >  Documentation/guides/introduction.rst                    | 2 +-
> >  Documentation/guides/ipa.rst                             | 2 +-
> >  Documentation/guides/pipeline-handler.rst                | 2 +-
> >  include/libcamera/ipa/meson.build                        | 2 +-
> >  meson.build                                              | 2 +-
> >  meson_options.txt                                        | 2 +-
> >  src/libcamera/pipeline/meson.build                       | 9 +++++++++
> >  .../{raspberrypi => rpi/common}/delayed_controls.cpp     | 0
> >  .../{raspberrypi => rpi/common}/delayed_controls.h       | 0
> >  src/libcamera/pipeline/rpi/common/meson.build            | 8 ++++++++
> >  .../pipeline/{raspberrypi => rpi/common}/rpi_stream.cpp  | 0
> >  .../pipeline/{raspberrypi => rpi/common}/rpi_stream.h    | 0
> >  .../pipeline/{raspberrypi => rpi/vc4}/data/example.yaml  | 0
> >  .../pipeline/{raspberrypi => rpi/vc4}/data/meson.build   | 2 +-
> >  .../pipeline/{raspberrypi => rpi/vc4}/dma_heaps.cpp      | 0
> >  .../pipeline/{raspberrypi => rpi/vc4}/dma_heaps.h        | 0
> >  .../pipeline/{raspberrypi => rpi/vc4}/meson.build        | 2 --
> >  .../pipeline/{raspberrypi => rpi/vc4}/raspberrypi.cpp    | 2 +-
> >  19 files changed, 26 insertions(+), 11 deletions(-)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/delayed_controls.cpp (100%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/delayed_controls.h (100%)
> >  create mode 100644 src/libcamera/pipeline/rpi/common/meson.build
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/rpi_stream.cpp (100%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/rpi_stream.h (100%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/data/example.yaml (100%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/data/meson.build (63%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/dma_heaps.cpp (100%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/dma_heaps.h (100%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/meson.build (71%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/raspberrypi.cpp (99%)
> >
> > diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst
> > index ceeb251a2ac0..4bf38b877897 100644
> > --- a/Documentation/environment_variables.rst
> > +++ b/Documentation/environment_variables.rst
> > @@ -40,7 +40,7 @@ LIBCAMERA_IPA_MODULE_PATH
> >  LIBCAMERA_RPI_CONFIG_FILE
> >     Define a custom configuration file to use in the Raspberry Pi pipeline handler.
> >
> > -   Example value: ``/usr/local/share/libcamera/pipeline/raspberrypi/minimal_mem.yaml``
> > +   Example value: ``/usr/local/share/libcamera/pipeline/rpi/vc4/minimal_mem.yaml``
> >
> >  Further details
> >  ---------------
> > diff --git a/Documentation/guides/introduction.rst b/Documentation/guides/introduction.rst
> > index 2d1760c1866b..700ec2d33c30 100644
> > --- a/Documentation/guides/introduction.rst
> > +++ b/Documentation/guides/introduction.rst
> > @@ -288,7 +288,7 @@ with dedicated pipeline handlers:
> >
> >     -  Intel IPU3 (ipu3)
> >     -  Rockchip RK3399 (rkisp1)
> > -   -  RaspberryPi 3 and 4 (raspberrypi)
> > +   -  RaspberryPi 3 and 4 (rpi/vc4)
> >
> >  Furthermore, generic platform support is provided for the following:
> >
> > diff --git a/Documentation/guides/ipa.rst b/Documentation/guides/ipa.rst
> > index 89839408672a..10301d89fc80 100644
> > --- a/Documentation/guides/ipa.rst
> > +++ b/Documentation/guides/ipa.rst
> > @@ -279,7 +279,7 @@ For example, adding the raspberrypi.mojom file to meson:
> >  .. code-block:: none
> >
> >          pipeline_ipa_mojom_mapping = [
> > -            'raspberrypi': 'raspberrypi.mojom',
> > +            'rpi/vc4': 'raspberrypi.mojom',
> >          ]
> >
> >  This will cause the mojo data definition file to be compiled. Specifically, it
> > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
> > index 4d38fa23fbcd..7d143b0eaecb 100644
> > --- a/Documentation/guides/pipeline-handler.rst
> > +++ b/Documentation/guides/pipeline-handler.rst
> > @@ -183,7 +183,7 @@ to the libcamera build options in the top level ``meson_options.txt``.
> >
> >     option('pipelines',
> >             type : 'array',
> > -           choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc', 'vivid'],
> > +           choices : ['ipu3', 'rpi/vc4', 'rkisp1', 'simple', 'uvcvideo', 'vimc', 'vivid'],
>
> 'rpi/vc4' should now go after 'rkisp1' in alphabetical order.
>
> >             description : 'Select which pipeline handlers to include')
> >
> >
> > diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
> > index 67c31cb04ccf..bae15d64d7d8 100644
> > --- a/include/libcamera/ipa/meson.build
> > +++ b/include/libcamera/ipa/meson.build
> > @@ -64,7 +64,7 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',
> >  pipeline_ipa_mojom_mapping = {
> >      'ipu3': 'ipu3.mojom',
> >      'rkisp1': 'rkisp1.mojom',
> > -    'raspberrypi': 'raspberrypi.mojom',
> > +    'rpi/vc4': 'raspberrypi.mojom',
> >      'vimc': 'vimc.mojom',
> >  }
> >
> > diff --git a/meson.build b/meson.build
> > index 8628e6acebee..f60da3e17719 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -179,7 +179,7 @@ arch_x86 = ['x86', 'x86_64']
> >  pipelines_support = {
> >      'imx8-isi':     arch_arm,
> >      'ipu3':         arch_x86,
> > -    'raspberrypi':  arch_arm,
> > +    'rpi/vc4':      arch_arm,
>
> This should also go after 'rkisp1'. Same below, and possibly in other
> patches in this series.
>
> >      'rkisp1':       arch_arm,
> >      'simple':       arch_arm,
> >      'uvcvideo':     ['any'],
> > diff --git a/meson_options.txt b/meson_options.txt
> > index 78a78b7214d5..e1f4c205aa94 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -43,7 +43,7 @@ option('pipelines',
> >              'auto',
> >              'imx8-isi',
> >              'ipu3',
> > -            'raspberrypi',
> > +            'rpi/vc4',
> >              'rkisp1',
> >              'simple',
> >              'uvcvideo',
> > diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build
> > index f14869f3a3c0..4f55611013db 100644
> > --- a/src/libcamera/pipeline/meson.build
> > +++ b/src/libcamera/pipeline/meson.build
> > @@ -3,6 +3,15 @@
> >  # Location of pipeline specific configuration files
> >  pipeline_data_dir = libcamera_datadir / 'pipeline'
> >
> > +# If the Raspberry Pi VC4 pipeline handler is enabled, ensure we include the
> > +# rpi/common subdirectory 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/common')
> > +endif
>
> I don't like how this becomes a special case :-S If we want to support
> multi-level directory trees instead of a flat hierarchy, this should be
> automatic.
>
> We could do something like this:
>
> ----
> subdirs = []
>
> foreach pipeline : pipelines
>     pipeline = pipeline.split('/')[0]
>     if pipeline in subdirs
>         continue
>     endif
>
>     subdir(pipeline)
>     subdirs += [pipeline]
> endforeach
> ----
>
> The logic in src/libcamera/pipeline/rpi/meson.build would be similar,
> iterating over all pipelines and selecting the ones that start with
> 'rpi' only. Something like the following should hopefully work:
>
> ----
> # SPDX-License-Identifier: CC0-1.0
>
> subdir('common')
>
> foreach pipeline : pipelines
>     pipeline = pipeline.split('/')
>     if pipeline.length() < 2 or pipeline[0] != 'rpi'
>         continue
>     endif
>
>     subdir(pipeline[1])
> endforeach
> ----

This should be fine for the pipeline handler case, but the IPA case is a bit
more awkward to do in the IPA case as it appends ipa_name and
enabled_ipa_modules
in the outer loop.  Can you suggest a neat way to fix that?

Regards,
Naush

>
> > +
> >  foreach pipeline : pipelines
> >      subdir(pipeline)
> >  endforeach
> > diff --git a/src/libcamera/pipeline/raspberrypi/delayed_controls.cpp b/src/libcamera/pipeline/rpi/common/delayed_controls.cpp
> > similarity index 100%
> > rename from src/libcamera/pipeline/raspberrypi/delayed_controls.cpp
> > rename to src/libcamera/pipeline/rpi/common/delayed_controls.cpp
> > diff --git a/src/libcamera/pipeline/raspberrypi/delayed_controls.h b/src/libcamera/pipeline/rpi/common/delayed_controls.h
> > similarity index 100%
> > rename from src/libcamera/pipeline/raspberrypi/delayed_controls.h
> > rename to src/libcamera/pipeline/rpi/common/delayed_controls.h
> > diff --git a/src/libcamera/pipeline/rpi/common/meson.build b/src/libcamera/pipeline/rpi/common/meson.build
> > new file mode 100644
> > index 000000000000..1dec6d3d028b
> > --- /dev/null
> > +++ b/src/libcamera/pipeline/rpi/common/meson.build
> > @@ -0,0 +1,8 @@
> > +# SPDX-License-Identifier: CC0-1.0
> > +
> > +libcamera_sources += files([
> > +    'delayed_controls.cpp',
> > +    'rpi_stream.cpp',
> > +])
> > +
> > +includes += include_directories('.')
> > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> > similarity index 100%
> > rename from src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> > rename to src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> > similarity index 100%
> > rename from src/libcamera/pipeline/raspberrypi/rpi_stream.h
> > rename to src/libcamera/pipeline/rpi/common/rpi_stream.h
> > diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/rpi/vc4/data/example.yaml
> > similarity index 100%
> > rename from src/libcamera/pipeline/raspberrypi/data/example.yaml
> > rename to src/libcamera/pipeline/rpi/vc4/data/example.yaml
> > diff --git a/src/libcamera/pipeline/raspberrypi/data/meson.build b/src/libcamera/pipeline/rpi/vc4/data/meson.build
> > similarity index 63%
> > rename from src/libcamera/pipeline/raspberrypi/data/meson.build
> > rename to src/libcamera/pipeline/rpi/vc4/data/meson.build
> > index 1c70433bbcbc..a7dfa02320e5 100644
> > --- a/src/libcamera/pipeline/raspberrypi/data/meson.build
> > +++ b/src/libcamera/pipeline/rpi/vc4/data/meson.build
> > @@ -5,4 +5,4 @@ conf_files = files([
> >  ])
> >
> >  install_data(conf_files,
> > -             install_dir : pipeline_data_dir / 'raspberrypi')
> > +             install_dir : pipeline_data_dir / 'rpi/vc4')
>
>              install_dir : pipeline_data_dir / 'rpi' / 'vc4')
>
> > diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp b/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
> > similarity index 100%
> > rename from src/libcamera/pipeline/raspberrypi/dma_heaps.cpp
> > rename to src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
> > diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.h b/src/libcamera/pipeline/rpi/vc4/dma_heaps.h
> > similarity index 100%
> > rename from src/libcamera/pipeline/raspberrypi/dma_heaps.h
> > rename to src/libcamera/pipeline/rpi/vc4/dma_heaps.h
> > diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/rpi/vc4/meson.build
> > similarity index 71%
> > rename from src/libcamera/pipeline/raspberrypi/meson.build
> > rename to src/libcamera/pipeline/rpi/vc4/meson.build
> > index 59e8fb18c555..228823f30922 100644
> > --- a/src/libcamera/pipeline/raspberrypi/meson.build
> > +++ b/src/libcamera/pipeline/rpi/vc4/meson.build
> > @@ -1,10 +1,8 @@
> >  # SPDX-License-Identifier: CC0-1.0
> >
> >  libcamera_sources += files([
> > -    'delayed_controls.cpp',
> >      'dma_heaps.cpp',
>
> I wondered if this could be a good candidate for the common directory,
> but I think we should turn it into a core libcamera helper if it gets
> shared by multiple pipeline handlers, so I'm happy to leave it here.
>
> >      'raspberrypi.cpp',
> > -    'rpi_stream.cpp',
> >  ])
> >
> >  subdir('data')
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
> > similarity index 99%
> > rename from src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > rename to src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
> > index a4fff28bf198..4595773d2517 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
> > @@ -2,7 +2,7 @@
> >  /*
> >   * Copyright (C) 2019-2021, Raspberry Pi Ltd
> >   *
> > - * raspberrypi.cpp - Pipeline handler for Raspberry Pi devices
> > + * raspberrypi.cpp - Pipeline handler for VC4 based Raspberry Pi devices
>
> s/VC4 based/VC4-based/
>
> >   */
> >  #include <algorithm>
> >  #include <assert.h>
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart April 27, 2023, 2:24 p.m. UTC | #4
Hi Naush,

On Thu, Apr 27, 2023 at 03:02:03PM +0100, Naushir Patuck wrote:
> On Thu, 27 Apr 2023 at 14:14, Laurent Pinchart wrote:
> > On Wed, Apr 26, 2023 at 02:10:47PM +0100, Naushir Patuck via libcamera-devel wrote:
> > > Split the Raspberry Pi pipeline handler code into common and VC4/BCM2835
> > > specific file structures.
> > >
> > > The common code files now live in src/libcamera/pipeline/rpi/common/
> > > and the vc4 specific files in src/libcamera/pipeline/rpi/vc4/.
> > >
> > > To build the pipeline handler, the meson configuration option to select
> > > the Raspberry Pi pipeline handler has now changed from "raspberrypi" to
> > > "rpi/vc4":
> > >
> > > meson setup build -Dpipelines=rpi/vc4
> >
> > This breaks the selection of IPA modules in src/ipa/meson.build:
> >
> > foreach pipeline : pipelines
> >     if ipa_modules.contains(pipeline)
> >         subdir(pipeline)
> >         ipa_names += ipa_install_dir / ipa_name + '.so'
> >         enabled_ipa_modules += pipeline
> >     endif
> > endforeach
> 
> Sorry, I forgot to note in my cover letter that patches 2 and 3 want to be
> squashed before merging for this very reason. I've keep them separate for
> easier review.

No worries. I appreciate the patches being separate for review :-)

> > > There are no functional changes in the pipeline handler code itself.
> > >
> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > > ---
> > >  Documentation/environment_variables.rst                  | 2 +-
> > >  Documentation/guides/introduction.rst                    | 2 +-
> > >  Documentation/guides/ipa.rst                             | 2 +-
> > >  Documentation/guides/pipeline-handler.rst                | 2 +-
> > >  include/libcamera/ipa/meson.build                        | 2 +-
> > >  meson.build                                              | 2 +-
> > >  meson_options.txt                                        | 2 +-
> > >  src/libcamera/pipeline/meson.build                       | 9 +++++++++
> > >  .../{raspberrypi => rpi/common}/delayed_controls.cpp     | 0
> > >  .../{raspberrypi => rpi/common}/delayed_controls.h       | 0
> > >  src/libcamera/pipeline/rpi/common/meson.build            | 8 ++++++++
> > >  .../pipeline/{raspberrypi => rpi/common}/rpi_stream.cpp  | 0
> > >  .../pipeline/{raspberrypi => rpi/common}/rpi_stream.h    | 0
> > >  .../pipeline/{raspberrypi => rpi/vc4}/data/example.yaml  | 0
> > >  .../pipeline/{raspberrypi => rpi/vc4}/data/meson.build   | 2 +-
> > >  .../pipeline/{raspberrypi => rpi/vc4}/dma_heaps.cpp      | 0
> > >  .../pipeline/{raspberrypi => rpi/vc4}/dma_heaps.h        | 0
> > >  .../pipeline/{raspberrypi => rpi/vc4}/meson.build        | 2 --
> > >  .../pipeline/{raspberrypi => rpi/vc4}/raspberrypi.cpp    | 2 +-
> > >  19 files changed, 26 insertions(+), 11 deletions(-)
> > >  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/delayed_controls.cpp (100%)
> > >  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/delayed_controls.h (100%)
> > >  create mode 100644 src/libcamera/pipeline/rpi/common/meson.build
> > >  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/rpi_stream.cpp (100%)
> > >  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/rpi_stream.h (100%)
> > >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/data/example.yaml (100%)
> > >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/data/meson.build (63%)
> > >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/dma_heaps.cpp (100%)
> > >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/dma_heaps.h (100%)
> > >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/meson.build (71%)
> > >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/raspberrypi.cpp (99%)
> > >
> > > diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst
> > > index ceeb251a2ac0..4bf38b877897 100644
> > > --- a/Documentation/environment_variables.rst
> > > +++ b/Documentation/environment_variables.rst
> > > @@ -40,7 +40,7 @@ LIBCAMERA_IPA_MODULE_PATH
> > >  LIBCAMERA_RPI_CONFIG_FILE
> > >     Define a custom configuration file to use in the Raspberry Pi pipeline handler.
> > >
> > > -   Example value: ``/usr/local/share/libcamera/pipeline/raspberrypi/minimal_mem.yaml``
> > > +   Example value: ``/usr/local/share/libcamera/pipeline/rpi/vc4/minimal_mem.yaml``
> > >
> > >  Further details
> > >  ---------------
> > > diff --git a/Documentation/guides/introduction.rst b/Documentation/guides/introduction.rst
> > > index 2d1760c1866b..700ec2d33c30 100644
> > > --- a/Documentation/guides/introduction.rst
> > > +++ b/Documentation/guides/introduction.rst
> > > @@ -288,7 +288,7 @@ with dedicated pipeline handlers:
> > >
> > >     -  Intel IPU3 (ipu3)
> > >     -  Rockchip RK3399 (rkisp1)
> > > -   -  RaspberryPi 3 and 4 (raspberrypi)
> > > +   -  RaspberryPi 3 and 4 (rpi/vc4)
> > >
> > >  Furthermore, generic platform support is provided for the following:
> > >
> > > diff --git a/Documentation/guides/ipa.rst b/Documentation/guides/ipa.rst
> > > index 89839408672a..10301d89fc80 100644
> > > --- a/Documentation/guides/ipa.rst
> > > +++ b/Documentation/guides/ipa.rst
> > > @@ -279,7 +279,7 @@ For example, adding the raspberrypi.mojom file to meson:
> > >  .. code-block:: none
> > >
> > >          pipeline_ipa_mojom_mapping = [
> > > -            'raspberrypi': 'raspberrypi.mojom',
> > > +            'rpi/vc4': 'raspberrypi.mojom',
> > >          ]
> > >
> > >  This will cause the mojo data definition file to be compiled. Specifically, it
> > > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
> > > index 4d38fa23fbcd..7d143b0eaecb 100644
> > > --- a/Documentation/guides/pipeline-handler.rst
> > > +++ b/Documentation/guides/pipeline-handler.rst
> > > @@ -183,7 +183,7 @@ to the libcamera build options in the top level ``meson_options.txt``.
> > >
> > >     option('pipelines',
> > >             type : 'array',
> > > -           choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc', 'vivid'],
> > > +           choices : ['ipu3', 'rpi/vc4', 'rkisp1', 'simple', 'uvcvideo', 'vimc', 'vivid'],
> >
> > 'rpi/vc4' should now go after 'rkisp1' in alphabetical order.
> >
> > >             description : 'Select which pipeline handlers to include')
> > >
> > >
> > > diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
> > > index 67c31cb04ccf..bae15d64d7d8 100644
> > > --- a/include/libcamera/ipa/meson.build
> > > +++ b/include/libcamera/ipa/meson.build
> > > @@ -64,7 +64,7 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',
> > >  pipeline_ipa_mojom_mapping = {
> > >      'ipu3': 'ipu3.mojom',
> > >      'rkisp1': 'rkisp1.mojom',
> > > -    'raspberrypi': 'raspberrypi.mojom',
> > > +    'rpi/vc4': 'raspberrypi.mojom',
> > >      'vimc': 'vimc.mojom',
> > >  }
> > >
> > > diff --git a/meson.build b/meson.build
> > > index 8628e6acebee..f60da3e17719 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -179,7 +179,7 @@ arch_x86 = ['x86', 'x86_64']
> > >  pipelines_support = {
> > >      'imx8-isi':     arch_arm,
> > >      'ipu3':         arch_x86,
> > > -    'raspberrypi':  arch_arm,
> > > +    'rpi/vc4':      arch_arm,
> >
> > This should also go after 'rkisp1'. Same below, and possibly in other
> > patches in this series.
> >
> > >      'rkisp1':       arch_arm,
> > >      'simple':       arch_arm,
> > >      'uvcvideo':     ['any'],
> > > diff --git a/meson_options.txt b/meson_options.txt
> > > index 78a78b7214d5..e1f4c205aa94 100644
> > > --- a/meson_options.txt
> > > +++ b/meson_options.txt
> > > @@ -43,7 +43,7 @@ option('pipelines',
> > >              'auto',
> > >              'imx8-isi',
> > >              'ipu3',
> > > -            'raspberrypi',
> > > +            'rpi/vc4',
> > >              'rkisp1',
> > >              'simple',
> > >              'uvcvideo',
> > > diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build
> > > index f14869f3a3c0..4f55611013db 100644
> > > --- a/src/libcamera/pipeline/meson.build
> > > +++ b/src/libcamera/pipeline/meson.build
> > > @@ -3,6 +3,15 @@
> > >  # Location of pipeline specific configuration files
> > >  pipeline_data_dir = libcamera_datadir / 'pipeline'
> > >
> > > +# If the Raspberry Pi VC4 pipeline handler is enabled, ensure we include the
> > > +# rpi/common subdirectory 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/common')
> > > +endif
> >
> > I don't like how this becomes a special case :-S If we want to support
> > multi-level directory trees instead of a flat hierarchy, this should be
> > automatic.
> >
> > We could do something like this:
> >
> > ----
> > subdirs = []
> >
> > foreach pipeline : pipelines
> >     pipeline = pipeline.split('/')[0]
> >     if pipeline in subdirs
> >         continue
> >     endif
> >
> >     subdir(pipeline)
> >     subdirs += [pipeline]
> > endforeach
> > ----
> >
> > The logic in src/libcamera/pipeline/rpi/meson.build would be similar,
> > iterating over all pipelines and selecting the ones that start with
> > 'rpi' only. Something like the following should hopefully work:
> >
> > ----
> > # SPDX-License-Identifier: CC0-1.0
> >
> > subdir('common')
> >
> > foreach pipeline : pipelines
> >     pipeline = pipeline.split('/')
> >     if pipeline.length() < 2 or pipeline[0] != 'rpi'
> >         continue
> >     endif
> >
> >     subdir(pipeline[1])
> > endforeach
> > ----
> 
> This should be fine for the pipeline handler case, but the IPA case is a bit
> more awkward to do in the IPA case as it appends ipa_name and enabled_ipa_modules
> in the outer loop.  Can you suggest a neat way to fix that?

I'll have a look once I finish reviewing the series. It may need to wait
until Monday I'm afraid.

> > > +
> > >  foreach pipeline : pipelines
> > >      subdir(pipeline)
> > >  endforeach
> > > diff --git a/src/libcamera/pipeline/raspberrypi/delayed_controls.cpp b/src/libcamera/pipeline/rpi/common/delayed_controls.cpp
> > > similarity index 100%
> > > rename from src/libcamera/pipeline/raspberrypi/delayed_controls.cpp
> > > rename to src/libcamera/pipeline/rpi/common/delayed_controls.cpp
> > > diff --git a/src/libcamera/pipeline/raspberrypi/delayed_controls.h b/src/libcamera/pipeline/rpi/common/delayed_controls.h
> > > similarity index 100%
> > > rename from src/libcamera/pipeline/raspberrypi/delayed_controls.h
> > > rename to src/libcamera/pipeline/rpi/common/delayed_controls.h
> > > diff --git a/src/libcamera/pipeline/rpi/common/meson.build b/src/libcamera/pipeline/rpi/common/meson.build
> > > new file mode 100644
> > > index 000000000000..1dec6d3d028b
> > > --- /dev/null
> > > +++ b/src/libcamera/pipeline/rpi/common/meson.build
> > > @@ -0,0 +1,8 @@
> > > +# SPDX-License-Identifier: CC0-1.0
> > > +
> > > +libcamera_sources += files([
> > > +    'delayed_controls.cpp',
> > > +    'rpi_stream.cpp',
> > > +])
> > > +
> > > +includes += include_directories('.')
> > > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> > > similarity index 100%
> > > rename from src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> > > rename to src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> > > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> > > similarity index 100%
> > > rename from src/libcamera/pipeline/raspberrypi/rpi_stream.h
> > > rename to src/libcamera/pipeline/rpi/common/rpi_stream.h
> > > diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/rpi/vc4/data/example.yaml
> > > similarity index 100%
> > > rename from src/libcamera/pipeline/raspberrypi/data/example.yaml
> > > rename to src/libcamera/pipeline/rpi/vc4/data/example.yaml
> > > diff --git a/src/libcamera/pipeline/raspberrypi/data/meson.build b/src/libcamera/pipeline/rpi/vc4/data/meson.build
> > > similarity index 63%
> > > rename from src/libcamera/pipeline/raspberrypi/data/meson.build
> > > rename to src/libcamera/pipeline/rpi/vc4/data/meson.build
> > > index 1c70433bbcbc..a7dfa02320e5 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/data/meson.build
> > > +++ b/src/libcamera/pipeline/rpi/vc4/data/meson.build
> > > @@ -5,4 +5,4 @@ conf_files = files([
> > >  ])
> > >
> > >  install_data(conf_files,
> > > -             install_dir : pipeline_data_dir / 'raspberrypi')
> > > +             install_dir : pipeline_data_dir / 'rpi/vc4')
> >
> >              install_dir : pipeline_data_dir / 'rpi' / 'vc4')
> >
> > > diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp b/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
> > > similarity index 100%
> > > rename from src/libcamera/pipeline/raspberrypi/dma_heaps.cpp
> > > rename to src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
> > > diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.h b/src/libcamera/pipeline/rpi/vc4/dma_heaps.h
> > > similarity index 100%
> > > rename from src/libcamera/pipeline/raspberrypi/dma_heaps.h
> > > rename to src/libcamera/pipeline/rpi/vc4/dma_heaps.h
> > > diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/rpi/vc4/meson.build
> > > similarity index 71%
> > > rename from src/libcamera/pipeline/raspberrypi/meson.build
> > > rename to src/libcamera/pipeline/rpi/vc4/meson.build
> > > index 59e8fb18c555..228823f30922 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/meson.build
> > > +++ b/src/libcamera/pipeline/rpi/vc4/meson.build
> > > @@ -1,10 +1,8 @@
> > >  # SPDX-License-Identifier: CC0-1.0
> > >
> > >  libcamera_sources += files([
> > > -    'delayed_controls.cpp',
> > >      'dma_heaps.cpp',
> >
> > I wondered if this could be a good candidate for the common directory,
> > but I think we should turn it into a core libcamera helper if it gets
> > shared by multiple pipeline handlers, so I'm happy to leave it here.
> >
> > >      'raspberrypi.cpp',
> > > -    'rpi_stream.cpp',
> > >  ])
> > >
> > >  subdir('data')
> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
> > > similarity index 99%
> > > rename from src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > rename to src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
> > > index a4fff28bf198..4595773d2517 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > +++ b/src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
> > > @@ -2,7 +2,7 @@
> > >  /*
> > >   * Copyright (C) 2019-2021, Raspberry Pi Ltd
> > >   *
> > > - * raspberrypi.cpp - Pipeline handler for Raspberry Pi devices
> > > + * raspberrypi.cpp - Pipeline handler for VC4 based Raspberry Pi devices
> >
> > s/VC4 based/VC4-based/
> >
> > >   */
> > >  #include <algorithm>
> > >  #include <assert.h>
Laurent Pinchart April 27, 2023, 3:59 p.m. UTC | #5
Hi Naush,

Another comment.

On Thu, Apr 27, 2023 at 04:14:47PM +0300, Laurent Pinchart via libcamera-devel wrote:
> On Wed, Apr 26, 2023 at 02:10:47PM +0100, Naushir Patuck via libcamera-devel wrote:
> > Split the Raspberry Pi pipeline handler code into common and VC4/BCM2835
> > specific file structures.
> > 
> > The common code files now live in src/libcamera/pipeline/rpi/common/
> > and the vc4 specific files in src/libcamera/pipeline/rpi/vc4/.
> > 
> > To build the pipeline handler, the meson configuration option to select
> > the Raspberry Pi pipeline handler has now changed from "raspberrypi" to
> > "rpi/vc4":
> > 
> > meson setup build -Dpipelines=rpi/vc4
> 
> This breaks the selection of IPA modules in src/ipa/meson.build:
> 
> foreach pipeline : pipelines
>     if ipa_modules.contains(pipeline)
>         subdir(pipeline)
>         ipa_names += ipa_install_dir / ipa_name + '.so'
>         enabled_ipa_modules += pipeline
>     endif
> endforeach
> 
> > There are no functional changes in the pipeline handler code itself.
> > 
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > ---
> >  Documentation/environment_variables.rst                  | 2 +-
> >  Documentation/guides/introduction.rst                    | 2 +-
> >  Documentation/guides/ipa.rst                             | 2 +-
> >  Documentation/guides/pipeline-handler.rst                | 2 +-
> >  include/libcamera/ipa/meson.build                        | 2 +-
> >  meson.build                                              | 2 +-
> >  meson_options.txt                                        | 2 +-
> >  src/libcamera/pipeline/meson.build                       | 9 +++++++++
> >  .../{raspberrypi => rpi/common}/delayed_controls.cpp     | 0
> >  .../{raspberrypi => rpi/common}/delayed_controls.h       | 0
> >  src/libcamera/pipeline/rpi/common/meson.build            | 8 ++++++++
> >  .../pipeline/{raspberrypi => rpi/common}/rpi_stream.cpp  | 0
> >  .../pipeline/{raspberrypi => rpi/common}/rpi_stream.h    | 0
> >  .../pipeline/{raspberrypi => rpi/vc4}/data/example.yaml  | 0
> >  .../pipeline/{raspberrypi => rpi/vc4}/data/meson.build   | 2 +-
> >  .../pipeline/{raspberrypi => rpi/vc4}/dma_heaps.cpp      | 0
> >  .../pipeline/{raspberrypi => rpi/vc4}/dma_heaps.h        | 0
> >  .../pipeline/{raspberrypi => rpi/vc4}/meson.build        | 2 --
> >  .../pipeline/{raspberrypi => rpi/vc4}/raspberrypi.cpp    | 2 +-
> >  19 files changed, 26 insertions(+), 11 deletions(-)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/delayed_controls.cpp (100%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/delayed_controls.h (100%)
> >  create mode 100644 src/libcamera/pipeline/rpi/common/meson.build
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/rpi_stream.cpp (100%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/common}/rpi_stream.h (100%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/data/example.yaml (100%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/data/meson.build (63%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/dma_heaps.cpp (100%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/dma_heaps.h (100%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/meson.build (71%)
> >  rename src/libcamera/pipeline/{raspberrypi => rpi/vc4}/raspberrypi.cpp (99%)

[snip]

> > diff --git a/src/libcamera/pipeline/rpi/common/meson.build b/src/libcamera/pipeline/rpi/common/meson.build
> > new file mode 100644
> > index 000000000000..1dec6d3d028b
> > --- /dev/null
> > +++ b/src/libcamera/pipeline/rpi/common/meson.build
> > @@ -0,0 +1,8 @@
> > +# SPDX-License-Identifier: CC0-1.0
> > +
> > +libcamera_sources += files([
> > +    'delayed_controls.cpp',
> > +    'rpi_stream.cpp',
> > +])
> > +
> > +includes += include_directories('.')

This isn't nice, especially given that the common directory will contain
a pipeline_base.h, which is a quite generic name. You can drop this and
use relative include paths in the source files.

[snip]

Patch
diff mbox series

diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst
index ceeb251a2ac0..4bf38b877897 100644
--- a/Documentation/environment_variables.rst
+++ b/Documentation/environment_variables.rst
@@ -40,7 +40,7 @@  LIBCAMERA_IPA_MODULE_PATH
 LIBCAMERA_RPI_CONFIG_FILE
    Define a custom configuration file to use in the Raspberry Pi pipeline handler.
 
-   Example value: ``/usr/local/share/libcamera/pipeline/raspberrypi/minimal_mem.yaml``
+   Example value: ``/usr/local/share/libcamera/pipeline/rpi/vc4/minimal_mem.yaml``
 
 Further details
 ---------------
diff --git a/Documentation/guides/introduction.rst b/Documentation/guides/introduction.rst
index 2d1760c1866b..700ec2d33c30 100644
--- a/Documentation/guides/introduction.rst
+++ b/Documentation/guides/introduction.rst
@@ -288,7 +288,7 @@  with dedicated pipeline handlers:
 
    -  Intel IPU3 (ipu3)
    -  Rockchip RK3399 (rkisp1)
-   -  RaspberryPi 3 and 4 (raspberrypi)
+   -  RaspberryPi 3 and 4 (rpi/vc4)
 
 Furthermore, generic platform support is provided for the following:
 
diff --git a/Documentation/guides/ipa.rst b/Documentation/guides/ipa.rst
index 89839408672a..10301d89fc80 100644
--- a/Documentation/guides/ipa.rst
+++ b/Documentation/guides/ipa.rst
@@ -279,7 +279,7 @@  For example, adding the raspberrypi.mojom file to meson:
 .. code-block:: none
 
         pipeline_ipa_mojom_mapping = [
-            'raspberrypi': 'raspberrypi.mojom',
+            'rpi/vc4': 'raspberrypi.mojom',
         ]
 
 This will cause the mojo data definition file to be compiled. Specifically, it
diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
index 4d38fa23fbcd..7d143b0eaecb 100644
--- a/Documentation/guides/pipeline-handler.rst
+++ b/Documentation/guides/pipeline-handler.rst
@@ -183,7 +183,7 @@  to the libcamera build options in the top level ``meson_options.txt``.
 
    option('pipelines',
            type : 'array',
-           choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc', 'vivid'],
+           choices : ['ipu3', 'rpi/vc4', 'rkisp1', 'simple', 'uvcvideo', 'vimc', 'vivid'],
            description : 'Select which pipeline handlers to include')
 
 
diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
index 67c31cb04ccf..bae15d64d7d8 100644
--- a/include/libcamera/ipa/meson.build
+++ b/include/libcamera/ipa/meson.build
@@ -64,7 +64,7 @@  libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',
 pipeline_ipa_mojom_mapping = {
     'ipu3': 'ipu3.mojom',
     'rkisp1': 'rkisp1.mojom',
-    'raspberrypi': 'raspberrypi.mojom',
+    'rpi/vc4': 'raspberrypi.mojom',
     'vimc': 'vimc.mojom',
 }
 
diff --git a/meson.build b/meson.build
index 8628e6acebee..f60da3e17719 100644
--- a/meson.build
+++ b/meson.build
@@ -179,7 +179,7 @@  arch_x86 = ['x86', 'x86_64']
 pipelines_support = {
     'imx8-isi':     arch_arm,
     'ipu3':         arch_x86,
-    'raspberrypi':  arch_arm,
+    'rpi/vc4':      arch_arm,
     'rkisp1':       arch_arm,
     'simple':       arch_arm,
     'uvcvideo':     ['any'],
diff --git a/meson_options.txt b/meson_options.txt
index 78a78b7214d5..e1f4c205aa94 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -43,7 +43,7 @@  option('pipelines',
             'auto',
             'imx8-isi',
             'ipu3',
-            'raspberrypi',
+            'rpi/vc4',
             'rkisp1',
             'simple',
             'uvcvideo',
diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build
index f14869f3a3c0..4f55611013db 100644
--- a/src/libcamera/pipeline/meson.build
+++ b/src/libcamera/pipeline/meson.build
@@ -3,6 +3,15 @@ 
 # Location of pipeline specific configuration files
 pipeline_data_dir = libcamera_datadir / 'pipeline'
 
+# If the Raspberry Pi VC4 pipeline handler is enabled, ensure we include the
+# rpi/common subdirectory 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/common')
+endif
+
 foreach pipeline : pipelines
     subdir(pipeline)
 endforeach
diff --git a/src/libcamera/pipeline/raspberrypi/delayed_controls.cpp b/src/libcamera/pipeline/rpi/common/delayed_controls.cpp
similarity index 100%
rename from src/libcamera/pipeline/raspberrypi/delayed_controls.cpp
rename to src/libcamera/pipeline/rpi/common/delayed_controls.cpp
diff --git a/src/libcamera/pipeline/raspberrypi/delayed_controls.h b/src/libcamera/pipeline/rpi/common/delayed_controls.h
similarity index 100%
rename from src/libcamera/pipeline/raspberrypi/delayed_controls.h
rename to src/libcamera/pipeline/rpi/common/delayed_controls.h
diff --git a/src/libcamera/pipeline/rpi/common/meson.build b/src/libcamera/pipeline/rpi/common/meson.build
new file mode 100644
index 000000000000..1dec6d3d028b
--- /dev/null
+++ b/src/libcamera/pipeline/rpi/common/meson.build
@@ -0,0 +1,8 @@ 
+# SPDX-License-Identifier: CC0-1.0
+
+libcamera_sources += files([
+    'delayed_controls.cpp',
+    'rpi_stream.cpp',
+])
+
+includes += include_directories('.')
diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
similarity index 100%
rename from src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
rename to src/libcamera/pipeline/rpi/common/rpi_stream.cpp
diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h
similarity index 100%
rename from src/libcamera/pipeline/raspberrypi/rpi_stream.h
rename to src/libcamera/pipeline/rpi/common/rpi_stream.h
diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/rpi/vc4/data/example.yaml
similarity index 100%
rename from src/libcamera/pipeline/raspberrypi/data/example.yaml
rename to src/libcamera/pipeline/rpi/vc4/data/example.yaml
diff --git a/src/libcamera/pipeline/raspberrypi/data/meson.build b/src/libcamera/pipeline/rpi/vc4/data/meson.build
similarity index 63%
rename from src/libcamera/pipeline/raspberrypi/data/meson.build
rename to src/libcamera/pipeline/rpi/vc4/data/meson.build
index 1c70433bbcbc..a7dfa02320e5 100644
--- a/src/libcamera/pipeline/raspberrypi/data/meson.build
+++ b/src/libcamera/pipeline/rpi/vc4/data/meson.build
@@ -5,4 +5,4 @@  conf_files = files([
 ])
 
 install_data(conf_files,
-             install_dir : pipeline_data_dir / 'raspberrypi')
+             install_dir : pipeline_data_dir / 'rpi/vc4')
diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp b/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
similarity index 100%
rename from src/libcamera/pipeline/raspberrypi/dma_heaps.cpp
rename to src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.h b/src/libcamera/pipeline/rpi/vc4/dma_heaps.h
similarity index 100%
rename from src/libcamera/pipeline/raspberrypi/dma_heaps.h
rename to src/libcamera/pipeline/rpi/vc4/dma_heaps.h
diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/rpi/vc4/meson.build
similarity index 71%
rename from src/libcamera/pipeline/raspberrypi/meson.build
rename to src/libcamera/pipeline/rpi/vc4/meson.build
index 59e8fb18c555..228823f30922 100644
--- a/src/libcamera/pipeline/raspberrypi/meson.build
+++ b/src/libcamera/pipeline/rpi/vc4/meson.build
@@ -1,10 +1,8 @@ 
 # SPDX-License-Identifier: CC0-1.0
 
 libcamera_sources += files([
-    'delayed_controls.cpp',
     'dma_heaps.cpp',
     'raspberrypi.cpp',
-    'rpi_stream.cpp',
 ])
 
 subdir('data')
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
similarity index 99%
rename from src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
rename to src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
index a4fff28bf198..4595773d2517 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/rpi/vc4/raspberrypi.cpp
@@ -2,7 +2,7 @@ 
 /*
  * Copyright (C) 2019-2021, Raspberry Pi Ltd
  *
- * raspberrypi.cpp - Pipeline handler for Raspberry Pi devices
+ * raspberrypi.cpp - Pipeline handler for VC4 based Raspberry Pi devices
  */
 #include <algorithm>
 #include <assert.h>