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; |
