I tried this patch with the i915 driver and encountered the following problem:

eglGetDisplay fails for the GBM device. The i915 driver (or something else?) apparently requires the EGL display to be retrieved with eglGetPlatformDisplayEXT and EGL_PLATFORM_GBM_KHR.

If SDL_EGL_LoadLibrary is expected to load a specific platform display, it probably has to be be informed of the target. Perhaps with an additional function argument?

Getting the eglGetPlatformDisplayEXT with eglGetProcAddress is no problem, but another extension (EGL_EXT_client_extensions) is required to check if it is available in the first place.

Fortunately, EGL_EXT_client_extensions just slightly modifies the behavior of the eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS) call and implementations that don't have the extension are easily detectable. SDL_EGL_HasExtension could be extended with these client extensions.

References:
https://www.khronos.org/registry/EGL/extensions/EXT/EGL_EXT_client_extensions.txt
https://www.khronos.org/registry/EGL/extensions/MESA/EGL_MESA_platform_gbm.txt

Going to try this on other systems as well now.

(In reply to Simon Hug from comment #1)
> I tried this patch with the i915 driver and encountered the following
> problem:
> 
> eglGetDisplay fails for the GBM device. The i915 driver (or something else?)
> apparently requires the EGL display to be retrieved with
> eglGetPlatformDisplayEXT and EGL_PLATFORM_GBM_KHR.
> 
> If SDL_EGL_LoadLibrary is expected to load a specific platform display, it
> probably has to be be informed of the target. Perhaps with an additional
> function argument?
> 
> Getting the eglGetPlatformDisplayEXT with eglGetProcAddress is no problem,
> but another extension (EGL_EXT_client_extensions) is required to check if it
> is available in the first place.
> 
> Fortunately, EGL_EXT_client_extensions just slightly modifies the behavior
> of the eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS) call and
> implementations that don't have the extension are easily detectable.
> SDL_EGL_HasExtension could be extended with these client extensions.
> 
> References:
> https://www.khronos.org/registry/EGL/extensions/EXT/
> EGL_EXT_client_extensions.txt
> https://www.khronos.org/registry/EGL/extensions/MESA/EGL_MESA_platform_gbm.
> txt
> 
> Going to try this on other systems as well now.

@Simon: I am on i915 myself, and it works perfectly here.
Are you sure you're using a recent kernel/mesa EGL implementation?
I am on 4.10.0-22-generic kernel and "pkg-config --version libegl1-mesa" says I'm on MESA EGL version 0.28.

Since it works on both i915 and Raspberry Pi here, I assume SDL_EGL_LoadLibrary does not neet to load a specific platform display.

Yes, very recent. I'm testing on Arch Linux.

Linux 4.11.7-1-ARCH #1 SMP PREEMPT Sat Jun 24 09:27:18 CEST 2017 i686 GNU/Linux
"pkg-config --version libegl1-mesa" -> 0.29.2 which is from Mesa 17.1.4.

The Raspberry Pi shows the same issue there. Perhaps they changed something recently in Mesa? I'll test it later on Raspbian.

Correction to my earlier comment: I think I meant to write EGL_PLATFORM_GBM_MESA. EGL_PLATFORM_GBM_KHR has the EGL 1.5 dependency.

@Simon: I am using Raspbian on the Pi, and Lubuntu on the i915 X86_64 laptop, and it works on both.
The thing is, it's hard for me to get it fixed if I can't reproduce the issue on my systems, but if you can test a possible solution, I will add it to the patch. Sounds like something simple anyway.

Created attachment 2790 [details]
Adds platform selection for the display. Apply after attachment 2785 [details].

Ok, here's what I came up with. This patch should apply after yours. If the right extensions or EGL 1.5 are present it will use the GBM platform to create the display. Should that fail, it will try it the old way. Not sure if this will cause issues. It can always be made stricter, I guess.

I tested it with i915 on Arch and vc4 on Raspbian. Going to test it with vc4 on Arch and perhaps with some nvidia driver on other popular distributions. Sadly, I don't have other systems to test this with.

The Mesa version on Arch has EGL 1.5 so that's probably why the platform is required.

I got some issues when building with the rpi driver. Including the broadcom EGL headers obviously isn't the best idea, but I'm sure the drivers can live side by side someday.

Your patch changes SDL_config.h. Is that intentional?

Created attachment 2792 [details]
V2 patch for KMS/DRM support, now using GBM platform usage for display creation if the needed extension is available

@Simon: I have tested your patch and it seems to work fine here, too, both on Raspbian and Lubuntu 17.04 with i915.
So I have integrated your patch with mine, and made a new version of the kmsdrm patch that uses the GBM platform if the extension is available, moves the headers to SDL_kmsdrmvideo.h as you did and patches the rest of the backends accodingly. In other words, I believe your patch is alright entirely.
About the config.h changes on the previous patch, they weren't intentional: I left those in accidentally, so I have removed them in this new version.

@Simon Hug & all: what's the problem with this patch? I think it's good and interesting enough to be merged, X-less accelerated SDL2 seems very desirable to me and I believe many developers will find it useful.

I think it's ok. Seems to compile properly with most distributions. Didn't have time to test the propietary NVIDIA driver yet, but going to do that today, probably.

A small bug: In KMSDRM_Create, the first 'goto cleanup;' will jump to the cleanup code when vdata is uninitialized, possbly calling 'free' on a garbage pointer. GCC doesn't catch this one for some reason.

