Re: [PATCH] Software Suspend: Fix suspend when console is in VT_AUTO/KD_GRAPHICS mode

[Pavel Machek <pavel@ucw.cz>]

Hi!

> > > So... if current console is graphical, we leave X accessing the
> > > console... That's bad, because video state is not going to be
> > > restored...?
> > 
> > A graphical console is not necessarily X. Is there any requirement for 
> > there to be a single VT that isn't in text mode? The vt switching is 
> > a hack, we shouldn't make life difficult for people who have their own 
> > userspace code that's entirely capable of restoring video state on its 
> > own.
> 
> I realised that the previous patch would disallow a console switch while
> running X.  Attached is an updated patch with this scenario fixed.
> 
> Another approach might be to fail in vt_waitactive() if a console switch
> is not going to occur.
> 
> -- Andrew
> 

>  
> -void set_console(int nr)
> +extern char vt_dont_switch;
> +

What does this variable do and why do we want to use it here?

> +int set_console(int nr)
>  {
> +	struct vc_data *vc = vc_cons[fg_console].d;
> +
> +	if(!vc_cons_allocated(nr) || vt_dont_switch || 

'if ('

> +		(vc->vt_mode.mode != VT_PROCESS && vc->vc_mode == KD_GRAPHICS)) {
> +
> +		return -EINVAL;
> +	}
> +

I assume you want ...mode == VT_AUTO here?

And big comment explaining why we want this behaviour?

And another big comment explaining why this will not break existing
set_console() users?

>  	want_console = nr;
>  	schedule_console_callback();
> +
> +	return 0;
>  }
>  
>  struct tty_driver *console_driver;
> diff -rup linux-2.6.20.1/drivers/char/vt_ioctl.c
> linux/drivers/char/vt_ioctl.c
> --- linux-2.6.20.1/drivers/char/vt_ioctl.c	2007-02-19 22:34:32.000000000
> -0800
> +++ linux/drivers/char/vt_ioctl.c	2007-03-08 14:15:41.000000000 -0800
> @@ -34,7 +34,7 @@
>  #include <linux/kbd_diacr.h>
>  #include <linux/selection.h>
>  
> -static char vt_dont_switch;
> +char vt_dont_switch;
>  extern struct tty_driver *console_driver;
>  
>  #define VT_IS_IN_USE(i)	(console_driver->ttys[i] &&
> console_driver->ttys[i]->count)
> diff -rup linux-2.6.20.1/include/linux/kbd_kern.h
> linux/include/linux/kbd_kern.h
> --- linux-2.6.20.1/include/linux/kbd_kern.h	2007-02-19
> 22:34:32.000000000 -0800
> +++ linux/include/linux/kbd_kern.h	2007-03-08 14:15:41.000000000 -0800
> @@ -75,7 +75,7 @@ extern int do_poke_blanked_console;
>  
>  extern void (*kbd_ledfunc)(unsigned int led);
>  
> -extern void set_console(int nr);
> +extern int set_console(int nr);
>  extern void schedule_console_callback(void);
>  
>  static inline void set_leds(void)
> diff -rup linux-2.6.20.1/kernel/power/console.c
> linux/kernel/power/console.c
> --- linux-2.6.20.1/kernel/power/console.c	2007-02-19 22:34:32.000000000
> -0800
> +++ linux/kernel/power/console.c	2007-03-08 14:15:41.000000000 -0800
> @@ -27,7 +27,11 @@ int pm_prepare_console(void)
>  		return 1;
>  	}
>  
> -	set_console(SUSPEND_CONSOLE);
> +	if (set_console(SUSPEND_CONSOLE)) {
> +		/* Unable to change to the new console */

That's not what the comment should say.

It should explain why it is okay to proceed when we can't change to
text console.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html