drm/fb-helper: Partially bring back workaround for bugs of SDL 1.2

commit 62d85b3bf9d978ed4b6b2aeef5cf0ccf1423906e upstream.

SDL 1.2 sets all fields related to the pixel format to zero in some
cases[1]. Prior to commit db05c48197759 ("drm: fb-helper: Reject all
pixel format changing requests"), there was an unintentional workaround
for this that existed for more than a decade. First in device-specific DRM
drivers, then here in drm_fb_helper.c.

Previous code containing this workaround just ignores pixel format fields
from userspace code. Not a good thing either, as this way, driver may
silently use pixel format different from what client actually requested,
and this in turn will lead to displaying garbage on the screen. I think
that returning EINVAL to userspace in this particular case is the right
option, so I decided to left code from problematic commit untouched
instead of just reverting it entirely.

Here is the steps required to reproduce this problem exactly:
	1) Compile fceux[2] with SDL 1.2.15 and without GTK or OpenGL
	   support. SDL should be compiled with fbdev support (which is
	   on by default).
	2) Create /etc/fb.modes with following contents (values seems
	   not used, and just required to trigger problematic code in
	   SDL):

		mode "test"
		    geometry 1 1 1 1 1
		    timings 1 1 1 1 1 1 1
		endmode

	3) Create ~/.fceux/fceux.cfg with following contents:

		SDL.Hotkeys.Quit = 27
		SDL.DoubleBuffering = 1

	4) Ensure that screen resolution is at least 1280x960 (e.g.
	   append "video=Virtual-1:1280x960-32" to the kernel cmdline
	   for qemu/QXL).

	5) Try to run fceux on VT with some ROM file[3]:

		# ./fceux color_test.nes

[1] SDL 1.2.15 source code, src/video/fbcon/SDL_fbvideo.c,
    FB_SetVideoMode()
[2] http://www.fceux.com
[3] Example ROM: https://github.com/bokuweb/rustynes/blob/master/roms/color_test.nes

Reported-by: saahriktu <mail@saahriktu.org>
Suggested-by: saahriktu <mail@saahriktu.org>
Cc: stable@vger.kernel.org
Fixes: db05c48197759 ("drm: fb-helper: Reject all pixel format changing requests")
Signed-off-by: Ivan Mironov <mironov.ivan@gmail.com>
[danvet: Delete misleading comment.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20190108072353.28078-2-mironov.ivan@gmail.com
Link: https://patchwork.freedesktop.org/patch/msgid/20190108072353.28078-2-mironov.ivan@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 29d1d3df31642f19d49e7c4ff04a0d25d2b0bdb9..ad6812baa6111b30f6123cf84501849fb4656a20 100644 (file)
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1509,6 +1509,64 @@ static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1,
               var_1->transp.msb_right == var_2->transp.msb_right;
 }
 
+static void drm_fb_helper_fill_pixel_fmt(struct fb_var_screeninfo *var,
+                                        u8 depth)
+{
+       switch (depth) {
+       case 8:
+               var->red.offset = 0;
+               var->green.offset = 0;
+               var->blue.offset = 0;
+               var->red.length = 8; /* 8bit DAC */
+               var->green.length = 8;
+               var->blue.length = 8;
+               var->transp.offset = 0;
+               var->transp.length = 0;
+               break;
+       case 15:
+               var->red.offset = 10;
+               var->green.offset = 5;
+               var->blue.offset = 0;
+               var->red.length = 5;
+               var->green.length = 5;
+               var->blue.length = 5;
+               var->transp.offset = 15;
+               var->transp.length = 1;
+               break;
+       case 16:
+               var->red.offset = 11;
+               var->green.offset = 5;
+               var->blue.offset = 0;
+               var->red.length = 5;
+               var->green.length = 6;
+               var->blue.length = 5;
+               var->transp.offset = 0;
+               break;
+       case 24:
+               var->red.offset = 16;
+               var->green.offset = 8;
+               var->blue.offset = 0;
+               var->red.length = 8;
+               var->green.length = 8;
+               var->blue.length = 8;
+               var->transp.offset = 0;
+               var->transp.length = 0;
+               break;
+       case 32:
+               var->red.offset = 16;
+               var->green.offset = 8;
+               var->blue.offset = 0;
+               var->red.length = 8;
+               var->green.length = 8;
+               var->blue.length = 8;
+               var->transp.offset = 24;
+               var->transp.length = 8;
+               break;
+       default:
+               break;
+       }
+}
+
 /**
  * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var
  * @var: screeninfo to check
@@ -1538,6 +1596,20 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
                return -EINVAL;
        }
 
+       /*
+        * Workaround for SDL 1.2, which is known to be setting all pixel format
+        * fields values to zero in some cases. We treat this situation as a
+        * kind of "use some reasonable autodetected values".
+        */
+       if (!var->red.offset     && !var->green.offset    &&
+           !var->blue.offset    && !var->transp.offset   &&
+           !var->red.length     && !var->green.length    &&
+           !var->blue.length    && !var->transp.length   &&
+           !var->red.msb_right  && !var->green.msb_right &&
+           !var->blue.msb_right && !var->transp.msb_right) {
+               drm_fb_helper_fill_pixel_fmt(var, fb->format->depth);
+       }
+
        /*
         * drm fbdev emulation doesn't support changing the pixel format at all,
         * so reject all pixel format changing requests.
@@ -1848,59 +1920,7 @@ void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helpe
        info->var.yoffset = 0;
        info->var.activate = FB_ACTIVATE_NOW;
 
-       switch (fb->format->depth) {
-       case 8:
-               info->var.red.offset = 0;
-               info->var.green.offset = 0;
-               info->var.blue.offset = 0;
-               info->var.red.length = 8; /* 8bit DAC */
-               info->var.green.length = 8;
-               info->var.blue.length = 8;
-               info->var.transp.offset = 0;
-               info->var.transp.length = 0;
-               break;
-       case 15:
-               info->var.red.offset = 10;
-               info->var.green.offset = 5;
-               info->var.blue.offset = 0;
-               info->var.red.length = 5;
-               info->var.green.length = 5;
-               info->var.blue.length = 5;
-               info->var.transp.offset = 15;
-               info->var.transp.length = 1;
-               break;
-       case 16:
-               info->var.red.offset = 11;
-               info->var.green.offset = 5;
-               info->var.blue.offset = 0;
-               info->var.red.length = 5;
-               info->var.green.length = 6;
-               info->var.blue.length = 5;
-               info->var.transp.offset = 0;
-               break;
-       case 24:
-               info->var.red.offset = 16;
-               info->var.green.offset = 8;
-               info->var.blue.offset = 0;
-               info->var.red.length = 8;
-               info->var.green.length = 8;
-               info->var.blue.length = 8;
-               info->var.transp.offset = 0;
-               info->var.transp.length = 0;
-               break;
-       case 32:
-               info->var.red.offset = 16;
-               info->var.green.offset = 8;
-               info->var.blue.offset = 0;
-               info->var.red.length = 8;
-               info->var.green.length = 8;
-               info->var.blue.length = 8;
-               info->var.transp.offset = 24;
-               info->var.transp.length = 8;
-               break;
-       default:
-               break;
-       }
+       drm_fb_helper_fill_pixel_fmt(&info->var, fb->format->depth);
 
        info->var.xres = fb_width;
        info->var.yres = fb_height;