[PATCH v2 1/2] drm/fb-helper: Partially bring back workaround for bugs of SDL 1.2

[PATCH v2 1/2] drm/fb-helper: Partially bring back workaround for bugs of SDL 1.2

[Ivan Mironov]

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>
---
 drivers/gpu/drm/drm_fb_helper.c | 142 ++++++++++++++++++++------------
 1 file changed, 89 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d3af098b0922..ed7e91423258 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,
 	       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
@@ -1654,6 +1712,36 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
 		return -EINVAL;
 	}
 
+	/*
+	 * 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) {
+		/*
+		 * There is no way to guess the right value for depth when
+		 * bits_per_pixel is 16 or 32. Instead of restoring the
+		 * behaviour previously introduced here by commit 785b93ef8c309,
+		 * we decided to just use the current depth and do not perform
+		 * any guessing.
+		 *
+		 * Also, if requested bits_per_pixel differs from current,
+		 * the next call to drm_fb_pixel_format_equal() will fail
+		 * resulting in EINVAL error from ioctl().
+		 *
+		 * However, this still leaves the theoretical possibility of
+		 * situation when userspace app requests different pixel format
+		 * and ioctl() call silently succeeds. Garbage will be displayed
+		 * in this case.
+		 */
+		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.
@@ -1967,59 +2055,7 @@ void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helpe
 	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;
-- 
2.20.1

Re: [PATCH v2 1/2] drm/fb-helper: Partially bring back workaround for bugs of SDL 1.2

[Daniel Vetter]

On Tue, Jan 08, 2019 at 12:23:52PM +0500, Ivan Mironov wrote:
> 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>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 142 ++++++++++++++++++++------------
>  1 file changed, 89 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index d3af098b0922..ed7e91423258 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,
>  	       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
> @@ -1654,6 +1712,36 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * 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) {
> +		/*
> +		 * There is no way to guess the right value for depth when
> +		 * bits_per_pixel is 16 or 32. Instead of restoring the
> +		 * behaviour previously introduced here by commit 785b93ef8c309,
> +		 * we decided to just use the current depth and do not perform
> +		 * any guessing.
> +		 *
> +		 * Also, if requested bits_per_pixel differs from current,
> +		 * the next call to drm_fb_pixel_format_equal() will fail
> +		 * resulting in EINVAL error from ioctl().
> +		 *
> +		 * However, this still leaves the theoretical possibility of
> +		 * situation when userspace app requests different pixel format
> +		 * and ioctl() call silently succeeds. Garbage will be displayed
> +		 * in this case.
> +		 */

I deleted this comment since I think it's misleading: We only need depth
to decided for the pixel format when userspace hasn't filled it out. We
could also change the function to instead pass fb->format, and use that as
the key to fill out the fbdev format information. That's why I requested
you keep Ville's change here, it was a bugfix.

So ther's no guessing going on here at all, and nothing else fancy: All we
do is fill out the fbdev pixel format information if userspace failed to
supply it. That's imo clear enough without a comment.

Applied both comments for 5.0 drm-misc-fixes, thanks a lot for your
patches and debug work.

Cheers, Daniel

> +		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.
> @@ -1967,59 +2055,7 @@ void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helpe
>  	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;
> -- 
> 2.20.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v2 2/2] drm/fb-helper: Ignore the value of fb_var_screeninfo.pixclock

[Ivan Mironov]

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)")
---
 drivers/gpu/drm/drm_fb_helper.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index ed7e91423258..2d4c2b38508e 100644
--- 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_var_screeninfo *var,
 	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;
+	}
+
 	if ((drm_format_info_block_width(fb->format, 0) > 1) ||
 	    (drm_format_info_block_height(fb->format, 0) > 1))
 		return -EINVAL;
-- 
2.20.1

Re: [PATCH v2 2/2] drm/fb-helper: Ignore the value of fb_var_screeninfo.pixclock

[Ivan Mironov]

