Wrong IOCTL used for VSYNC

[SDLBugzilla]

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Patch to address the issue (sdl-fbcon-waitforvsync.patch, text/plain, 2013-05-20 19:04:37 +0000, 624 bytes)

Reported in version: 1.2.15
Reported for operating system, platform: Linux, All

Comments on the original bug report:

On 2013-05-20 19:04:37 +0000, Paul Cercueil wrote:

Created attachment 1145
Patch to address the issue

The fbcon backend uses a non-standard IOCTL to wait for the vertical refresh. The Linux kernel provides a standard IOCTL for that task that should be used instead.
See the attached patch.

On 2015-08-25 09:38:22 +0000, Ryan C. Gordon wrote:

Hello, and sorry if you're getting several copies of this message by email, since we are closing many bugs at once here.

We have decided to mark all SDL 1.2-related bugs as RESOLVED ENDOFLIFE, as we don't intend to work on SDL 1.2 any further, but didn't want to mark a large quantity of bugs as RESOLVED WONTFIX, to clearly show what was left unattended to and make it easily searchable.

Our current focus is on SDL 2.0.

If you are still having problems with an ENDOFLIFE bug, your absolute best option is to move your program to SDL2, as it will likely fix the problem by default, and give you access to modern platforms and tons of super-cool new features.

Failing that, we will accept small patches to fix these issues, and put them in revision control, although we do not intend to do any further official 1.2 releases.

Failing that, please feel free to contact me directly by email (icculus@icculus.org) and we'll try to find some way to help you out of your situation.

Thank you,
--ryan.

[sezero]

@pcercuei: You removed non-existent FBIOWAITRETRACE with commit
98cd856
But you did not apply your FBIO_WAITFORVSYNC patch from this bug:
Was it intentional? Should we not apply this?

[pcercuei]

It was intentional. Our fbdev driver at that time had the buffer flip synchronized to VSYNC, so using FBIO_WAITFORVSYNC would halve the framerate to half the screen's refresh rate (by waiting on VSYNC twice). The reason for that is that using two separate IOCTLs to vsync and flip buffers meant that there was no guarantee that the buffer flip would happen soon enough after vsync, sometimes resulting in visible tearing.

While this was a specificity of our fbdev driver, it is now the default behaviour of all atomic DRM drivers, so this still applies.

[sezero]

@pcercuei: Confused a little - should the patch be applied or not needed now?

(Inlined the patch below for convenience)

diff --git a/src/video/fbcon/SDL_fbvideo.c b/src/video/fbcon/SDL_fbvideo.c
index 27fcceb..ed1a06a 100644
--- a/src/video/fbcon/SDL_fbvideo.c
+++ b/src/video/fbcon/SDL_fbvideo.c
@@ -34,6 +34,7 @@
 #include <asm/page.h>		/* For definition of PAGE_SIZE */
 #endif
 
+#include <linux/fb.h>
 #include <linux/vt.h>
 
 #include "SDL_video.h"
@@ -1461,6 +1462,10 @@ static void FB_UnlockHWSurface(_THIS, SDL_Surface *surface)
 
 static void FB_WaitVBL(_THIS)
 {
+#ifdef FBIO_WAITFORVSYNC
+	unsigned int screen = 0;
+	ioctl(console_fd, FBIO_WAITFORVSYNC, &screen);
+#endif
 }
 
 static void FB_WaitIdle(_THIS)

[pcercuei]

@sezero no need.

[sezero]

OK, closing as outdated