[PATCH] VT: Restore VT fonts on switch

[PATCH] VT: Restore VT fonts on switch

[Matthew Garrett]

Not all X drivers save and restore fonts on text VTs. Add code to the
kernel to explicitly save and restore them on VT switches.

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>
Signed-off-by: Ben Collins <ben.collins@canonical.com>
---
 drivers/char/vt_ioctl.c |   27 ++++++++++++++++++++++++++-
 1 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/drivers/char/vt_ioctl.c b/drivers/char/vt_ioctl.c
index 3211afd..b2ac3d1 100644
--- a/drivers/char/vt_ioctl.c
+++ b/drivers/char/vt_ioctl.c
@@ -35,6 +35,8 @@
 #include <linux/kbd_diacr.h>
 #include <linux/selection.h>
 
+#define max_font_size 65536
+
 char vt_dont_switch;
 extern struct tty_driver *console_driver;
 
@@ -1250,6 +1252,7 @@ void vc_SAK(struct work_struct *work)
 static void complete_change_console(struct vc_data *vc)
 {
 	unsigned char old_vc_mode;
+	struct vc_data *oldvc = vc_cons[fg_console].d;
 
 	last_console = fg_console;
 
@@ -1258,9 +1261,31 @@ static void complete_change_console(struct vc_data *vc)
 	 * KD_TEXT mode or vice versa, which means we need to blank or
 	 * unblank the screen later.
 	 */
-	old_vc_mode = vc_cons[fg_console].d->vc_mode;
+	old_vc_mode = oldvc->vc_mode;
+
+#if defined(CONFIG_VGA_CONSOLE)
+	if (old_vc_mode == KD_TEXT && oldvc->vc_sw == &vga_con &&
+	    oldvc->vc_sw->con_font_get) {
+		if (!oldvc->vc_font.data)
+			oldvc->vc_font.data = kmalloc(max_font_size, 
+						      GFP_KERNEL);
+		lock_kernel();
+		oldvc->vc_sw->con_font_get(oldvc, &oldvc->vc_font);
+		unlock_kernel();
+	}
+#endif
 	switch_screen(vc);
 
+#if defined(CONFIG_VGA_CONSOLE)
+	if (vc->vc_mode == KD_TEXT && vc->vc_sw == &vga_con &&
+	    vc->vc_sw->con_font_set) {
+		if (vc->vc_font.data) {
+			lock_kernel();
+			vc->vc_sw->con_font_set(vc, &vc->vc_font, 0);
+			unlock_kernel();
+		}
+	}
+#endif
 	/*
 	 * This can't appear below a successful kill_pid().  If it did,
 	 * then the *blank_screen operation could occur while X, having
-- 
1.5.4.3

Re: [PATCH] VT: Restore VT fonts on switch

[Alan Cox]

On Mon, 21 Jul 2008 15:39:44 -0400
Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> Not all X drivers save and restore fonts on text VTs. Add code to the
> kernel to explicitly save and restore them on VT switches.

Please explain why this cannot be done by HAL.

Please further explain the use of lock_kernel and why you think that is
sufficient and safe.

For bonus points explain how the behaviour of the kmalloc failing is
sufficient.

NAK

Alan

Re: [PATCH] VT: Restore VT fonts on switch

[Benjamin Herrenschmidt]

On Mon, 2008-07-21 at 21:40 +0100, Alan Cox wrote:
> On Mon, 21 Jul 2008 15:39:44 -0400
> Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> 
> > Not all X drivers save and restore fonts on text VTs. Add code to the
> > kernel to explicitly save and restore them on VT switches.
> 
> Please explain why this cannot be done by HAL.

To give you a chance to see the console messages during resume
in case something goes wrong before HAL kicks in ? :-)

Ben.

Re: [PATCH] VT: Restore VT fonts on switch

[Arjan van de Ven]

On Mon, 21 Jul 2008 15:39:44 -0400
Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> Not all X drivers save and restore fonts on text VTs. Add code to the
> kernel to explicitly save and restore them on VT switches.


do you have a list of which ones don't ?

Re: [PATCH] VT: Restore VT fonts on switch

[Ben Collins]

On Mon, 2008-07-21 at 13:43 -0700, Arjan van de Ven wrote:
> On Mon, 21 Jul 2008 15:39:44 -0400
> Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> 
> > Not all X drivers save and restore fonts on text VTs. Add code to the
> > kernel to explicitly save and restore them on VT switches.
> 
> 
> do you have a list of which ones don't ?

I don't, but perhaps Matthew does.

Re: [PATCH] VT: Restore VT fonts on switch

[Alan Cox]

> > do you have a list of which ones don't ?
> 
> I don't, but perhaps Matthew does.

Perhaps Matthew would care to file X bugs against those that get it wrong
then.

Alan

Re: [PATCH] VT: Restore VT fonts on switch

[Matthew Garrett]

On Mon, Jul 21, 2008 at 10:12:04PM +0100, Alan Cox wrote:
> > > do you have a list of which ones don't ?
> > 
> > I don't, but perhaps Matthew does.
> 
> Perhaps Matthew would care to file X bugs against those that get it wrong
> then.

It's hardly limited to X. Fonts are set through the kernel - why 
shouldn't it be the kernel's responsibility to ensure that they're 
restored?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] VT: Restore VT fonts on switch

[Alan Cox]

On Mon, 21 Jul 2008 22:37:35 +0100
Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Mon, Jul 21, 2008 at 10:12:04PM +0100, Alan Cox wrote:
> > > > do you have a list of which ones don't ?
> > > 
> > > I don't, but perhaps Matthew does.
> > 
> > Perhaps Matthew would care to file X bugs against those that get it wrong
> > then.
> 
> It's hardly limited to X. Fonts are set through the kernel - why 
> shouldn't it be the kernel's responsibility to ensure that they're 
> restored?

Why should it be the kernels problem to clean up after buggy X drivers ?

There are cases the kernel probably should handle font restore: the
obvious one being suspend/resume. X is not one of those cases and wasting
memory on fonts on embedded boxes that will never be seen anyway seems
silly.

For suspend/resume it might be far better to do it in user space,
although I'm not sure what the interface would look like.

Alan

Re: [PATCH] VT: Restore VT fonts on switch

[Pavel Machek]

Hi!

> > > > > do you have a list of which ones don't ?
> > > > 
> > > > I don't, but perhaps Matthew does.
> > > 
> > > Perhaps Matthew would care to file X bugs against those that get it wrong
> > > then.
> > 
> > It's hardly limited to X. Fonts are set through the kernel - why 
> > shouldn't it be the kernel's responsibility to ensure that they're 
> > restored?
> 
> Why should it be the kernels problem to clean up after buggy X drivers ?
> 
> There are cases the kernel probably should handle font restore: the
> obvious one being suspend/resume. X is not one of those cases and wasting
> memory on fonts on embedded boxes that will never be seen anyway seems
> silly.
> 
> For suspend/resume it might be far better to do it in user space,
> although I'm not sure what the interface would look like.

This does not parse. 'kernel should handle font restore' .... 'but it
might be better to do that in userspace'.

Actually I'd prefer to do this in kernel, so it works in cases like
suspend self-test (done from kernel), from single user mode, etc...

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

Re: [PATCH] VT: Restore VT fonts on switch

[Arjan van de Ven]

On Mon, 21 Jul 2008 16:53:04 -0400
Ben Collins <bcollins@swissdisk.com> wrote:

> On Mon, 2008-07-21 at 13:43 -0700, Arjan van de Ven wrote:
> > On Mon, 21 Jul 2008 15:39:44 -0400
> > Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > 
> > > Not all X drivers save and restore fonts on text VTs. Add code to
> > > the kernel to explicitly save and restore them on VT switches.
> > 
> > 
> > do you have a list of which ones don't ?
> 
> I don't, but perhaps Matthew does.
> 
I was assuming Matthew would respond since he sent this patch to lkml
after all...

Re: [PATCH] VT: Restore VT fonts on switch

[Ben Collins]

On Mon, 2008-07-21 at 14:23 -0700, Arjan van de Ven wrote:
> On Mon, 21 Jul 2008 16:53:04 -0400
> Ben Collins <bcollins@swissdisk.com> wrote:
> 
> > On Mon, 2008-07-21 at 13:43 -0700, Arjan van de Ven wrote:
> > > On Mon, 21 Jul 2008 15:39:44 -0400
> > > Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > > 
> > > > Not all X drivers save and restore fonts on text VTs. Add code to
> > > > the kernel to explicitly save and restore them on VT switches.
> > > 
> > > 
> > > do you have a list of which ones don't ?
> > 
> > I don't, but perhaps Matthew does.
> > 
> I was assuming Matthew would respond since he sent this patch to lkml
> after all...

Actually, I sent it on Matthews behalf, so perhaps you could understand
why I answered.

Re: [PATCH] VT: Restore VT fonts on switch

[Arjan van de Ven]

On Mon, 21 Jul 2008 17:40:09 -0400
Ben Collins <ben.collins@canonical.com> wrote:does.
> > > 
> > I was assuming Matthew would respond since he sent this patch to
> > lkml after all...
> 
> Actually, I sent it on Matthews behalf, so perhaps you could
> understand why I answered.
> 

then I'd humbly request that you stop doing such things.
If you send teh email, the mail should say "From" you.
Anything else is forgery!

Now inside the body of your email you can put a 
From: Matthew Garrett

to indicate the patch is from him... but please don't forge emails.


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH] VT: Restore VT fonts on switch

[Ben Collins]

On Mon, 2008-07-21 at 15:18 -0700, Arjan van de Ven wrote:
> On Mon, 21 Jul 2008 17:40:09 -0400
> Ben Collins <ben.collins@canonical.com> wrote:does.
> > > > 
> > > I was assuming Matthew would respond since he sent this patch to
> > > lkml after all...
> > 
> > Actually, I sent it on Matthews behalf, so perhaps you could
> > understand why I answered.
> > 
> 
> then I'd humbly request that you stop doing such things.
> If you send teh email, the mail should say "From" you.
> Anything else is forgery!
> 
> Now inside the body of your email you can put a 
> From: Matthew Garrett
> 
> to indicate the patch is from him... but please don't forge emails.

I'm just piping the output for git-format-patch. Maybe the output of
that program should look less like a raw email, or perhaps use the local
person who is sending the patch as the From and add the author to the
body under a separate From. Your call, but I didn't "decide" to send the
patches this way.

Besides all that, I asked Matthew if I could send this patch upstream,
and I would be surprised if he was angry about the way I sent it.

Re: [PATCH] VT: Restore VT fonts on switch

[Jan Engelhardt]

On Monday 2008-07-21 22:43, Arjan van de Ven wrote:

>On Mon, 21 Jul 2008 15:39:44 -0400
>Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>
>> Not all X drivers save and restore fonts on text VTs. Add code to the
>> kernel to explicitly save and restore them on VT switches.
>
>
>do you have a list of which ones don't ?

This piece of code may be needed anyway for swsusp, which does not
restore fonts yet.

Re: [PATCH] VT: Restore VT fonts on switch

[Arkadiusz Miskiewicz]

On Monday 21 July 2008, Jan Engelhardt wrote:
> On Monday 2008-07-21 22:43, Arjan van de Ven wrote:
> >On Mon, 21 Jul 2008 15:39:44 -0400
> >
> >Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> >> Not all X drivers save and restore fonts on text VTs. Add code to the
> >> kernel to explicitly save and restore them on VT switches.
> >
> >do you have a list of which ones don't ?
>
> This piece of code may be needed anyway for swsusp, which does not
> restore fonts yet.

Yeah, console is a mess after resume from ram (using s2ram).

-- 
Arkadiusz Miśkiewicz        PLD/Linux Team
arekm / maven.pl            http://ftp.pld-linux.org/

Re: [PATCH] VT: Restore VT fonts on switch

[Pavel Machek]

Hi!

> Not all X drivers save and restore fonts on text VTs. Add code to the
> kernel to explicitly save and restore them on VT switches.
> 
> Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>
> Signed-off-by: Ben Collins <ben.collins@canonical.com>


...plus fonts are lost over suspend/resume in vgacon case; I guess
this code could be reused for that?

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