[PATCH 1/2] uninorth: Use same sizes array everywhere

[PATCH 1/2] uninorth: Use same sizes array everywhere

[Michel Dänzer]

Until we have evidence of the opposite, we'll just assume for the sake of
simplicity that all revisions of UniNorth support the same aperture sizes.

Also derive the number of supported sizes from the array definition.

Signed-off-by: Michel Dänzer <michel@tungstengraphics.com>
---
 drivers/char/agp/uninorth-agp.c |   27 ++++++---------------------
 1 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/drivers/char/agp/uninorth-agp.c b/drivers/char/agp/uninorth-agp.c
index 91b71e7..78c8cb2 100644
--- a/drivers/char/agp/uninorth-agp.c
+++ b/drivers/char/agp/uninorth-agp.c
@@ -450,24 +450,7 @@ void null_cache_flush(void)
 
 /* Setup function */
 
-static struct aper_size_info_32 uninorth_sizes[7] =
-{
-#if 0 /* Not sure uninorth supports that high aperture sizes */
-	{256, 65536, 6, 64},
-	{128, 32768, 5, 32},
-	{64, 16384, 4, 16},
-#endif
-	{32, 8192, 3, 8},
-	{16, 4096, 2, 4},
-	{8, 2048, 1, 2},
-	{4, 1024, 0, 1}
-};
-
-/*
- * Not sure that u3 supports that high aperture sizes but it
- * would strange if it did not :)
- */
-static struct aper_size_info_32 u3_sizes[8] =
+static struct aper_size_info_32 uninorth_sizes[8] =
 {
 	{512, 131072, 7, 128},
 	{256, 65536, 6, 64},
@@ -483,7 +466,8 @@ struct agp_bridge_driver uninorth_agp_dr
 	.owner			= THIS_MODULE,
 	.aperture_sizes		= (void *)uninorth_sizes,
 	.size_type		= U32_APER_SIZE,
-	.num_aperture_sizes	= 4,
+	.num_aperture_sizes	= sizeof(uninorth_sizes)
+				/ sizeof(struct aper_size_info_32),
 	.configure		= uninorth_configure,
 	.fetch_size		= uninorth_fetch_size,
 	.cleanup		= uninorth_cleanup,
@@ -505,9 +489,10 @@ struct agp_bridge_driver uninorth_agp_dr
 
 struct agp_bridge_driver u3_agp_driver = {
 	.owner			= THIS_MODULE,
-	.aperture_sizes		= (void *)u3_sizes,
+	.aperture_sizes		= (void *)uninorth_sizes,
 	.size_type		= U32_APER_SIZE,
-	.num_aperture_sizes	= 8,
+	.num_aperture_sizes	= sizeof(uninorth_sizes)
+				/ sizeof(struct aper_size_info_32),
 	.configure		= uninorth_configure,
 	.fetch_size		= uninorth_fetch_size,
 	.cleanup		= uninorth_cleanup,


-- 
Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer

[PATCH 2/2] uninorth: Add module param 'aperture' for aperture size

[Michel Dänzer]

In contrast to most if not all PC BIOSes, OpenFirmware (OF) on PowerMacs with
UniNorth bridges does not allow changing the aperture size. The size set up by
OF is usually 16 MB, which is too low for graphics intensive environments.
Hence, add a module parameter that allows changing the aperture size at driver
initialization time. When the parameter is not specified, the default is still
to leave the size unchanged, usually as set up by OF.

Signed-off-by: Michel Dänzer <michel@tungstengraphics.com>
---
 drivers/char/agp/uninorth-agp.c |   59 ++++++++++++++++++++++++++-------------
 1 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/drivers/char/agp/uninorth-agp.c b/drivers/char/agp/uninorth-agp.c
index 78c8cb2..ef2010c 100644
--- a/drivers/char/agp/uninorth-agp.c
+++ b/drivers/char/agp/uninorth-agp.c
@@ -27,32 +27,48 @@ #include "agp.h"
 static int uninorth_rev;
 static int is_u3;
 
+static char __devinitdata *aperture = NULL;
 
 static int uninorth_fetch_size(void)
 {
-	int i;
-	u32 temp;
-	struct aper_size_info_32 *values;
-
-	pci_read_config_dword(agp_bridge->dev, UNI_N_CFG_GART_BASE, &temp);
-	temp &= ~(0xfffff000);
-	values = A_SIZE_32(agp_bridge->driver->aperture_sizes);
-
-	for (i = 0; i < agp_bridge->driver->num_aperture_sizes; i++) {
-		if (temp == values[i].size_value) {
-			agp_bridge->previous_size =
-			    agp_bridge->current_size = (void *) (values + i);
-			agp_bridge->aperture_size_idx = i;
-			return values[i].size;
+	int i = 0, size = 0;
+	struct aper_size_info_32 *values =
+	    A_SIZE_32(agp_bridge->driver->aperture_sizes);
+
+	if (aperture) {
+		char *save = aperture;
+
+		size = memparse(aperture, &aperture) >> 20;
+		aperture = save;
+
+		for (i = 0; i < agp_bridge->driver->num_aperture_sizes; i++)
+			if (size == values[i].size)
+				break;
+
+		if (i == agp_bridge->driver->num_aperture_sizes) {
+			printk(KERN_ERR PFX "Invalid aperture size, using"
+			       " default\n");
+			size = 0;
+			aperture = NULL;
 		}
 	}
 
-	agp_bridge->previous_size =
-	    agp_bridge->current_size = (void *) (values + 1);
-	agp_bridge->aperture_size_idx = 1;
-	return values[1].size;
+	if (!size) {
+		u32 temp;
 
-	return 0;
+		pci_read_config_dword(agp_bridge->dev, UNI_N_CFG_GART_BASE,
+				      &temp);
+		temp &= ~(0xfffff000);
+
+		for (i = 0; i < agp_bridge->driver->num_aperture_sizes; i++)
+			if (temp == values[i].size_value)
+				break;
+	}
+
+	agp_bridge->previous_size =
+	    agp_bridge->current_size = (void *)(values + i);
+	agp_bridge->aperture_size_idx = i;
+	return values[i].size;
 }
 
 static void uninorth_tlbflush(struct agp_memory *mem)
@@ -668,5 +684,10 @@ static void __exit agp_uninorth_cleanup(
 module_init(agp_uninorth_init);
 module_exit(agp_uninorth_cleanup);
 
+module_param(aperture, charp, 0);
+MODULE_PARM_DESC(aperture,
+		 "Aperture size, must be power of two between 4MB and 512MB.\n"
+		 "Default: Leave unchanged, e.g. as set up by firmware");
+
 MODULE_AUTHOR("Ben Herrenschmidt & Paul Mackerras");
 MODULE_LICENSE("GPL");


-- 
Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer

Re: [PATCH 2/2] uninorth: Add module param 'aperture' for aperture size

[Benjamin Herrenschmidt]

On Mon, 2006-08-21 at 18:09 +0200, Michel Dänzer wrote:

> In contrast to most if not all PC BIOSes, OpenFirmware (OF) on PowerMacs with
> UniNorth bridges does not allow changing the aperture size. The size set up by
> OF is usually 16 MB, which is too low for graphics intensive environments.
> Hence, add a module parameter that allows changing the aperture size at driver
> initialization time. When the parameter is not specified, the default is still
> to leave the size unchanged, usually as set up by OF.

The right fix is to allow changing it by the driver ... can't we do it
such that AGPSize will work & change it ? The firmware doesn't actually
configure it at all I suspect... those 16M are basically 
what the HW comes up with as a default setting (unless the firmware runs
some diagnostics and sets it as a result of these). It's basically
expected that we set it ourselves and a module option doesn't seem like
a terribly good idea to me...

Maybe we should just have the driver default to something more sensible
in addition to what AGPSize comes from userland ? like 64M ?

Ben.

Re: [PATCH 2/2] uninorth: Add module param 'aperture' for aperture size

[Michel Dänzer]

On Tue, 2006-08-22 at 17:25 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2006-08-21 at 18:09 +0200, Michel Dänzer wrote:
> 
> > In contrast to most if not all PC BIOSes, OpenFirmware (OF) on PowerMacs with
> > UniNorth bridges does not allow changing the aperture size. The size set up by
> > OF is usually 16 MB, which is too low for graphics intensive environments.
> > Hence, add a module parameter that allows changing the aperture size at driver
> > initialization time. When the parameter is not specified, the default is still
> > to leave the size unchanged, usually as set up by OF.
> 
> The right fix is to allow changing it by the driver ... can't we do it
> such that AGPSize will work & change it ? 

Not sure, but it seems like the aperture size is fixed at backend
initialization time, i.e. in the driver's probe hook.


> It's basically expected that we set it ourselves and a module option 
> doesn't seem like a terribly good idea to me...

Well, wasn't it you who suggested this? :)


> Maybe we should just have the driver default to something more sensible
> in addition to what AGPSize comes from userland ? like 64M ?

Might be a good idea.


-- 
Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer

Re: [PATCH 2/2] uninorth: Add module param 'aperture' for aperture size

[Benjamin Herrenschmidt]

> > It's basically expected that we set it ourselves and a module option 
> > doesn't seem like a terribly good idea to me...
> 
> Well, wasn't it you who suggested this? :)

Possibly, I don't always have good ideas :) I would rather have that
size be configured by the client driver... oh well, let's workaround
that for now by using a better default _and_ the module option.

> > Maybe we should just have the driver default to something more sensible
> > in addition to what AGPSize comes from userland ? like 64M ?
> 
> Might be a good idea.
> 
> 

Re: [PATCH 2/2] uninorth: Add module param 'aperture' for aperture size

[Michel Dänzer]

On Wed, 2006-08-23 at 08:17 +1000, Benjamin Herrenschmidt wrote:
> > > It's basically expected that we set it ourselves and a module option 
> > > doesn't seem like a terribly good idea to me...
> > 
> > Well, wasn't it you who suggested this? :)
> 
> Possibly, I don't always have good ideas :) I would rather have that
> size be configured by the client driver... 

That would certainly be ideal. Digging into the code a little deeper, it
seems like the backend's insert_memory hook should be able to adjust the
GATT table size on the fly. Not sure __get_free_pages() is expected to
succeed for dozens of pages at any given time though.


> oh well, let's workaround that for now by using a better default _and_ 
> the module option.

Agreed, I'll re-send the patch with the default changed to 64M.


-- 
Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer

[PATCH] uninorth: Add module param 'aperture' for aperture size

[Michel Dänzer]

In contrast to most if not all PC BIOSes, OpenFirmware (OF) on PowerMacs with
UniNorth bridges does not allow changing the aperture size. The size set up by
OF is usually 16 MB, which is too low for graphics intensive environments.
Hence, add a module parameter that allows changing the aperture size at driver
initialization time. When the parameter is not specified, the default is 32 MB.

Signed-off-by: Michel Dänzer <michel@tungstengraphics.com>
---

Note that I made the default size 32 MB (instead of 64 MB as discussed on the
linuxppc-dev list) to make this patch independent of the other uninorth patch I
sent out.

 drivers/char/agp/uninorth-agp.c |   54 +++++++++++++++++++++++++--------------
 1 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/drivers/char/agp/uninorth-agp.c b/drivers/char/agp/uninorth-agp.c
index 78c8cb2..4770187 100644
--- a/drivers/char/agp/uninorth-agp.c
+++ b/drivers/char/agp/uninorth-agp.c
@@ -27,32 +27,42 @@ #include "agp.h"
 static int uninorth_rev;
 static int is_u3;
 
+static char __devinitdata *aperture = NULL;
 
 static int uninorth_fetch_size(void)
 {
-	int i;
-	u32 temp;
-	struct aper_size_info_32 *values;
-
-	pci_read_config_dword(agp_bridge->dev, UNI_N_CFG_GART_BASE, &temp);
-	temp &= ~(0xfffff000);
-	values = A_SIZE_32(agp_bridge->driver->aperture_sizes);
-
-	for (i = 0; i < agp_bridge->driver->num_aperture_sizes; i++) {
-		if (temp == values[i].size_value) {
-			agp_bridge->previous_size =
-			    agp_bridge->current_size = (void *) (values + i);
-			agp_bridge->aperture_size_idx = i;
-			return values[i].size;
+	int i, size = 0;
+	struct aper_size_info_32 *values =
+	    A_SIZE_32(agp_bridge->driver->aperture_sizes);
+
+	if (aperture) {
+		char *save = aperture;
+
+		size = memparse(aperture, &aperture) >> 20;
+		aperture = save;
+
+		for (i = 0; i < agp_bridge->driver->num_aperture_sizes; i++)
+			if (size == values[i].size)
+				break;
+
+		if (i == agp_bridge->driver->num_aperture_sizes) {
+			printk(KERN_ERR PFX "Invalid aperture size, using"
+			       " default\n");
+			size = 0;
+			aperture = NULL;
 		}
 	}
 
-	agp_bridge->previous_size =
-	    agp_bridge->current_size = (void *) (values + 1);
-	agp_bridge->aperture_size_idx = 1;
-	return values[1].size;
+	if (!size) {
+		for (i = 0; i < agp_bridge->driver->num_aperture_sizes; i++)
+			if (32 == values[i].size)
+				break;
+	}
 
-	return 0;
+	agp_bridge->previous_size =
+	    agp_bridge->current_size = (void *)(values + i);
+	agp_bridge->aperture_size_idx = i;
+	return values[i].size;
 }
 
 static void uninorth_tlbflush(struct agp_memory *mem)
@@ -668,5 +678,11 @@ static void __exit agp_uninorth_cleanup(
 module_init(agp_uninorth_init);
 module_exit(agp_uninorth_cleanup);
 
+module_param(aperture, charp, 0);
+MODULE_PARM_DESC(aperture,
+		 "Aperture size, must be power of two between 4MB and an\n"
+		 "\t\tupper limit specific to the UniNorth revision.\n"
+		 "\t\tDefault: 32M");
+
 MODULE_AUTHOR("Ben Herrenschmidt & Paul Mackerras");
 MODULE_LICENSE("GPL");
-- 
1.4.1.1



-- 
Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer

[PATCH] uninorth: Add module param 'aperture' for aperture size

[Michel Dänzer]

In contrast to most if not all PC BIOSes, OpenFirmware (OF) on PowerMacs with
UniNorth bridges does not allow changing the aperture size. The size set up by
OF is usually 16 MB, which is too low for graphics intensive environments.
Hence, add a module parameter that allows changing the aperture size at driver
initialization time. When the parameter is not specified, the default is 32 MB.

Signed-off-by: Michel Dänzer <michel@tungstengraphics.com>
---

Any (N)ACKs on this would be much appreciated.

Note that I made the default size 32 MB (instead of 64 MB as discussed on the
linuxppc-dev list) to make this patch independent of the other uninorth patch I
sent out.

 drivers/char/agp/uninorth-agp.c |   54 +++++++++++++++++++++++++--------------
 1 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/drivers/char/agp/uninorth-agp.c b/drivers/char/agp/uninorth-agp.c
index 78c8cb2..4770187 100644
--- a/drivers/char/agp/uninorth-agp.c
+++ b/drivers/char/agp/uninorth-agp.c
@@ -27,32 +27,42 @@ #include "agp.h"
 static int uninorth_rev;
 static int is_u3;
 
+static char __devinitdata *aperture = NULL;
 
 static int uninorth_fetch_size(void)
 {
-	int i;
-	u32 temp;
-	struct aper_size_info_32 *values;
-
-	pci_read_config_dword(agp_bridge->dev, UNI_N_CFG_GART_BASE, &temp);
-	temp &= ~(0xfffff000);
-	values = A_SIZE_32(agp_bridge->driver->aperture_sizes);
-
-	for (i = 0; i < agp_bridge->driver->num_aperture_sizes; i++) {
-		if (temp == values[i].size_value) {
-			agp_bridge->previous_size =
-			    agp_bridge->current_size = (void *) (values + i);
-			agp_bridge->aperture_size_idx = i;
-			return values[i].size;
+	int i, size = 0;
+	struct aper_size_info_32 *values =
+	    A_SIZE_32(agp_bridge->driver->aperture_sizes);
+
+	if (aperture) {
+		char *save = aperture;
+
+		size = memparse(aperture, &aperture) >> 20;
+		aperture = save;
+
+		for (i = 0; i < agp_bridge->driver->num_aperture_sizes; i++)
+			if (size == values[i].size)
+				break;
+
+		if (i == agp_bridge->driver->num_aperture_sizes) {
+			printk(KERN_ERR PFX "Invalid aperture size, using"
+			       " default\n");
+			size = 0;
+			aperture = NULL;
 		}
 	}
 
-	agp_bridge->previous_size =
-	    agp_bridge->current_size = (void *) (values + 1);
-	agp_bridge->aperture_size_idx = 1;
-	return values[1].size;
+	if (!size) {
+		for (i = 0; i < agp_bridge->driver->num_aperture_sizes; i++)
+			if (32 == values[i].size)
+				break;
+	}
 
-	return 0;
+	agp_bridge->previous_size =
+	    agp_bridge->current_size = (void *)(values + i);
+	agp_bridge->aperture_size_idx = i;
+	return values[i].size;
 }
 
 static void uninorth_tlbflush(struct agp_memory *mem)
@@ -668,5 +678,11 @@ static void __exit agp_uninorth_cleanup(
 module_init(agp_uninorth_init);
 module_exit(agp_uninorth_cleanup);
 
+module_param(aperture, charp, 0);
+MODULE_PARM_DESC(aperture,
+		 "Aperture size, must be power of two between 4MB and an\n"
+		 "\t\tupper limit specific to the UniNorth revision.\n"
+		 "\t\tDefault: 32M");
+
 MODULE_AUTHOR("Ben Herrenschmidt & Paul Mackerras");
 MODULE_LICENSE("GPL");
-- 
1.4.1.1



-- 
Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer

Re: [PATCH] uninorth: Add module param 'aperture' for aperture size

[Andreas Schwab]

Michel Dänzer <michel@tungstengraphics.com> writes:

> +			if (32 == values[i].size)

variable == value, please.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."