| Message ID | 20260110221337.145378-1-robert.mader@collabora.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
P.S.: whops, forgot to add: the proprietary Nvidia driver also supports GBM - but apparently implements the surfaceless platform as well. On 10.01.26 23:13, Robert Mader wrote: > Which appears to be a better fit for the use-case at hand: > 1. Like GBM it is Mesa specific, so no change in supported setups is > expected. If ever required, a fallback to the generic device platform > could be added on top. > 2. It leaves the complexity of selecting a renderer device to the > driver, reducing code and dependencies. > 3. It allows to use llvmpipe / software drivers without dri device, > which can be useful on CI or debugging (with LIBGL_ALWAYS_SOFTWARE=1). > > Signed-off-by: Robert Mader <robert.mader@collabora.com> > > --- > > Should be applied on to of > https://patchwork.libcamera.org/cover/25706/ > --- > include/libcamera/internal/egl.h | 4 +--- > src/libcamera/egl.cpp | 12 +++++------- > src/libcamera/software_isp/debayer_egl.cpp | 7 ++----- > src/libcamera/software_isp/debayer_egl.h | 1 - > 4 files changed, 8 insertions(+), 16 deletions(-) > > diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h > index f007f448a..630345ea7 100644 > --- a/include/libcamera/internal/egl.h > +++ b/include/libcamera/internal/egl.h > @@ -16,8 +16,6 @@ > #include <libcamera/base/span.h> > #include <libcamera/base/utils.h> > > -#include "libcamera/internal/gbm.h" > - > #define EGL_EGLEXT_PROTOTYPES > #include <EGL/egl.h> > #include <EGL/eglext.h> > @@ -96,7 +94,7 @@ public: > eGL(); > ~eGL(); > > - int initEGLContext(GBM *gbmContext); > + int initEGLContext(); > > int createInputDMABufTexture2D(eGLImage &eglImage, int fd); > int createOutputDMABufTexture2D(eGLImage &eglImage, int fd); > diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp > index 0ffd008c7..19c5ff48f 100644 > --- a/src/libcamera/egl.cpp > +++ b/src/libcamera/egl.cpp > @@ -256,24 +256,22 @@ void eGL::createTexture2D(eGLImage &eglImage, GLint format, uint32_t width, uint > > /** > * \brief Initialise the EGL context > - * \param[in] gbmContext Pointer to initialised GBM context > * > - * Sets up the EGL display from the GBM device, creates an OpenGL ES 2.0 > - * context, and retrieves function pointers for required extensions > - * including: > + * Sets up the EGL display, creates an OpenGL ES 2.0 context, and retrieves > + * function pointers for required extensions including: > * - eglCreateImageKHR / eglDestroyImageKHR > * - glEGLImageTargetTexture2DOES > * > * \return 0 on success, or -ENODEV on failure > */ > -int eGL::initEGLContext(GBM *gbmContext) > +int eGL::initEGLContext() > { > EGLint configAttribs[] = { > EGL_RED_SIZE, 8, > EGL_GREEN_SIZE, 8, > EGL_BLUE_SIZE, 8, > EGL_ALPHA_SIZE, 8, > - EGL_SURFACE_TYPE, EGL_WINDOW_BIT, > + EGL_SURFACE_TYPE, EGL_PBUFFER_BIT, > EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT, > EGL_NONE > }; > @@ -297,7 +295,7 @@ int eGL::initEGLContext(GBM *gbmContext) > goto fail; > } > > - display_ = eglGetDisplay(gbmContext->device()); > + display_ = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA, nullptr, nullptr); > if (display_ == EGL_NO_DISPLAY) { > LOG(eGL, Error) << "Unable to get EGL display"; > goto fail; > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > index 9693d7252..c85c7d6cd 100644 > --- a/src/libcamera/software_isp/debayer_egl.cpp > +++ b/src/libcamera/software_isp/debayer_egl.cpp > @@ -142,7 +142,7 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm > > /* > * Tell shaders how to re-order output taking account of how the > - * pixels are actually stored by GBM > + * pixels are actually stored by EGL > */ > switch (outputFormat) { > case formats::ARGB8888: > @@ -586,10 +586,7 @@ int DebayerEGL::start() > { > GLint maxTextureImageUnits; > > - if (gbmSurface_.createDevice()) > - return -ENODEV; > - > - if (egl_.initEGLContext(&gbmSurface_)) > + if (egl_.initEGLContext()) > return -ENODEV; > > glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &maxTextureImageUnits); > diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h > index a5033bc63..cfbaf8e9d 100644 > --- a/src/libcamera/software_isp/debayer_egl.h > +++ b/src/libcamera/software_isp/debayer_egl.h > @@ -113,7 +113,6 @@ private: > Rectangle window_; > std::unique_ptr<SwStatsCpu> stats_; > eGL egl_; > - GBM gbmSurface_; > uint32_t width_; > uint32_t height_; > GLint glFormat_;
On 10/01/2026 22:13, Robert Mader wrote: > Which appears to be a better fit for the use-case at hand: > 1. Like GBM it is Mesa specific, so no change in supported setups is > expected. If ever required, a fallback to the generic device platform > could be added on top. > 2. It leaves the complexity of selecting a renderer device to the > driver, reducing code and dependencies. > 3. It allows to use llvmpipe / software drivers without dri device, > which can be useful on CI or debugging (with LIBGL_ALWAYS_SOFTWARE=1). > > Signed-off-by: Robert Mader <robert.mader@collabora.com> > > --- > > Should be applied on to of > https://patchwork.libcamera.org/cover/25706/ > --- > include/libcamera/internal/egl.h | 4 +--- > src/libcamera/egl.cpp | 12 +++++------- > src/libcamera/software_isp/debayer_egl.cpp | 7 ++----- > src/libcamera/software_isp/debayer_egl.h | 1 - > 4 files changed, 8 insertions(+), 16 deletions(-) > > diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h > index f007f448a..630345ea7 100644 > --- a/include/libcamera/internal/egl.h > +++ b/include/libcamera/internal/egl.h > @@ -16,8 +16,6 @@ > #include <libcamera/base/span.h> > #include <libcamera/base/utils.h> > > -#include "libcamera/internal/gbm.h" > - > #define EGL_EGLEXT_PROTOTYPES > #include <EGL/egl.h> > #include <EGL/eglext.h> > @@ -96,7 +94,7 @@ public: > eGL(); > ~eGL(); > > - int initEGLContext(GBM *gbmContext); > + int initEGLContext(); > > int createInputDMABufTexture2D(eGLImage &eglImage, int fd); > int createOutputDMABufTexture2D(eGLImage &eglImage, int fd); > diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp > index 0ffd008c7..19c5ff48f 100644 > --- a/src/libcamera/egl.cpp > +++ b/src/libcamera/egl.cpp > @@ -256,24 +256,22 @@ void eGL::createTexture2D(eGLImage &eglImage, GLint format, uint32_t width, uint > > /** > * \brief Initialise the EGL context > - * \param[in] gbmContext Pointer to initialised GBM context > * > - * Sets up the EGL display from the GBM device, creates an OpenGL ES 2.0 > - * context, and retrieves function pointers for required extensions > - * including: > + * Sets up the EGL display, creates an OpenGL ES 2.0 context, and retrieves > + * function pointers for required extensions including: > * - eglCreateImageKHR / eglDestroyImageKHR > * - glEGLImageTargetTexture2DOES > * > * \return 0 on success, or -ENODEV on failure > */ > -int eGL::initEGLContext(GBM *gbmContext) > +int eGL::initEGLContext() > { > EGLint configAttribs[] = { > EGL_RED_SIZE, 8, > EGL_GREEN_SIZE, 8, > EGL_BLUE_SIZE, 8, > EGL_ALPHA_SIZE, 8, > - EGL_SURFACE_TYPE, EGL_WINDOW_BIT, > + EGL_SURFACE_TYPE, EGL_PBUFFER_BIT, > EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT, > EGL_NONE > }; > @@ -297,7 +295,7 @@ int eGL::initEGLContext(GBM *gbmContext) > goto fail; > } > > - display_ = eglGetDisplay(gbmContext->device()); > + display_ = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA, nullptr, nullptr); > if (display_ == EGL_NO_DISPLAY) { > LOG(eGL, Error) << "Unable to get EGL display"; > goto fail; > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > index 9693d7252..c85c7d6cd 100644 > --- a/src/libcamera/software_isp/debayer_egl.cpp > +++ b/src/libcamera/software_isp/debayer_egl.cpp > @@ -142,7 +142,7 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm > > /* > * Tell shaders how to re-order output taking account of how the > - * pixels are actually stored by GBM > + * pixels are actually stored by EGL > */ > switch (outputFormat) { > case formats::ARGB8888: > @@ -586,10 +586,7 @@ int DebayerEGL::start() > { > GLint maxTextureImageUnits; > > - if (gbmSurface_.createDevice()) > - return -ENODEV; > - > - if (egl_.initEGLContext(&gbmSurface_)) > + if (egl_.initEGLContext()) > return -ENODEV; > > glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &maxTextureImageUnits); > diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h > index a5033bc63..cfbaf8e9d 100644 > --- a/src/libcamera/software_isp/debayer_egl.h > +++ b/src/libcamera/software_isp/debayer_egl.h > @@ -113,7 +113,6 @@ private: > Rectangle window_; > std::unique_ptr<SwStatsCpu> stats_; > eGL egl_; > - GBM gbmSurface_; > uint32_t width_; > uint32_t height_; > GLint glFormat_; > -- > 2.52.0 > Are we 100% sure the pixel format will be _stored_ by all GPUs as RGBA8888 ? What lead me to use GBM surface was specifying that directly. --- bod
On 12.01.26 13:45, Bryan O'Donoghue wrote: > Are we 100% sure the pixel format will be _stored_ by all GPUs as > RGBA8888 ? > > What lead me to use GBM surface was specifying that directly. As far as I know this is controlled by the EGL_LINUX_DRM_FOURCC_EXT attribute when creating the image with eglCreateImageKHR(). As we create the target texture from that, it would be very surprising to me if a driver would suddenly use a different format. Best regards, Robert > > --- > bod
On 12.01.26 13:51, Robert Mader wrote: > > On 12.01.26 13:45, Bryan O'Donoghue wrote: >> Are we 100% sure the pixel format will be _stored_ by all GPUs as >> RGBA8888 ? >> >> What lead me to use GBM surface was specifying that directly. > > As far as I know this is controlled by the EGL_LINUX_DRM_FOURCC_EXT > attribute when creating the image with eglCreateImageKHR(). As we > create the target texture from that, it would be very surprising to me > if a driver would suddenly use a different format. P.S.: IIRC you mentioned that you ran into cases where the output ended up using tiling/modifiers, however IIRC that was before before adding the EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT / EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT attributes which explicitly request linear. > > Best regards, > > Robert > >> >> --- >> bod >
On 12/01/2026 12:51, Robert Mader wrote: > As far as I know this is controlled by the EGL_LINUX_DRM_FOURCC_EXT > attribute when creating the image with eglCreateImageKHR(). As we create > the target texture from that, it would be very surprising to me if a > driver would suddenly use a different format. > > Best regards, > > Robert > >> >> --- >> bod > > -- > Robert Mader > Consultant Software Developer > > Collabora Ltd. > Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK > Registered in England & Wales, no. 5513718 OK great, good enough for me. I did a a bit of DD on this. We are specifying the FOURCC but also MODFIFIER_HI/LO = 0 which gives us untiled... Good stuff so. Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sm8250/rb5, x1e/Dell Insprion14p --- bod
On 12/01/2026 12:59, Robert Mader wrote: > P.S.: IIRC you mentioned that you ran into cases where the output ended > up using tiling/modifiers, however IIRC that was before before adding > the EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT / > EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT attributes which explicitly request > linear. I'll pretend I knew that all along :) --- bod
Robert Mader <robert.mader@collabora.com> writes: > Which appears to be a better fit for the use-case at hand: > 1. Like GBM it is Mesa specific, so no change in supported setups is > expected. If ever required, a fallback to the generic device platform > could be added on top. > 2. It leaves the complexity of selecting a renderer device to the > driver, reducing code and dependencies. > 3. It allows to use llvmpipe / software drivers without dri device, > which can be useful on CI or debugging (with LIBGL_ALWAYS_SOFTWARE=1). > > Signed-off-by: Robert Mader <robert.mader@collabora.com> Tested-by: Milan Zamazal <mzamazal@redhat.com> # TI AM69 (LIBGL_ALWAYS_SOFTWARE=1 fails but apparently I don't have llvmpipe drivers on the system.) > --- > > Should be applied on to of > https://patchwork.libcamera.org/cover/25706/ > --- > include/libcamera/internal/egl.h | 4 +--- > src/libcamera/egl.cpp | 12 +++++------- > src/libcamera/software_isp/debayer_egl.cpp | 7 ++----- > src/libcamera/software_isp/debayer_egl.h | 1 - > 4 files changed, 8 insertions(+), 16 deletions(-) > > diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h > index f007f448a..630345ea7 100644 > --- a/include/libcamera/internal/egl.h > +++ b/include/libcamera/internal/egl.h > @@ -16,8 +16,6 @@ > #include <libcamera/base/span.h> > #include <libcamera/base/utils.h> > > -#include "libcamera/internal/gbm.h" > - > #define EGL_EGLEXT_PROTOTYPES > #include <EGL/egl.h> > #include <EGL/eglext.h> > @@ -96,7 +94,7 @@ public: > eGL(); > ~eGL(); > > - int initEGLContext(GBM *gbmContext); > + int initEGLContext(); > > int createInputDMABufTexture2D(eGLImage &eglImage, int fd); > int createOutputDMABufTexture2D(eGLImage &eglImage, int fd); > diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp > index 0ffd008c7..19c5ff48f 100644 > --- a/src/libcamera/egl.cpp > +++ b/src/libcamera/egl.cpp > @@ -256,24 +256,22 @@ void eGL::createTexture2D(eGLImage &eglImage, GLint format, uint32_t width, uint > > /** > * \brief Initialise the EGL context > - * \param[in] gbmContext Pointer to initialised GBM context > * > - * Sets up the EGL display from the GBM device, creates an OpenGL ES 2.0 > - * context, and retrieves function pointers for required extensions > - * including: > + * Sets up the EGL display, creates an OpenGL ES 2.0 context, and retrieves > + * function pointers for required extensions including: > * - eglCreateImageKHR / eglDestroyImageKHR > * - glEGLImageTargetTexture2DOES > * > * \return 0 on success, or -ENODEV on failure > */ > -int eGL::initEGLContext(GBM *gbmContext) > +int eGL::initEGLContext() > { > EGLint configAttribs[] = { > EGL_RED_SIZE, 8, > EGL_GREEN_SIZE, 8, > EGL_BLUE_SIZE, 8, > EGL_ALPHA_SIZE, 8, > - EGL_SURFACE_TYPE, EGL_WINDOW_BIT, > + EGL_SURFACE_TYPE, EGL_PBUFFER_BIT, > EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT, > EGL_NONE > }; > @@ -297,7 +295,7 @@ int eGL::initEGLContext(GBM *gbmContext) > goto fail; > } > > - display_ = eglGetDisplay(gbmContext->device()); > + display_ = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA, nullptr, nullptr); > if (display_ == EGL_NO_DISPLAY) { > LOG(eGL, Error) << "Unable to get EGL display"; > goto fail; > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > index 9693d7252..c85c7d6cd 100644 > --- a/src/libcamera/software_isp/debayer_egl.cpp > +++ b/src/libcamera/software_isp/debayer_egl.cpp > @@ -142,7 +142,7 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm > > /* > * Tell shaders how to re-order output taking account of how the > - * pixels are actually stored by GBM > + * pixels are actually stored by EGL > */ > switch (outputFormat) { > case formats::ARGB8888: > @@ -586,10 +586,7 @@ int DebayerEGL::start() > { > GLint maxTextureImageUnits; > > - if (gbmSurface_.createDevice()) > - return -ENODEV; > - > - if (egl_.initEGLContext(&gbmSurface_)) > + if (egl_.initEGLContext()) > return -ENODEV; > > glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &maxTextureImageUnits); > diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h > index a5033bc63..cfbaf8e9d 100644 > --- a/src/libcamera/software_isp/debayer_egl.h > +++ b/src/libcamera/software_isp/debayer_egl.h > @@ -113,7 +113,6 @@ private: > Rectangle window_; > std::unique_ptr<SwStatsCpu> stats_; > eGL egl_; > - GBM gbmSurface_; > uint32_t width_; > uint32_t height_; > GLint glFormat_;
2026. 01. 10. 23:13 keltezéssel, Robert Mader írta: > Which appears to be a better fit for the use-case at hand: > 1. Like GBM it is Mesa specific, so no change in supported setups is > expected. If ever required, a fallback to the generic device platform > could be added on top. > 2. It leaves the complexity of selecting a renderer device to the > driver, reducing code and dependencies. > 3. It allows to use llvmpipe / software drivers without dri device, > which can be useful on CI or debugging (with LIBGL_ALWAYS_SOFTWARE=1). > > Signed-off-by: Robert Mader <robert.mader@collabora.com> > > --- > It seems to work fine, LIBGL_ALWAYS_SOFTWARE=1 also works. Tested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> # ThinkPad X1 Yoga Gen 7 + ov2740 > Should be applied on to of > https://patchwork.libcamera.org/cover/25706/ > --- > include/libcamera/internal/egl.h | 4 +--- > src/libcamera/egl.cpp | 12 +++++------- > src/libcamera/software_isp/debayer_egl.cpp | 7 ++----- > src/libcamera/software_isp/debayer_egl.h | 1 - > 4 files changed, 8 insertions(+), 16 deletions(-) > > diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h > index f007f448a..630345ea7 100644 > --- a/include/libcamera/internal/egl.h > +++ b/include/libcamera/internal/egl.h > @@ -16,8 +16,6 @@ > #include <libcamera/base/span.h> > #include <libcamera/base/utils.h> > > -#include "libcamera/internal/gbm.h" > - > #define EGL_EGLEXT_PROTOTYPES > #include <EGL/egl.h> > #include <EGL/eglext.h> > @@ -96,7 +94,7 @@ public: > eGL(); > ~eGL(); > > - int initEGLContext(GBM *gbmContext); > + int initEGLContext(); > > int createInputDMABufTexture2D(eGLImage &eglImage, int fd); > int createOutputDMABufTexture2D(eGLImage &eglImage, int fd); > diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp > index 0ffd008c7..19c5ff48f 100644 > --- a/src/libcamera/egl.cpp > +++ b/src/libcamera/egl.cpp > @@ -256,24 +256,22 @@ void eGL::createTexture2D(eGLImage &eglImage, GLint format, uint32_t width, uint > > /** > * \brief Initialise the EGL context > - * \param[in] gbmContext Pointer to initialised GBM context > * > - * Sets up the EGL display from the GBM device, creates an OpenGL ES 2.0 > - * context, and retrieves function pointers for required extensions > - * including: > + * Sets up the EGL display, creates an OpenGL ES 2.0 context, and retrieves > + * function pointers for required extensions including: > * - eglCreateImageKHR / eglDestroyImageKHR > * - glEGLImageTargetTexture2DOES > * > * \return 0 on success, or -ENODEV on failure > */ > -int eGL::initEGLContext(GBM *gbmContext) > +int eGL::initEGLContext() > { > EGLint configAttribs[] = { > EGL_RED_SIZE, 8, > EGL_GREEN_SIZE, 8, > EGL_BLUE_SIZE, 8, > EGL_ALPHA_SIZE, 8, > - EGL_SURFACE_TYPE, EGL_WINDOW_BIT, > + EGL_SURFACE_TYPE, EGL_PBUFFER_BIT, > EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT, > EGL_NONE > }; > @@ -297,7 +295,7 @@ int eGL::initEGLContext(GBM *gbmContext) > goto fail; > } > > - display_ = eglGetDisplay(gbmContext->device()); > + display_ = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA, nullptr, nullptr); > if (display_ == EGL_NO_DISPLAY) { > LOG(eGL, Error) << "Unable to get EGL display"; > goto fail; > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > index 9693d7252..c85c7d6cd 100644 > --- a/src/libcamera/software_isp/debayer_egl.cpp > +++ b/src/libcamera/software_isp/debayer_egl.cpp > @@ -142,7 +142,7 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm > > /* > * Tell shaders how to re-order output taking account of how the > - * pixels are actually stored by GBM > + * pixels are actually stored by EGL > */ > switch (outputFormat) { > case formats::ARGB8888: > @@ -586,10 +586,7 @@ int DebayerEGL::start() > { > GLint maxTextureImageUnits; > > - if (gbmSurface_.createDevice()) > - return -ENODEV; > - > - if (egl_.initEGLContext(&gbmSurface_)) > + if (egl_.initEGLContext()) > return -ENODEV; > > glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &maxTextureImageUnits); > diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h > index a5033bc63..cfbaf8e9d 100644 > --- a/src/libcamera/software_isp/debayer_egl.h > +++ b/src/libcamera/software_isp/debayer_egl.h > @@ -113,7 +113,6 @@ private: > Rectangle window_; > std::unique_ptr<SwStatsCpu> stats_; > eGL egl_; > - GBM gbmSurface_; > uint32_t width_; > uint32_t height_; > GLint glFormat_;
Hi, On 15-Jan-26 10:44, Barnabás Pőcze wrote: > 2026. 01. 10. 23:13 keltezéssel, Robert Mader írta: >> Which appears to be a better fit for the use-case at hand: >> 1. Like GBM it is Mesa specific, so no change in supported setups is >> expected. If ever required, a fallback to the generic device platform >> could be added on top. >> 2. It leaves the complexity of selecting a renderer device to the >> driver, reducing code and dependencies. >> 3. It allows to use llvmpipe / software drivers without dri device, >> which can be useful on CI or debugging (with LIBGL_ALWAYS_SOFTWARE=1). >> >> Signed-off-by: Robert Mader <robert.mader@collabora.com> >> >> --- >> > > It seems to work fine, LIBGL_ALWAYS_SOFTWARE=1 also works. > > Tested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> # ThinkPad X1 Yoga Gen 7 + ov2740 Thank you for testing this on an IPU6 device. I was supposed to test this yesterday but I thought this was part of the cleanup series. Anyways it is tested now, thank you. Regards, Hans > > >> Should be applied on to of >> https://patchwork.libcamera.org/cover/25706/ >> --- >> include/libcamera/internal/egl.h | 4 +--- >> src/libcamera/egl.cpp | 12 +++++------- >> src/libcamera/software_isp/debayer_egl.cpp | 7 ++----- >> src/libcamera/software_isp/debayer_egl.h | 1 - >> 4 files changed, 8 insertions(+), 16 deletions(-) >> >> diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h >> index f007f448a..630345ea7 100644 >> --- a/include/libcamera/internal/egl.h >> +++ b/include/libcamera/internal/egl.h >> @@ -16,8 +16,6 @@ >> #include <libcamera/base/span.h> >> #include <libcamera/base/utils.h> >> -#include "libcamera/internal/gbm.h" >> - >> #define EGL_EGLEXT_PROTOTYPES >> #include <EGL/egl.h> >> #include <EGL/eglext.h> >> @@ -96,7 +94,7 @@ public: >> eGL(); >> ~eGL(); >> - int initEGLContext(GBM *gbmContext); >> + int initEGLContext(); >> int createInputDMABufTexture2D(eGLImage &eglImage, int fd); >> int createOutputDMABufTexture2D(eGLImage &eglImage, int fd); >> diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp >> index 0ffd008c7..19c5ff48f 100644 >> --- a/src/libcamera/egl.cpp >> +++ b/src/libcamera/egl.cpp >> @@ -256,24 +256,22 @@ void eGL::createTexture2D(eGLImage &eglImage, GLint format, uint32_t width, uint >> /** >> * \brief Initialise the EGL context >> - * \param[in] gbmContext Pointer to initialised GBM context >> * >> - * Sets up the EGL display from the GBM device, creates an OpenGL ES 2.0 >> - * context, and retrieves function pointers for required extensions >> - * including: >> + * Sets up the EGL display, creates an OpenGL ES 2.0 context, and retrieves >> + * function pointers for required extensions including: >> * - eglCreateImageKHR / eglDestroyImageKHR >> * - glEGLImageTargetTexture2DOES >> * >> * \return 0 on success, or -ENODEV on failure >> */ >> -int eGL::initEGLContext(GBM *gbmContext) >> +int eGL::initEGLContext() >> { >> EGLint configAttribs[] = { >> EGL_RED_SIZE, 8, >> EGL_GREEN_SIZE, 8, >> EGL_BLUE_SIZE, 8, >> EGL_ALPHA_SIZE, 8, >> - EGL_SURFACE_TYPE, EGL_WINDOW_BIT, >> + EGL_SURFACE_TYPE, EGL_PBUFFER_BIT, >> EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT, >> EGL_NONE >> }; >> @@ -297,7 +295,7 @@ int eGL::initEGLContext(GBM *gbmContext) >> goto fail; >> } >> - display_ = eglGetDisplay(gbmContext->device()); >> + display_ = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA, nullptr, nullptr); >> if (display_ == EGL_NO_DISPLAY) { >> LOG(eGL, Error) << "Unable to get EGL display"; >> goto fail; >> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp >> index 9693d7252..c85c7d6cd 100644 >> --- a/src/libcamera/software_isp/debayer_egl.cpp >> +++ b/src/libcamera/software_isp/debayer_egl.cpp >> @@ -142,7 +142,7 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm >> /* >> * Tell shaders how to re-order output taking account of how the >> - * pixels are actually stored by GBM >> + * pixels are actually stored by EGL >> */ >> switch (outputFormat) { >> case formats::ARGB8888: >> @@ -586,10 +586,7 @@ int DebayerEGL::start() >> { >> GLint maxTextureImageUnits; >> - if (gbmSurface_.createDevice()) >> - return -ENODEV; >> - >> - if (egl_.initEGLContext(&gbmSurface_)) >> + if (egl_.initEGLContext()) >> return -ENODEV; >> glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &maxTextureImageUnits); >> diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h >> index a5033bc63..cfbaf8e9d 100644 >> --- a/src/libcamera/software_isp/debayer_egl.h >> +++ b/src/libcamera/software_isp/debayer_egl.h >> @@ -113,7 +113,6 @@ private: >> Rectangle window_; >> std::unique_ptr<SwStatsCpu> stats_; >> eGL egl_; >> - GBM gbmSurface_; >> uint32_t width_; >> uint32_t height_; >> GLint glFormat_; >
Robert Mader <robert.mader@collabora.com> writes: > Which appears to be a better fit for the use-case at hand: > 1. Like GBM it is Mesa specific, so no change in supported setups is > expected. If ever required, a fallback to the generic device platform > could be added on top. > 2. It leaves the complexity of selecting a renderer device to the > driver, reducing code and dependencies. > 3. It allows to use llvmpipe / software drivers without dri device, > which can be useful on CI or debugging (with LIBGL_ALWAYS_SOFTWARE=1). > > Signed-off-by: Robert Mader <robert.mader@collabora.com> Makes sense and works: Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > --- > > Should be applied on to of > https://patchwork.libcamera.org/cover/25706/ > --- > include/libcamera/internal/egl.h | 4 +--- > src/libcamera/egl.cpp | 12 +++++------- > src/libcamera/software_isp/debayer_egl.cpp | 7 ++----- > src/libcamera/software_isp/debayer_egl.h | 1 - > 4 files changed, 8 insertions(+), 16 deletions(-) > > diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h > index f007f448a..630345ea7 100644 > --- a/include/libcamera/internal/egl.h > +++ b/include/libcamera/internal/egl.h > @@ -16,8 +16,6 @@ > #include <libcamera/base/span.h> > #include <libcamera/base/utils.h> > > -#include "libcamera/internal/gbm.h" > - > #define EGL_EGLEXT_PROTOTYPES > #include <EGL/egl.h> > #include <EGL/eglext.h> > @@ -96,7 +94,7 @@ public: > eGL(); > ~eGL(); > > - int initEGLContext(GBM *gbmContext); > + int initEGLContext(); > > int createInputDMABufTexture2D(eGLImage &eglImage, int fd); > int createOutputDMABufTexture2D(eGLImage &eglImage, int fd); > diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp > index 0ffd008c7..19c5ff48f 100644 > --- a/src/libcamera/egl.cpp > +++ b/src/libcamera/egl.cpp > @@ -256,24 +256,22 @@ void eGL::createTexture2D(eGLImage &eglImage, GLint format, uint32_t width, uint > > /** > * \brief Initialise the EGL context > - * \param[in] gbmContext Pointer to initialised GBM context > * > - * Sets up the EGL display from the GBM device, creates an OpenGL ES 2.0 > - * context, and retrieves function pointers for required extensions > - * including: > + * Sets up the EGL display, creates an OpenGL ES 2.0 context, and retrieves > + * function pointers for required extensions including: > * - eglCreateImageKHR / eglDestroyImageKHR > * - glEGLImageTargetTexture2DOES > * > * \return 0 on success, or -ENODEV on failure > */ > -int eGL::initEGLContext(GBM *gbmContext) > +int eGL::initEGLContext() > { > EGLint configAttribs[] = { > EGL_RED_SIZE, 8, > EGL_GREEN_SIZE, 8, > EGL_BLUE_SIZE, 8, > EGL_ALPHA_SIZE, 8, > - EGL_SURFACE_TYPE, EGL_WINDOW_BIT, > + EGL_SURFACE_TYPE, EGL_PBUFFER_BIT, > EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT, > EGL_NONE > }; > @@ -297,7 +295,7 @@ int eGL::initEGLContext(GBM *gbmContext) > goto fail; > } > > - display_ = eglGetDisplay(gbmContext->device()); > + display_ = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA, nullptr, nullptr); > if (display_ == EGL_NO_DISPLAY) { > LOG(eGL, Error) << "Unable to get EGL display"; > goto fail; > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > index 9693d7252..c85c7d6cd 100644 > --- a/src/libcamera/software_isp/debayer_egl.cpp > +++ b/src/libcamera/software_isp/debayer_egl.cpp > @@ -142,7 +142,7 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm > > /* > * Tell shaders how to re-order output taking account of how the > - * pixels are actually stored by GBM > + * pixels are actually stored by EGL > */ > switch (outputFormat) { > case formats::ARGB8888: > @@ -586,10 +586,7 @@ int DebayerEGL::start() > { > GLint maxTextureImageUnits; > > - if (gbmSurface_.createDevice()) > - return -ENODEV; > - > - if (egl_.initEGLContext(&gbmSurface_)) > + if (egl_.initEGLContext()) > return -ENODEV; > > glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &maxTextureImageUnits); > diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h > index a5033bc63..cfbaf8e9d 100644 > --- a/src/libcamera/software_isp/debayer_egl.h > +++ b/src/libcamera/software_isp/debayer_egl.h > @@ -113,7 +113,6 @@ private: > Rectangle window_; > std::unique_ptr<SwStatsCpu> stats_; > eGL egl_; > - GBM gbmSurface_; > uint32_t width_; > uint32_t height_; > GLint glFormat_;
Quoting Milan Zamazal (2026-01-19 11:37:47) > Robert Mader <robert.mader@collabora.com> writes: > > > Which appears to be a better fit for the use-case at hand: > > 1. Like GBM it is Mesa specific, so no change in supported setups is > > expected. If ever required, a fallback to the generic device platform > > could be added on top. > > 2. It leaves the complexity of selecting a renderer device to the > > driver, reducing code and dependencies. > > 3. It allows to use llvmpipe / software drivers without dri device, > > which can be useful on CI or debugging (with LIBGL_ALWAYS_SOFTWARE=1). > > Do we need to document these additional environment variables somewhere/somehow [0]? - or are they just 'external' to libcamera? [0] https://git.libcamera.org/libcamera/libcamera.git/tree/Documentation/runtime_configuration.rst -- Kieran > > Signed-off-by: Robert Mader <robert.mader@collabora.com> > > Makes sense and works: > > Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > > > --- > > > > Should be applied on to of > > https://patchwork.libcamera.org/cover/25706/ > > --- > > include/libcamera/internal/egl.h | 4 +--- > > src/libcamera/egl.cpp | 12 +++++------- > > src/libcamera/software_isp/debayer_egl.cpp | 7 ++----- > > src/libcamera/software_isp/debayer_egl.h | 1 - > > 4 files changed, 8 insertions(+), 16 deletions(-) > > > > diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h > > index f007f448a..630345ea7 100644 > > --- a/include/libcamera/internal/egl.h > > +++ b/include/libcamera/internal/egl.h > > @@ -16,8 +16,6 @@ > > #include <libcamera/base/span.h> > > #include <libcamera/base/utils.h> > > > > -#include "libcamera/internal/gbm.h" > > - > > #define EGL_EGLEXT_PROTOTYPES > > #include <EGL/egl.h> > > #include <EGL/eglext.h> > > @@ -96,7 +94,7 @@ public: > > eGL(); > > ~eGL(); > > > > - int initEGLContext(GBM *gbmContext); > > + int initEGLContext(); > > > > int createInputDMABufTexture2D(eGLImage &eglImage, int fd); > > int createOutputDMABufTexture2D(eGLImage &eglImage, int fd); > > diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp > > index 0ffd008c7..19c5ff48f 100644 > > --- a/src/libcamera/egl.cpp > > +++ b/src/libcamera/egl.cpp > > @@ -256,24 +256,22 @@ void eGL::createTexture2D(eGLImage &eglImage, GLint format, uint32_t width, uint > > > > /** > > * \brief Initialise the EGL context > > - * \param[in] gbmContext Pointer to initialised GBM context > > * > > - * Sets up the EGL display from the GBM device, creates an OpenGL ES 2.0 > > - * context, and retrieves function pointers for required extensions > > - * including: > > + * Sets up the EGL display, creates an OpenGL ES 2.0 context, and retrieves > > + * function pointers for required extensions including: > > * - eglCreateImageKHR / eglDestroyImageKHR > > * - glEGLImageTargetTexture2DOES > > * > > * \return 0 on success, or -ENODEV on failure > > */ > > -int eGL::initEGLContext(GBM *gbmContext) > > +int eGL::initEGLContext() > > { > > EGLint configAttribs[] = { > > EGL_RED_SIZE, 8, > > EGL_GREEN_SIZE, 8, > > EGL_BLUE_SIZE, 8, > > EGL_ALPHA_SIZE, 8, > > - EGL_SURFACE_TYPE, EGL_WINDOW_BIT, > > + EGL_SURFACE_TYPE, EGL_PBUFFER_BIT, > > EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT, > > EGL_NONE > > }; > > @@ -297,7 +295,7 @@ int eGL::initEGLContext(GBM *gbmContext) > > goto fail; > > } > > > > - display_ = eglGetDisplay(gbmContext->device()); > > + display_ = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA, nullptr, nullptr); > > if (display_ == EGL_NO_DISPLAY) { > > LOG(eGL, Error) << "Unable to get EGL display"; > > goto fail; > > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > > index 9693d7252..c85c7d6cd 100644 > > --- a/src/libcamera/software_isp/debayer_egl.cpp > > +++ b/src/libcamera/software_isp/debayer_egl.cpp > > @@ -142,7 +142,7 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm > > > > /* > > * Tell shaders how to re-order output taking account of how the > > - * pixels are actually stored by GBM > > + * pixels are actually stored by EGL > > */ > > switch (outputFormat) { > > case formats::ARGB8888: > > @@ -586,10 +586,7 @@ int DebayerEGL::start() > > { > > GLint maxTextureImageUnits; > > > > - if (gbmSurface_.createDevice()) > > - return -ENODEV; > > - > > - if (egl_.initEGLContext(&gbmSurface_)) > > + if (egl_.initEGLContext()) > > return -ENODEV; > > > > glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &maxTextureImageUnits); > > diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h > > index a5033bc63..cfbaf8e9d 100644 > > --- a/src/libcamera/software_isp/debayer_egl.h > > +++ b/src/libcamera/software_isp/debayer_egl.h > > @@ -113,7 +113,6 @@ private: > > Rectangle window_; > > std::unique_ptr<SwStatsCpu> stats_; > > eGL egl_; > > - GBM gbmSurface_; > > uint32_t width_; > > uint32_t height_; > > GLint glFormat_; >
2026. 01. 19. 13:42 keltezéssel, Kieran Bingham írta: > Quoting Milan Zamazal (2026-01-19 11:37:47) >> Robert Mader <robert.mader@collabora.com> writes: >> >>> Which appears to be a better fit for the use-case at hand: >>> 1. Like GBM it is Mesa specific, so no change in supported setups is >>> expected. If ever required, a fallback to the generic device platform >>> could be added on top. >>> 2. It leaves the complexity of selecting a renderer device to the >>> driver, reducing code and dependencies. >>> 3. It allows to use llvmpipe / software drivers without dri device, >>> which can be useful on CI or debugging (with LIBGL_ALWAYS_SOFTWARE=1). >>> > > Do we need to document these additional environment variables > somewhere/somehow [0]? - or are they just 'external' to libcamera? > > [0] https://git.libcamera.org/libcamera/libcamera.git/tree/Documentation/runtime_configuration.rst I wouldn't list it there because it is an external thing, but maybe it could be mentioned in a separate document specifically about the software isp. > > -- > Kieran > >>> Signed-off-by: Robert Mader <robert.mader@collabora.com> >> >> Makes sense and works: >> >> Reviewed-by: Milan Zamazal <mzamazal@redhat.com> >> >>> --- >>> >>> Should be applied on to of >>> https://patchwork.libcamera.org/cover/25706/ >>> --- >>> include/libcamera/internal/egl.h | 4 +--- >>> src/libcamera/egl.cpp | 12 +++++------- >>> src/libcamera/software_isp/debayer_egl.cpp | 7 ++----- >>> src/libcamera/software_isp/debayer_egl.h | 1 - >>> 4 files changed, 8 insertions(+), 16 deletions(-) >>> >>> diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h >>> index f007f448a..630345ea7 100644 >>> --- a/include/libcamera/internal/egl.h >>> +++ b/include/libcamera/internal/egl.h >>> @@ -16,8 +16,6 @@ >>> #include <libcamera/base/span.h> >>> #include <libcamera/base/utils.h> >>> >>> -#include "libcamera/internal/gbm.h" >>> - >>> #define EGL_EGLEXT_PROTOTYPES >>> #include <EGL/egl.h> >>> #include <EGL/eglext.h> >>> @@ -96,7 +94,7 @@ public: >>> eGL(); >>> ~eGL(); >>> >>> - int initEGLContext(GBM *gbmContext); >>> + int initEGLContext(); >>> >>> int createInputDMABufTexture2D(eGLImage &eglImage, int fd); >>> int createOutputDMABufTexture2D(eGLImage &eglImage, int fd); >>> diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp >>> index 0ffd008c7..19c5ff48f 100644 >>> --- a/src/libcamera/egl.cpp >>> +++ b/src/libcamera/egl.cpp >>> @@ -256,24 +256,22 @@ void eGL::createTexture2D(eGLImage &eglImage, GLint format, uint32_t width, uint >>> >>> /** >>> * \brief Initialise the EGL context >>> - * \param[in] gbmContext Pointer to initialised GBM context >>> * >>> - * Sets up the EGL display from the GBM device, creates an OpenGL ES 2.0 >>> - * context, and retrieves function pointers for required extensions >>> - * including: >>> + * Sets up the EGL display, creates an OpenGL ES 2.0 context, and retrieves >>> + * function pointers for required extensions including: >>> * - eglCreateImageKHR / eglDestroyImageKHR >>> * - glEGLImageTargetTexture2DOES >>> * >>> * \return 0 on success, or -ENODEV on failure >>> */ >>> -int eGL::initEGLContext(GBM *gbmContext) >>> +int eGL::initEGLContext() >>> { >>> EGLint configAttribs[] = { >>> EGL_RED_SIZE, 8, >>> EGL_GREEN_SIZE, 8, >>> EGL_BLUE_SIZE, 8, >>> EGL_ALPHA_SIZE, 8, >>> - EGL_SURFACE_TYPE, EGL_WINDOW_BIT, >>> + EGL_SURFACE_TYPE, EGL_PBUFFER_BIT, >>> EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT, >>> EGL_NONE >>> }; >>> @@ -297,7 +295,7 @@ int eGL::initEGLContext(GBM *gbmContext) >>> goto fail; >>> } >>> >>> - display_ = eglGetDisplay(gbmContext->device()); >>> + display_ = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA, nullptr, nullptr); >>> if (display_ == EGL_NO_DISPLAY) { >>> LOG(eGL, Error) << "Unable to get EGL display"; >>> goto fail; >>> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp >>> index 9693d7252..c85c7d6cd 100644 >>> --- a/src/libcamera/software_isp/debayer_egl.cpp >>> +++ b/src/libcamera/software_isp/debayer_egl.cpp >>> @@ -142,7 +142,7 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm >>> >>> /* >>> * Tell shaders how to re-order output taking account of how the >>> - * pixels are actually stored by GBM >>> + * pixels are actually stored by EGL >>> */ >>> switch (outputFormat) { >>> case formats::ARGB8888: >>> @@ -586,10 +586,7 @@ int DebayerEGL::start() >>> { >>> GLint maxTextureImageUnits; >>> >>> - if (gbmSurface_.createDevice()) >>> - return -ENODEV; >>> - >>> - if (egl_.initEGLContext(&gbmSurface_)) >>> + if (egl_.initEGLContext()) >>> return -ENODEV; >>> >>> glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &maxTextureImageUnits); >>> diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h >>> index a5033bc63..cfbaf8e9d 100644 >>> --- a/src/libcamera/software_isp/debayer_egl.h >>> +++ b/src/libcamera/software_isp/debayer_egl.h >>> @@ -113,7 +113,6 @@ private: >>> Rectangle window_; >>> std::unique_ptr<SwStatsCpu> stats_; >>> eGL egl_; >>> - GBM gbmSurface_; >>> uint32_t width_; >>> uint32_t height_; >>> GLint glFormat_; >>
Quoting Robert Mader (2026-01-10 22:13:36) > Which appears to be a better fit for the use-case at hand: > 1. Like GBM it is Mesa specific, so no change in supported setups is > expected. If ever required, a fallback to the generic device platform > could be added on top. > 2. It leaves the complexity of selecting a renderer device to the > driver, reducing code and dependencies. > 3. It allows to use llvmpipe / software drivers without dri device, > which can be useful on CI or debugging (with LIBGL_ALWAYS_SOFTWARE=1). > > Signed-off-by: Robert Mader <robert.mader@collabora.com> > > --- > > Should be applied on to of > https://patchwork.libcamera.org/cover/25706/ Now that the above series is merged, I kicked these patches into CI but I'm afraid they failed to compile on all targets. Can you take a look please? https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1586726 https://gitlab.freedesktop.org/camera/libcamera/-/jobs/91366140 -- Kieran > --- > include/libcamera/internal/egl.h | 4 +--- > src/libcamera/egl.cpp | 12 +++++------- > src/libcamera/software_isp/debayer_egl.cpp | 7 ++----- > src/libcamera/software_isp/debayer_egl.h | 1 - > 4 files changed, 8 insertions(+), 16 deletions(-) > > diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h > index f007f448a..630345ea7 100644 > --- a/include/libcamera/internal/egl.h > +++ b/include/libcamera/internal/egl.h > @@ -16,8 +16,6 @@ > #include <libcamera/base/span.h> > #include <libcamera/base/utils.h> > > -#include "libcamera/internal/gbm.h" > - > #define EGL_EGLEXT_PROTOTYPES > #include <EGL/egl.h> > #include <EGL/eglext.h> > @@ -96,7 +94,7 @@ public: > eGL(); > ~eGL(); > > - int initEGLContext(GBM *gbmContext); > + int initEGLContext(); > > int createInputDMABufTexture2D(eGLImage &eglImage, int fd); > int createOutputDMABufTexture2D(eGLImage &eglImage, int fd); > diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp > index 0ffd008c7..19c5ff48f 100644 > --- a/src/libcamera/egl.cpp > +++ b/src/libcamera/egl.cpp > @@ -256,24 +256,22 @@ void eGL::createTexture2D(eGLImage &eglImage, GLint format, uint32_t width, uint > > /** > * \brief Initialise the EGL context > - * \param[in] gbmContext Pointer to initialised GBM context > * > - * Sets up the EGL display from the GBM device, creates an OpenGL ES 2.0 > - * context, and retrieves function pointers for required extensions > - * including: > + * Sets up the EGL display, creates an OpenGL ES 2.0 context, and retrieves > + * function pointers for required extensions including: > * - eglCreateImageKHR / eglDestroyImageKHR > * - glEGLImageTargetTexture2DOES > * > * \return 0 on success, or -ENODEV on failure > */ > -int eGL::initEGLContext(GBM *gbmContext) > +int eGL::initEGLContext() > { > EGLint configAttribs[] = { > EGL_RED_SIZE, 8, > EGL_GREEN_SIZE, 8, > EGL_BLUE_SIZE, 8, > EGL_ALPHA_SIZE, 8, > - EGL_SURFACE_TYPE, EGL_WINDOW_BIT, > + EGL_SURFACE_TYPE, EGL_PBUFFER_BIT, > EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT, > EGL_NONE > }; > @@ -297,7 +295,7 @@ int eGL::initEGLContext(GBM *gbmContext) > goto fail; > } > > - display_ = eglGetDisplay(gbmContext->device()); > + display_ = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA, nullptr, nullptr); > if (display_ == EGL_NO_DISPLAY) { > LOG(eGL, Error) << "Unable to get EGL display"; > goto fail; > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > index 9693d7252..c85c7d6cd 100644 > --- a/src/libcamera/software_isp/debayer_egl.cpp > +++ b/src/libcamera/software_isp/debayer_egl.cpp > @@ -142,7 +142,7 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm > > /* > * Tell shaders how to re-order output taking account of how the > - * pixels are actually stored by GBM > + * pixels are actually stored by EGL > */ > switch (outputFormat) { > case formats::ARGB8888: > @@ -586,10 +586,7 @@ int DebayerEGL::start() > { > GLint maxTextureImageUnits; > > - if (gbmSurface_.createDevice()) > - return -ENODEV; > - > - if (egl_.initEGLContext(&gbmSurface_)) > + if (egl_.initEGLContext()) > return -ENODEV; > > glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &maxTextureImageUnits); > diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h > index a5033bc63..cfbaf8e9d 100644 > --- a/src/libcamera/software_isp/debayer_egl.h > +++ b/src/libcamera/software_isp/debayer_egl.h > @@ -113,7 +113,6 @@ private: > Rectangle window_; > std::unique_ptr<SwStatsCpu> stats_; > eGL egl_; > - GBM gbmSurface_; > uint32_t width_; > uint32_t height_; > GLint glFormat_; > -- > 2.52.0 >
On 19.01.26 13:47, Barnabás Pőcze wrote: > 2026. 01. 19. 13:42 keltezéssel, Kieran Bingham írta: >> Quoting Milan Zamazal (2026-01-19 11:37:47) >>> Robert Mader <robert.mader@collabora.com> writes: >>> >>>> Which appears to be a better fit for the use-case at hand: >>>> 1. Like GBM it is Mesa specific, so no change in supported setups is >>>> expected. If ever required, a fallback to the generic device >>>> platform >>>> could be added on top. >>>> 2. It leaves the complexity of selecting a renderer device to the >>>> driver, reducing code and dependencies. >>>> 3. It allows to use llvmpipe / software drivers without dri device, >>>> which can be useful on CI or debugging (with >>>> LIBGL_ALWAYS_SOFTWARE=1). >>>> >> >> Do we need to document these additional environment variables >> somewhere/somehow [0]? - or are they just 'external' to libcamera? >> >> [0] >> https://git.libcamera.org/libcamera/libcamera.git/tree/Documentation/runtime_configuration.rst > > I wouldn't list it there because it is an external thing, but maybe it > could be > mentioned in a separate document specifically about the software isp. I'd also argue it's an external thing that 1. is quite specific to developers wanting to debug driver issues. This small group will usually want to learn about various Mesa env variables eventually - and LIBGL_ALWAYS_SOFTWARE is probably the best known one amongst them. 2. may not work if llvmpipe isn't installed or a proprietary (but compatible) driver is used, such as Nvidia. But if requested I'd be happy to add it in some SW/GPU-ISP specific part, like Barnabás suggested.
On 19.01.26 13:51, Kieran Bingham wrote: > Now that the above series is merged, I kicked these patches into CI but > I'm afraid they failed to compile on all targets. Can you take a look > please? > > https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1586726 > > https://gitlab.freedesktop.org/camera/libcamera/-/jobs/91366140 Sure, will do! And urg, Debian 11 - happy to see it going EOL later this year. > > -- > Kieran >
Quoting Robert Mader (2026-01-19 13:09:37) > On 19.01.26 13:51, Kieran Bingham wrote: > > Now that the above series is merged, I kicked these patches into CI but > > I'm afraid they failed to compile on all targets. Can you take a look > > please? > > > > https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1586726 > > > > https://gitlab.freedesktop.org/camera/libcamera/-/jobs/91366140 > Sure, will do! And urg, Debian 11 - happy to see it going EOL later this > year. Indeed, we'll drop it from our CI once it's EOL I believe. But until then ... :-S -- Kieran > > > > -- > > Kieran > > > -- > Robert Mader > Consultant Software Developer > > Collabora Ltd. > Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK > Registered in England & Wales, no. 5513718
On 19.01.26 14:34, Kieran Bingham wrote: > Quoting Robert Mader (2026-01-19 13:09:37) >> On 19.01.26 13:51, Kieran Bingham wrote: >>> Now that the above series is merged, I kicked these patches into CI but >>> I'm afraid they failed to compile on all targets. Can you take a look >>> please? >>> >>> https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1586726 >>> >>> https://gitlab.freedesktop.org/camera/libcamera/-/jobs/91366140 >> Sure, will do! And urg, Debian 11 - happy to see it going EOL later this >> year. > Indeed, we'll drop it from our CI once it's EOL I believe. But until > then ... :-S Had a quick look and the issue appears to be the revert: previously the EGL parts were not build on Debian 11 because of Run-time dependency gbm found: NO (tried pkgconfig and cmake) Check usable header "gbm.h" : NO ... |SoftISP GPU acceleration : False| i.e. rather by mistake if IIUC. Not sure yet how to handle that - maybe just require a minimum g++ version, if used? I tried wrapping the imports in 'external "C"' but that didn't work :/ > > -- > Kieran > >>> -- >>> Kieran >>> >> -- >> Robert Mader >> Consultant Software Developer >> >> Collabora Ltd. >> Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK >> Registered in England & Wales, no. 5513718
2026. 01. 19. 17:12 keltezéssel, Robert Mader írta: > > On 19.01.26 14:34, Kieran Bingham wrote: >> Quoting Robert Mader (2026-01-19 13:09:37) >>> On 19.01.26 13:51, Kieran Bingham wrote: >>>> Now that the above series is merged, I kicked these patches into CI but >>>> I'm afraid they failed to compile on all targets. Can you take a look >>>> please? >>>> >>>> https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1586726 >>>> >>>> https://gitlab.freedesktop.org/camera/libcamera/-/jobs/91366140 >>> Sure, will do! And urg, Debian 11 - happy to see it going EOL later this >>> year. >> Indeed, we'll drop it from our CI once it's EOL I believe. But until >> then ... :-S > > Had a quick look and the issue appears to be the revert: previously the EGL parts were not build on Debian 11 because of > > Run-time dependency gbm found: NO (tried pkgconfig and cmake) > Check usable header "gbm.h" : NO > ... > |SoftISP GPU acceleration : False| > > i.e. rather by mistake if IIUC. Not sure yet how to handle that - maybe just require a minimum g++ version, if used? > > I tried wrapping the imports in 'external "C"' but that didn't work :/ Based on https://github.com/KhronosGroup/EGL-Registry/pull/130, have you tried `#define EGL_NO_X11` before all includes in egl.cpp? > >> -- >> Kieran >> >>>> -- >>>> Kieran >>>> >>> -- >>> Robert Mader >>> Consultant Software Developer >>> >>> Collabora Ltd. >>> Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK >>> Registered in England & Wales, no. 5513718 > > -- > Robert Mader > Consultant Software Developer > > Collabora Ltd. > Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK > Registered in England & Wales, no. 5513718 >
On 19.01.26 18:46, Barnabás Pőcze wrote: > Based on https://github.com/KhronosGroup/EGL-Registry/pull/130, > have you tried `#define EGL_NO_X11` before all includes in egl.cpp? > Nice, thanks a lot for digging that out! Did that now in v2/v3.
diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h index f007f448a..630345ea7 100644 --- a/include/libcamera/internal/egl.h +++ b/include/libcamera/internal/egl.h @@ -16,8 +16,6 @@ #include <libcamera/base/span.h> #include <libcamera/base/utils.h> -#include "libcamera/internal/gbm.h" - #define EGL_EGLEXT_PROTOTYPES #include <EGL/egl.h> #include <EGL/eglext.h> @@ -96,7 +94,7 @@ public: eGL(); ~eGL(); - int initEGLContext(GBM *gbmContext); + int initEGLContext(); int createInputDMABufTexture2D(eGLImage &eglImage, int fd); int createOutputDMABufTexture2D(eGLImage &eglImage, int fd); diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp index 0ffd008c7..19c5ff48f 100644 --- a/src/libcamera/egl.cpp +++ b/src/libcamera/egl.cpp @@ -256,24 +256,22 @@ void eGL::createTexture2D(eGLImage &eglImage, GLint format, uint32_t width, uint /** * \brief Initialise the EGL context - * \param[in] gbmContext Pointer to initialised GBM context * - * Sets up the EGL display from the GBM device, creates an OpenGL ES 2.0 - * context, and retrieves function pointers for required extensions - * including: + * Sets up the EGL display, creates an OpenGL ES 2.0 context, and retrieves + * function pointers for required extensions including: * - eglCreateImageKHR / eglDestroyImageKHR * - glEGLImageTargetTexture2DOES * * \return 0 on success, or -ENODEV on failure */ -int eGL::initEGLContext(GBM *gbmContext) +int eGL::initEGLContext() { EGLint configAttribs[] = { EGL_RED_SIZE, 8, EGL_GREEN_SIZE, 8, EGL_BLUE_SIZE, 8, EGL_ALPHA_SIZE, 8, - EGL_SURFACE_TYPE, EGL_WINDOW_BIT, + EGL_SURFACE_TYPE, EGL_PBUFFER_BIT, EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT, EGL_NONE }; @@ -297,7 +295,7 @@ int eGL::initEGLContext(GBM *gbmContext) goto fail; } - display_ = eglGetDisplay(gbmContext->device()); + display_ = eglGetPlatformDisplay(EGL_PLATFORM_SURFACELESS_MESA, nullptr, nullptr); if (display_ == EGL_NO_DISPLAY) { LOG(eGL, Error) << "Unable to get EGL display"; goto fail; diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp index 9693d7252..c85c7d6cd 100644 --- a/src/libcamera/software_isp/debayer_egl.cpp +++ b/src/libcamera/software_isp/debayer_egl.cpp @@ -142,7 +142,7 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm /* * Tell shaders how to re-order output taking account of how the - * pixels are actually stored by GBM + * pixels are actually stored by EGL */ switch (outputFormat) { case formats::ARGB8888: @@ -586,10 +586,7 @@ int DebayerEGL::start() { GLint maxTextureImageUnits; - if (gbmSurface_.createDevice()) - return -ENODEV; - - if (egl_.initEGLContext(&gbmSurface_)) + if (egl_.initEGLContext()) return -ENODEV; glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &maxTextureImageUnits); diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h index a5033bc63..cfbaf8e9d 100644 --- a/src/libcamera/software_isp/debayer_egl.h +++ b/src/libcamera/software_isp/debayer_egl.h @@ -113,7 +113,6 @@ private: Rectangle window_; std::unique_ptr<SwStatsCpu> stats_; eGL egl_; - GBM gbmSurface_; uint32_t width_; uint32_t height_; GLint glFormat_;
Which appears to be a better fit for the use-case at hand: 1. Like GBM it is Mesa specific, so no change in supported setups is expected. If ever required, a fallback to the generic device platform could be added on top. 2. It leaves the complexity of selecting a renderer device to the driver, reducing code and dependencies. 3. It allows to use llvmpipe / software drivers without dri device, which can be useful on CI or debugging (with LIBGL_ALWAYS_SOFTWARE=1). Signed-off-by: Robert Mader <robert.mader@collabora.com> --- Should be applied on to of https://patchwork.libcamera.org/cover/25706/ --- include/libcamera/internal/egl.h | 4 +--- src/libcamera/egl.cpp | 12 +++++------- src/libcamera/software_isp/debayer_egl.cpp | 7 ++----- src/libcamera/software_isp/debayer_egl.h | 1 - 4 files changed, 8 insertions(+), 16 deletions(-)