From 6e1d889cfec9ccc4d21833340b23c9a8bd2b30de Mon Sep 17 00:00:00 2001 From: Milan Zamazal Date: Fri, 12 Sep 2025 16:29:11 +0200 Subject: [PATCH] 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 Reviewed-by: Paul Elder Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- Documentation/runtime_configuration.rst | 13 +++++++++++++ src/libcamera/software_isp/TODO | 11 ----------- src/libcamera/software_isp/debayer_cpu.cpp | 10 ++++++++-- src/libcamera/software_isp/debayer_cpu.h | 3 ++- src/libcamera/software_isp/software_isp.cpp | 3 ++- 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/Documentation/runtime_configuration.rst b/Documentation/runtime_configuration.rst index bad0297c..ea5bd55e 100644 --- a/Documentation/runtime_configuration.rst +++ b/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 --------------- diff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO index a50db668..2c919f44 100644 --- a/src/libcamera/software_isp/TODO +++ b/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 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) diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp index 66f6038c..a5f915b6 100644 --- a/src/libcamera/software_isp/debayer_cpu.cpp +++ b/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 stats) +DebayerCpu::DebayerCpu(std::unique_ptr stats, const GlobalConfiguration &configuration) : stats_(std::move(stats)) { /* @@ -49,8 +51,12 @@ DebayerCpu::DebayerCpu(std::unique_ptr 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({ "software_isp", "copy_input_buffer" }).value_or(true); /* Initialize color lookup tables */ for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) { diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h index 926195e9..2f35aa18 100644 --- a/src/libcamera/software_isp/debayer_cpu.h +++ b/src/libcamera/software_isp/debayer_cpu.h @@ -18,6 +18,7 @@ #include #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 stats); + DebayerCpu(std::unique_ptr stats, const GlobalConfiguration &configuration); ~DebayerCpu(); int configure(const StreamConfiguration &inputCfg, diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp index 28e2a360..b7651b7d 100644 --- a/src/libcamera/software_isp/software_isp.cpp +++ b/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(std::move(stats)); + const GlobalConfiguration &configuration = pipe->cameraManager()->_d()->configuration(); + debayer_ = std::make_unique(std::move(stats), configuration); debayer_->inputBufferReady.connect(this, &SoftwareIsp::inputReady); debayer_->outputBufferReady.connect(this, &SoftwareIsp::outputReady);