Re: [PATCH 2.4.29-pre1] radeonfb: don't try to ioreamp the entire VRAM [was: Re: [Linux-fbdev-devel] why does radeonfb work fine in 2.6, but not in 2.4.29-pre1?]
From: Kronos
Date: Thu Dec 02 2004 - 11:01:09 EST
Il Thu, Dec 02, 2004 at 04:39:25PM +0100, Geert Uytterhoeven ha scritto:
> On Thu, 2 Dec 2004, Kronos wrote:
> > Il Wed, Dec 01, 2004 at 10:25:38PM +0100, Geert Uytterhoeven ha scritto:
> > > On Wed, 1 Dec 2004, Kronos wrote:
> > > > Il Wed, Dec 01, 2004 at 05:25:52PM +0100, Geert Uytterhoeven ha scritto:
> > > > > > Make fb layer aware of the difference between the ioremap()'ed VRAM and
> > > > > > total available VRAM.
> > > > > > smem_len in struct fb_fix_screeninfo still contains the amount of
> > > > > > physical VRAM (reported to userspace via FBIOGET_FSCREENINFO ioctl) and
> > > > > > the new field mapped_vram contains the amount of VRAM actually
> > > > > > ioremap()'ed by drivers (used in read/write/mmap operations).
> > > > > > Since there was unused padding at the end of struct fb_fix_screeninfo
> > > > > > binary compatibility with userspace utilities is retained.
> > > > > > If mapped_vram is not set it's assumed that the entire framebuffer is
> > > > > > mapped, thus other drivers are unaffected by this patch.
> > > > > >
> > > > > > The patch has been tested by Jurriaan <thunder7@xxxxxxxxx>.
> > > > > >
> > > > > > Signed-off-by: Luca Tettamanti <kronos@xxxxxxxxx>
> > > > > >
> > > > > > --- a/include/linux/fb.h 2004-11-30 18:30:08.000000000 +0100
> > > > > > +++ b/include/linux/fb.h 2004-11-30 18:33:00.000000000 +0100
> > > > > > @@ -126,7 +126,8 @@
> > > > > > /* (physical address) */
> > > > > > __u32 mmio_len; /* Length of Memory Mapped I/O */
> > > > > > __u32 accel; /* Type of acceleration available */
> > > > > > - __u16 reserved[3]; /* Reserved for future compatibility */
> > > > > > + __u32 mapped_vram; /* Amount of ioremap()'ed VRAM */
> > > > > > + __u16 reserved[1]; /* Reserved for future compatibility */
> > > > > > };
> > > > >
> > > > > I don't really like this patch. mapped_vram doesn't matter for user space at
> > > > > all, so it does not belong to fb_fix_screeninfo.
> > > >
> > > > Hmm, it looked sensible to me since it's the max amount of data that
> > > > userspace can read (or write) from /dev/fb%d.
> > >
> > > That's not really a user space limitation, but a kernel `bug'.
> > >
> > > I.e. it could be solved in the kernel if larger ioremap()s would be allowed, or
> > > if some sliding window mapping would be used. User space shouldn't need to know
> > > about this.
> >
> > I see you point, but users see that their video board does not work if
> > the system has 1GB (or more) of RAM and they aren't happy.
> > With current VM split (ie. 1GB/3GB) and with that much RAM there's no
> > space left to map the entire FB. And AFAIK VM hacking in 2.4 is not
> > doable (not that I would be able to do it ;)
> >
> > > > Putting mapped_vram in fb_info would be acceptable?
> > >
> > > That would make sure this stays in-kernel, but my other reasoning stays the
> > > same: it's some kind of kernel bug.
>
> Looks OK at first sight.
>
> > --- a/include/linux/fb.h 2004-11-30 18:30:08.000000000 +0100
> > +++ b/include/linux/fb.h 2004-12-02 14:20:50.000000000 +0100
> > @@ -323,6 +323,7 @@
> > struct fb_cmap cmap; /* Current cmap */
> > struct fb_ops *fbops;
> > char *screen_base; /* Virtual address */
> > + unsigned int mapped_vram; /* ioremap()'ed VRAM */
>
> But perhaps you want to make this unsigned long? I don't think it'll take long
> before we'll see graphics cards with 4 GiB or more memory, and it would be good
> if 64-bit platforms could take advantage of them.
>
> Oh no, we already have __u32 smem_len in struct fb_fix_screeninfo :-(
> So you can forget my comment about 64-bit above...
>
> Hence, you best make it `u32' (this is kernel internal) for consistency.
Ok, here we go:
Make fb layer aware of the difference between the ioremap()'ed VRAM and
total available VRAM.
smem_len in struct fb_fix_screeninfo contains the amount of physical
VRAM (reported to userspace via FBIOGET_FSCREENINFO ioctl) while the new
field mapped_vram in struct fb_info contains the amount of VRAM actually
ioremap()'ed by drivers (used in read/write/mmap operations).
If mapped_vram is not set it's assumed that the entire framebuffer is
mapped, thus other drivers are unaffected by this patch.
Signed-off-by: Luca Tettamanti <kronos@xxxxxxxxx>
--- a/include/linux/fb.h 2004-11-30 18:30:08.000000000 +0100
+++ b/include/linux/fb.h 2004-12-02 14:20:50.000000000 +0100
@@ -323,6 +323,7 @@
struct fb_cmap cmap; /* Current cmap */
struct fb_ops *fbops;
char *screen_base; /* Virtual address */
+ u32 mapped_vram; /* ioremap()'ed VRAM */
struct display *disp; /* initial display variable */
struct vc_data *display_fg; /* Console visible on this display */
char fontname[40]; /* default font name */
--- a/drivers/video/fbmem.c 2004-11-30 18:30:00.000000000 +0100
+++ b/drivers/video/fbmem.c 2004-12-02 14:29:44.000000000 +0100
@@ -410,6 +410,7 @@
struct fb_info *info = registered_fb[fbidx];
struct fb_ops *fb = info->fbops;
struct fb_fix_screeninfo fix;
+ unsigned int size;
if (! fb || ! info->disp)
return -ENODEV;
@@ -418,10 +419,12 @@
return -EINVAL;
fb->fb_get_fix(&fix,PROC_CONSOLE(info), info);
- if (p >= fix.smem_len)
+ size = info->mapped_vram ? info->mapped_vram : fix.smem_len;
+
+ if (p >= size)
return 0;
- if (count > fix.smem_len - p)
- count = fix.smem_len - p;
+ if (count > size - p)
+ count = size - p;
if (count) {
char *base_addr;
@@ -444,6 +447,7 @@
struct fb_ops *fb = info->fbops;
struct fb_fix_screeninfo fix;
int err;
+ unsigned int size;
if (! fb || ! info->disp)
return -ENODEV;
@@ -452,11 +456,13 @@
return -EINVAL;
fb->fb_get_fix(&fix, PROC_CONSOLE(info), info);
- if (p > fix.smem_len)
+ size = info->mapped_vram ? info->mapped_vram : fix.smem_len;
+
+ if (p > size)
return -ENOSPC;
err = 0;
- if (count > fix.smem_len - p) {
- count = fix.smem_len - p;
+ if (count > size - p) {
+ count = size - p;
err = -ENOSPC;
}
if (count) {
@@ -619,7 +625,10 @@
/* frame buffer memory */
start = fix.smem_start;
- len = PAGE_ALIGN((start & ~PAGE_MASK)+fix.smem_len);
+ if (info->mapped_vram)
+ len = PAGE_ALIGN((start & ~PAGE_MASK) + info->mapped_vram);
+ else
+ len = PAGE_ALIGN((start & ~PAGE_MASK) + fix.smem_len);
if (off >= len) {
/* memory mapped io */
off -= len;
--- a/drivers/video/radeonfb.c 2004-11-30 18:06:45.000000000 +0100
+++ b/drivers/video/radeonfb.c 2004-12-02 14:28:10.000000000 +0100
@@ -176,7 +176,8 @@
#define RTRACE if(0) printk
#endif
-
+#define MAX_MAPPED_VRAM (2048*2048*4)
+#define MIN_MAPPED_VRAM (1024*768*1)
enum radeon_chips {
RADEON_QD,
@@ -499,7 +500,7 @@
short chipset;
unsigned char arch;
- int video_ram;
+ unsigned int video_ram;
u8 rev;
int pitch, bpp, depth;
int xres, yres, pixclock;
@@ -1626,6 +1627,7 @@
const struct pci_device_id *ent)
{
struct radeonfb_info *rinfo;
+ struct fb_info *fb_info;
struct radeon_chip_info *rci = &radeon_chip_info[ent->driver_data];
u32 tmp;
int i, j;
@@ -1640,6 +1642,7 @@
memset (rinfo, 0, sizeof (struct radeonfb_info));
+ fb_info = (struct fb_info *)rinfo;
rinfo->pdev = pdev;
strncpy(rinfo->name, rci->name, 16);
rinfo->arch = rci->arch;
@@ -1824,8 +1827,16 @@
}
}
- rinfo->fb_base = (unsigned long) ioremap (rinfo->fb_base_phys,
- rinfo->video_ram);
+ fb_info->mapped_vram = min_t(unsigned int, MAX_MAPPED_VRAM, rinfo->video_ram);
+ do {
+ rinfo->fb_base = (unsigned long) ioremap (rinfo->fb_base_phys,
+ fb_info->mapped_vram);
+ if (rinfo->fb_base)
+ break;
+
+ fb_info->mapped_vram /= 2;
+ } while(fb_info->mapped_vram > MIN_MAPPED_VRAM);
+
if (!rinfo->fb_base) {
printk ("radeonfb: cannot map FB\n");
iounmap ((void*)rinfo->mmio_base);
@@ -1836,6 +1847,7 @@
kfree (rinfo);
return -ENODEV;
}
+ RTRACE(KERN_INFO "radeonfb: mapped %dk videoram\n", fb_info->mapped_vram/1024);
/* currcon not yet configured, will be set by first switch */
rinfo->currcon = -1;
@@ -2199,13 +2211,14 @@
{-1, -1}
};
int i;
+ struct fb_info *fb_info = (struct fb_info *)rinfo;
/* use highest possible virtual resolution */
if (v->xres_virtual == -1 && v->yres_virtual == -1) {
printk("radeonfb: using max available virtual resolution\n");
for (i=0; modes[i].xres != -1; i++) {
if (modes[i].xres * nom / den * modes[i].yres <
- rinfo->video_ram / 2)
+ fb_info->mapped_vram / 2)
break;
}
if (modes[i].xres == -1) {
@@ -2218,15 +2231,15 @@
printk("radeonfb: virtual resolution set to max of %dx%d\n",
v->xres_virtual, v->yres_virtual);
} else if (v->xres_virtual == -1) {
- v->xres_virtual = (rinfo->video_ram * den /
+ v->xres_virtual = (fb_info->mapped_vram * den /
(nom * v->yres_virtual * 2)) & ~15;
} else if (v->yres_virtual == -1) {
v->xres_virtual = (v->xres_virtual + 15) & ~15;
- v->yres_virtual = rinfo->video_ram * den /
+ v->yres_virtual = fb_info->mapped_vram * den /
(nom * v->xres_virtual *2);
} else {
if (v->xres_virtual * nom / den * v->yres_virtual >
- rinfo->video_ram) {
+ fb_info->mapped_vram) {
return -EINVAL;
}
}
@@ -2430,6 +2443,9 @@
return -EINVAL;
}
+ if (((v.xres_virtual * v.yres_virtual * nom) / den) > info->mapped_vram)
+ return -EINVAL;
+
if (radeonfb_do_maximize(rinfo, var, &v, nom, den) < 0)
return -EINVAL;
Luca
--
Home: http://kronoz.cjb.net
Una donna sposa un uomo sperando che cambi, e lui non cambiera`. Un
uomo sposa una donna sperando che non cambi, e lei cambiera`.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/