On Wed, 2019-01-09 at 15:52 +0000, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: 79e539453b34 DRM: i915: add mode setting support.
> 
> The bot has tested the following trees: v4.20.0, v4.19.13, v4.14.91, v4.9.148, v4.4.169, v3.18.131.
> 
> v4.20.0: Failed to apply! Possible dependencies:
>     042bf753842d ("drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info")
>     9c71a6686bfa ("drm: fourcc: Convert drm_format_info kerneldoc to in-line member documentation")
> 
> v4.19.13: Failed to apply! Possible dependencies:
>     042bf753842d ("drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info")
>     9c71a6686bfa ("drm: fourcc: Convert drm_format_info kerneldoc to in-line member documentation")
>     c76abab59b3c ("drm: Use horizontal and vertical chroma subsampling factor while calculating offsets in the physical address of framebuffer")
> 
> v4.14.91: Failed to apply! Possible dependencies:
>     042bf753842d ("drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info")
>     4cc4e1b40f3f ("drm/fourcc: Add a alpha field to drm_format_info")
>     9c71a6686bfa ("drm: fourcc: Convert drm_format_info kerneldoc to in-line member documentation")
>     c76abab59b3c ("drm: Use horizontal and vertical chroma subsampling factor while calculating offsets in the physical address of framebuffer")
>     ce2d54619a10 ("drm/fourcc: Add is_yuv field to drm_format_info to denote if the format is yuv")
> 
> v4.9.148: Failed to apply! Possible dependencies:
>     042bf753842d ("drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info")
>     05fc03217e08 ("drm/mm: Some doc polish")
>     06df8ac682e6 ("drm: kselftest for drm_mm_debug()")
>     14d7f96f90fb ("drm/fb_cma_helper: Add drm_fb_cma_prepare_fb() helper")
>     2bd966d106e3 ("drm: kselftest for drm_mm_replace_node()")
>     2fba0de0a9ec ("drm: kselftest for drm_mm_insert_node_in_range()")
>     393b50f30566 ("drm: kselftest for drm_mm_init()")
>     4636ce93d5b2 ("drm/fb-cma-helper: Add drm_fb_cma_get_gem_addr()")
>     50f0033d1a0f ("drm: Add some kselftests for the DRM range manager (struct drm_mm)")
>     5628648df755 ("drm/fb-cma-helper: Use drm_gem_framebuffer_helper")
>     5705670d0463 ("drm: Track drm_mm allocators and show leaks on shutdown")
>     6259a56ba0e1 ("drm: Add asserts to catch overflow in drm_mm_init() and drm_mm_init_scan()")
>     62a0d98a188c ("drm: allow to use mmuless SoC")
>     72a93e8dd52c ("drm: Take ownership of the dmabuf->obj when exporting")
>     7886692a5804 ("drm: kselftest for drm_mm_insert_node()")
>     900537dc3889 ("drm: kselftest for drm_mm_reserve_node()")
>     940eba2d58a7 ("drm/gem|prime|mm: Use recommened kerneldoc for struct member refs")
>     9a71e277888b ("drm: Extract struct drm_mm_scan from struct drm_mm")
>     9b26f2ed29f8 ("drm: kselftest for drm_mm and alignment")
>     b112481bb327 ("drm/cma-helper: simplify setup for drivers with ->dirty callbacks")
>     b3ee963fe41d ("drm: Compile time enabling for asserts in drm_mm")
>     ba004e39b199 ("drm: Fix kerneldoc for drm_mm_scan_remove_block()")
>     c76abab59b3c ("drm: Use horizontal and vertical chroma subsampling factor while calculating offsets in the physical address of framebuffer")
>     e6b62714e87c ("drm: Introduce drm_gem_object_{get,put}()")
> 
> v4.4.169: Failed to apply! Possible dependencies:
>     042bf753842d ("drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info")
>     14d7f96f90fb ("drm/fb_cma_helper: Add drm_fb_cma_prepare_fb() helper")
>     199c77179c87 ("drm/fb-cma-helper: Add fb_deferred_io support")
>     1eb83451ba55 ("drm: Pass the user drm_mode_fb_cmd2 as const to .fb_create()")
>     4636ce93d5b2 ("drm/fb-cma-helper: Add drm_fb_cma_get_gem_addr()")
>     5628648df755 ("drm/fb-cma-helper: Use drm_gem_framebuffer_helper")
>     70c0616d5a84 ("drm/fb_cma_helper: remove duplicate const from drm_fb_cma_alloc")
>     b112481bb327 ("drm/cma-helper: simplify setup for drivers with ->dirty callbacks")
>     c76abab59b3c ("drm: Use horizontal and vertical chroma subsampling factor while calculating offsets in the physical address of framebuffer")
>     ce0c57576810 ("drm/fb_cma_helper: Implement fb_mmap callback")
>     fdce184609ee ("drm/fb-cma-helper: Use const for drm_framebuffer_funcs argument")
> 
> v3.18.131: Failed to apply! Possible dependencies:
>     042bf753842d ("drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info")
>     14d7f96f90fb ("drm/fb_cma_helper: Add drm_fb_cma_prepare_fb() helper")
>     199c77179c87 ("drm/fb-cma-helper: Add fb_deferred_io support")
>     1a396789f65a ("drm: add Atmel HLCDC Display Controller support")
>     1eb83451ba55 ("drm: Pass the user drm_mode_fb_cmd2 as const to .fb_create()")
>     2a8cb4894540 ("drm/exynos: merge exynos_drm_buf.c to exynos_drm_gem.c")
>     2b8376c803c4 ("drm/exynos: remove struct exynos_drm_encoder layer")
>     39a839f2e651 ("drm/exynos: s/exynos_gem_obj/obj in exynos_drm_fbdev.c")
>     421ee18d4e04 ("drm/exynos: fix null pointer dereference issue")
>     4636ce93d5b2 ("drm/fb-cma-helper: Add drm_fb_cma_get_gem_addr()")
>     4846e4520849 ("drm/exynos: clean up machine compatible string check")
>     5628648df755 ("drm/fb-cma-helper: Use drm_gem_framebuffer_helper")
>     5cbb37df378d ("drm/exynos: resolve infinite loop issue on multi-platform")
>     70c0616d5a84 ("drm/fb_cma_helper: remove duplicate const from drm_fb_cma_alloc")
>     7239067795dc ("drm/exynos: remove ifdeferry from initialization code")
>     7ded85885d49 ("drm/exynos: remove superfluous error messages")
>     813fd67b57ff ("drm/exynos: cleanup name of gem object for exynos_drm")
>     820687befec4 ("drm/exynos: move Exynos platform drivers registration to init")
>     94e30d93f936 ("drm/exynos: remove exynos_drm_fb_set_buf_cnt()")
>     96976c3d9aff ("drm/exynos: Add DECON driver")
>     b74ea6a97e82 ("drm/exynos: remove DRM_EXYNOS_DMABUF config")
>     c76abab59b3c ("drm: Use horizontal and vertical chroma subsampling factor while calculating offsets in the physical address of framebuffer")
>     ce0c57576810 ("drm/fb_cma_helper: Implement fb_mmap callback")
>     cf67cc9a29ac ("drm/exynos: remove struct exynos_drm_display")
>     d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
>     d56125afcbdf ("drm/exynos: update exynos_drm_framebuffer_init() for multiple buffers")
>     e9fbdcb45a36 ("drm/exynos: fix possible infinite loop issue")
> 
> 
> How should we proceed with this patch?
> 
> --
> Thanks,
> Sasha

