diff mbox

fbdev: Distinguish between interlaced and progressive modes

Message ID 20180622183754.GB17701@localhost.localdomain (mailing list archive)
State New, archived
Headers show

Commit Message

Fredrik Noring June 22, 2018, 6:37 p.m. UTC
With this change fb_find_mode can match interlaced and progressive
modes equally well, and distinguish between for example 1920x1080i
and 1920x1080p.

Signed-off-by: Fredrik Noring <noring@nocrew.org>
---
 drivers/video/fbdev/core/modedb.c | 41 ++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Bartlomiej Zolnierkiewicz July 4, 2018, 5:07 p.m. UTC | #1
Hi,

On Friday, June 22, 2018 08:37:54 PM Fredrik Noring wrote:
> With this change fb_find_mode can match interlaced and progressive
> modes equally well, and distinguish between for example 1920x1080i
> and 1920x1080p.
> 
> Signed-off-by: Fredrik Noring <noring@nocrew.org>

Could you please give some examples of use cases that need this change
(i.e. which fbdev drivers need it for which modes etc.)?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Fredrik Noring July 4, 2018, 7:08 p.m. UTC | #2
Hi Bartlomiej,

> > With this change fb_find_mode can match interlaced and progressive
> > modes equally well, and distinguish between for example 1920x1080i
> > and 1920x1080p.
> > 
> > Signed-off-by: Fredrik Noring <noring@nocrew.org>
> 
> Could you please give some examples of use cases that need this change
> (i.e. which fbdev drivers need it for which modes etc.)?

I discovered the problem when developing a frame buffer driver for the
PlayStation 2 (not yet merged), using the following video modes for the
PlayStation 3 in drivers/video/fbdev/ps3fb.c:

    }, {
	/* 1080if */
	"1080if", 50, 1920, 1080, 13468, 148, 484, 36, 4, 88, 5,
	FB_SYNC_BROADCAST, FB_VMODE_INTERLACED
    }, {
	/* 1080pf */
	"1080pf", 50, 1920, 1080, 6734, 148, 484, 36, 4, 88, 5,
	FB_SYNC_BROADCAST, FB_VMODE_NONINTERLACED
    },

In ps3fb_probe, the mode_option module parameter is used with fb_find_mode
but it can only select the interlaced variant of 1920x1080 since the loop
matching the modes does not take the difference between interlaced and
progressive modes into account.

In short, without the patch, progressive 1920x1080 cannot be chosen as a
mode_option parameter since fb_find_mode (falsely) thinks interlace is a
perfect match.

Fredrik
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bartlomiej Zolnierkiewicz July 24, 2018, 2:38 p.m. UTC | #3
On Wednesday, July 04, 2018 09:08:48 PM Fredrik Noring wrote:
> Hi Bartlomiej,
> 
> > > With this change fb_find_mode can match interlaced and progressive
> > > modes equally well, and distinguish between for example 1920x1080i
> > > and 1920x1080p.
> > > 
> > > Signed-off-by: Fredrik Noring <noring@nocrew.org>
> > 
> > Could you please give some examples of use cases that need this change
> > (i.e. which fbdev drivers need it for which modes etc.)?
> 
> I discovered the problem when developing a frame buffer driver for the
> PlayStation 2 (not yet merged), using the following video modes for the
> PlayStation 3 in drivers/video/fbdev/ps3fb.c:
> 
>     }, {
> 	/* 1080if */
> 	"1080if", 50, 1920, 1080, 13468, 148, 484, 36, 4, 88, 5,
> 	FB_SYNC_BROADCAST, FB_VMODE_INTERLACED
>     }, {
> 	/* 1080pf */
> 	"1080pf", 50, 1920, 1080, 6734, 148, 484, 36, 4, 88, 5,
> 	FB_SYNC_BROADCAST, FB_VMODE_NONINTERLACED
>     },
> 
> In ps3fb_probe, the mode_option module parameter is used with fb_find_mode
> but it can only select the interlaced variant of 1920x1080 since the loop
> matching the modes does not take the difference between interlaced and
> progressive modes into account.
> 
> In short, without the patch, progressive 1920x1080 cannot be chosen as a
> mode_option parameter since fb_find_mode (falsely) thinks interlace is a
> perfect match.

