We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4432

Summary: The recent changes in drivers/gpu/drm/drm_fb_helper.c (Linux kernel) broke FB_SetVideoMode() for DRM Framebuffers
Product: SDL Reporter: mail
Component: videoAssignee: Ozkan Sezer <sezeroz>
Status: ASSIGNED --- QA Contact: Sam Lantinga <slouken>
Severity: critical    
Priority: P2 CC: sezeroz
Version: 1.2.15   
Hardware: All   
OS: Linux   

Description mail 2018-12-20 11:36:25 UTC 
The commit that broke how it was works: https://github.com/torvalds/linux/commit/db05c481977599236f12a85e55de9f5ab37b0a2c#diff-22d9bb21c28bb1fcddde6c4db23cb3bc

How it works for me (radeondrmfb) now:
diff -ru SDL-1.2.15/src/video/fbcon/SDL_fbvideo.c SDL-1.2.15_fixed/src/video/fbcon/SDL_fbvideo.c
--- SDL-1.2.15/src/video/fbcon/SDL_fbvideo.c    2012-01-19 10:30:06.000000000 +0400
+++ SDL-1.2.15_fixed/src/video/fbcon/SDL_fbvideo.c      2018-12-20 11:54:11.620271894 +0300
@@ -1058,10 +1058,6 @@
                if ( !shadow_fb &&
                                ioctl(console_fd, FBIOPUT_VSCREENINFO, &vinfo) < 0 ) {
                        vinfo.yres_virtual = height;
-                       if ( ioctl(console_fd, FBIOPUT_VSCREENINFO, &vinfo) < 0 ) {
-                               SDL_SetError("Couldn't set console screen info");
-                               return(NULL);
-                       }
                }
        } else {
                int maxheight;

This four lines causes a segmentation fault in the software based on SDL1 when running on the DRM framebuffers now. But all works fine without this lines for DRM framebuffers now.
Comment 1 Ozkan Sezer 2018-12-27 05:33:52 UTC 
AFAICS, that kernel change forbids bits_per_pixel change.
So, how about ignoring the caller's bpp request, something
like the below one-liner? (completely untested !)

diff --git a/src/video/fbcon/SDL_fbvideo.c b/src/video/fbcon/SDL_fbvideo.c
--- a/src/video/fbcon/SDL_fbvideo.c
+++ b/src/video/fbcon/SDL_fbvideo.c
@@ -1031,11 +1031,11 @@ static SDL_Surface *FB_SetVideoMode(_THI
 
        if ( (vinfo.xres != width) || (vinfo.yres != height) ||
             (vinfo.bits_per_pixel != bpp) || (flags & SDL_DOUBLEBUF) ) {
                vinfo.activate = FB_ACTIVATE_NOW;
                vinfo.accel_flags = 0;
-               vinfo.bits_per_pixel = bpp;
+               /*vinfo.bits_per_pixel = bpp;*/
                vinfo.xres = width;
                vinfo.xres_virtual = width;
                vinfo.yres = height;
                if ( flags & SDL_DOUBLEBUF ) {
                        vinfo.yres_virtual = height*2;
Comment 2 mail 2018-12-27 17:05:32 UTC 
It doesn't works in my case. By the way, requested pixel depth the same as actual framebuffer pixel depth.

This bug somehow connected to double buffering. With disabled double buffering the segmentation fault disappears, but the picture turns to mess.
Comment 3 mail 2018-12-27 19:26:31 UTC 
The second patch that works for me:
diff -ru SDL-1.2.15_orig/src/video/fbcon/SDL_fbvideo.c SDL-1.2.15/src/video/fbcon/SDL_fbvideo.c
--- SDL-1.2.15_orig/src/video/fbcon/SDL_fbvideo.c       2012-01-19 10:30:06.000000000 +0400
+++ SDL-1.2.15/src/video/fbcon/SDL_fbvideo.c    2018-12-27 21:59:42.853402012 +0300
@@ -1044,9 +1044,6 @@
                }
                vinfo.xoffset = 0;
                vinfo.yoffset = 0;
-               vinfo.red.length = vinfo.red.offset = 0;
-               vinfo.green.length = vinfo.green.offset = 0;
-               vinfo.blue.length = vinfo.blue.offset = 0;
                vinfo.transp.length = vinfo.transp.offset = 0;
                if ( ! choose_fbmodes_mode(&vinfo) ) {
                        choose_vesa_mode(&vinfo);

Actual values are:
vinfo.red.length = 8
vinfo.green.length = 8
vinfo.blue.length = 8
vinfo.red.offset = 16
vinfo.green.offset = 8
vinfo.blue.offset = 0
Comment 4 Ozkan Sezer 2019-01-06 11:49:13 UTC 
(In reply to mail from comment #3)
> The second patch that works for me:
> diff -ru SDL-1.2.15_orig/src/video/fbcon/SDL_fbvideo.c
> SDL-1.2.15/src/video/fbcon/SDL_fbvideo.c
> --- SDL-1.2.15_orig/src/video/fbcon/SDL_fbvideo.c       2012-01-19
> 10:30:06.000000000 +0400
> +++ SDL-1.2.15/src/video/fbcon/SDL_fbvideo.c    2018-12-27
> 21:59:42.853402012 +0300
> @@ -1044,9 +1044,6 @@
>                 }
>                 vinfo.xoffset = 0;
>                 vinfo.yoffset = 0;
> -               vinfo.red.length = vinfo.red.offset = 0;
> -               vinfo.green.length = vinfo.green.offset = 0;
> -               vinfo.blue.length = vinfo.blue.offset = 0;
>                 vinfo.transp.length = vinfo.transp.offset = 0;
>                 if ( ! choose_fbmodes_mode(&vinfo) ) {
>                         choose_vesa_mode(&vinfo);
> 
> Actual values are:
> vinfo.red.length = 8
> vinfo.green.length = 8
> vinfo.blue.length = 8
> vinfo.red.offset = 16
> vinfo.green.offset = 8
> vinfo.blue.offset = 0


Sam, Ryan:  Is this correct?  Why did we need to zero
those members?
Comment 5 Ozkan Sezer 2019-09-03 20:38:00 UTC 
PING:  Is the patch from comment #3 correct?
Comment 6 Ozkan Sezer 2020-09-22 02:35:15 UTC 
Note: Paul Cercueil <paul@crapouillou.net> reports that:

".. can't reproduce , fbdev has been working fine on top of DRM in our case."