[v4] libcamera: software_isp: Probe EGL availability before creating DebayerEGL
diff mbox series

Message ID 20260616093604.201689-1-qi.hou@nxp.com
State New
Headers show
Series
  • [v4] libcamera: software_isp: Probe EGL availability before creating DebayerEGL
Related show

Commit Message

Qi Hou June 16, 2026, 9:36 a.m. UTC
When HAVE_DEBAYER_EGL is enabled, the SoftwareIsp constructor
unconditionally creates a DebayerEGL instance. If the platform lacks
EGL surfaceless support, DebayerEGL::start() fails with -ENODEV after
SwStatsCpu ownership has already been moved, making fallback to
DebayerCpu impossible.

Add a static eGL::isAvailable() method that probes whether EGL
surfaceless rendering can be initialised. Extract the common display
obtaining logic (eglBindAPI, eglGetPlatformDisplay, eglInitialize)
into a private eGL::probeDisplay() helper that is shared by both
isAvailable() and initEGLContext(), avoiding code duplication.

The SoftwareIsp constructor now calls eGL::isAvailable() before
creating DebayerEGL. If EGL is not available, the existing fallback
path creates a DebayerCpu instance instead. Also add an item to the
TODO list about adding GBM surface as a progressive fallback.

Signed-off-by: Qi Hou <qi.hou@nxp.com>
---
 include/libcamera/internal/egl.h            |  2 +
 src/libcamera/egl.cpp                       | 60 ++++++++++++++++-----
 src/libcamera/software_isp/TODO.md          |  8 +++
 src/libcamera/software_isp/software_isp.cpp | 10 +++-
 4 files changed, 64 insertions(+), 16 deletions(-)

Comments

Milan Zamazal June 16, 2026, 9:57 a.m. UTC | #1
Qi Hou <qi.hou@nxp.com> writes:

> When HAVE_DEBAYER_EGL is enabled, the SoftwareIsp constructor
> unconditionally creates a DebayerEGL instance. If the platform lacks
> EGL surfaceless support, DebayerEGL::start() fails with -ENODEV after
> SwStatsCpu ownership has already been moved, making fallback to
> DebayerCpu impossible.
>
> Add a static eGL::isAvailable() method that probes whether EGL
> surfaceless rendering can be initialised. Extract the common display
> obtaining logic (eglBindAPI, eglGetPlatformDisplay, eglInitialize)
> into a private eGL::probeDisplay() helper that is shared by both
> isAvailable() and initEGLContext(), avoiding code duplication.
>
> The SoftwareIsp constructor now calls eGL::isAvailable() before
> creating DebayerEGL. If EGL is not available, the existing fallback
> path creates a DebayerCpu instance instead. Also add an item to the
> TODO list about adding GBM surface as a progressive fallback.
>
> Signed-off-by: Qi Hou <qi.hou@nxp.com>
> ---
>  include/libcamera/internal/egl.h            |  2 +
>  src/libcamera/egl.cpp                       | 60 ++++++++++++++++-----
>  src/libcamera/software_isp/TODO.md          |  8 +++
>  src/libcamera/software_isp/software_isp.cpp | 10 +++-
>  4 files changed, 64 insertions(+), 16 deletions(-)
>
> diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h
> index 57f90d93..f7bfb28d 100644
> --- a/include/libcamera/internal/egl.h
> +++ b/include/libcamera/internal/egl.h
> @@ -103,6 +103,7 @@ public:
>  	~eGL();
>  
>  	int initEGLContext();
> +	static bool isAvailable();
>  
>  	int createInputDMABufTexture2D(eGLImage &eglImage, int fd);
>  	int createOutputDMABufTexture2D(eGLImage &eglImage, int fd);
> @@ -133,6 +134,7 @@ private:
>  	EGLContext context_ = EGL_NO_CONTEXT;
>  	EGLSurface surface_ = EGL_NO_SURFACE;
>  
> +	static EGLDisplay probeDisplay();
>  	int compileShader(int shaderType, GLuint &shaderId, const unsigned char *shaderData,
>  			  unsigned int shaderDataLen,
>  			  Span<const std::string> shaderEnv);
> diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp
> index c185bb7a..860f18dd 100644
> --- a/src/libcamera/egl.cpp
> +++ b/src/libcamera/egl.cpp
> @@ -267,6 +267,50 @@ void eGL::createTexture2D(eGLImage &eglImage, void *data)
>  	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
>  }
>  
> +EGLDisplay eGL::probeDisplay()
> +{
> +	EGLDisplay display;
> +
> +	if (!eglBindAPI(EGL_OPENGL_ES_API)) {
> +		LOG(eGL, Info) << "API bind fail";
> +		return EGL_NO_DISPLAY;
> +	}
> +
> +	display = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA,
> +					 EGL_DEFAULT_DISPLAY,
> +					 nullptr);

