linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] VT: Restore VT fonts on switch
@ 2008-07-21 19:39 Matthew Garrett
  2008-07-21 20:40 ` Alan Cox
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Matthew Garrett @ 2008-07-21 19:39 UTC (permalink / raw)
  To: linux-kernel

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 19:39 [PATCH] VT: Restore VT fonts on switch Matthew Garrett
@ 2008-07-21 20:40 ` Alan Cox
  2008-08-08  9:32   ` Benjamin Herrenschmidt
  2008-07-21 20:43 ` Arjan van de Ven
  2008-08-08  7:44 ` Pavel Machek
  2 siblings, 1 reply; 16+ messages in thread
From: Alan Cox @ 2008-07-21 20:40 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 19:39 [PATCH] VT: Restore VT fonts on switch Matthew Garrett
  2008-07-21 20:40 ` Alan Cox
@ 2008-07-21 20:43 ` Arjan van de Ven
  2008-07-21 20:53   ` Ben Collins
  2008-07-21 20:54   ` Jan Engelhardt
  2008-08-08  7:44 ` Pavel Machek
  2 siblings, 2 replies; 16+ messages in thread
From: Arjan van de Ven @ 2008-07-21 20:43 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: linux-kernel

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 ?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 20:43 ` Arjan van de Ven
@ 2008-07-21 20:53   ` Ben Collins
  2008-07-21 21:12     ` Alan Cox
  2008-07-21 21:23     ` Arjan van de Ven
  2008-07-21 20:54   ` Jan Engelhardt
  1 sibling, 2 replies; 16+ messages in thread
From: Ben Collins @ 2008-07-21 20:53 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: Matthew Garrett, linux-kernel

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.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 20:43 ` Arjan van de Ven
  2008-07-21 20:53   ` Ben Collins
@ 2008-07-21 20:54   ` Jan Engelhardt
  2008-07-22  7:59     ` Arkadiusz Miskiewicz
  1 sibling, 1 reply; 16+ messages in thread
From: Jan Engelhardt @ 2008-07-21 20:54 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: Matthew Garrett, linux-kernel


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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 20:53   ` Ben Collins
@ 2008-07-21 21:12     ` Alan Cox
  2008-07-21 21:37       ` Matthew Garrett
  2008-07-21 21:23     ` Arjan van de Ven
  1 sibling, 1 reply; 16+ messages in thread
From: Alan Cox @ 2008-07-21 21:12 UTC (permalink / raw)
  To: Ben Collins; +Cc: Arjan van de Ven, Matthew Garrett, linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 20:53   ` Ben Collins
  2008-07-21 21:12     ` Alan Cox
@ 2008-07-21 21:23     ` Arjan van de Ven
  2008-07-21 21:40       ` Ben Collins
  1 sibling, 1 reply; 16+ messages in thread
From: Arjan van de Ven @ 2008-07-21 21:23 UTC (permalink / raw)
  To: Ben Collins; +Cc: Matthew Garrett, linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 21:37       ` Matthew Garrett
@ 2008-07-21 21:24         ` Alan Cox
  2008-08-08  7:49           ` Pavel Machek
  0 siblings, 1 reply; 16+ messages in thread
From: Alan Cox @ 2008-07-21 21:24 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Ben Collins, Arjan van de Ven, linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 21:12     ` Alan Cox
@ 2008-07-21 21:37       ` Matthew Garrett
  2008-07-21 21:24         ` Alan Cox
  0 siblings, 1 reply; 16+ messages in thread
From: Matthew Garrett @ 2008-07-21 21:37 UTC (permalink / raw)
  To: Alan Cox; +Cc: Ben Collins, Arjan van de Ven, linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 21:23     ` Arjan van de Ven
@ 2008-07-21 21:40       ` Ben Collins
  2008-07-21 22:18         ` Arjan van de Ven
  0 siblings, 1 reply; 16+ messages in thread
From: Ben Collins @ 2008-07-21 21:40 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: Matthew Garrett, linux-kernel

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.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 21:40       ` Ben Collins
@ 2008-07-21 22:18         ` Arjan van de Ven
  2008-07-21 22:29           ` Ben Collins
  0 siblings, 1 reply; 16+ messages in thread
From: Arjan van de Ven @ 2008-07-21 22:18 UTC (permalink / raw)
  To: Ben Collins; +Cc: Matthew Garrett, linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 22:18         ` Arjan van de Ven
@ 2008-07-21 22:29           ` Ben Collins
  0 siblings, 0 replies; 16+ messages in thread
From: Ben Collins @ 2008-07-21 22:29 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: Matthew Garrett, linux-kernel

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.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 20:54   ` Jan Engelhardt
@ 2008-07-22  7:59     ` Arkadiusz Miskiewicz
  0 siblings, 0 replies; 16+ messages in thread
From: Arkadiusz Miskiewicz @ 2008-07-22  7:59 UTC (permalink / raw)
  To: linux-kernel

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/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 19:39 [PATCH] VT: Restore VT fonts on switch Matthew Garrett
  2008-07-21 20:40 ` Alan Cox
  2008-07-21 20:43 ` Arjan van de Ven
@ 2008-08-08  7:44 ` Pavel Machek
  2 siblings, 0 replies; 16+ messages in thread
From: Pavel Machek @ 2008-08-08  7:44 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 21:24         ` Alan Cox
@ 2008-08-08  7:49           ` Pavel Machek
  0 siblings, 0 replies; 16+ messages in thread
From: Pavel Machek @ 2008-08-08  7:49 UTC (permalink / raw)
  To: Alan Cox; +Cc: Matthew Garrett, Ben Collins, Arjan van de Ven, linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] VT: Restore VT fonts on switch
  2008-07-21 20:40 ` Alan Cox
@ 2008-08-08  9:32   ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 16+ messages in thread
From: Benjamin Herrenschmidt @ 2008-08-08  9:32 UTC (permalink / raw)
  To: Alan Cox; +Cc: Matthew Garrett, linux-kernel

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.



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2008-08-08  9:32 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-07-21 19:39 [PATCH] VT: Restore VT fonts on switch Matthew Garrett
2008-07-21 20:40 ` Alan Cox
2008-08-08  9:32   ` Benjamin Herrenschmidt
2008-07-21 20:43 ` Arjan van de Ven
2008-07-21 20:53   ` Ben Collins
2008-07-21 21:12     ` Alan Cox
2008-07-21 21:37       ` Matthew Garrett
2008-07-21 21:24         ` Alan Cox
2008-08-08  7:49           ` Pavel Machek
2008-07-21 21:23     ` Arjan van de Ven
2008-07-21 21:40       ` Ben Collins
2008-07-21 22:18         ` Arjan van de Ven
2008-07-21 22:29           ` Ben Collins
2008-07-21 20:54   ` Jan Engelhardt
2008-07-22  7:59     ` Arkadiusz Miskiewicz
2008-08-08  7:44 ` Pavel Machek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).