lkml.org 
[lkml]   [2019]   [Jan]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.20 111/111] drm/fb-helper: Ignore the value of fb_var_screeninfo.pixclock
    Date
    4.20-stable review patch.  If anyone has any objections, please let me know.

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

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

    commit 66a8d5bfb518f9f12d47e1d2dce1732279f9451e upstream.

    Strict requirement of pixclock to be zero breaks support of SDL 1.2
    which contains hardcoded table of supported video modes with non-zero
    pixclock values[1].

    To better understand which pixclock values are considered valid and how
    driver should handle these values, I briefly examined few existing fbdev
    drivers and documentation in Documentation/fb/. And it looks like there
    are no strict rules on that and actual behaviour varies:

    * some drivers treat (pixclock == 0) as "use defaults" (uvesafb.c);
    * some treat (pixclock == 0) as invalid value which leads to
    -EINVAL (clps711x-fb.c);
    * some pass converted pixclock value to hardware (uvesafb.c);
    * some are trying to find nearest value from predefined table
    (vga16fb.c, video_gx.c).

    Given this, I believe that it should be safe to just ignore this value if
    changing is not supported. It seems that any portable fbdev application
    which was not written only for one specific device working under one
    specific kernel version should not rely on any particular behaviour of
    pixclock anyway.

    However, while enabling SDL1 applications to work out of the box when
    there is no /etc/fb.modes with valid settings, this change affects the
    video mode choosing logic in SDL. Depending on current screen
    resolution, contents of /etc/fb.modes and resolution requested by
    application, this may lead to user-visible difference (not always):
    image will be displayed in a right way, but it will be aligned to the
    left instead of center. There is no "right behaviour" here as well, as
    emulated fbdev, opposing to old fbdev drivers, simply ignores any
    requsts of video mode changes with resolutions smaller than current.

    The easiest way to reproduce this problem is to install sdl-sopwith[2],
    remove /etc/fb.modes file if it exists, and then try to run sopwith
    from console without X. At least in Fedora 29, sopwith may be simply
    installed from standard repositories.

    [1] SDL 1.2.15 source code, src/video/fbcon/SDL_fbvideo.c, vesa_timings
    [2] http://sdl-sopwith.sourceforge.net/

    Signed-off-by: Ivan Mironov <mironov.ivan@gmail.com>
    Cc: stable@vger.kernel.org
    Fixes: 79e539453b34e ("DRM: i915: add mode setting support")
    Fixes: 771fe6b912fca ("drm/radeon: introduce kernel modesetting for radeon hardware")
    Fixes: 785b93ef8c309 ("drm/kms: move driver specific fb common code to helper functions (v2)")
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Link: https://patchwork.freedesktop.org/patch/msgid/20190108072353.28078-3-mironov.ivan@gmail.com
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    drivers/gpu/drm/drm_fb_helper.c | 7 ++++++-
    1 file changed, 6 insertions(+), 1 deletion(-)

    --- a/drivers/gpu/drm/drm_fb_helper.c
    +++ b/drivers/gpu/drm/drm_fb_helper.c
    @@ -1690,9 +1690,14 @@ int drm_fb_helper_check_var(struct fb_va
    struct drm_fb_helper *fb_helper = info->par;
    struct drm_framebuffer *fb = fb_helper->fb;

    - if (var->pixclock != 0 || in_dbg_master())
    + if (in_dbg_master())
    return -EINVAL;

    + if (var->pixclock != 0) {
    + DRM_DEBUG("fbdev emulation doesn't support changing the pixel clock, value of pixclock is ignored\n");
    + var->pixclock = 0;
    + }
    +
    /*
    * Changes struct fb_var_screeninfo are currently not pushed back
    * to KMS, hence fail if different settings are requested.

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