2003-05-01 11:48:17

by Thomas Backlund

[permalink] [raw]
Subject: Re: [PATCH 2.4.21-rc1] vesafb with large memory

O.Sezer wrote:

> So far, so good...
>
> I can happily boot, halt, play some opengl games, perform my daily
> routines, etc. This should also be related to the bug recorded at
> Mandrake-bugzilla: http://qa.mandrakesoft.com/show_bug.cgi?id=3198 .
> I also reported this to [email protected] .
>

No need ;-)....
We already have a (IMHO) better patch in current Cooker...
(and if you need it to install MDK 9.1, head over to http://www.iki.fi/tmb,
grab the install disks, and follow the instructions...)

The problem with the patch you discussed, is that it allocates
framebuffer memory only according to the display mode...,
but there are programs that benefits from the "extra" memory...
this according to Antonino Daplas...
(AFAIK double/triple buffering is one thing...)

The patch that we have in current Cooker, allows the user to
request the size himself by a command line option...
and it does not change the setup/configuration for users that
does not suffer from this problem...


the patch works like this:

you request the framebuffer size with "video=vesa:vram:32"
where 32 is the requested size in MB...
(you can choose the size yourself)
but a gemeric rule IMHO is to not go beyond the amount
of video memory you have...

and when the system boots:

- if no vram option, boot as normal (like without the patch)
- if probed memory > requested, use requested
- if probed memory < requested, use probed (like without the patch)

As a test I used this "video=vesa:vram:32" on a card
with 64MB of ram, and on a card with only 4MB of ram
to se if I could mess it up, with the following results:

64MB card: allocated FB -> 32MB
(this is also confirmed on a 128MB card, by other Cooker users)
4MB card: allocated FB -> 4MB
(here the original probe worked, so we use it...)


So this way we wont break current systems,
but have an option for those cards that has problem...
(seems to be AGP + >=128MB VideoRAM + >=1024MB system RAM only)

and finally... the patch itself:

----- cut -----
diff -Naru tmb7/Documentation/fb/vesafb.txt tmb9/Documentation/fb/vesafb.txt
--- tmb7/Documentation/fb/vesafb.txt 2000-07-28 22:50:51.000000000 +0300
+++ tmb9/Documentation/fb/vesafb.txt 2003-04-03 10:48:50.000000000 +0300
@@ -146,6 +146,10 @@

mtrr setup memory type range registers for the vesafb framebuffer.

+vram:n remap 'n' MiB of video RAM. If 0 or not specified, remap all
+ available video RAM. (2.5.66 patch by Antonino Daplas backported
+ to 2.4 by [email protected])
+

Have fun!

diff -Naru tmb7/drivers/video/vesafb.c tmb9/drivers/video/vesafb.c
--- tmb7/drivers/video/vesafb.c 2002-11-29 01:53:15.000000000 +0200
+++ tmb9/drivers/video/vesafb.c 2003-04-03 10:46:52.000000000 +0300
@@ -94,6 +94,7 @@

static int inverse = 0;
static int mtrr = 0;
+static int vram __initdata = 0; /* needed for vram boot option */
static int currcon = 0;

static int pmi_setpal = 0; /* pmi for palette changes ??? */
@@ -479,6 +480,10 @@
pmi_setpal=1;
else if (! strcmp(this_opt, "mtrr"))
mtrr=1;
+ /* checks for vram boot option */
+ else if (! strncmp(this_opt, "vram:", 5))
+ vram = simple_strtoul(this_opt+5, NULL, 0);
+
else if (!strncmp(this_opt, "font:", 5))
strcpy(fb_info.fontname, this_opt+5);
}
@@ -521,6 +526,9 @@
video_height = screen_info.lfb_height;
video_linelength = screen_info.lfb_linelength;
video_size = screen_info.lfb_size * 65536;
+ /* sets video_size according to boot option */
+ if (vram && vram * 1024 * 1024 < video_size)
+ video_size = vram * 1024 * 1024;
video_visual = (video_bpp == 8) ?
FB_VISUAL_PSEUDOCOLOR : FB_VISUAL_TRUECOLOR;
----- cut -----


Best Regards

Thomas Backlund

[email protected]
http://www.iki.fi/tmb


2003-05-01 12:54:21

by Alan

[permalink] [raw]
Subject: Re: [PATCH 2.4.21-rc1] vesafb with large memory

On Iau, 2003-05-01 at 13:00, Thomas Backlund wrote:
> but there are programs that benefits from the "extra" memory...
> this according to Antonino Daplas...
> (AFAIK double/triple buffering is one thing...)

I've actually looke through the traces for some situations
and the "how much memory" case is reporting banked RAM on some
cards so you don't know that RAM exists.

The change proposed is definitely correct for the default. Whether
you want to support overriding it I don't know - I'm not worried
either way.

2003-05-01 14:49:28

by Thomas Backlund

[permalink] [raw]
Subject: Re: [PATCH 2.4.21-rc1] vesafb with large memory

Viestissä Torstai 1. Toukokuuta 2003 15:07, Alan Cox kirjoitti:
> On Iau, 2003-05-01 at 13:00, Thomas Backlund wrote:
> > but there are programs that benefits from the "extra" memory...
> > this according to Antonino Daplas...
> > (AFAIK double/triple buffering is one thing...)
>
> I've actually looke through the traces for some situations
> and the "how much memory" case is reporting banked RAM on some
> cards so you don't know that RAM exists.
>
> The change proposed is definitely correct for the default. Whether
> you want to support overriding it I don't know - I'm not worried
> either way.

You mean the patch that only looks at videomode, dont you...

Well maybe it's best to use it as default, to avoid this bug
"out of the box"...

But I will remake a patch to ovverride it so that the users who
need/want the extra memory to be able to allocate it...
since I like the idea of giving the user the choice...

The next thing that comes to mind is of course that
it need to be done so that you can't kill the system with the
vram setting... but that should not be to hard...

Thomas

--
Thomas Backlund

[email protected]
http://www.iki.fi/tmb

2003-05-01 15:35:49

by Alan

[permalink] [raw]
Subject: Re: [PATCH 2.4.21-rc1] vesafb with large memory

On Iau, 2003-05-01 at 16:01, Thomas Backlund wrote:
> You mean the patch that only looks at videomode, dont you...

I do

> Well maybe it's best to use it as default, to avoid this bug
> "out of the box"...
>
> But I will remake a patch to ovverride it so that the users who
> need/want the extra memory to be able to allocate it...
> since I like the idea of giving the user the choice...

Sounds right to me


2003-05-01 17:27:46

by Thomas Backlund

[permalink] [raw]
Subject: Re: [PATCH 2.4.21-rc1] vesafb with large memory

Viestissä Torstai 1. Toukokuuta 2003 17:49, Alan Cox kirjoitti:
> On Iau, 2003-05-01 at 16:01, Thomas Backlund wrote:
> > You mean the patch that only looks at videomode, dont you...
>
> I do
>
> > Well maybe it's best to use it as default, to avoid this bug
> > "out of the box"...
> >
> > But I will remake a patch to ovverride it so that the users who
> > need/want the extra memory to be able to allocate it...
> > since I like the idea of giving the user the choice...
>
> Sounds right to me

Yeah, so merging the two patches ends up like this:
----- cut -----
--- tmb3/Documentation/fb/vesafb.txt 2003-05-01 19:53:26.000000000 +0300
+++ tmb3/Documentation/fb/vesafb.txt.vram 2003-05-01 20:11:08.000000000
+0300
@@ -146,6 +146,11 @@

mtrr setup memory type range registers for the vesafb framebuffer.