Patch queued for 4.19 (with the above explanation used as patch description),
thanks!

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c
index 2510fa728d77..de119f11b78f 100644
--- a/drivers/video/fbdev/core/modedb.c
+++ b/drivers/video/fbdev/core/modedb.c
@@ -644,7 +644,7 @@  static int fb_try_mode(struct fb_var_screeninfo *var, struct fb_info *info,
  *
  *     Valid mode specifiers for @mode_option:
  *
- *     <xres>x<yres>[M][R][-<bpp>][@<refresh>][i][m] or
+ *     <xres>x<yres>[M][R][-<bpp>][@<refresh>][i][p][m] or
  *     <name>[-<bpp>][@<refresh>]
  *
  *     with <xres>, <yres>, <bpp> and <refresh> decimal numbers and
@@ -653,10 +653,10 @@  static int fb_try_mode(struct fb_var_screeninfo *var, struct fb_info *info,
  *      If 'M' is present after yres (and before refresh/bpp if present),
  *      the function will compute the timings using VESA(tm) Coordinated
  *      Video Timings (CVT).  If 'R' is present after 'M', will compute with
- *      reduced blanking (for flatpanels).  If 'i' is present, compute
- *      interlaced mode.  If 'm' is present, add margins equal to 1.8%
- *      of xres rounded down to 8 pixels, and 1.8% of yres. The char
- *      'i' and 'm' must be after 'M' and 'R'. Example:
+ *      reduced blanking (for flatpanels).  If 'i' or 'p' are present, compute
+ *      interlaced or progressive mode.  If 'm' is present, add margins equal
+ *      to 1.8% of xres rounded down to 8 pixels, and 1.8% of yres. The chars
+ *      'i', 'p' and 'm' must be after 'M' and 'R'. Example:
  *
  *      1024x768MR-8@60m - Reduced blank with margins at 60Hz.
  *
@@ -697,7 +697,8 @@  int fb_find_mode(struct fb_var_screeninfo *var,
 		unsigned int namelen = strlen(name);
 		int res_specified = 0, bpp_specified = 0, refresh_specified = 0;
 		unsigned int xres = 0, yres = 0, bpp = default_bpp, refresh = 0;
-		int yres_specified = 0, cvt = 0, rb = 0, interlace = 0;
+		int yres_specified = 0, cvt = 0, rb = 0;
+		int interlace_specified = 0, interlace = 0;
 		int margins = 0;
 		u32 best, diff, tdiff;
 
@@ -748,9 +749,17 @@  int fb_find_mode(struct fb_var_screeninfo *var,
 				if (!cvt)
 					margins = 1;
 				break;
+			case 'p':
+				if (!cvt) {
+					interlace = 0;
+					interlace_specified = 1;
+				}
+				break;
 			case 'i':
-				if (!cvt)
+				if (!cvt) {
 					interlace = 1;
+					interlace_specified = 1;
+				}
 				break;
 			default:
 				goto done;
@@ -819,11 +828,21 @@  int fb_find_mode(struct fb_var_screeninfo *var,
 			if ((name_matches(db[i], name, namelen) ||
 			     (res_specified && res_matches(db[i], xres, yres))) &&
 			    !fb_try_mode(var, info, &db[i], bpp)) {
-				if (refresh_specified && db[i].refresh == refresh)
-					return 1;
+				const int db_interlace = (db[i].vmode &
+					FB_VMODE_INTERLACED ? 1 : 0);
+				int score = abs(db[i].refresh - refresh);
+
+				if (interlace_specified)
+					score += abs(db_interlace - interlace);
+
+				if (!interlace_specified ||
+				    db_interlace == interlace)
+					if (refresh_specified &&
+					    db[i].refresh == refresh)
+						return 1;
 
-				if (abs(db[i].refresh - refresh) < diff) {
-					diff = abs(db[i].refresh - refresh);
+				if (score < diff) {
+					diff = score;
 					best = i;
 				}
 			}