[v18,07/12] pipeline: simple: Allow enabling software ISP via config file
diff mbox series

Message ID 20250912142915.53949-9-mzamazal@redhat.com
State Accepted
Headers show
Series
  • Add global configuration file
Related show

Commit Message

Milan Zamazal Sept. 12, 2025, 2:29 p.m. UTC
This patch allows enabling or disabling software ISP via config file in
addition to compile time.  This can be useful for software ISP testing
on various platforms as well as for overriding the defaults in case the
defaults don't work well (e.g. hardware ISP may or may not work on
i.MX8MP depending on the kernel and libcamera patches present in the
given system).

The configuration is specified as follows:

  configuration:
    pipelines:
      simple:
        supported_devices:
        - driver: DRIVER-NAME
          software_isp: BOOLEAN
        - ...

For example:

  configuration:
    pipelines:
      simple:
        supported_devices:
        - driver: mxc-isi
          software_isp: true

The overall configuration of enabling or disabling software ISP may get
dropped in future but this patch is still useful in the meantime.

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/libcamera/pipeline/simple/simple.cpp | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Laurent Pinchart Sept. 19, 2025, 6:57 p.m. UTC | #1
On Fri, Sep 12, 2025 at 04:29:09PM +0200, Milan Zamazal wrote:
> This patch allows enabling or disabling software ISP via config file in
> addition to compile time.  This can be useful for software ISP testing
> on various platforms as well as for overriding the defaults in case the
> defaults don't work well (e.g. hardware ISP may or may not work on
> i.MX8MP depending on the kernel and libcamera patches present in the
> given system).
> 
> The configuration is specified as follows:
> 
>   configuration:
>     pipelines:
>       simple:
>         supported_devices:
>         - driver: DRIVER-NAME
>           software_isp: BOOLEAN
>         - ...

This should be

        supported_devices:
          - driver: DRIVER-NAME
            software_isp: BOOLEAN
          - ...

I'll fix it.

> 
> For example:
> 
>   configuration:
>     pipelines:
>       simple:
>         supported_devices:
>         - driver: mxc-isi
>           software_isp: true

Same here.

> The overall configuration of enabling or disabling software ISP may get
> dropped in future but this patch is still useful in the meantime.
> 
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>

I have a few ideas for improvement, but they can be implemented on top.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/libcamera/pipeline/simple/simple.cpp | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 4f914be80..c816cffc9 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -30,11 +30,13 @@
>  #include <libcamera/stream.h>
>  
>  #include "libcamera/internal/camera.h"
> +#include "libcamera/internal/camera_manager.h"
>  #include "libcamera/internal/camera_sensor.h"
>  #include "libcamera/internal/camera_sensor_properties.h"
>  #include "libcamera/internal/converter.h"
>  #include "libcamera/internal/delayed_controls.h"
>  #include "libcamera/internal/device_enumerator.h"
> +#include "libcamera/internal/global_configuration.h"
>  #include "libcamera/internal/media_device.h"
>  #include "libcamera/internal/pipeline_handler.h"
>  #include "libcamera/internal/software_isp/software_isp.h"
> @@ -1682,6 +1684,19 @@ bool SimplePipelineHandler::matchDevice(MediaDevice *media,
>  	}
>  
>  	swIspEnabled_ = info.swIspEnabled;
> +	const GlobalConfiguration &configuration = cameraManager()->_d()->configuration();
> +	for (GlobalConfiguration::Configuration entry :
> +	     configuration.configuration()["pipelines"]["simple"]["supported_devices"]
> +		     .asList()) {
> +		auto name = entry["driver"].get<std::string>();
> +		if (name == info.driver) {
> +			swIspEnabled_ = entry["software_isp"].get<bool>().value_or(swIspEnabled_);
> +			LOG(SimplePipeline, Debug)
> +				<< "Configuration file overrides software ISP for "
> +				<< info.driver << " to " << swIspEnabled_;
> +			break;
> +		}
> +	}
>  
>  	/* Locate the sensors. */
>  	std::vector<MediaEntity *> sensors = locateSensors(media);

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 4f914be80..c816cffc9 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -30,11 +30,13 @@ 
 #include <libcamera/stream.h>
 
 #include "libcamera/internal/camera.h"
+#include "libcamera/internal/camera_manager.h"
 #include "libcamera/internal/camera_sensor.h"
 #include "libcamera/internal/camera_sensor_properties.h"
 #include "libcamera/internal/converter.h"
 #include "libcamera/internal/delayed_controls.h"
 #include "libcamera/internal/device_enumerator.h"
+#include "libcamera/internal/global_configuration.h"
 #include "libcamera/internal/media_device.h"
 #include "libcamera/internal/pipeline_handler.h"
 #include "libcamera/internal/software_isp/software_isp.h"
@@ -1682,6 +1684,19 @@  bool SimplePipelineHandler::matchDevice(MediaDevice *media,
 	}
 
 	swIspEnabled_ = info.swIspEnabled;
+	const GlobalConfiguration &configuration = cameraManager()->_d()->configuration();
+	for (GlobalConfiguration::Configuration entry :
+	     configuration.configuration()["pipelines"]["simple"]["supported_devices"]
+		     .asList()) {
+		auto name = entry["driver"].get<std::string>();
+		if (name == info.driver) {
+			swIspEnabled_ = entry["software_isp"].get<bool>().value_or(swIspEnabled_);
+			LOG(SimplePipeline, Debug)
+				<< "Configuration file overrides software ISP for "
+				<< info.driver << " to " << swIspEnabled_;
+			break;
+		}
+	}
 
 	/* Locate the sensors. */
 	std::vector<MediaEntity *> sensors = locateSensors(media);