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

[Willy TARREAU <willy@w.ods.org>]

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;