nvidiafb: ioremap and i2c fixes

- Add 'vram' option to specify amount of video RAM to remap
- Limit remap size to 128 MIB
- Use info->screen_size for remapped RAM
- Fix misplaced label in failure path

From: Antonino Daplas <[email protected]>

---

 nvidia.c |   26 +++++++++++++++++++++-----
 1 files changed, 21 insertions(+), 5 deletions(-)

diff -Nru a/drivers/video/nvidia/nvidia.c b/drivers/video/nvidia/nvidia.c
@@ -408,6 +408,7 @@
 static int noaccel __devinitdata = 0;
 static int noscale __devinitdata = 0;
 static int paneltweak __devinitdata = 0;
+static int vram __devinitdata = 0;
 #ifdef CONFIG_MTRR
 static int nomtrr __devinitdata = 0;
 #endif

@@ -1180,7 +1181,7 @@
 
 	var->xres_virtual = (var->xres_virtual + 63) & ~63;
 
-	vramlen = info->fix.smem_len;
+	vramlen = info->screen_size;
 	pitch = ((var->xres_virtual * var->bits_per_pixel) + 7) / 8;
 	memlen = pitch * var->yres_virtual;
 

@@ -1343,7 +1344,7 @@
 	/* maximize virtual vertical length */
 	lpitch = info->var.xres_virtual *
 		((info->var.bits_per_pixel + 7) >> 3);
-	info->var.yres_virtual = info->fix.smem_len / lpitch;
+	info->var.yres_virtual = info->screen_size / lpitch;
 
 	info->pixmap.scan_align = 4;
 	info->pixmap.buf_align = 4;

@@ -1507,12 +1508,20 @@
 
 	par->FbAddress = nvidiafb_fix.smem_start;
 	par->FbMapSize = par->RamAmountKBytes * 1024;
+	if (vram && vram * 1024 * 1024 < par->FbMapSize)
+		par->FbMapSize = vram * 1024 * 1024;
+
+	/* Limit amount of vram to 128 MB */
+	if (par->FbMapSize > 128 * 1024 * 1024)
+		par->FbMapSize = 128 * 1024 * 1024;
+
 	par->FbUsableSize = par->FbMapSize - (128 * 1024);
 	par->ScratchBufferSize = (par->Architecture < NV_ARCH_10) ? 8 * 1024 :
 	    16 * 1024;
 	par->ScratchBufferStart = par->FbUsableSize - par->ScratchBufferSize;
 	info->screen_base = ioremap(nvidiafb_fix.smem_start, par->FbMapSize);
-	nvidiafb_fix.smem_len = par->FbUsableSize;
+	info->screen_size = par->FbUsableSize;
+	nvidiafb_fix.smem_len = par->RamAmountKBytes * 1024;
 
 	if (!info->screen_base) {
 		printk(KERN_ERR PFX "cannot ioremap FB base\n");

@@ -1524,7 +1533,8 @@
 #ifdef CONFIG_MTRR
 	if (!nomtrr) {
 		par->mtrr.vram = mtrr_add(nvidiafb_fix.smem_start,
-					  par->FbMapSize, MTRR_TYPE_WRCOMB, 1);
+					  par->RamAmountKBytes * 1024,
+					  MTRR_TYPE_WRCOMB, 1);
 		if (par->mtrr.vram < 0) {
 			printk(KERN_ERR PFX "unable to setup MTRR\n");
 		} else {

@@ -1566,9 +1576,9 @@
 
       err_out_iounmap_fb:
 	iounmap(info->screen_base);
+      err_out_free_base1:
 	fb_destroy_modedb(info->monspecs.modedb);
 	nvidia_delete_i2c_busses(par);
-      err_out_free_base1:
 	iounmap(par->REGS);
       err_out_free_base0:
 	pci_release_regions(pd);

@@ -1645,6 +1655,8 @@
 			noscale = 1;
 		} else if (!strncmp(this_opt, "paneltweak:", 11)) {
 			paneltweak = simple_strtoul(this_opt+11, NULL, 0);
+		} else if (!strncmp(this_opt, "vram:", 5)) {
+			vram = simple_strtoul(this_opt+5, NULL, 0);
 #ifdef CONFIG_MTRR
 		} else if (!strncmp(this_opt, "nomtrr", 6)) {
 			nomtrr = 1;

@@ -1716,6 +1728,10 @@
 MODULE_PARM_DESC(forceCRTC,
 		 "Forces usage of a particular CRTC in case autodetection "
 		 "fails. (0 or 1) (default=autodetect)");
+module_param(vram, int, 0);
+MODULE_PARM_DESC(vram,
+		 "amount of framebuffer memory to remap in MiB"
+		 "(default=0 - remap entire memory)");
 #ifdef CONFIG_MTRR
 module_param(nomtrr, bool, 0);
 MODULE_PARM_DESC(nomtrr, "Disables MTRR support (0 or 1=disabled) "

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Am I right in believing that if someone has a 256MB card and doesn't know about the `vram' option, their card will fail to work, because the 128MB vmalloc() will fail?

If so, that seems a bit user-hostile to me. Perhaps we should make it 64MB?

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

I thought that a 128MiB vmalloc will work.

Okay, dropping maximum to 64 MiB.

From: Antonino Daplas <[email protected]>

 nvidia.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff -Nru a/drivers/video/nvidia/nvidia.c b/drivers/video/nvidia/nvidia.c
@@ -1511,9 +1511,9 @@
 	if (vram && vram * 1024 * 1024 < par->FbMapSize)
 		par->FbMapSize = vram * 1024 * 1024;
 
-	/* Limit amount of vram to 128 MB */
-	if (par->FbMapSize > 128 * 1024 * 1024)
-		par->FbMapSize = 128 * 1024 * 1024;
+	/* Limit amount of vram to 64 MB */
+	if (par->FbMapSize > 64 * 1024 * 1024)
+		par->FbMapSize = 64 * 1024 * 1024;
 
 	par->FbUsableSize = par->FbMapSize - (128 * 1024);
 	par->ScratchBufferSize = (par->Architecture < NV_ARCH_10) ? 8 * 1024 :

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Well only if there are zero other current vmallocations and if we didn't have the guard page.

	unsigned int __VMALLOC_RESERVE = 128 << 20;

Fair enough. I guess we should check the vmalloc max on all other supported 32-bit architectures.

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

On Mon, 25 Apr 2005 11:21:20 +0800 Antonino A. Daplas wrote:

| On Sunday 24 April 2005 08:40, Andrew Morton wrote:
| > "Antonino A. Daplas" <[email protected]> wrote:
| > > - Add 'vram' option to specify amount of video RAM to remap
| > >  - Limit remap size to 128 MIB
| >
| > Am I right in believing that if someone has a 256MB card and doesn't know
| > about the `vram' option, their card will fail to work, because the 128MB
| > vmalloc() will fail?
| >
| 
| I thought that a 128MiB vmalloc will work.

The default vmalloc total size is 128 MB, but the kernel always requires a "guard page", which cannot be allocated, so the total allocation fails.

I tried to make a patch that sets the default vmalloc total size to 128 MB + PAGE_SIZE, but Miles reported that the driver's vmalloc() call still failed...

| > If so, that seems a bit user-hostile to me.  Perhaps we should make it
| > 64MB?
| 
| Okay, dropping maximum to 64 MiB.
| 
| From: Antonino Daplas <[email protected]>
| Signed-off-by: Antonino Daplas <[email protected]>
| 
|  nvidia.c |    6 +++---
|  1 files changed, 3 insertions(+), 3 deletions(-)
| 
| diff -Nru a/drivers/video/nvidia/nvidia.c b/drivers/video/nvidia/nvidia.c
| --- a/drivers/video/nvidia/nvidia.c	2005-04-16 11:54:44 +08:00
| +++ b/drivers/video/nvidia/nvidia.c	2005-04-25 10:39:45 +08:00
| @@ -1511,9 +1511,9 @@
|  	if (vram && vram * 1024 * 1024 < par->FbMapSize)
|  		par->FbMapSize = vram * 1024 * 1024;
|  
| -	/* Limit amount of vram to 128 MB */
| -	if (par->FbMapSize > 128 * 1024 * 1024)
| -		par->FbMapSize = 128 * 1024 * 1024;
| +	/* Limit amount of vram to 64 MB */
| +	if (par->FbMapSize > 64 * 1024 * 1024)
| +		par->FbMapSize = 64 * 1024 * 1024;
|  
|  	par->FbUsableSize = par->FbMapSize - (128 * 1024);
|  	par->ScratchBufferSize = (par->Architecture < NV_ARCH_10) ? 8 * 1024 :

---
~Randy

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Which seems to beg a couple of questions:
How much memory is _really_ trying to get allocated when I boot with no vmalloc argument. Does this point to some problem in the nvidiafb code? Does the fact that specifying vmalloc=128M does not fix the problem indicate that something about my video card won't work with less than 256Mb vmalloced, or is this a driver issue?

Thanks,
Miles

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id396&op=click

On Sun, 24 Apr 2005 22:36:42 -0700 Miles Lane wrote:

| On 4/24/05, Randy.Dunlap <[email protected]> wrote:
| > On Mon, 25 Apr 2005 11:21:20 +0800 Antonino A. Daplas wrote:
| > 
| > | On Sunday 24 April 2005 08:40, Andrew Morton wrote:
| > | > "Antonino A. Daplas" <[email protected]> wrote:
| > | > > - Add 'vram' option to specify amount of video RAM to remap
| > | > >  - Limit remap size to 128 MIB
| > | >
| > | > Am I right in believing that if someone has a 256MB card and doesn't know
| > | > about the `vram' option, their card will fail to work, because the 128MB
| > | > vmalloc() will fail?
| > | >
| > |
| > | I thought that a 128MiB vmalloc will work.
| > 
| > The default vmalloc total size is 128 MB, but the kernel always
| > requires a "guard page", which cannot be allocated, so the
| > total allocation fails.
| > 
| > I tried to make a patch that sets the default vmalloc total size
| > to 128 MB + PAGE_SIZE, but Miles reported that the driver's
| > vmalloc() call still failed...
| 
| Which seems to beg a couple of questions:
| How much memory is _really_ trying to get allocated when I boot
| with no vmalloc argument.  Does this point to some problem in 
| the nvidiafb code?  Does the fact that specifying vmalloc=128M
| does not fix the problem indicate that something about my video
| card won't work with less than 256Mb vmalloced, or is this a
| driver issue?

or simply that some other code is also vmalloc-ing some memory, so that nvidiafb's request can't succeed due to that?

You could go back to the very first test patch that I sent to you to see if other vmalloc's are happening. I don't recall the result of that one, but it seems like it was "Nope."

---
~Randy

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click