diff options
author | Ivan Mironov <mironov.ivan@gmail.com> | 2019-01-08 02:23:52 -0500 |
---|---|---|
committer | Daniel Vetter <daniel.vetter@ffwll.ch> | 2019-01-10 02:25:36 -0500 |
commit | 62d85b3bf9d978ed4b6b2aeef5cf0ccf1423906e (patch) | |
tree | 03039326cb0cb55336bc5af760d3db5ccea7402b | |
parent | f8c15790e4d8bdf2d21a5e9d43b5f97983af1222 (diff) |
drm/fb-helper: Partially bring back workaround for bugs of SDL 1.2
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
-rw-r--r-- | drivers/gpu/drm/drm_fb_helper.c | 126 |
1 files changed, 73 insertions, 53 deletions
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index d3af098b0922..2d7ce9d3143f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c | |||
@@ -1621,6 +1621,64 @@ static bool drm_fb_pixel_format_equal(const struct fb_var_screeninfo *var_1, | |||
1621 | var_1->transp.msb_right == var_2->transp.msb_right; | 1621 | var_1->transp.msb_right == var_2->transp.msb_right; |
1622 | } | 1622 | } |
1623 | 1623 | ||
1624 | static void drm_fb_helper_fill_pixel_fmt(struct fb_var_screeninfo *var, | ||
1625 | u8 depth) | ||
1626 | { | ||
1627 | switch (depth) { | ||
1628 | case 8: | ||
1629 | var->red.offset = 0; | ||
1630 | var->green.offset = 0; | ||
1631 | var->blue.offset = 0; | ||
1632 | var->red.length = 8; /* 8bit DAC */ | ||
1633 | var->green.length = 8; | ||
1634 | var->blue.length = 8; | ||
1635 | var->transp.offset = 0; | ||
1636 | var->transp.length = 0; | ||
1637 | break; | ||
1638 | case 15: | ||
1639 | var->red.offset = 10; | ||
1640 | var->green.offset = 5; | ||
1641 | var->blue.offset = 0; | ||
1642 | var->red.length = 5; | ||
1643 | var->green.length = 5; | ||
1644 | var->blue.length = 5; | ||
1645 | var->transp.offset = 15; | ||
1646 | var->transp.length = 1; | ||
1647 | break; | ||
1648 | case 16: | ||
1649 | var->red.offset = 11; | ||
1650 | var->green.offset = 5; | ||
1651 | var->blue.offset = 0; | ||
1652 | var->red.length = 5; | ||
1653 | var->green.length = 6; | ||
1654 | var->blue.length = 5; | ||
1655 | var->transp.offset = 0; | ||
1656 | break; | ||
1657 | case 24: | ||
1658 | var->red.offset = 16; | ||
1659 | var->green.offset = 8; | ||
1660 | var->blue.offset = 0; | ||
1661 | var->red.length = 8; | ||
1662 | var->green.length = 8; | ||
1663 | var->blue.length = 8; | ||
1664 | var->transp.offset = 0; | ||
1665 | var->transp.length = 0; | ||
1666 | break; | ||
1667 | case 32: | ||
1668 | var->red.offset = 16; | ||
1669 | var->green.offset = 8; | ||
1670 | var->blue.offset = 0; | ||
1671 | var->red.length = 8; | ||
1672 | var->green.length = 8; | ||
1673 | var->blue.length = 8; | ||
1674 | var->transp.offset = 24; | ||
1675 | var->transp.length = 8; | ||
1676 | break; | ||
1677 | default: | ||
1678 | break; | ||
1679 | } | ||
1680 | } | ||
1681 | |||
1624 | /** | 1682 | /** |
1625 | * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var | 1683 | * drm_fb_helper_check_var - implementation for &fb_ops.fb_check_var |
1626 | * @var: screeninfo to check | 1684 | * @var: screeninfo to check |
@@ -1655,6 +1713,20 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, | |||
1655 | } | 1713 | } |
1656 | 1714 | ||
1657 | /* | 1715 | /* |
1716 | * Workaround for SDL 1.2, which is known to be setting all pixel format | ||
1717 | * fields values to zero in some cases. We treat this situation as a | ||
1718 | * kind of "use some reasonable autodetected values". | ||
1719 | */ | ||
1720 | if (!var->red.offset && !var->green.offset && | ||
1721 | !var->blue.offset && !var->transp.offset && | ||
1722 | !var->red.length && !var->green.length && | ||
1723 | !var->blue.length && !var->transp.length && | ||
1724 | !var->red.msb_right && !var->green.msb_right && | ||
1725 | !var->blue.msb_right && !var->transp.msb_right) { | ||
1726 | drm_fb_helper_fill_pixel_fmt(var, fb->format->depth); | ||
1727 | } | ||
1728 | |||
1729 | /* | ||
1658 | * drm fbdev emulation doesn't support changing the pixel format at all, | 1730 | * drm fbdev emulation doesn't support changing the pixel format at all, |
1659 | * so reject all pixel format changing requests. | 1731 | * so reject all pixel format changing requests. |
1660 | */ | 1732 | */ |
@@ -1967,59 +2039,7 @@ void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helpe | |||
1967 | info->var.yoffset = 0; | 2039 | info->var.yoffset = 0; |
1968 | info->var.activate = FB_ACTIVATE_NOW; | 2040 | info->var.activate = FB_ACTIVATE_NOW; |
1969 | 2041 | ||
1970 | switch (fb->format->depth) { | 2042 | drm_fb_helper_fill_pixel_fmt(&info->var, fb->format->depth); |
1971 | case 8: | ||
1972 | info->var.red.offset = 0; | ||
1973 | info->var.green.offset = 0; | ||
1974 | info->var.blue.offset = 0; | ||
1975 | info->var.red.length = 8; /* 8bit DAC */ | ||
1976 | info->var.green.length = 8; | ||
1977 | info->var.blue.length = 8; | ||
1978 | info->var.transp.offset = 0; | ||
1979 | info->var.transp.length = 0; | ||
1980 | break; | ||
1981 | case 15: | ||
1982 | info->var.red.offset = 10; | ||
1983 | info->var.green.offset = 5; | ||
1984 | info->var.blue.offset = 0; | ||
1985 | info->var.red.length = 5; | ||
1986 | info->var.green.length = 5; | ||
1987 | info->var.blue.length = 5; | ||
1988 | info->var.transp.offset = 15; | ||
1989 | info->var.transp.length = 1; | ||
1990 | break; | ||
1991 | case 16: | ||
1992 | info->var.red.offset = 11; | ||
1993 | info->var.green.offset = 5; | ||
1994 | info->var.blue.offset = 0; | ||
1995 | info->var.red.length = 5; | ||
1996 | info->var.green.length = 6; | ||
1997 | info->var.blue.length = 5; | ||
1998 | info->var.transp.offset = 0; | ||
1999 | break; | ||
2000 | case 24: | ||
2001 | info->var.red.offset = 16; | ||
2002 | info->var.green.offset = 8; | ||
2003 | info->var.blue.offset = 0; | ||
2004 | info->var.red.length = 8; | ||
2005 | info->var.green.length = 8; | ||
2006 | info->var.blue.length = 8; | ||
2007 | info->var.transp.offset = 0; | ||
2008 | info->var.transp.length = 0; | ||
2009 | break; | ||
2010 | case 32: | ||
2011 | info->var.red.offset = 16; | ||
2012 | info->var.green.offset = 8; | ||
2013 | info->var.blue.offset = 0; | ||
2014 | info->var.red.length = 8; | ||
2015 | info->var.green.length = 8; | ||
2016 | info->var.blue.length = 8; | ||
2017 | info->var.transp.offset = 24; | ||
2018 | info->var.transp.length = 8; | ||
2019 | break; | ||
2020 | default: | ||
2021 | break; | ||
2022 | } | ||
2023 | 2043 | ||
2024 | info->var.xres = fb_width; | 2044 | info->var.xres = fb_width; |
2025 | info->var.yres = fb_height; | 2045 | info->var.yres = fb_height; |