This has been committed, thanks guys!
https://hg.libsdl.org/SDL/rev/cbc6a8a5b701
https://hg.libsdl.org/SDL/rev/9397a2d41d6b

I couldn't get it to work yet with the proprietary NVIDIA driver. Doesn't give me an EGL display. It did trigger a bug in the code, though. The encoder pointer doesn't get set to NULL and a double free happens on certain errors. I'll poke around some more and make a bug report later.

Doesn't work on NVIDIA driver

Oh. Looks like NVIDIA doesn't do GBM and the EGLStreams stuff has to be used. They have an example here.

https://github.com/aritger/eglstreams-kms-example

Nice, yeah I can dig around egl streams (unless Manuel has started already). Have been debugging an issue with custom cursor not showing up on my intel machine. Ive not tested any other cards, but would like to see a nice custom cursor.

Was all testing Terraria and it does not seem to like evdev... seems to render just fine but events are very delayed. Though Hammerwatch worked just fine and was doing well.

Also an issue with touch pads, I changed the evdev backend to include a _TOUCHPAD. The issue though is they are not synthesised so each mouse move from a new position on the touchpad causes jumping. Was going to check around a bit more on that.

There was another issue when changing the configuration of the screen to a different resolution causes the FB to be cleaned up then black screening. Though we should just stick to the default fullscreen mode size its possible to blackscreen which sucks when running through gbm :). Not 100% on the fix here, but something to take into account. 

Otherwise the backend seems very nice so far on intel! Need to add set/get gamma, though not super important as well as make a test for gamma.

Manuel, are these patches still relevant?

@Sam Lantiga: No, they are not relevant anymore. Since this, the KMSDRM backend was rewritten into ATOMIC KMSDRM. This is of no use anymore.

(In reply to Manuel Alfayate Corchete from comment #16)
> @Sam Lantiga: No, they are not relevant anymore. Since this, the KMSDRM
> backend was rewritten into ATOMIC KMSDRM. This is of no use anymore.


Anyway, I would love testing from people and bug catching. I don't have much time, but I would like to hear from @Simon Hug, who originally wrote the KMSDRM-legacy KMSDRM backend, he's very good detecting pointer problems, etc.

I think you're confusing me with someone else. The only thing I contributed to the KMSDRM driver is the patch I uploaded in this issue. At least that's what my memory tells me.

Are we talking about testing the current kmsdrm driver in tip? I have to admit that I didn't look at KMS, DRM, or OpenGL since then, so probably can't be of much help with those parts.

Just to see if I can test it at all I compiled SDL2 with kmsdrm and tried testrendercopyex, but it throws a segfault in SDL_kmsdrmvideo.c:1411. dispdata->crtc is 0. Didn't poke it any further yet. I'm guessing initialization fails somewhere and KMSDRM_VideoQuit gets called anyway. I'll report back if I have more information.

It seems that this new KMSDRM driver drops support for drivers without atomic mode setting. Is this intended? I tested the i915 and nouveau drivers and they apparently can't do it and initialization fails with "no atomic modesetting support". I got it working on the Raspberry Pi with the vc4 driver, but only did some short testing with the existing test applications.

Other small things I noticed while quickly looking through the code:

* Mixed spaces and tabs for indentation.
* C99 syntax. (Is SDL still resisting or is it accepted now?)
* Missing checks after calloc allocations.

@Simon Hug

I believe I have mistaken you, yes. Sorry!

Thanks a lot for your report. Let's see.

-The new ATOMIC interface leaves out hardware not supporting the ATOMIC interface, yes. The non-ATOMIC functions are considered "legacy", and mixing ATOMIC and no-ATOMIC would increase the backend an insane lot. Anyway, ATOMIC is already available on most hardware. This is mainly an Rpi-related effort (dispmanX is gone for good!).
Anyway, it worries me to hear that Nouveau is not working. Doesn't Nouveau support ATOMIC? Are you using a recent kernel and MESA?

-I have just fixed the mixed spaces and tabs for indentation: ONLY four-spaces for indentation now, all over the backend.

-I have also added the missing checks after calloc allocations.

-About the C99 syntax. Do you mean the KMSDRM backend is using some C99 syntax? Is that supposed to be incompatible with any recent GCC? I am not sure what the idea regarding C99 in SDL2 currently is.

@Simon Hug: Also, libSDL should NOT segfault now if the ATOMIC interface is not available: it will gracefully exit with an error telling you about it instead.

It surprised me as well that nouveau doesn't do it. kmscube does show its cube (without the --atomic setting of course), so I know that KMS works to some extent. Kernel is 5.9.1, Mesa is 20.2.1. Maybe it has to do with the older hardware, it's a GeForce GTX 260. I'll try to test it on some newer NVIDIA hardware. Well, the more direct approach would be to look at the nouveau code, but I don't know enough about KMS or DRM to make that search efficient.

And the fix worked. SDL now properly returns the "no atomic modesetting support" error and the test application exits.

Regarding C99: Oh no, GCC does it just fine. I thought SDL is still trying to keep as C90 as possible for compatibility reasons. You'd have to be for people still using the compilers from VS 2010 (obviously not relevant for KMSDRM).

I'll try to read a bit more into the KMSDRM code and report back if I find anything else.