2005-01-11 15:29:21

by Egbert Eich

[permalink] [raw]
Subject: vgacon fixes to help font restauration in X11

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 */
}


Attachments:
diff.vgacon (1.84 kB)

2005-01-15 01:48:18

by Alan

[permalink] [raw]
Subject: Re: vgacon fixes to help font restauration in X11

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.

2005-01-17 09:08:38

by Egbert Eich

[permalink] [raw]
Subject: Re: vgacon fixes to help font restauration in X11

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

2005-01-17 13:28:10

by Alan

[permalink] [raw]
Subject: Re: vgacon fixes to help font restauration in X11

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.

2005-01-17 14:30:05

by Egbert Eich

[permalink] [raw]
Subject: Re: vgacon fixes to help font restauration in X11

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.

2005-01-17 17:35:07

by Alan

[permalink] [raw]
Subject: Re: vgacon fixes to help font restauration in X11

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

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

Alan

2005-01-18 08:32:09

by Egbert Eich

[permalink] [raw]
Subject: Re: vgacon fixes to help font restauration in X11

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

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.