Hi,

I'm new to kernel development, so: what exactly I'm supposed to do in
such case? Rebase my patch on top of older versions and then resend
patches somewhere?

Just checked the v3.18.131. Apparently code in question was not changed
since then, so manual rebase would be trivial.

Re: [PATCH v2 2/2] drm/fb-helper: Ignore the value of fb_var_screeninfo.pixclock

[Daniel Vetter]

On Fri, Jan 11, 2019 at 9:19 AM Ivan Mironov <mironov.ivan@gmail.com> wrote:
>
> On Wed, 2019-01-09 at 15:52 +0000, Sasha Levin wrote:
> > Hi,
> >
> > [This is an automated email]
> >
> > This commit has been processed because it contains a "Fixes:" tag,
> > fixing commit: 79e539453b34 DRM: i915: add mode setting support.
> >
> > The bot has tested the following trees: v4.20.0, v4.19.13, v4.14.91, v4.9.148, v4.4.169, v3.18.131.
> >
> > v4.20.0: Failed to apply! Possible dependencies:
> >     042bf753842d ("drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info")
> >     9c71a6686bfa ("drm: fourcc: Convert drm_format_info kerneldoc to in-line member documentation")
> >
> > v4.19.13: Failed to apply! Possible dependencies:
> >     042bf753842d ("drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info")
> >     9c71a6686bfa ("drm: fourcc: Convert drm_format_info kerneldoc to in-line member documentation")
> >     c76abab59b3c ("drm: Use horizontal and vertical chroma subsampling factor while calculating offsets in the physical address of framebuffer")
> >
> > v4.14.91: Failed to apply! Possible dependencies:
> >     042bf753842d ("drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info")
> >     4cc4e1b40f3f ("drm/fourcc: Add a alpha field to drm_format_info")
> >     9c71a6686bfa ("drm: fourcc: Convert drm_format_info kerneldoc to in-line member documentation")
> >     c76abab59b3c ("drm: Use horizontal and vertical chroma subsampling factor while calculating offsets in the physical address of framebuffer")
> >     ce2d54619a10 ("drm/fourcc: Add is_yuv field to drm_format_info to denote if the format is yuv")
> >
> > v4.9.148: Failed to apply! Possible dependencies:
> >     042bf753842d ("drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info")
> >     05fc03217e08 ("drm/mm: Some doc polish")
> >     06df8ac682e6 ("drm: kselftest for drm_mm_debug()")
> >     14d7f96f90fb ("drm/fb_cma_helper: Add drm_fb_cma_prepare_fb() helper")
> >     2bd966d106e3 ("drm: kselftest for drm_mm_replace_node()")
> >     2fba0de0a9ec ("drm: kselftest for drm_mm_insert_node_in_range()")
> >     393b50f30566 ("drm: kselftest for drm_mm_init()")
> >     4636ce93d5b2 ("drm/fb-cma-helper: Add drm_fb_cma_get_gem_addr()")
> >     50f0033d1a0f ("drm: Add some kselftests for the DRM range manager (struct drm_mm)")
> >     5628648df755 ("drm/fb-cma-helper: Use drm_gem_framebuffer_helper")
> >     5705670d0463 ("drm: Track drm_mm allocators and show leaks on shutdown")
> >     6259a56ba0e1 ("drm: Add asserts to catch overflow in drm_mm_init() and drm_mm_init_scan()")
> >     62a0d98a188c ("drm: allow to use mmuless SoC")
> >     72a93e8dd52c ("drm: Take ownership of the dmabuf->obj when exporting")
> >     7886692a5804 ("drm: kselftest for drm_mm_insert_node()")
> >     900537dc3889 ("drm: kselftest for drm_mm_reserve_node()")
> >     940eba2d58a7 ("drm/gem|prime|mm: Use recommened kerneldoc for struct member refs")
> >     9a71e277888b ("drm: Extract struct drm_mm_scan from struct drm_mm")
> >     9b26f2ed29f8 ("drm: kselftest for drm_mm and alignment")
> >     b112481bb327 ("drm/cma-helper: simplify setup for drivers with ->dirty callbacks")
> >     b3ee963fe41d ("drm: Compile time enabling for asserts in drm_mm")
> >     ba004e39b199 ("drm: Fix kerneldoc for drm_mm_scan_remove_block()")
> >     c76abab59b3c ("drm: Use horizontal and vertical chroma subsampling factor while calculating offsets in the physical address of framebuffer")
> >     e6b62714e87c ("drm: Introduce drm_gem_object_{get,put}()")
> >
> > v4.4.169: Failed to apply! Possible dependencies:
> >     042bf753842d ("drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info")
> >     14d7f96f90fb ("drm/fb_cma_helper: Add drm_fb_cma_prepare_fb() helper")
> >     199c77179c87 ("drm/fb-cma-helper: Add fb_deferred_io support")
> >     1eb83451ba55 ("drm: Pass the user drm_mode_fb_cmd2 as const to .fb_create()")
> >     4636ce93d5b2 ("drm/fb-cma-helper: Add drm_fb_cma_get_gem_addr()")
> >     5628648df755 ("drm/fb-cma-helper: Use drm_gem_framebuffer_helper")
> >     70c0616d5a84 ("drm/fb_cma_helper: remove duplicate const from drm_fb_cma_alloc")
> >     b112481bb327 ("drm/cma-helper: simplify setup for drivers with ->dirty callbacks")
> >     c76abab59b3c ("drm: Use horizontal and vertical chroma subsampling factor while calculating offsets in the physical address of framebuffer")
> >     ce0c57576810 ("drm/fb_cma_helper: Implement fb_mmap callback")
> >     fdce184609ee ("drm/fb-cma-helper: Use const for drm_framebuffer_funcs argument")
> >
> > v3.18.131: Failed to apply! Possible dependencies:
> >     042bf753842d ("drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info")
> >     14d7f96f90fb ("drm/fb_cma_helper: Add drm_fb_cma_prepare_fb() helper")
> >     199c77179c87 ("drm/fb-cma-helper: Add fb_deferred_io support")
> >     1a396789f65a ("drm: add Atmel HLCDC Display Controller support")
> >     1eb83451ba55 ("drm: Pass the user drm_mode_fb_cmd2 as const to .fb_create()")
> >     2a8cb4894540 ("drm/exynos: merge exynos_drm_buf.c to exynos_drm_gem.c")
> >     2b8376c803c4 ("drm/exynos: remove struct exynos_drm_encoder layer")
> >     39a839f2e651 ("drm/exynos: s/exynos_gem_obj/obj in exynos_drm_fbdev.c")
> >     421ee18d4e04 ("drm/exynos: fix null pointer dereference issue")
> >     4636ce93d5b2 ("drm/fb-cma-helper: Add drm_fb_cma_get_gem_addr()")
> >     4846e4520849 ("drm/exynos: clean up machine compatible string check")
> >     5628648df755 ("drm/fb-cma-helper: Use drm_gem_framebuffer_helper")
> >     5cbb37df378d ("drm/exynos: resolve infinite loop issue on multi-platform")
> >     70c0616d5a84 ("drm/fb_cma_helper: remove duplicate const from drm_fb_cma_alloc")
> >     7239067795dc ("drm/exynos: remove ifdeferry from initialization code")
> >     7ded85885d49 ("drm/exynos: remove superfluous error messages")
> >     813fd67b57ff ("drm/exynos: cleanup name of gem object for exynos_drm")
> >     820687befec4 ("drm/exynos: move Exynos platform drivers registration to init")
> >     94e30d93f936 ("drm/exynos: remove exynos_drm_fb_set_buf_cnt()")
> >     96976c3d9aff ("drm/exynos: Add DECON driver")
> >     b74ea6a97e82 ("drm/exynos: remove DRM_EXYNOS_DMABUF config")
> >     c76abab59b3c ("drm: Use horizontal and vertical chroma subsampling factor while calculating offsets in the physical address of framebuffer")
> >     ce0c57576810 ("drm/fb_cma_helper: Implement fb_mmap callback")
> >     cf67cc9a29ac ("drm/exynos: remove struct exynos_drm_display")
> >     d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
> >     d56125afcbdf ("drm/exynos: update exynos_drm_framebuffer_init() for multiple buffers")
> >     e9fbdcb45a36 ("drm/exynos: fix possible infinite loop issue")
> >
> >
> > How should we proceed with this patch?
> >
> > --
> > Thanks,
> > Sasha
>
> Hi,
>
> I'm new to kernel development, so: what exactly I'm supposed to do in
> such case? Rebase my patch on top of older versions and then resend
> patches somewhere?
>
> Just checked the v3.18.131. Apparently code in question was not changed
> since then, so manual rebase would be trivial.

If you want to do that work of backporting to older kernels, then:
- grab latest point release of each such kernel you care about.
- cherry-pick the upstream commit with git cherry-pick -x (the -x is
required by stable kernel release rules)
- add a note at the bottom that this is the manual backport for $kernel_release
- submit to stable team

https://www.kernel.org/doc/html/v4.14/process/stable-kernel-rules.html#option-3

Cheers, Daniel

>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch