2008-07-21 20:06:33

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH] VT: Restore VT fonts on switch

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 <[email protected]>
Signed-off-by: Ben Collins <[email protected]>
---
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


2008-07-21 20:43:40

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch

On Mon, 21 Jul 2008 15:39:44 -0400
Matthew Garrett <[email protected]> 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 ?

2008-07-21 20:53:33

by Ben Collins

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch

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 <[email protected]> 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.

2008-07-21 20:55:05

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch


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

>On Mon, 21 Jul 2008 15:39:44 -0400
>Matthew Garrett <[email protected]> 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.

2008-07-21 20:58:17

by Alan

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch

On Mon, 21 Jul 2008 15:39:44 -0400
Matthew Garrett <[email protected]> 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

2008-07-21 21:23:42

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch

On Mon, 21 Jul 2008 16:53:04 -0400
Ben Collins <[email protected]> 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 <[email protected]> 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...

2008-07-21 21:30:16

by Alan

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch

> > 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

2008-07-21 21:37:53

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch

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 | [email protected]

2008-07-21 21:40:21

by Ben Collins

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch

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 <[email protected]> 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 <[email protected]> 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.

2008-07-21 21:42:23

by Alan

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch

On Mon, 21 Jul 2008 22:37:35 +0100
Matthew Garrett <[email protected]> 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

2008-07-21 22:19:11

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch

On Mon, 21 Jul 2008 17:40:09 -0400
Ben Collins <[email protected]> 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 [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-07-21 22:33:27

by Ben Collins

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch

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 <[email protected]> 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.

2008-07-22 07:59:57

by Arkadiusz Miśkiewicz

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch

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 <[email protected]> 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/

2008-08-08 07:52:31

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch

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 <[email protected]>
> Signed-off-by: Ben Collins <[email protected]>


...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

2008-08-08 07:52:44

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch

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

2008-08-08 09:32:28

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH] VT: Restore VT fonts on switch

On Mon, 2008-07-21 at 21:40 +0100, Alan Cox wrote:
> On Mon, 21 Jul 2008 15:39:44 -0400
> Matthew Garrett <[email protected]> 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.