| Message ID | 20260617033748.312358-1-qi.hou@nxp.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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> Reviewed-by: Milan Zamazal <mzamazal@redhat.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 1479df8b..f03abb8a 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 19f83e61..1b316c54 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_)
Quoting Qi Hou (2026-06-17 04:37:48) > 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> Please collect tags that are already given to you. Thanks for fixing the indentation, and I see Milan has already re-posted his tag again anyway. I'll merge this now. Thanks. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.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 1479df8b..f03abb8a 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 19f83e61..1b316c54 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_) > -- > 2.34.1 >
On Wed, Jun 17, 2026 at 12:37:48PM +0900, Qi Hou wrote: > 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 1479df8b..f03abb8a 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); Could we please avoid doing a full initialization and teardown to check availability, to then reinitialize when instantiating the eGL class ? > + 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 19f83e61..1b316c54 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_)
On Wed, Jun 17, 2026 at 12:44:59PM +0300, Laurent Pinchart wrote: > On Wed, Jun 17, 2026 at 12:37:48PM +0900, Qi Hou wrote: > > 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 1479df8b..f03abb8a 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); > > Could we please avoid doing a full initialization and teardown to check > availability, to then reinitialize when instantiating the eGL class ? I see this patch just got merged. Please send patches on top to fix this. > > + 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 19f83e61..1b316c54 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_)
Hi Laurent Pinchart, Sorry for the late reply, the patch to fix this issue has been sent out with title " libcamera: egl: Cache probed EGLDisplay to avoid redundant init/teardown ". Please help review. Thanks. Regards, Qi Hou -----Original Message----- From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Sent: Wednesday, June 17, 2026 5:46 PM To: Qi Hou <qi.hou@nxp.com> Cc: libcamera-devel@lists.libcamera.org; Jared Hu <jared.hu@nxp.com>; Julien Vuillaumier <julien.vuillaumier@nxp.com> Subject: [EXT] Re: [PATCH v5] libcamera: software_isp: Probe EGL availability before creating DebayerEGL Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button On Wed, Jun 17, 2026 at 12:44:59PM +0300, Laurent Pinchart wrote: > On Wed, Jun 17, 2026 at 12:37:48PM +0900, Qi Hou wrote: > > 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 > > 1479df8b..f03abb8a 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); > > Could we please avoid doing a full initialization and teardown to > check availability, to then reinitialize when instantiating the eGL class ? I see this patch just got merged. Please send patches on top to fix this. > > + 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 19f83e61..1b316c54 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_) -- Regards, Laurent Pinchart
Hello, On Wed, Jul 22, 2026 at 08:37:47AM +0000, Qi Hou wrote: > Hi Laurent Pinchart, > > Sorry for the late reply, the patch to fix this issue has been sent > out with title " libcamera: egl: Cache probed EGLDisplay to avoid > redundant init/teardown ". I don't see that patch in the mailing list archives or in patchwork. > Please help review. Thanks. > > Regards, > Qi Hou > > -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Sent: Wednesday, June 17, 2026 5:46 PM > To: Qi Hou <qi.hou@nxp.com> > Cc: libcamera-devel@lists.libcamera.org; Jared Hu <jared.hu@nxp.com>; Julien Vuillaumier <julien.vuillaumier@nxp.com> > Subject: [EXT] Re: [PATCH v5] libcamera: software_isp: Probe EGL availability before creating DebayerEGL > > On Wed, Jun 17, 2026 at 12:44:59PM +0300, Laurent Pinchart wrote: > > On Wed, Jun 17, 2026 at 12:37:48PM +0900, Qi Hou wrote: > > > 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 > > > 1479df8b..f03abb8a 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); > > > > Could we please avoid doing a full initialization and teardown to > > check availability, to then reinitialize when instantiating the eGL class ? > > I see this patch just got merged. Please send patches on top to fix this. > > > > + 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 19f83e61..1b316c54 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_)
Hi Laurent Pinchart， Thanks for your reminder. It's strange that the email can only be sent internally, not externally today. I'll check and resend the patch. Regards, Qi Hou -----Original Message----- From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Sent: Wednesday, July 22, 2026 5:35 PM To: Qi Hou <qi.hou@nxp.com> Cc: libcamera-devel@lists.libcamera.org; Jared Hu <jared.hu@nxp.com>; Julien Vuillaumier <julien.vuillaumier@nxp.com> Subject: Re: [EXT] Re: [PATCH v5] libcamera: software_isp: Probe EGL availability before creating DebayerEGL Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button Hello, On Wed, Jul 22, 2026 at 08:37:47AM +0000, Qi Hou wrote: > Hi Laurent Pinchart, > > Sorry for the late reply, the patch to fix this issue has been sent > out with title " libcamera: egl: Cache probed EGLDisplay to avoid > redundant init/teardown ". I don't see that patch in the mailing list archives or in patchwork. > Please help review. Thanks. > > Regards, > Qi Hou > > -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Sent: Wednesday, June 17, 2026 5:46 PM > To: Qi Hou <qi.hou@nxp.com> > Cc: libcamera-devel@lists.libcamera.org; Jared Hu <jared.hu@nxp.com>; > Julien Vuillaumier <julien.vuillaumier@nxp.com> > Subject: [EXT] Re: [PATCH v5] libcamera: software_isp: Probe EGL > availability before creating DebayerEGL > > On Wed, Jun 17, 2026 at 12:44:59PM +0300, Laurent Pinchart wrote: > > On Wed, Jun 17, 2026 at 12:37:48PM +0900, Qi Hou wrote: > > > 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 > > > 1479df8b..f03abb8a 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); > > > > Could we please avoid doing a full initialization and teardown to > > check availability, to then reinitialize when instantiating the eGL class ? > > I see this patch just got merged. Please send patches on top to fix this. > > > > + 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 19f83e61..1b316c54 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_) -- Regards, Laurent Pinchart
Hi Laurent Pinchart, It seems the reason my email couldn't be sent a few days ago was that I hadn't subscribed to the mailing list. Now patch titled " libcamera: egl: Cache probed EGLDisplay to avoid redundant init/teardown " has been sent out. I see it in patchwork. Please help review. Thanks. Regards, Qi Hou -----Original Message----- From: Qi Hou Sent: Wednesday, July 22, 2026 6:22 PM To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: libcamera-devel@lists.libcamera.org; Jared Hu <jared.hu@nxp.com>; Julien Vuillaumier <julien.vuillaumier@nxp.com> Subject: RE: [EXT] Re: [PATCH v5] libcamera: software_isp: Probe EGL availability before creating DebayerEGL Hi Laurent Pinchart， Thanks for your reminder. It's strange that the email can only be sent internally, not externally today. I'll check and resend the patch. Regards, Qi Hou -----Original Message----- From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Sent: Wednesday, July 22, 2026 5:35 PM To: Qi Hou <qi.hou@nxp.com> Cc: libcamera-devel@lists.libcamera.org; Jared Hu <jared.hu@nxp.com>; Julien Vuillaumier <julien.vuillaumier@nxp.com> Subject: Re: [EXT] Re: [PATCH v5] libcamera: software_isp: Probe EGL availability before creating DebayerEGL Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button Hello, On Wed, Jul 22, 2026 at 08:37:47AM +0000, Qi Hou wrote: > Hi Laurent Pinchart, > > Sorry for the late reply, the patch to fix this issue has been sent > out with title " libcamera: egl: Cache probed EGLDisplay to avoid > redundant init/teardown ". I don't see that patch in the mailing list archives or in patchwork. > Please help review. Thanks. > > Regards, > Qi Hou > > -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Sent: Wednesday, June 17, 2026 5:46 PM > To: Qi Hou <qi.hou@nxp.com> > Cc: libcamera-devel@lists.libcamera.org; Jared Hu <jared.hu@nxp.com>; > Julien Vuillaumier <julien.vuillaumier@nxp.com> > Subject: [EXT] Re: [PATCH v5] libcamera: software_isp: Probe EGL > availability before creating DebayerEGL > > On Wed, Jun 17, 2026 at 12:44:59PM +0300, Laurent Pinchart wrote: > > On Wed, Jun 17, 2026 at 12:37:48PM +0900, Qi Hou wrote: > > > 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 > > > 1479df8b..f03abb8a 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); > > > > Could we please avoid doing a full initialization and teardown to > > check availability, to then reinitialize when instantiating the eGL class ? > > I see this patch just got merged. Please send patches on top to fix this. > > > > + 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 19f83e61..1b316c54 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_) -- Regards, Laurent Pinchart
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 1479df8b..f03abb8a 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 19f83e61..1b316c54 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_)
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(-)