[PATCH] fbdev: find mode with highest refresh rate in fb_find_mode()

Currently if the refresh rate is not specified fb_find_mode() returns
the first known video mode with the requested resoluion, which provides
no guarantees wrt the refresh rate.  Change this so that the mode with
the highest refresh rate is returned instead.

Signed-off-by: Michal Januszewski <spock@gentoo.org>
---

diff --git a/drivers/video/modedb.c b/drivers/video/modedb.c
index 3741ad7..f70143a 100644
--- a/drivers/video/modedb.c
+++ b/drivers/video/modedb.c

@@ -606,26 +606,29 @@ done:
 	DPRINTK("Trying specified video mode%s %ix%i\n",
 	    refresh_specified ? "" : " (ignoring refresh rate)", xres, yres);
 
-	diff = refresh;
+	if (!refresh_specified)
+		diff = 0
+	else
+		diff = refresh;
+
 	best = -1;
 	for (i = 0; i < dbsize; i++) {
-		if (name_matches(db[i], name, namelen) ||
-		    (res_specified && res_matches(db[i], xres, yres))) {
-			if(!fb_try_mode(var, info, &db[i], bpp)) {
-				if(!refresh_specified || db[i].refresh == refresh)
-					return 1;
-				else {
-					if(diff > abs(db[i].refresh - refresh)) {
-						diff = abs(db[i].refresh - refresh);
-						best = i;
-					}
+		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;
+			} else {
+				if (diff < db[i].refresh) {
+					diff = db[i].refresh;
+					best = i;
 				}
 			}
 		}
 	}
 	if (best != -1) {
 		fb_try_mode(var, info, &db[best], bpp);
-		return 2;
+		return (refresh_specified) ? 2 : 1;
 	}
 
 	diff = xres + yres;

On Wed, Jul 18, 2007 at 10:41:02AM +0200, Michal Januszewski wrote:

Currently if the refresh rate is not specified fb_find_mode() returns
the first known video mode with the requested resoluion, which provides
no guarantees wrt the refresh rate.  Change this so that the mode with
the highest refresh rate is returned instead.

What refresh rate it sets when used on card or monitor without DDC?

-- 
Elen sila lumenn' omentielvo

Ondrej 'SanTiago' Zajicek (email: santiago@crfreenet.org, jabber: santiago@njs.netlab.cz)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."

On Wed, 2007-07-18 at 16:38 +0200, Ondrej Zajicek wrote:

On Wed, Jul 18, 2007 at 10:41:02AM +0200, Michal Januszewski wrote:

quoted

Currently if the refresh rate is not specified fb_find_mode() returns
the first known video mode with the requested resoluion, which provides
no guarantees wrt the refresh rate.  Change this so that the mode with
the highest refresh rate is returned instead.

What refresh rate it sets when used on card or monitor without DDC?

Yes, I noted this also while reviewing patches.  fb_find_mode() is used
predominantly with the 'generic' modedb which contains modes that are
not specific to the card or monitor.  And fb_try_mode() is not a
guarantee that the returned refresh rate will be safe (we have a lot of
drivers that do not check the timings against the display capabilities).

It would be best that fb_find_mode() return the safest refresh rate
(60Hz) instead of the highest.

Tony

On Wed, Jul 18, 2007 at 11:18:15PM +0800, Antonino A. Daplas wrote:

quoted

quoted

Currently if the refresh rate is not specified fb_find_mode() returns
the first known video mode with the requested resoluion, which provides
no guarantees wrt the refresh rate.  Change this so that the mode with
the highest refresh rate is returned instead.

What refresh rate it sets when used on card or monitor without DDC?

Yes, I noted this also while reviewing patches.  fb_find_mode() is used
predominantly with the 'generic' modedb which contains modes that are
not specific to the card or monitor.  And fb_try_mode() is not a
guarantee that the returned refresh rate will be safe (we have a lot of
drivers that do not check the timings against the display capabilities).

It would be best that fb_find_mode() return the safest refresh rate
(60Hz) instead of the highest.

How about modifying it so that it looks for a mode with the highest
refresh rate if either a non-generic modedb is used or
info.monspecs.{vfmin,vfmax,hfmin,hfmax,dclkmax} are all non-zero,
and for a mode with refresh rate closest to 60 Hz otherwise?

I'm sorry for the delayed reply.

Best regards.
-- 
Michal Januszewski                              JID: spock@im.gentoo.org
Gentoo Linux Developer                    http://people.gentoo.org/spock


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

On Sun, 2007-08-26 at 21:09 +0200, Michal Januszewski wrote:

On Wed, Jul 18, 2007 at 11:18:15PM +0800, Antonino A. Daplas wrote:

How about modifying it so that it looks for a mode with the highest
refresh rate if either a non-generic modedb is used or
info.monspecs.{vfmin,vfmax,hfmin,hfmax,dclkmax} are all non-zero,
and for a mode with refresh rate closest to 60 Hz otherwise?

I would opt for combining the two, return the highest refresh rate only
if the modedb was built by the driver and with the mode checked against
the display's operating limits.

Tony



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Currently, if the refresh rate is not specified, fb_find_mode() returns
the first known video mode with the requested resolution, which provides
no guarantees wrt the refresh rate.  Change this so that the mode with
the highest refresh rate is returned when the driver provides a custom
video mode database and the monitor limits, and a mode with the safe
60 Hz refresh rate otherwise.

Signed-off-by: Michal Januszewski <spock@gentoo.org>
---

diff --git a/drivers/video/modedb.c b/drivers/video/modedb.c
index 3741ad7..9598c46 100644
--- a/drivers/video/modedb.c
+++ b/drivers/video/modedb.c

@@ -606,26 +606,43 @@ done:
 	DPRINTK("Trying specified video mode%s %ix%i\n",
 	    refresh_specified ? "" : " (ignoring refresh rate)", xres, yres);
 
-	diff = refresh;
+	if (!refresh_specified) {
+		/*
+		 * If the caller has provided a custom mode database and a
+		 * valid monspecs structure, we look for the mode with the
+		 * highest refresh rate.  Otherwise we play it safe it and
+		 * try to find a mode with a refresh rate closest to the
+		 * standard 60 Hz.
+		 */
+		if (db != modedb &&
+		    info->monspecs.vfmin && info->monspecs.vfmax &&
+		    info->monspecs.hfmin && info->monspecs.hfmax &&
+		    info->monspecs.dclkmax) {
+			refresh = 1000;
+		} else {
+			refresh = 60;
+		}
+	}
+
+	diff = -1;
 	best = -1;
 	for (i = 0; i < dbsize; i++) {
-		if (name_matches(db[i], name, namelen) ||
-		    (res_specified && res_matches(db[i], xres, yres))) {
-			if(!fb_try_mode(var, info, &db[i], bpp)) {
-				if(!refresh_specified || db[i].refresh == refresh)
-					return 1;
-				else {
-					if(diff > abs(db[i].refresh - refresh)) {
-						diff = abs(db[i].refresh - refresh);
-						best = i;
-					}
+		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;
+			} else {
+				if (abs(db[i].refresh - refresh) < diff) {
+					diff = abs(db[i].refresh - refresh);
+					best = i;
 				}
 			}
 		}
 	}
 	if (best != -1) {
 		fb_try_mode(var, info, &db[best], bpp);
-		return 2;
+		return (refresh_specified) ? 2 : 1;
 	}
 
 	diff = xres + yres;


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

This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/