Make glamor egl more flexible by stefan11111 · Pull Request #2167 · X11Libre/xserver

stefan11111 · 2026-04-09T12:03:09Z

No description provided.

stefan11111 · 2026-04-10T17:22:59Z

metux · 2026-04-13T09:33:42Z

+    if (glamor_egl->fd >= 0) {
+        glamor_egl->gbm = gbm_create_device(glamor_egl->fd);
+        if (!glamor_egl->gbm) {
+            glamor_egl->gbm = gbm_create_device_by_name(glamor_egl->fd, "dumb");


I'm a bit confused. Isn't that a separate issue / feature that should be a separate commit ?

split into a separate commit

metux · 2026-04-13T09:35:13Z

 static void
 glamor_filter_modifiers(uint32_t *num_modifiers, uint64_t **modifiers,
-                        EGLBoolean *external_only)
+                        EGLBoolean *external_only, int linear_only)


Not sure, but looks like a separate topic (-> separate commit ?) to me.
Can you please give some bits documentation on what that's actually doing ?

Split into a separate commit.

This filters all modifiers except DRM_FORMAT_MOD_{LINEAR,INVALID}, when using the dumb gbm backend.

IOW: when using dumb gbm, it's restricting to linear framebuffer only ?

Yes.
We still have to read the modifier list, because the linear modifier may not be in the list at all, or it may be external_only, which means we cannot use it.

If the linear modifier cannot be used, we disable glamor's dri3 implementation when we check for at least a supported (format, modifier) pair and find none.

ok. maybe we should have some little docs on that.

This the current commit message:

https://github.com/stefan11111/dumb_gbm is a gbm backend that only uses dumb buffers. As such, it is very portable, allowing libgbm to be used on devices where a proper gbm backend with tiled buffer support isn't available.

What should I add to it?

Also, my earlier response was perhaps a bit incomplete.

With dumb gbm, all buffers created are linear, so they can be thought of as linear framebuffers.

However, it's not like traditional unaccelerated fbdev, with a single front framebuffer where everything is drawn.

With dumb gbm, glamor draws into the (linear) front buffer using gl, and X clients can also draw into the (linear) buffers they get from the X server through DRI3 with gl.

If we still have an egl context when `closeScreen` is called, we destroy it here and set it to `EGL_NO_CONTEXT` Signed-off-by: stefan11111 <stefan11111@shitposting.expert>

metux · 2026-04-15T09:22:21Z

This github review & commenting functionality is quite crap ... totally mixes up context :(
Maybe we should only 1-commit-PRs ... :o grmpf

Just reviewed directly via git ... only one little nitpick:
The commit "glamor/glamor_egl.c: extend glamor_egl_make_current" could deserve a bit better description on what/why it's actually doing. Anything else looks good to me.

The change is mostly cosmetic, bringing this in line with Xephyr's `egl_make_current`. We'll see if we need to do anything fancier if/when we decide to add EGLStream support Signed-off-by: stefan11111 <stefan11111@shitposting.expert>

glamor_egl's glvnd vendor option used to only work with libraries. Now, it will work with driver names too (e.g. intel/iris vs mesa). This code doesn't do much currently, it will be used by subsequent commits. Signed-off-by: stefan11111 <stefan11111@shitposting.expert>

Before this, the generic glamor egl code from xf86 only worked with a dri fd. However, this is only needed for glamor_egl's dri3 support. Glamor supports having no DRI3 support at all, even if the X server / xf86 ddx driver doesn't implement DRI3 itself. When using glamor's DRI3 implementation, we create a gbm bo for each pixmap we want to export using DRI3. From that bo, we create an `EGLImageKHR`. From this `EGLImageKHR`, we create a texture using `glEGLImageTargetTexture2DOES` and set this newly created texture as the pixmap's texture, for glamor to render into. When not using DRI3, glamor allocates all textures itself, using `glTexImage2D` in `_glamor_create_tex` from `glamor/glamor_fbo.c`. As-is, the generic glamor_egl code doesn't work without a dri fd, because it only tries to get an `EGLDisplay` on the GBM platform. This commit only make glamor handle the case if no fd (-1) is passed to it. It will currently error out cleanly at runtime if this actually happens. Subsequent commits will add support for more egl platforms and drop existing extension requirements, enabling support for more hardware. Signed-off-by: stefan11111 <stefan11111@shitposting.expert>

https://github.com/stefan11111/dumb_gbm is a gbm backend that only uses dumb buffers. As such, it is very portable, allowing libgbm to be used on devices where a proper gbm backend with tiled buffer support isn't available. Signed-off-by: stefan11111 <stefan11111@shitposting.expert>

stefan11111 · 2026-04-15T12:03:34Z

This github review & commenting functionality is quite crap ... totally mixes up context :( Maybe we should only 1-commit-PRs ... :o grmpf

Just reviewed directly via git ... only one little nitpick: The commit "glamor/glamor_egl.c: extend glamor_egl_make_current" could deserve a bit better description on what/why it's actually doing. Anything else looks good to me.

Done.
Added some more description to the other commits too.

stefan11111 · 2026-04-15T12:15:31Z

@cepelinas9000 Any concerns on your side?

cepelinas9000 · 2026-04-16T16:36:58Z

applying on master, i getting small leak:


Indirect leak of 4128 byte(s) in 43 object(s) allocated from:
    #0 0x7ffff792220b in realloc /usr/src/debug/sys-devel/gcc-15.2.1_p20251122/gcc-15-20251122/libsanitizer/asan/asan_malloc_linux.cpp:81
    #1 0x7bfff285a188 in glamor_filter_modifiers ../glamor/glamor_egl.c:897
    #2 0x7bfff285aacc in glamor_get_modifiers_internal ../glamor/glamor_egl.c:949
    #3 0x7bfff285ac39 in glamor_get_modifiers ../glamor/glamor_egl.c:970
    #4 0x5555572a8080 in cache_formats_and_modifiers ../dri3/dri3_screen.c:198
    #5 0x5555572a877d in dri3_get_supported_modifiers ../dri3/dri3_screen.c:236
    #6 0x55555729e364 in proc_dri3_get_supported_modifiers ../dri3/dri3_request.c:399
    #7 0x5555572a5818 in proc_dri3_dispatch ../dri3/dri3_request.c:680
    #8 0x555556a03e93 in Dispatch ../dix/dispatch.c:566
    #9 0x555556a70b15 in dix_main ../dix/main.c:288
    #10 0x55555760b25a in main ../dix/stubmain.c:34
    #11 0x7ffff6c4d3fa  (/usr/lib64/libc.so.6+0x273fa)
    #12 0x7fffffffdb38  ([stack]+0x28b38)

This was when i ran glxgears something 40 times. Or this leak fix should be seperate pr?

stefan11111 · 2026-04-16T18:37:34Z

This leak is not related to this patch.
It's dri3 allocating a cache of formats and modifiers, so that it doesn't have to
call into egl every time an X client requests it.
It's not really a leak, the X server never looses track of the cache,
it just doesn't free it at closeScreen time.

This patch should fix it: #2198

cepelinas9000

LGTM

stefan11111 requested review from a team, ONykyf, algrid, b-aaz, cepelinas9000, josephcrowell and metux April 9, 2026 12:03

stefan11111 mentioned this pull request Apr 9, 2026

Glamor egl merge and split #2143

Open

metux reviewed Apr 13, 2026

View reviewed changes

Comment thread glamor/glamor_egl.h

metux reviewed Apr 13, 2026

View reviewed changes

Comment thread glamor/glamor_egl.c

metux reviewed Apr 13, 2026

View reviewed changes

Comment thread glamor/glamor_egl.c

metux reviewed Apr 13, 2026

View reviewed changes

Comment thread glamor/glamor_egl.c

stefan11111 force-pushed the glamor-egl-prelims branch from 2f3a754 to eef0014 Compare April 13, 2026 11:34

glamor/glamor_egl.c: Clean up more in glamor_egl_cleanup

6d06d8a

If we still have an egl context when `closeScreen` is called, we destroy it here and set it to `EGL_NO_CONTEXT` Signed-off-by: stefan11111 <stefan11111@shitposting.expert>

stefan11111 force-pushed the glamor-egl-prelims branch from eef0014 to 84f3b38 Compare April 13, 2026 16:29

stefan11111 added 2 commits April 15, 2026 14:33

glamor/glamor_egl.c: extend glamor_egl_make_current

be01925

The change is mostly cosmetic, bringing this in line with Xephyr's `egl_make_current`. We'll see if we need to do anything fancier if/when we decide to add EGLStream support Signed-off-by: stefan11111 <stefan11111@shitposting.expert>

stefan11111 force-pushed the glamor-egl-prelims branch from 84f3b38 to c81badf Compare April 15, 2026 12:00

stefan11111 added 2 commits April 15, 2026 15:02

stefan11111 force-pushed the glamor-egl-prelims branch from c81badf to d56eb16 Compare April 15, 2026 12:03

cepelinas9000 approved these changes Apr 16, 2026

View reviewed changes

metux merged commit 8ba3f3a into X11Libre:master Apr 17, 2026

stefan11111 deleted the glamor-egl-prelims branch April 18, 2026 23:54

Conversation

stefan11111 commented Apr 9, 2026

Uh oh!

stefan11111 commented Apr 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

metux commented Apr 15, 2026

Uh oh!

stefan11111 commented Apr 15, 2026

Uh oh!

stefan11111 commented Apr 15, 2026

Uh oh!

cepelinas9000 commented Apr 16, 2026

Uh oh!

stefan11111 commented Apr 16, 2026

Uh oh!

cepelinas9000 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants