[PATCH 2/2] intelfb/fbdev: Save info->flags in a local variable

Antonino A. Daplas

author

2005-08-15 13:38 UTC #0 lore raw

PATCH 2/2 intelfb/fbdev: Save info->flags in a local variable

Reported by: Pavel Kysilka (Bugzilla Bug 5059)

Problem Description:
intelfb driver do not keep resolution set with fbset after switching to anot console and back.

Steps to reproduce:
initial options: tty1,tty2 - 1024x768-60
1) tty1 - fbset after booting (1024x768-60)
2) tty1 - fbset 800x600-100
tty1: 800x600-100
3) swith to tty2, swith to tty1
tty1: 1024x768-60 (the same resolution as default from kernel booting)

This bug is caused by intelfb unintentionally destroying info->flags in set_par(). Therefore the flag, FBINFO_MISC_USEREVENT used to notify fbcon of a mode change was cleared causing the above problem. This bug though is not intelfb specific, as other drivers may also be affected.

The fix is to save info->flags in a local variable before calling any of the driver hooks. A more definitive fix (for post 2.6.13) is to separate info->flags into one that is set by the driver and another that is set by core fbdev/fbcon.

Signed-off-by Antonino Daplas <[email protected]>

---

fbmem.c | 6 +++---

1 files changed, 3 insertions(+), 3 deletions(-)

drivers/video/fbmem.c +0 -0

@@ -643,8 +643,8 @@ fb_pan_display(struct fb_info *info, str

int
fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var) {
- int err;
-
+ int err, flags = info->flags;
+

	if (var->activate & FB_ACTIVATE_INV_MODE) {
		struct fb_videomode mode1, mode2;
		int ret = 0;
@@ -697,7 +697,7 @@ fb_set_var(struct fb_info *info, struct 
			    !list_empty(&info->modelist))
				err = fb_add_videomode(&mode, &info->modelist);

- if (!err && info->flags & FBINFO_MISC_USEREVENT) {

+			if (!err && flags & FBINFO_MISC_USEREVENT) {
				struct fb_event event;
				int evnt = (var->activate & FB_ACTIVATE_ALL) ?
					FB_EVENT_MODE_CHANGE_ALL :

-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

James Simmons

2005-08-15 16:28 UTC #1 lore raw

Quoted 10 lines

This bug is caused by intelfb unintentionally destroying info->flags in set_par(). Therefore the flag, FBINFO_MISC_USEREVENT used to notify fbcon of a mode change was cleared causing the above problem. This bug though is not intelfb specific, as other drivers may also be affected.
The fix is to save info->flags in a local variable before calling any of the driver hooks. A more definitive fix (for post 2.6.13) is to separate info->flags into one that is set by the driver and another that is set by core fbdev/fbcon.

The only problem I have with that patch is that the flags variable can also contain what accelerated features the driver supports in that mode. It is possible to have a old mode that supports no hardware acceleration and then change to a mode that does. Save the whole flag has us lose that information. What I suggest is that we mask the flag to save the upper bits and save only those. Then after mode switching OR it with the new mask.

-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

James Simmons

2005-08-15 16:31 UTC #2 lore raw

In reply to Antonino A. Daplas

Quoted 19 lines

fbmem.c | 6 +++---

1 files changed, 3 insertions(+), 3 deletions(-)

drivers/video/fbmem.c +2 -2

@@ -643,8 +643,8 @@ fb_pan_display(struct fb_info *info, str
 int
 fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 {
-	int err;
-
+	int err, flags = info->flags;
+
 	if (var->activate & FB_ACTIVATE_INV_MODE) {
 		struct fb_videomode mode1, mode2;
 		int ret = 0;

drivers/video/fbmem.c fb_set_var(struct fb_info *info, struct · line 703

Quoted 8 lines

drivers/video/fbmem.c +1 -1

@@ -697,7 +697,7 @@ fb_set_var(struct fb_info *info, struct 
 			    !list_empty(&info->modelist))
 				err = fb_add_videomode(&mode, &info->modelist);
 
-			if (!err && info->flags & FBINFO_MISC_USEREVENT) {
+			if (!err && flags & FBINFO_MISC_USEREVENT) {
 				struct fb_event event;
 				int evnt = (var->activate & FB_ACTIVATE_ALL) ?
 					FB_EVENT_MODE_CHANGE_ALL :

I take that back. I misread the code. You are not overwriting the new flag. The patch is good :-)

-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Linus Torvalds

2005-08-15 16:58 UTC #3 lore raw

In reply to Antonino A. Daplas

Review fb_set_var(struct fb_info *info, struct · line 703

Quoted 10 lines · Antonino A. Daplas

(quoted diff) +1 -1

@@ -697,7 +697,7 @@ fb_set_var(struct fb_info *info, struct 
 			    !list_empty(&info->modelist))
 				err = fb_add_videomode(&mode, &info->modelist);
 
-			if (!err && info->flags & FBINFO_MISC_USEREVENT) {
+			if (!err && flags & FBINFO_MISC_USEREVENT) {
 				struct fb_event event;
 				int evnt = (var->activate & FB_ACTIVATE_ALL) ?
 					FB_EVENT_MODE_CHANGE_ALL :

This doesn't match my tree - I assume we're talking about the -mm tree here.

I can/will fix up the thing by hand, but somebody should check it.

Linus

-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Antonino A. Daplas

author

2005-08-15 21:52 UTC #4 lore raw

Quoted 14 lines · Linus Torvalds

Review fb_set_var(struct fb_info *info, struct · line 703

Quoted 9 lines · Antonino A. Daplas

(quoted diff) +1 -1

@@ -697,7 +697,7 @@ fb_set_var(struct fb_info *info, struct 
 			    !list_empty(&info->modelist))
 				err = fb_add_videomode(&mode, &info->modelist);
 
-			if (!err && info->flags & FBINFO_MISC_USEREVENT) {
+			if (!err && flags & FBINFO_MISC_USEREVENT) {
 				struct fb_event event;
 				int evnt = (var->activate & FB_ACTIVATE_ALL) ?
 					FB_EVENT_MODE_CHANGE_ALL :

This doesn't match my tree - I assume we're talking about the -mm tree here.

Quoted 4 lines

I can/will fix up the thing by hand, but somebody should check it.
Linus

Sorry about that, I forgot. I checked your git tree, and it looks good.

Thanks.

Tony

-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf