[v1] software_isp: Request input buffer alignment of 256 bytes
diff mbox series

Message ID 20260527150715.67574-1-robert.mader@collabora.com
State New
Headers show
Series
  • [v1] software_isp: Request input buffer alignment of 256 bytes
Related show

Commit Message

Robert Mader May 27, 2026, 3:07 p.m. UTC 
When using the GPU-ISP. This should increase the chances that input
buffers can be imported and directly be used by the GPU as stride
requirements are the most common reason for import fails.

Notes:
 - 256 is already used for output buffers and known to work across
   most hardware. Unfortunately there is no API to query the optimal
   value from the GL driver.
 - The V4L2 API does not require drivers to honor stride request, thus
   we log those cases.

Signed-off-by: Robert Mader <robert.mader@collabora.com>

---

Follow-up to https://patchwork.libcamera.org/project/libcamera/list/?series=5965
---
 .../libcamera/internal/software_isp/software_isp.h  |  1 +
 src/libcamera/pipeline/simple/simple.cpp            | 12 ++++++++++++
 src/libcamera/software_isp/debayer.cpp              |  8 ++++++++
 src/libcamera/software_isp/debayer.h                |  1 +
 src/libcamera/software_isp/debayer_egl.cpp          |  7 +++++++
 src/libcamera/software_isp/debayer_egl.h            |  1 +
 src/libcamera/software_isp/software_isp.cpp         | 13 +++++++++++++
 7 files changed, 43 insertions(+)

Comments

Robert Mader May 27, 2026, 8:55 p.m. UTC | #1 
On 27.05.26 17:07, Robert Mader wrote:
> ...
>   
> +uint32_t DebayerEGL::getPreferredInputStride(const PixelFormat &inputFormat, const Size &size)
> +{
> +	const PixelFormatInfo &info = PixelFormatInfo::info(inputFormat);
> +	return info.stride(size.width, 0, 256);
> +}
> +

Sorry for talking to myself, however I guess it would be a good call to 
turn the 256 into a `static constexpr` variable and use that here and 
for the other already present occurrence in strideAndFrameSize(). And 
while on it strideAndFrameSize() could be updated to use 
`info.stride(size.width, 0, 256)` instead of 
`libcamera::utils::alignUp(size.width * config.bpp / 8, 256)` as well. 
Will do that in v2.

Patch
diff mbox series 

diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
index 86cb8f8de..7440937c2 100644
--- a/include/libcamera/internal/software_isp/software_isp.h
+++ b/include/libcamera/internal/software_isp/software_isp.h
@@ -61,6 +61,7 @@  public:
 
 	std::tuple<unsigned int, unsigned int>
 	strideAndFrameSize(const PixelFormat &outputFormat, const Size &size);
+	uint32_t getPreferredInputStride(const PixelFormat &inputFormat, const Size &size);
 
 	int configure(const StreamConfiguration &inputCfg,
 		      const std::vector<std::reference_wrapper<const StreamConfiguration>> &outputCfgs,
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index c6fe12d65..0de2fe695 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -1542,6 +1542,11 @@  int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
 	captureFormat.fourcc = videoFormat;
 	captureFormat.size = pipeConfig->captureSize;
 
+	uint32_t requested_bpl = 0;
+	if (data->swIsp_)
+		requested_bpl = data->swIsp_->getPreferredInputStride(videoFormat.toPixelFormat(), pipeConfig->captureSize);
+	captureFormat.planes[0].bpl = requested_bpl;
+
 	ret = video->setFormat(&captureFormat);
 	if (ret)
 		return ret;
@@ -1561,6 +1566,13 @@  int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
 		return -EINVAL;
 	}
 
+	if (requested_bpl && captureFormat.planes[0].bpl != requested_bpl) {
+		LOG(SimplePipeline, Info)
+			<< "Input buffer stride ignored by the driver. "
+			<< "Requested " << requested_bpl
+			<< ", got " << captureFormat.planes[0].bpl;
+	}
+
 	/* Configure the converter if needed. */
 	std::vector<std::reference_wrapper<const StreamConfiguration>> outputCfgs;
 	data->useConversion_ = config->needConversion();
diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
index 2d7abfb83..858c9b97c 100644
--- a/src/libcamera/software_isp/debayer.cpp
+++ b/src/libcamera/software_isp/debayer.cpp
@@ -103,6 +103,14 @@  Debayer::~Debayer()
  * there is no valid output config
  */
 
+/**
+ * \fn uint32_t Debayer::getPreferredInputStride(const PixelFormat &inputFormat, const Size &size)
+ * Get the preferred input stride in bytes for the given input format and size
+ * \param[in] inputFormat The input format
+ * \param[in] size The input size (width and height in pixels)
+ * \return The preferred input stride in bytes
+ */
+
 /**
  * \fn void Debayer::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params)
  * \brief Process the bayer data into the requested format
diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h
index a2a17ec18..136374b8d 100644
--- a/src/libcamera/software_isp/debayer.h
+++ b/src/libcamera/software_isp/debayer.h
@@ -46,6 +46,7 @@  public:
 
 	virtual std::tuple<unsigned int, unsigned int>
 	strideAndFrameSize(const PixelFormat &outputFormat, const Size &size) = 0;
+	virtual uint32_t getPreferredInputStride(const PixelFormat &inputFormat, const Size &size) { return 0; }
 
 	virtual void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, const DebayerParams &params) = 0;
 	virtual int start() { return 0; }
diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
index 7b9e02d90..b7ddec17e 100644
--- a/src/libcamera/software_isp/debayer_egl.cpp
+++ b/src/libcamera/software_isp/debayer_egl.cpp
@@ -20,6 +20,7 @@ 
 
 #include <libcamera/formats.h>
 
+#include "libcamera/internal/formats.h"
 #include "libcamera/internal/framebuffer.h"
 
 #include "../glsl_shaders.h"
@@ -377,6 +378,12 @@  DebayerEGL::strideAndFrameSize(const PixelFormat &outputFormat, const Size &size
 	return std::make_tuple(stride, stride * size.height);
 }
 
+uint32_t DebayerEGL::getPreferredInputStride(const PixelFormat &inputFormat, const Size &size)
+{
+	const PixelFormatInfo &info = PixelFormatInfo::info(inputFormat);
+	return info.stride(size.width, 0, 256);
+}
+
 void DebayerEGL::setShaderVariableValues(const DebayerParams &params)
 {
 	/*
diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h
index 141fb288f..1b0153853 100644
--- a/src/libcamera/software_isp/debayer_egl.h
+++ b/src/libcamera/software_isp/debayer_egl.h
@@ -50,6 +50,7 @@  public:
 
 	std::vector<PixelFormat> formats(PixelFormat input) override;
 	std::tuple<unsigned int, unsigned int> strideAndFrameSize(const PixelFormat &outputFormat, const Size &size) override;
+	uint32_t getPreferredInputStride(const PixelFormat &inputFormat, const Size &size) override;
 
 	void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, const DebayerParams &params) override;
 	int start() override;
diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
index 781cf02f8..f6c8f498e 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -255,6 +255,19 @@  SoftwareIsp::strideAndFrameSize(const PixelFormat &outputFormat, const Size &siz
 	return debayer_->strideAndFrameSize(outputFormat, size);
 }
 
+/**
+ * Get the preferred input stride in bytes for the given input format and size
+ * \param[in] inputFormat The input format
+ * \param[in] size The input size (width and height in pixels)
+ * \return The preferred input stride in bytes
+ */
+uint32_t SoftwareIsp::getPreferredInputStride(const PixelFormat &inputFormat, const Size &size)
+{
+	ASSERT(debayer_);
+
+	return debayer_->getPreferredInputStride(inputFormat, size);
+}
+
 /**
  * \brief Configure the SoftwareIsp object according to the passed in parameters
  * \param[in] inputCfg The input configuration