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

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

[Antonino A. Daplas]

  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 <adaplas@pol.net>
---

fbmem.c |    6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -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

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

[James Simmons]

>   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

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

[James Simmons]

> fbmem.c |    6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -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 :

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

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

[Linus Torvalds]

On Mon, 15 Aug 2005, Antonino A. Daplas wrote:
>
> @@ -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

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

[Antonino A. Daplas]

Linus Torvalds wrote:
> 
> On Mon, 15 Aug 2005, Antonino A. Daplas wrote:
>> @@ -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
> 

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