+vram:n remap 'n' MiB of video RAM. If 0 or not specified, remap memory
+ according to video mode. (2.5.66 patch/idea by Antonino Daplas
+ reversed to give override possibility (allocate more fb memory
+ than the kernel would) to 2.4 by [email protected])
+

Have fun!

--- tmb3/drivers/video/vesafb.c 2003-05-01 19:49:34.000000000 +0300
+++ tmb3/drivers/video/vesafb.c.vram 2003-05-01 20:07:57.000000000 +0300
@@ -94,6 +94,7 @@

static int inverse = 0;
static int mtrr = 0;
+static int vram __initdata = 0; /* needed for vram boot option */
static int currcon = 0;

static int pmi_setpal = 0; /* pmi for palette changes ??? */
@@ -479,6 +480,10 @@
pmi_setpal=1;
else if (! strcmp(this_opt, "mtrr"))
mtrr=1;
+ /* checks for vram boot option */
+ else if (! strncmp(this_opt, "vram:", 5))
+ vram = simple_strtoul(this_opt+5, NULL, 0);
+
else if (!strncmp(this_opt, "font:", 5))
strcpy(fb_info.fontname, this_opt+5);
}
@@ -520,7 +525,10 @@
video_width = screen_info.lfb_width;
video_height = screen_info.lfb_height;
video_linelength = screen_info.lfb_linelength;
- video_size = screen_info.lfb_size * 65536;
+ video_size = screen_info.lfb_size * screen_info.lfb_height *
video_bpp;
+ /* sets video_size according to boot option */
+ if (vram && vram * 1024 * 1024 > video_size)
+ video_size = vram * 1024 * 1024;
video_visual = (video_bpp == 8) ?
FB_VISUAL_PSEUDOCOLOR : FB_VISUAL_TRUECOLOR;
----- cut -----

And yes...
This is still without a "failsafe" since I don't have an upper limit,
but it occurred to me that maybe it isn't that easy after all...
- I can't trust the videocard memory probe...,
- to just calculate the video_size * 3 (for tripple buffering) is bad ;-)
- videocards with everyhing between 256k and 256 MB of memory,
makes choosing the limit somewhat impossible...
- of course there is also endless range of installed system memory...

So, any thoughts...?
Or should one just leave it open due to:
" it's your system, feel free to break it..." ;-)

Oh well,
I'm building my testkernel now with the above patch,
and will be testing different vram options to "kill" my system...

--
Thomas Backlund

[email protected]
http://www.iki.fi/tmb

2003-05-02 00:02:12

by Thomas Backlund

[permalink] [raw]
Subject: Re: [PATCH 2.4.21-rc1] vesafb with large memory

Thomas Backlund wrote:
>Viestissä Torstai 1. Toukokuuta 2003 17:49, Alan Cox kirjoitti:
>> On Iau, 2003-05-01 at 16:01, Thomas Backlund wrote:
>> > You mean the patch that only looks at videomode, dont you...
>>
>> I do
>>
>> > Well maybe it's best to use it as default, to avoid this bug
>> > "out of the box"...
>> >
>> > But I will remake a patch to ovverride it so that the users who
>> > need/want the extra memory to be able to allocate it...
>> > since I like the idea of giving the user the choice...
>>
>> Sounds right to me
>
>>Oh well,
>I'm building my testkernel now with the above patch,
>and will be testing different vram options to "kill" my system...

And here are the results...

the testsystem:
Amd XP 1800+
nForce2 m/b
256MB DDR RAM
Geforce4 Ti4200 64MB, videomode 1024x768x32
kernel-2.4.21-0.16mdk (i586 optimized)
kernel-2.4.21-0.16mdkent (i686 optimized + HIGHMEM)

without any vram option it allocated:
vesafb: framebuffer at 0xc0000000, mapped to 0xd0800000, size 24576k
as it should...

the standard kernel maxed out at about 755 MB
the enterprise kernel maxed out at about 720 MB

