lkml.org 
[lkml]   [2019]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.19 33/50] drm/fb-helper: Partially bring back workaround for bugs of SDL 1.2
    Date
    4.19-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Ivan Mironov <mironov.ivan@gmail.com>

    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>

    ---
    drivers/gpu/drm/drm_fb_helper.c | 126 +++++++++++++++++++++++-----------------
    1 file changed, 73 insertions(+), 53 deletions(-)

    --- 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(co
    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
    @@ -1651,6 +1709,20 @@ int drm_fb_helper_check_var(struct fb_va
    }

    /*
    + * 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.
    */
    @@ -1961,59 +2033,7 @@ void drm_fb_helper_fill_var(struct fb_in
    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;

    \
     
     \ /
      Last update: 2019-01-15 17:56    [W:3.116 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site