libcamera: software_isp: Make input buffer copying configurable

On some platforms, working directly on the input buffer is very slow due
to disabled caching.  This is why we copy the input buffer into standard
(cached) memory.  This is an unnecessary overhead on platforms with
cached buffers.

Let's make input buffer copying configurable.  The default is still
copying, as its overhead is much lower than contingent operations on
non-cached memory.  Ideally, we should improve this in future to set the
default to non-copying if we can be sure under observable circumstances
that we are working with cached buffers.

Completes software ISP TODO #6.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Documentation/runtime_configuration.rst

@@ -46,6 +46,8 @@ file structure:
         supported_devices:
           - driver: # driver name, e.g. `mxc-isi`
             software_isp: # true/false
+    software_isp:
+      copy_input_buffer: # true/false
 
 Configuration file example
 --------------------------
@@ -74,6 +76,8 @@ Configuration file example
          supported_devices:
            - driver: mxc-isi
              software_isp: true
+     software_isp:
+       copy_input_buffer: false
 
 List of variables and configuration options
 -------------------------------------------
@@ -136,6 +140,15 @@ pipelines.simple.supported_devices.driver, pipelines.simple.supported_devices.so
 
    Example `software_isp` value: ``true``
 
+software_isp.copy_input_buffer
+   Define whether input buffers should be copied into standard (cached)
+   memory in software ISP. This is done by default to prevent very slow
+   processing on platforms with non-cached buffers. It can be set to
+   false on platforms with cached buffers to avoid an unnecessary
+   overhead.
+
+   Example value: ``false``
+
 Further details
 ---------------
 

src/libcamera/software_isp/TODO

@@ -71,17 +71,6 @@ per-frame buffers like we do for hardware ISPs.
 
 ---
 
-6. Input buffer copying configuration
-
-> DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats)
-> 	: stats_(std::move(stats)), gammaCorrection_(1.0)
-> {
-> 	enableInputMemcpy_ = true;
-
-Set this appropriately and/or make it configurable.
-
----
-
 7. Performance measurement configuration
 
 > void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams params)

src/libcamera/software_isp/debayer_cpu.cpp

@@ -24,6 +24,7 @@
 #include "libcamera/internal/bayer_format.h"
 #include "libcamera/internal/dma_buf_allocator.h"
 #include "libcamera/internal/framebuffer.h"
+#include "libcamera/internal/global_configuration.h"
 #include "libcamera/internal/mapped_framebuffer.h"
 
 namespace libcamera {
@@ -38,8 +39,9 @@ namespace libcamera {
 /**
  * \brief Constructs a DebayerCpu object
  * \param[in] stats Pointer to the stats object to use
+ * \param[in] configuration The global configuration
  */
-DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats)
+DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats, const GlobalConfiguration &configuration)
 	: stats_(std::move(stats))
 {
 	/*
@@ -49,8 +51,12 @@ DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats)
 	 * enable_input_memcpy_ makes this behavior configurable.  At the moment, we
 	 * always set it to true as the safer choice but this should be changed in
 	 * future.
+	 *
+	 * \todo Make memcpy automatic based on runtime detection of platform
+	 * capabilities.
 	 */
-	enableInputMemcpy_ = true;
+	enableInputMemcpy_ =
+		configuration.option<bool>({ "software_isp", "copy_input_buffer" }).value_or(true);
 
 	/* Initialize color lookup tables */
 	for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {

src/libcamera/software_isp/debayer_cpu.h

@@ -18,6 +18,7 @@
 #include <libcamera/base/object.h>
 
 #include "libcamera/internal/bayer_format.h"
+#include "libcamera/internal/global_configuration.h"
 
 #include "debayer.h"
 #include "swstats_cpu.h"
@@ -27,7 +28,7 @@ namespace libcamera {
 class DebayerCpu : public Debayer, public Object
 {
 public:
-	DebayerCpu(std::unique_ptr<SwStatsCpu> stats);
+	DebayerCpu(std::unique_ptr<SwStatsCpu> stats, const GlobalConfiguration &configuration);
 	~DebayerCpu();
 
 	int configure(const StreamConfiguration &inputCfg,

src/libcamera/software_isp/software_isp.cpp

@@ -114,7 +114,8 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
 	}
 	stats->statsReady.connect(this, &SoftwareIsp::statsReady);
 
-	debayer_ = std::make_unique<DebayerCpu>(std::move(stats));
+	const GlobalConfiguration &configuration = pipe->cameraManager()->_d()->configuration();
+	debayer_ = std::make_unique<DebayerCpu>(std::move(stats), configuration);
 	debayer_->inputBufferReady.connect(this, &SoftwareIsp::inputReady);
 	debayer_->outputBufferReady.connect(this, &SoftwareIsp::outputReady);