vgacon fixes to help font restauration in X11

vgacon fixes to help font restauration in X11

[Egbert Eich]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 1809 bytes --]


[Please leave me in Cc as I'm not subscribed to this list]

So far the X.Org/XFree86 Xservers use internal functions to 
perform saving/restoring of console fonts. This doesn't work 
under all circumstances and has disadvantages:

1. On some platforms (IA64) X needs to POST the BIOS before
   it even has a chance to access the hardware itself. This POSTing 
   will usually undo any changes to the graphics hardware that the 
   kernel may have done.
2. More and more drivers fully rely on BIOS support. However
   the BIOS functions which could be used to save/restore 
   register settings may be broken so the only way of mode
   save/restore is getting/setting the BIOS mode ID.
   This will restore the BIOSes default fonts, not the custom font
   that the user may have loaded.

I would like to utilize the kernels save/restore console 
fonts ioctls to save/restore the text mode console fonts 
from inside X. 

The patch does two things:

1. Eliminates the optimization that returns from vgacon_adjust_height()
   early (without reprogramming the HW) when the hight of the font hasn't
   changed.
2. Resets the stored 'from' and 'to' lines for the text cursor in
   vgacon_adjust_height() to cause vgacon_set_cursor_size() reprogram 
   the textcursor start and end line.

These are necessary to sanitize the HW in case something other than
the kernel has changed the register values without restoring them 
properly.

I'm fully aware that in the long run we will need to look into a new 
driver model for graphics where no two instances fight over who gets
register access. However such a model won't be created nor will we get 
the majority of the drivers ported over night.
Therefore we need to find an interim solution for the most pressing 
problems.


Egbert.

<Patch against latest snapshot>

[-- Attachment #2: diff.vgacon --]
[-- Type: text/plain, Size: 1880 bytes --]

Index: drivers/video/console/vgacon.c
===================================================================
RCS file: /work/kernelcvs/linux/drivers/video/console/vgacon.c,v
retrieving revision 1.1.1.2
diff -u -p -r1.1.1.2 vgacon.c
--- drivers/video/console/vgacon.c	11 Jan 2005 10:27:11 -0000	1.1.1.2
+++ drivers/video/console/vgacon.c	11 Jan 2005 13:40:19 -0000
@@ -54,6 +54,7 @@
 #include <asm/io.h>
 
 static spinlock_t vga_lock = SPIN_LOCK_UNLOCKED;
+static int cursor_size_lastfrom = 0, cursor_size_lastto = 0;
 static struct vgastate state;
 
 #define BLANK 0x0020
@@ -409,17 +410,16 @@ static void vgacon_set_cursor_size(int x
 {
 	unsigned long flags;
 	int curs, cure;
-	static int lastfrom, lastto;
 
 #ifdef TRIDENT_GLITCH
 	if (xpos < 16)
 		from--, to--;
 #endif
 
-	if ((from == lastfrom) && (to == lastto))
+	if ((from == cursor_size_lastfrom) && (to == cursor_size_lastto))
 		return;
-	lastfrom = from;
-	lastto = to;
+	cursor_size_lastfrom = from;
+	cursor_size_lastto = to;
 
 	spin_lock_irqsave(&vga_lock, flags);
 	outb_p(0x0a, vga_video_port_reg);	/* Cursor start */
@@ -862,11 +862,6 @@ static int vgacon_adjust_height(struct v
 	unsigned char ovr, vde, fsr;
 	int rows, maxscan, i;
 
-	if (fontheight == vc->vc_font.height)
-		return 0;
-
-	vc->vc_font.height = fontheight;
-
 	rows = vc->vc_scan_lines / fontheight;	/* Number of video rows we end up with */
 	maxscan = rows * fontheight - 1;	/* Scan lines to actually display-1 */
 
@@ -905,6 +900,11 @@ static int vgacon_adjust_height(struct v
 		struct vc_data *c = vc_cons[i].d;
 
 		if (c && c->vc_sw == &vga_con) {
+			if( CON_IS_VISIBLE(c)) {
+			        /* void size to cause regs to be rewritten */
+				cursor_size_lastfrom = cursor_size_lastto = 0;
+				c->vc_sw->con_cursor(c, CM_DRAW);
+			}
 			c->vc_font.height = fontheight;
 			vc_resize(c->vc_num, 0, rows);	/* Adjust console size */
 		}

Re: vgacon fixes to help font restauration in X11

[Alan Cox]

On Maw, 2005-01-11 at 14:28, Egbert Eich wrote:
> I'm fully aware that in the long run we will need to look into a new 
> driver model for graphics where no two instances fight over who gets
> register access. However such a model won't be created nor will we get 
> the majority of the drivers ported over night.
> Therefore we need to find an interim solution for the most pressing 
> problems.

This doesn't appear to work as a solution because the functionality
changes won't be in all the existing kernel, and also because the kernel
font save has a couple of bugs reported against it with regards to
saving the right data that might need looking at anyway.

It seems it would be neccessary for X to have a way to know whether the
feature is present.

Re: vgacon fixes to help font restauration in X11

[Egbert Eich]

[Please leave me in Cc: as I'm not subscribed to this list]

Alan Cox writes:
 > On Maw, 2005-01-11 at 14:28, Egbert Eich wrote:
 > > I'm fully aware that in the long run we will need to look into a new 
 > > driver model for graphics where no two instances fight over who gets
 > > register access. However such a model won't be created nor will we get 
 > > the majority of the drivers ported over night.
 > > Therefore we need to find an interim solution for the most pressing 
 > > problems.
 > 
 > This doesn't appear to work as a solution because the functionality
 > changes won't be in all the existing kernel, and also because the kernel
 > font save has a couple of bugs reported against it with regards to
 > saving the right data that might need looking at anyway.

Can you point me to these reports?
I tested with a couple chipsets here and didn't find any problems.

Maybe we can compare the code in X with the code in the kernel
for the amount of data to save.
However we don't know if the X font code is completely without
problems. I remeber fixing a problem in X years ago - a case
where the kernel got it right ;-)
X VGA font code involves a lot of magic. Also things might differ
slightly form HW vendor to vendor and it will be extremely hard
to get it right for all chipset models therefore I would not even 
talk about 'bugs' ;-)

 > 
 > It seems it would be neccessary for X to have a way to know whether the
 > feature is present.
 > 

We could check for the kernel version. This could be done during build
time - assuming we don't ship generic binaries or during run time if we
want to provide binaries that work everywhere.
In reality the former would be sufficient for a lot of cases - especially
for vendor supplied binaries.
Once X.Org starts shipping binaries it should definitely provide a version
that fits everywhere.
But maybe you have a better suggestion.

Anyway, would my patch be acceptable for the kernel?

Cheers,
	Egbert.

Re: vgacon fixes to help font restauration in X11

[Alan Cox]

On Llu, 2005-01-17 at 09:07, Egbert Eich wrote:
> Can you point me to these reports?
> I tested with a couple chipsets here and didn't find any problems.

I'll take a dig. The ones I've got are for 2.4 so relate to old code.

> We could check for the kernel version. This could be done during build
> time - assuming we don't ship generic binaries or during run time if we
> want to provide binaries that work everywhere.
> In reality the former would be sufficient for a lot of cases - especially
> for vendor supplied binaries.

The former would be a disaster for Fedora for example - we ship
'current' kernels and having kernel upgrades require a new X11 won't
endear users . A runtime check on version might work I was wondering if
it would be better to have an actual interface that said "do/do not
restore the extra bits in kernel".

That also avoids any suprises and regressions ?

> Anyway, would my patch be acceptable for the kernel?

I'm not video maintainer but other than the detection question it looks
sensible to me.

Re: vgacon fixes to help font restauration in X11

[Egbert Eich]

Alan Cox writes:
 > On Llu, 2005-01-17 at 09:07, Egbert Eich wrote:
 > > Can you point me to these reports?
 > > I tested with a couple chipsets here and didn't find any problems.
 > 
 > I'll take a dig. The ones I've got are for 2.4 so relate to old code.
 > 
 > > We could check for the kernel version. This could be done during build
 > > time - assuming we don't ship generic binaries or during run time if we
 > > want to provide binaries that work everywhere.
 > > In reality the former would be sufficient for a lot of cases - especially
 > > for vendor supplied binaries.
 > 
 > The former would be a disaster for Fedora for example - we ship
 > 'current' kernels and having kernel upgrades require a new X11 won't
 > endear users . A runtime check on version might work I was wondering if

No, it would rather be the other way around. A new version of X would
require a certain version of the kernel - unless you plan to drop the 
feature again.
This however will not be necessary until 6.9/7 (however it will be named)
comes out.
I can implement both ways. Since the new font code lives in the OS dependent
part this should not be a problem at all.
The only disadvantage may be that I may not be able to turn off the old
font code in the generic vgaHW stuff.

 > it would be better to have an actual interface that said "do/do not
 > restore the extra bits in kernel".
 > 
 > That also avoids any suprises and regressions ?

I used to have a patch like that. But kernel people I've talked to told
me that it would be preferrable not to change the API if not necessary.

In my opinion it is not. The changes only affect cases where a new font
gets written or restored.

 > > Anyway, would my patch be acceptable for the kernel?
 > 
 > I'm not video maintainer but other than the detection question it looks
 > sensible to me.
 > 

OK, sounds promising. The changed Xserver pieces are in HEAD of the 
X.Org tree. I'll see that I make the necessary adjustments to have
a soft detection if you can give me a version number of the kernel
which will have the new features.

Egbert.

Re: vgacon fixes to help font restauration in X11

[Alan Cox]

On Llu, 2005-01-17 at 14:29, Egbert Eich wrote:
> OK, sounds promising. The changed Xserver pieces are in HEAD of the 
> X.Org tree. I'll see that I make the necessary adjustments to have
> a soft detection if you can give me a version number of the kernel
> which will have the new features.

Send a copy directly to torvalds@osdl.org and akpm@osdl.org for merging.
It looks fine to me. You'll need to include a "Signed-off-by: .." line
to indicate you are submitting it and have the rights to do so. You can
tag it with "Approved-by: Alan Cox <alan@redhat.com>".

If you mention you need to know what kernel merges it for the X11 check
I'm sure you'll get an answer.

Alan

Re: vgacon fixes to help font restauration in X11

[Egbert Eich]

Alan Cox writes:
 > On Llu, 2005-01-17 at 14:29, Egbert Eich wrote:
 > > OK, sounds promising. The changed Xserver pieces are in HEAD of the 
 > > X.Org tree. I'll see that I make the necessary adjustments to have
 > > a soft detection if you can give me a version number of the kernel
 > > which will have the new features.
 > 
 > Send a copy directly to torvalds@osdl.org and akpm@osdl.org for merging.
 > It looks fine to me. You'll need to include a "Signed-off-by: .." line
 > to indicate you are submitting it and have the rights to do so. You can
 > tag it with "Approved-by: Alan Cox <alan@redhat.com>".

Thanks, I will do that.
I have attached the patch for kernel version detection to the 
freedesktop.org bugzilla (#2277) assuming this feature will be
in the kernel starting with version 2.6.11.

 > 
 > If you mention you need to know what kernel merges it for the X11 check
 > I'm sure you'll get an answer.
 > 

OK, will do.

Thanks!
	Egbert.