with the enterprise kernel I managed to get it to boot
with an allocation request for 1536MB, but the screen
blacked out at the berginning, even if the kernel seemed
to boot without errors....

booting with 800MB or 1024MB request resulted in an direct
lockup with flashing keyboard leds....

so it seems to be the 1GB magical limit again...

So it seems useless to try and set an upper limit,
instead just leving it to the users choice to set whatever
he/she wants...


Now I have to get some sleep....

--
Thomas Backlund

[email protected]
http://www.iki.fi/tmb

2003-05-02 12:51:22

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH 2.4.21-rc1] vesafb with large memory

On Fri, May 02, 2003 at 03:14:01AM +0300, Thomas Backlund wrote:
> And here are the results...
[snip]

Hi Thomas,

With this patch, vesafb doesn't work anymore on my vaio notebook in 1400x1050
nor 1280x1024, because scree_info.lfb_size is reported to be 127, while it
should be 175 and 160 instead ! So I modified your patch a little bit to get it
right :

- video_size = screen_info.lfb_size * screen_info.lfb_height * video_bpp;
+ video_size = screen_info.lfb_width/8 * screen_info.lfb_height * video_bpp;

and it now works again.
Maybe I have a broken bios, but other people might have the problem too.

Cheers,
Willy

2003-05-03 12:34:41

by Thomas Backlund

[permalink] [raw]
Subject: Re: [PATCH 2.4.21-rc1] vesafb with large memory

Viestiss? Perjantai 2. Toukokuuta 2003 16:03, Willy Tarreau kirjoitti:
> On Fri, May 02, 2003 at 03:14:01AM +0300, Thomas Backlund wrote:
> > And here are the results...
>
> [snip]
>
> Hi Thomas,
>
> With this patch, vesafb doesn't work anymore on my vaio notebook in
> 1400x1050 nor 1280x1024, because scree_info.lfb_size is reported to be
> 127, while it should be 175 and 160 instead ! So I modified your patch a
> little bit to get it right :
>
> - video_size = screen_info.lfb_size * screen_info.lfb_height *
> video_bpp; + video_size = screen_info.lfb_width/8 *
> screen_info.lfb_height * video_bpp;
>
> and it now works again.
> Maybe I have a broken bios, but other people might have the problem too.
>
> Cheers,
> Willy

Oh man...
I must have been sleeping when I posted that patch...

the correct line should AFAIK be:
video_size = screen_info.lfb_width * screen_info.lfb_height * video_bpp;

(AFAIK we are calculating bits here, not bytes so the '/8' you used is
wrong... could you try without it, and let me know...)

or even shorter:

video_size = video_width * video_height * video_bpp;


I'll be rebuilding and retesting my system today or tommorrow,
but I wuold like to hear if it works for you...


--
Thomas Backlund

[email protected]
http://www.iki.fi/tmb

2003-05-03 21:46:22

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH 2.4.21-rc1] vesafb with large memory

On Sat, May 03, 2003 at 03:46:57PM +0300, Thomas Backlund wrote:

> Oh man...
> I must have been sleeping when I posted that patch...

no problem, we all need some sleep at least once a week ;-)

> the correct line should AFAIK be:
> video_size = screen_info.lfb_width * screen_info.lfb_height * video_bpp;
>
> (AFAIK we are calculating bits here, not bytes so the '/8' you used is
> wrong... could you try without it, and let me know...)

OK, I was assuming that video_size was in bytes. I will retry without the /8.

> I'll be rebuilding and retesting my system today or tommorrow,
> but I wuold like to hear if it works for you...

I just come back home this evening, so let's say you'll get a feedback tomorrow
morning.

Regards,
Willy

2003-05-04 09:36:42

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH 2.4.21-rc1] vesafb with large memory

Hi Thomas,


> the correct line should AFAIK be:
> video_size = screen_info.lfb_width * screen_info.lfb_height * video_bpp;
>
> (AFAIK we are calculating bits here, not bytes so the '/8' you used is
> wrong... could you try without it, and let me know...)

