[PATCH] udlfb - Fix for problem with monitor resolution detection /

[PATCH] udlfb - Fix for problem with monitor resolution detection /

[Katsak, William Alexander (Bill)]

[-- Attachment #1: Type: text/plain, Size: 1211 bytes --]

Hello,

This is my first contribution on this list, so please let me know if I am off with any procedures or anything.

This patch fixes a problem where a DisplayLink device autoselects a suboptimal framebuffer resolution. The problem occured with a Plugable UGA-2K-A connected to a Samsung EX2220X display. In this situation, the driver always allocates a 1600x1200 framebuffer, even after it indicates in its output that 1920x1080 is a valid mode. If my interpretation of the code is correct, the problem was in the block that selects the best mode. The existing code unset the FB_MISC_1ST_DETAIL flag if ANY of the modes is determined to be invalid (on my adapter, 1680x1680 is invalid). This in turn causes the fb_find_best_display() function to disregard the first mode (1920x1080). The comment for the line that unsets the flag implies that this should happen if we have removed the top/best mode...however, this else block is not qualified, and happens if we remove any mode. I simply put in a condition so that we only unset this flag if the first mode is invalid.

Thanks in advance for your feedback.

Bill Katsak
Bell Laboratories
Alcatel-Lucent, Inc.
william.katsak@alcatel-lucent.com


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: udlfb_resolution_fix.patch --]
[-- Type: text/x-patch; name="udlfb_resolution_fix.patch", Size: 1499 bytes --]

From c3327c41877637e80f9f06a8ce3bdcfe8b4e6fa4 Mon Sep 17 00:00:00 2001
From: William Katsak <william.katsak@alcatel-lucent.com>
Date: Wed, 22 Jun 2011 12:48:11 -0400
Subject: [PATCH] This patch fixes a problem where a DisplayLink device autoselects a
  suboptimal framebuffer resolution.

The situation in which the problem occurred was with a Plugable UGA-2K-A
connected to a Samsung EX2220X display. The driver indicates that
1920x1080 is a valid mode (the first mode available, in fact), but
proceeds to set the framebuffer size to 1600x1200.

The patch corrects what seems to be a logic error, regarding unsetting
the FB_MISC_1ST_DETAIL flag, if the first (top/best) mode is invalid.
The existing code unset the flag if ANY mode was invalid.
---
 drivers/video/udlfb.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c
index 52b0f3e..816a4fd 100644
--- a/drivers/video/udlfb.c
+++ b/drivers/video/udlfb.c
@@ -1233,8 +1233,12 @@ static int dlfb_setup_modes(struct dlfb_data *dev,
 			if (dlfb_is_valid_mode(&info->monspecs.modedb[i], info))
 				fb_add_videomode(&info->monspecs.modedb[i],
 					&info->modelist);
-			else /* if we've removed top/best mode */
-				info->monspecs.misc &= ~FB_MISC_1ST_DETAIL;
+			else {
+				if (i == 0)
+					/* if we've removed top/best mode */
+					info->monspecs.misc
+						&= ~FB_MISC_1ST_DETAIL;
+			}
 		}
 
 		default_vmode = fb_find_best_display(&info->monspecs,
-- 
1.7.3.4

Re: [PATCH] udlfb - Fix for problem with monitor resolution detection

[Florian Tobias Schandinat]

Hi,

On 06/22/2011 05:15 PM, Katsak, William Alexander (Bill) wrote:
> This is my first contribution on this list, so please let me know if I am off with any procedures or anything.

Please send patches inline to make review easier, preferably using "git 
send-email" to avoid broken patches.

>  From c3327c41877637e80f9f06a8ce3bdcfe8b4e6fa4 Mon Sep 17 00:00:00 2001
> From: William Katsak<william.katsak@alcatel-lucent.com>
> Date: Wed, 22 Jun 2011 12:48:11 -0400
> Subject: [PATCH] This patch fixes a problem where a DisplayLink device autoselects a
>    suboptimal framebuffer resolution.
>
> The situation in which the problem occurred was with a Plugable UGA-2K-A
> connected to a Samsung EX2220X display. The driver indicates that
> 1920x1080 is a valid mode (the first mode available, in fact), but
> proceeds to set the framebuffer size to 1600x1200.
>
> The patch corrects what seems to be a logic error, regarding unsetting
> the FB_MISC_1ST_DETAIL flag, if the first (top/best) mode is invalid.
> The existing code unset the flag if ANY mode was invalid.

You're missing a Signed-off-line, see Documentation/SubmittingPatches in your 
kernel sources.


Best regards,

Florian Tobias Schandinat

> ---
>   drivers/video/udlfb.c |    8 ++++++--
>   1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c
> index 52b0f3e..816a4fd 100644
> --- a/drivers/video/udlfb.c
> +++ b/drivers/video/udlfb.c
> @@ -1233,8 +1233,12 @@ static int dlfb_setup_modes(struct dlfb_data *dev,
>   			if (dlfb_is_valid_mode(&info->monspecs.modedb[i], info))
>   				fb_add_videomode(&info->monspecs.modedb[i],
>   					&info->modelist);
> -			else /* if we've removed top/best mode */
> -				info->monspecs.misc&= ~FB_MISC_1ST_DETAIL;
> +			else {
> +				if (i = 0)
> +					/* if we've removed top/best mode */
> +					info->monspecs.misc
> +						&= ~FB_MISC_1ST_DETAIL;
> +			}
>   		}
>
>   		default_vmode = fb_find_best_display(&info->monspecs,