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

From: Pavel Machek
Date: Fri Mar 09 2007 - 16:19:49 EST


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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/