No, after verification, I insist, we're really calculating BYTES here. Please
take a look :

================== with lfb_width/8 :

wtap:~$ dmesg|grep vesafb
vesafb: framebuffer at 0xe9000000, mapped to 0xe0825000, size 1435k
vesafb: mode is 1400x1050x8, linelength=1400, pages=4
vesafb: protected mode interface info at c000:50ae
vesafb: scrolling: redraw

wtap:~$ grep vesafb /proc/iomem
e9000000-e9166e2f : vesafb

wtap:~$ cat /proc/mtrr
reg00: base=0x00000000 ( 0MB), size= 512MB: write-back, count=1
reg01: base=0xe9000000 (3728MB), size= 1MB: write-combining, count=1

================== with lfb_width :

wtap:~$ dmesg|grep vesafb
vesafb: framebuffer at 0xe9000000, mapped to 0xe0825000, size 11484k
vesafb: mode is 1400x1050x8, linelength=1400, pages=4
vesafb: protected mode interface info at c000:50ae
vesafb: scrolling: redraw

wtap:~$ grep vesafb /proc/iomem
e9000000-e9b3717f : vesafb

wtap:~$ cat /proc/mtrr
reg00: base=0x00000000 ( 0MB), size= 512MB: write-back, count=1
reg01: base=0xe9000000 (3728MB), size= 8MB: write-combining, count=1

============

So I think that the correct line really is :
video_size = screen_info.lfb_width * screen_info.lfb_height * video_bpp / 8;

(Which also handles line widths which are not multiple of 8).

BTW, I wonder why we truncate the mtrr size to the highest lower power of 2.
Shouldn't we round it up to the next one ?

Personnally, I would find this change more logical. For instance, it returns
1 MB for video_size between 1MB and 2MB-1 inclusive.

Any comments ?

Willy

--- ./drivers/video/vesafb.c
+++ ./drivers/video/vesafb.c
@@ -647,4 +647,5 @@
- int temp_size = video_size;
+ int temp_size = video_size - 1;
/* Find the largest power-of-two */
while (temp_size & (temp_size - 1))
temp_size &= (temp_size - 1);
+ temp_size <<= 1;

2003-05-06 00:09:13

by Thomas Backlund

[permalink] [raw]
Subject: Re: [PATCH 2.4.21-rc1] vesafb with large memory

Viestiss? Sunnuntai 4. Toukokuuta 2003 12:49, Willy TARREAU kirjoitti:
> Hi Thomas,
>
> > the correct line should AFAIK be:
> > video_size = screen_info.lfb_width * screen_info.lfb_height *
> > video_bpp;
> >
> > (AFAIK we are calculating bits here, not bytes so the '/8' you used is
> > wrong... could you try without it, and let me know...)
>
> No, after verification, I insist, we're really calculating BYTES here.
> Please take a look :

Yes,
you are right...

[...]
>
> So I think that the correct line really is :
> video_size = screen_info.lfb_width * screen_info.lfb_height *
> video_bpp / 8;

There is a problem with this, ...
If we calculate the exact memory like this, there wont be any
memory remapped to do double/tripple buffering...
So the question is: shoud one take the formula and add ' * 2' to
atleast get the double buffering supported...
(in the patch I made for mdk, I kept a modified override part so that
the user can change this, if he needs it....)

Alan,
any comments on this?

> (Which also handles line widths which are not multiple of 8).

Well actually, in that formula it does not matter where you put the '/8',
the result is always the same...

> BTW, I wonder why we truncate the mtrr size to the highest lower power
> of 2. Shouldn't we round it up to the next one ?
>

Isn't it a hardware requirement?
I think I read it in a nvidia document once...
(of course they may be wrong...)

--
Thomas Backlund

[email protected]
http://www.iki.fi/tmb