The two lines above are misaligned, they should be shifted one character
left.

With this fixed:

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>

> +
> +	if (display == EGL_NO_DISPLAY) {
> +		LOG(eGL, Info) << "Unable to get EGL display";
> +		return EGL_NO_DISPLAY;
> +	}
> +
> +	if (eglInitialize(display, nullptr, nullptr) != EGL_TRUE) {
> +		LOG(eGL, Error) << "eglInitialize fail";
> +		return EGL_NO_DISPLAY;
> +	}
> +
> +	return display;
> +}
> +
> +/**
> + * \brief Probe whether EGL surfaceless rendering is available
> + *
> + * Checks if an EGL surfaceless display can be obtained and initialised.
> + * The display is immediately terminated so that no resources are leaked.
> + *
> + * \return true if EGL surfaceless rendering is available, false otherwise
> + */
> +bool eGL::isAvailable()
> +{
> +	EGLDisplay display = probeDisplay();
> +	if (display == EGL_NO_DISPLAY)
> +		return false;
> +
> +	eglTerminate(display);
> +	return true;
> +}
> +
>  /**
>   * \brief Initialise the EGL context
>   *
> @@ -297,21 +341,9 @@ int eGL::initEGLContext()
>  	EGLint numConfigs;
>  	EGLConfig config;
>  
> -	if (!eglBindAPI(EGL_OPENGL_ES_API)) {
> -		LOG(eGL, Error) << "API bind fail";
> -		goto fail;
> -	}
> -
> -	display_ = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA,
> -					 EGL_DEFAULT_DISPLAY,
> -					 nullptr);
> +	display_ = probeDisplay();
>  	if (display_ == EGL_NO_DISPLAY) {
> -		LOG(eGL, Error) << "Unable to get EGL display";
> -		goto fail;
> -	}
> -
> -	if (eglInitialize(display_, nullptr, nullptr) != EGL_TRUE) {
> -		LOG(eGL, Error) << "eglInitialize fail";
> +		LOG(eGL, Error) << "Unable to probe display";
>  		goto fail;
>  	}
>  
> diff --git a/src/libcamera/software_isp/TODO.md b/src/libcamera/software_isp/TODO.md
> index 0f27da2d..b6805908 100644
> --- a/src/libcamera/software_isp/TODO.md
> +++ b/src/libcamera/software_isp/TODO.md
> @@ -189,6 +189,14 @@ by correctness, as that's not very maintainable.
>  
>  The TODO items are listed in perceived order of ease.
>  
> +### Add GBM surface as a progressive fallback
> +
> +Some platforms (e.g. i.MX8MM) support EGL with GBM surfaces but lack
> +`EGL_PLATFORM_SURFACELESS_MESA` support. The GPU ISP originally used a GBM
> +surface before transitioning to MESA surfaceless. Adding GBM surface back as a
> +fallback path would allow GPU-accelerated debayering on these platforms without
> +requiring surfaceless support.
> +
>  ### Denoising
>  
>  WIP
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index 781cf02f..ff8c3465 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -119,8 +119,14 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
>  		}
>  	}
>  
> -	if (!softISPMode || softISPMode == "gpu")
> -		debayer_ = std::make_unique<DebayerEGL>(std::move(stats), cm);
> +	if (!softISPMode || softISPMode == "gpu") {
> +		if (eGL::isAvailable()) {
> +			debayer_ = std::make_unique<DebayerEGL>(std::move(stats), cm);
> +		} else {
> +			LOG(SoftwareIsp, Info)
> +				<< "EGL not available, falling back to CPU debayer";
> +		}
> +	}
>  
>  #endif
>  	if (!debayer_)

Patch
diff mbox series

diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h
index 57f90d93..f7bfb28d 100644
--- a/include/libcamera/internal/egl.h
+++ b/include/libcamera/internal/egl.h
@@ -103,6 +103,7 @@  public:
 	~eGL();
 
 	int initEGLContext();
+	static bool isAvailable();
 
 	int createInputDMABufTexture2D(eGLImage &eglImage, int fd);
 	int createOutputDMABufTexture2D(eGLImage &eglImage, int fd);
@@ -133,6 +134,7 @@  private:
 	EGLContext context_ = EGL_NO_CONTEXT;
 	EGLSurface surface_ = EGL_NO_SURFACE;
 
+	static EGLDisplay probeDisplay();
 	int compileShader(int shaderType, GLuint &shaderId, const unsigned char *shaderData,
 			  unsigned int shaderDataLen,
 			  Span<const std::string> shaderEnv);
diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp
index c185bb7a..860f18dd 100644
--- a/src/libcamera/egl.cpp
+++ b/src/libcamera/egl.cpp
@@ -267,6 +267,50 @@  void eGL::createTexture2D(eGLImage &eglImage, void *data)
 	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
 }
 
+EGLDisplay eGL::probeDisplay()
+{
+	EGLDisplay display;
+
+	if (!eglBindAPI(EGL_OPENGL_ES_API)) {
+		LOG(eGL, Info) << "API bind fail";
+		return EGL_NO_DISPLAY;
+	}
+
+	display = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA,
+					 EGL_DEFAULT_DISPLAY,
+					 nullptr);
+
+	if (display == EGL_NO_DISPLAY) {
+		LOG(eGL, Info) << "Unable to get EGL display";
+		return EGL_NO_DISPLAY;
+	}
+
+	if (eglInitialize(display, nullptr, nullptr) != EGL_TRUE) {
+		LOG(eGL, Error) << "eglInitialize fail";
+		return EGL_NO_DISPLAY;
+	}
+
+	return display;
+}
+
+/**
+ * \brief Probe whether EGL surfaceless rendering is available
+ *
+ * Checks if an EGL surfaceless display can be obtained and initialised.
+ * The display is immediately terminated so that no resources are leaked.
+ *
+ * \return true if EGL surfaceless rendering is available, false otherwise
+ */
+bool eGL::isAvailable()
+{
+	EGLDisplay display = probeDisplay();
+	if (display == EGL_NO_DISPLAY)
+		return false;
+
+	eglTerminate(display);
+	return true;
+}
+
 /**
  * \brief Initialise the EGL context
  *
@@ -297,21 +341,9 @@  int eGL::initEGLContext()
 	EGLint numConfigs;
 	EGLConfig config;
 
-	if (!eglBindAPI(EGL_OPENGL_ES_API)) {
-		LOG(eGL, Error) << "API bind fail";
-		goto fail;
-	}
-
-	display_ = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA,
-					 EGL_DEFAULT_DISPLAY,
-					 nullptr);
+	display_ = probeDisplay();
 	if (display_ == EGL_NO_DISPLAY) {
-		LOG(eGL, Error) << "Unable to get EGL display";
-		goto fail;
-	}
-
-	if (eglInitialize(display_, nullptr, nullptr) != EGL_TRUE) {
-		LOG(eGL, Error) << "eglInitialize fail";
+		LOG(eGL, Error) << "Unable to probe display";
 		goto fail;
 	}
 
diff --git a/src/libcamera/software_isp/TODO.md b/src/libcamera/software_isp/TODO.md
index 0f27da2d..b6805908 100644
--- a/src/libcamera/software_isp/TODO.md
+++ b/src/libcamera/software_isp/TODO.md
@@ -189,6 +189,14 @@  by correctness, as that's not very maintainable.
 
 The TODO items are listed in perceived order of ease.
 
+### Add GBM surface as a progressive fallback
+
+Some platforms (e.g. i.MX8MM) support EGL with GBM surfaces but lack
+`EGL_PLATFORM_SURFACELESS_MESA` support. The GPU ISP originally used a GBM
+surface before transitioning to MESA surfaceless. Adding GBM surface back as a
+fallback path would allow GPU-accelerated debayering on these platforms without
+requiring surfaceless support.
+
 ### Denoising
 
 WIP
diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
index 781cf02f..ff8c3465 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -119,8 +119,14 @@  SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
 		}
 	}
 
-	if (!softISPMode || softISPMode == "gpu")
-		debayer_ = std::make_unique<DebayerEGL>(std::move(stats), cm);
+	if (!softISPMode || softISPMode == "gpu") {
+		if (eGL::isAvailable()) {
+			debayer_ = std::make_unique<DebayerEGL>(std::move(stats), cm);
+		} else {
+			LOG(SoftwareIsp, Info)
+				<< "EGL not available, falling back to CPU debayer";
+		}
+	}
 
 #endif
 	if (!debayer_)