• Ivan Mironov's avatar
    drm/fb-helper: Ignore the value of fb_var_screeninfo.pixclock · 66a8d5bf
    Ivan Mironov authored
    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: default avatarIvan Mironov <mironov.ivan@gmail.com>
    Cc: stable@vger.kernel.org
    Fixes: 79e53945 ("DRM: i915: add mode setting support")
    Fixes: 771fe6b9 ("drm/radeon: introduce kernel modesetting for radeon hardware")
    Fixes: 785b93ef ("drm/kms: move driver specific fb common code to helper functions (v2)")
    Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
    Link: https://patchwork.freedesktop.org/patch/msgid/20190108072353.28078-3-mironov.ivan@gmail.com
    66a8d5bf
drm_fb_helper.c 91.8 KB