[PATCH 1/4] gxfb: Replace FBSIZE config option with a kernel argument

[PATCH 1/4] gxfb: Replace FBSIZE config option with a kernel argument

[Andres Salomon]

Use a command line option (fbsize:) rather than hardcoding the vram size.
LxFB already does this; it's useful for machines that can't query the
BIOS for fb size.  This patch originated from David Woodhouse, was
modified by Jordan Crouse, and was then modified further by me.

Signed-off-by: Andres Salomon <dilinger@debian.org>
Acked-by: Jordan Crouse <jordan.crouse@amd.com>
---
 drivers/video/geode/Kconfig      |   20 --------------------
 drivers/video/geode/display_gx.c |    7 -------
 drivers/video/geode/gxfb_core.c  |   19 ++++++++++++-------
 3 files changed, 12 insertions(+), 34 deletions(-)

diff --git a/drivers/video/geode/Kconfig b/drivers/video/geode/Kconfig
index 7608429..c5d8ba4 100644
--- a/drivers/video/geode/Kconfig
+++ b/drivers/video/geode/Kconfig
@@ -38,26 +38,6 @@ config FB_GEODE_GX
 
 	  If unsure, say N.
 
-config FB_GEODE_GX_SET_FBSIZE
-	bool "Manually specify the Geode GX framebuffer size"
-	depends on FB_GEODE_GX
-	default n
-	---help---
-	  If you want to manually specify the size of your GX framebuffer,
-	  say Y here, otherwise say N to dynamically probe it.
-
-	  Say N unless you know what you are doing.
-
-config FB_GEODE_GX_FBSIZE
-	hex "Size of the GX framebuffer, in bytes"
-	depends on FB_GEODE_GX_SET_FBSIZE
-	default "0x1600000"
-	---help---
-	  Specify the size of the GX framebuffer.  Normally, you will
-	  want this to be MB aligned.  Common values are 0x80000 (8MB)
-	  and 0x1600000 (16MB).  Don't change this unless you know what
-	  you are doing
-
 config FB_GEODE_GX1
 	tristate "AMD Geode GX1 framebuffer support (EXPERIMENTAL)"
 	depends on FB && FB_GEODE && EXPERIMENTAL
diff --git a/drivers/video/geode/display_gx.c b/drivers/video/geode/display_gx.c
index 0f16e4b..8cd7572 100644
--- a/drivers/video/geode/display_gx.c
+++ b/drivers/video/geode/display_gx.c
@@ -21,12 +21,6 @@
 #include "geodefb.h"
 #include "display_gx.h"
 
-#ifdef CONFIG_FB_GEODE_GX_SET_FBSIZE
-unsigned int gx_frame_buffer_size(void)
-{
-	return CONFIG_FB_GEODE_GX_FBSIZE;
-}
-#else
 unsigned int gx_frame_buffer_size(void)
 {
 	unsigned int val;
@@ -41,7 +35,6 @@ unsigned int gx_frame_buffer_size(void)
 	val = (unsigned int)(inw(0xAC1E)) & 0xFFl;
 	return (val << 19);
 }
-#endif
 
 int gx_line_delta(int xres, int bpp)
 {
diff --git a/drivers/video/geode/gxfb_core.c b/drivers/video/geode/gxfb_core.c
index cf841ef..07ff4bc 100644
--- a/drivers/video/geode/gxfb_core.c
+++ b/drivers/video/geode/gxfb_core.c
@@ -36,6 +36,7 @@
 #include "video_gx.h"
 
 static char *mode_option;
+static int fbsize;
 
 /* Modes relevant to the GX (taken from modedb.c) */
 static const struct fb_videomode gx_modedb[] __initdata = {
@@ -207,7 +208,6 @@ static int gxfb_blank(int blank_mode, struct fb_info *info)
 static int __init gxfb_map_video_memory(struct fb_info *info, struct pci_dev *dev)
 {
 	struct geodefb_par *par = info->par;
-	int fb_len;
 	int ret;
 
 	ret = pci_enable_device(dev);
@@ -232,21 +232,20 @@ static int __init gxfb_map_video_memory(struct fb_info *info, struct pci_dev *de
 	ret = pci_request_region(dev, 0, "gxfb (framebuffer)");
 	if (ret < 0)
 		return ret;
-	if ((fb_len = gx_frame_buffer_size()) < 0)
-		return -ENOMEM;
+
 	info->fix.smem_start = pci_resource_start(dev, 0);
-	info->fix.smem_len = fb_len;
+	info->fix.smem_len = fbsize ? fbsize : gx_frame_buffer_size();
 	info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
 	if (!info->screen_base)
 		return -ENOMEM;
 
-	/* Set the 16MB aligned base address of the graphics memory region
+	/* Set the 16MiB aligned base address of the graphics memory region
 	 * in the display controller */
 
 	writel(info->fix.smem_start & 0xFF000000,
 			par->dc_regs + DC_GLIU0_MEM_OFFSET);
 
-	dev_info(&dev->dev, "%d Kibyte of video memory at 0x%lx\n",
+	dev_info(&dev->dev, "%d KiB of video memory at 0x%lx\n",
 		 info->fix.smem_len / 1024, info->fix.smem_start);
 
 	return 0;
@@ -425,7 +424,10 @@ static int __init gxfb_setup(char *options)
 		if (!*opt)
 			continue;
 
-		mode_option = opt;
+		if (!strncmp(opt, "fbsize:", 7))
+			fbsize = simple_strtoul(opt+7, NULL, 0);
+		else
+			mode_option = opt;
 	}
 
 	return 0;
@@ -456,5 +458,8 @@ module_exit(gxfb_cleanup);
 module_param(mode_option, charp, 0);
 MODULE_PARM_DESC(mode_option, "video mode (<x>x<y>[-<bpp>][@<refr>])");
 
+module_param(fbsize, int, 0);
+MODULE_PARM_DESC(fbsize, "video memory size");
+
 MODULE_DESCRIPTION("Framebuffer driver for the AMD Geode GX");
 MODULE_LICENSE("GPL");
-- 
1.5.3.7

Re: [PATCH 1/4] gxfb: Replace FBSIZE config option with a kernel argument

[Andrew Morton]

On Sat, 23 Feb 2008 01:10:45 -0500
Andres Salomon <dilinger@queued.net> wrote:

> @@ -425,7 +424,10 @@ static int __init gxfb_setup(char *options)
>  		if (!*opt)
>  			continue;
>  
> -		mode_option = opt;
> +		if (!strncmp(opt, "fbsize:", 7))
> +			fbsize = simple_strtoul(opt+7, NULL, 0);
> +		else
> +			mode_option = opt;
>  	}

The above shouldn't be necessary.

And it should have been documented in Documentation/kernel-parameters.txt.

And "fbsize=N" would be a lot more conventional than "fbsize:N"

I suspect that the formulation you have here will not permit "fbsize:128k",
whereas "fbsize=128k" or "gxfb.fbsize=128k" should work.  Needs checking.

>  	return 0;
> @@ -456,5 +458,8 @@ module_exit(gxfb_cleanup);
>  module_param(mode_option, charp, 0);
>  MODULE_PARM_DESC(mode_option, "video mode (<x>x<y>[-<bpp>][@<refr>])");
>  
> +module_param(fbsize, int, 0);
> +MODULE_PARM_DESC(fbsize, "video memory size");
> +

Because the module_param() already makes fbsize available on the kernel
boot command line via gxfb.fbsize=42 (or something similar).



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/4] gxfb: Replace FBSIZE config option with a kernel argument

[Andres Salomon]

On Wed, 27 Feb 2008 16:31:05 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Sat, 23 Feb 2008 01:10:45 -0500
> Andres Salomon <dilinger@queued.net> wrote:
> 
> > @@ -425,7 +424,10 @@ static int __init gxfb_setup(char *options)
> >  		if (!*opt)
> >  			continue;
> >  
> > -		mode_option = opt;
> > +		if (!strncmp(opt, "fbsize:", 7))
> > +			fbsize = simple_strtoul(opt+7, NULL, 0);
> > +		else
> > +			mode_option = opt;
> >  	}
> 
> The above shouldn't be necessary.

It looks like that's done in other drivers in case MODULE isn't defined.  I'm
assuming this is historical at this point, and manual options parsing can
be removed from all fb drivers at this point, or is there another reason
why manual parsing would be necessary?


> 
> And it should have been documented in Documentation/kernel-parameters.txt.

Yeah, I wasn't actually sure about that; I did check for other fb drivers
documenting stuff in kernel-parameters.txt, and didn't see it.  It looks
like they instead document stuff in Documentation/fb/.  Which is preferred?



> 
> And "fbsize=N" would be a lot more conventional than "fbsize:N"
> 

I can certainly change that.  Regarding convention, I toyed with renaming
it 'vram' (as most of the fb drivers use that), and will probably do
that unless Jordan objects.


> I suspect that the formulation you have here will not permit "fbsize:128k",
> whereas "fbsize=128k" or "gxfb.fbsize=128k" should work.  Needs checking.
> 
> >  	return 0;
> > @@ -456,5 +458,8 @@ module_exit(gxfb_cleanup);
> >  module_param(mode_option, charp, 0);
> >  MODULE_PARM_DESC(mode_option, "video mode (<x>x<y>[-<bpp>][@<refr>])");
> >  
> > +module_param(fbsize, int, 0);
> > +MODULE_PARM_DESC(fbsize, "video memory size");
> > +
> 
> Because the module_param() already makes fbsize available on the kernel
> boot command line via gxfb.fbsize=42 (or something similar).
> 
> 

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: gxfb: Replace FBSIZE config option with a kernel argument

[Jordan Crouse]

> > And "fbsize=N" would be a lot more conventional than "fbsize:N"
> > 
> 
> I can certainly change that.  Regarding convention, I toyed with renaming
> it 'vram' (as most of the fb drivers use that), and will probably do
> that unless Jordan objects.

vram is fine.

Jordan

-- 
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/4] gxfb: Replace FBSIZE config option with a kernel argument

[Andres Salomon]

On Wed, 27 Feb 2008 19:58:39 -0500
Andres Salomon <dilinger@queued.net> wrote:

> On Wed, 27 Feb 2008 16:31:05 -0800
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Sat, 23 Feb 2008 01:10:45 -0500
> > Andres Salomon <dilinger@queued.net> wrote:
> > 
> > > @@ -425,7 +424,10 @@ static int __init gxfb_setup(char *options)
> > >  		if (!*opt)
> > >  			continue;
> > >  
> > > -		mode_option = opt;
> > > +		if (!strncmp(opt, "fbsize:", 7))
> > > +			fbsize = simple_strtoul(opt+7, NULL, 0);
> > > +		else
> > > +			mode_option = opt;
> > >  	}
> > 
> > The above shouldn't be necessary.
> 
> It looks like that's done in other drivers in case MODULE isn't defined.  I'm
> assuming this is historical at this point, and manual options parsing can
> be removed from all fb drivers at this point, or is there another reason
> why manual parsing would be necessary?
> 


Could I get an answer from the fbdevel folks about this?  It looks like
the fb_get_options stuff is there for backwards compatibility.
gxfb.fbsize=16777216 (for example) works regardless of whether or not
CONFIG_MODULES is set.  

It would appear that the only reason for fb_get_options() calling would
be to support video=gxfb:<options>.  Is this used any more?  Should it
be marked deprecated, and dropped from the geode fb drivers?


> 
> > 
> > And it should have been documented in Documentation/kernel-parameters.txt.
> 
> Yeah, I wasn't actually sure about that; I did check for other fb drivers
> documenting stuff in kernel-parameters.txt, and didn't see it.  It looks
> like they instead document stuff in Documentation/fb/.  Which is preferred?
> 


I'd also like an opinion about this.


> 
> 
> > 
> > And "fbsize=N" would be a lot more conventional than "fbsize:N"
> > 
> 
> I can certainly change that.  Regarding convention, I toyed with renaming
> it 'vram' (as most of the fb drivers use that), and will probably do
> that unless Jordan objects.
> 
> 
> > I suspect that the formulation you have here will not permit "fbsize:128k",
> > whereas "fbsize=128k" or "gxfb.fbsize=128k" should work.  Needs checking.
> > 
> > >  	return 0;
> > > @@ -456,5 +458,8 @@ module_exit(gxfb_cleanup);
> > >  module_param(mode_option, charp, 0);
> > >  MODULE_PARM_DESC(mode_option, "video mode (<x>x<y>[-<bpp>][@<refr>])");
> > >  
> > > +module_param(fbsize, int, 0);
> > > +MODULE_PARM_DESC(fbsize, "video memory size");
> > > +
> > 
> > Because the module_param() already makes fbsize available on the kernel
> > boot command line via gxfb.fbsize=42 (or something similar).
> > 
> > 

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/4] gxfb: Replace FBSIZE config option with a kernel argument

[Andres Salomon]

On Wed, 27 Feb 2008 19:58:39 -0500
Andres Salomon <dilinger@queued.net> wrote:

> On Wed, 27 Feb 2008 16:31:05 -0800
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Sat, 23 Feb 2008 01:10:45 -0500
> > Andres Salomon <dilinger@queued.net> wrote:
> > 
> > > @@ -425,7 +424,10 @@ static int __init gxfb_setup(char *options)
> > >  		if (!*opt)
> > >  			continue;
> > >  
> > > -		mode_option = opt;
> > > +		if (!strncmp(opt, "fbsize:", 7))
> > > +			fbsize = simple_strtoul(opt+7, NULL, 0);
> > > +		else
> > > +			mode_option = opt;
> > >  	}
> > 
> > The above shouldn't be necessary.
> 
> It looks like that's done in other drivers in case MODULE isn't defined.  I'm
> assuming this is historical at this point, and manual options parsing can
> be removed from all fb drivers at this point, or is there another reason
> why manual parsing would be necessary?
> 

Could I get an answer from the fbdevel folks about this?  It looks like
the fb_get_options stuff is there for backwards compatibility.
gxfb.fbsize=16777216 (for example) works regardless of whether or not
CONFIG_MODULES is set.  



> 
> > 
> > And it should have been documented in Documentation/kernel-parameters.txt.
> 
> Yeah, I wasn't actually sure about that; I did check for other fb drivers
> documenting stuff in kernel-parameters.txt, and didn't see it.  It looks
> like they instead document stuff in Documentation/fb/.  Which is preferred?
> 

I'd also like an opinion on this.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/4] gxfb: Replace FBSIZE config option with a kernel argument

[Randy Dunlap]

On Sat, 8 Mar 2008 20:19:50 -0500 Andres Salomon wrote:

> On Wed, 27 Feb 2008 19:58:39 -0500
> Andres Salomon <dilinger@queued.net> wrote:
> 
> > On Wed, 27 Feb 2008 16:31:05 -0800
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > On Sat, 23 Feb 2008 01:10:45 -0500
> > > Andres Salomon <dilinger@queued.net> wrote:
> > > 
> > > > @@ -425,7 +424,10 @@ static int __init gxfb_setup(char *options)
> > > >  		if (!*opt)
> > > >  			continue;
> > > >  
> > > > -		mode_option = opt;
> > > > +		if (!strncmp(opt, "fbsize:", 7))
> > > > +			fbsize = simple_strtoul(opt+7, NULL, 0);
> > > > +		else
> > > > +			mode_option = opt;
> > > >  	}
> > > 
> > > The above shouldn't be necessary.
> > 
> > It looks like that's done in other drivers in case MODULE isn't defined.  I'm
> > assuming this is historical at this point, and manual options parsing can
> > be removed from all fb drivers at this point, or is there another reason
> > why manual parsing would be necessary?
> > 
> 
> Could I get an answer from the fbdevel folks about this?  It looks like
> the fb_get_options stuff is there for backwards compatibility.
> gxfb.fbsize=16777216 (for example) works regardless of whether or not
> CONFIG_MODULES is set.  
> 

It would be very nice to get Mr. Daplas back.

> > 
> > > 
> > > And it should have been documented in Documentation/kernel-parameters.txt.
> > 
> > Yeah, I wasn't actually sure about that; I did check for other fb drivers
> > documenting stuff in kernel-parameters.txt, and didn't see it.  It looks
> > like they instead document stuff in Documentation/fb/.  Which is preferred?
> > 
> 
> I'd also like an opinion on this.

My opinion (only) is that any subsystem that has lots of doc and/or
kernel parameters should put that info into Documentation/<subdir>/,
so adding to Documentation/fb/ is good.  (sound is another big
candidate for this.  All of those "snd-xyz=" entries in
kernel-parameters.txt don't actually help other than to say that there
are some user-settable parameters that someone has to go search for.)
We should probably also extend this entry in kernel-parameters.txt:

 	video=		[FB] Frame buffer configuration
			See Documentation/fb/modedb.txt.

to point to Documentation/fb/ for all frame buffer docs.

---
~Randy

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/4] gxfb: Replace FBSIZE config option with a kernel argument

[Ondrej Zajicek]

On Sat, Mar 08, 2008 at 08:19:50PM -0500, Andres Salomon wrote:
> > assuming this is historical at this point, and manual options parsing can
> > be removed from all fb drivers at this point, or is there another reason
> > why manual parsing would be necessary?
> > 
> 
> Could I get an answer from the fbdevel folks about this?  It looks like
> the fb_get_options stuff is there for backwards compatibility.

I think so. I used fb_get_options() in arkfb and vt8623fb for mode option
only. Universal notation module.option=value is much nicer than manual
options parsing.  

-- 
Elen sila lumenn' omentielvo

Ondrej 'SanTiago' Zajicek (email: santiago@crfreenet.org, jabber: santiago@njs.netlab.cz)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/4] gxfb: Replace FBSIZE config option with a kernel argument

[Andres Salomon]

On Sun, 9 Mar 2008 23:00:25 +0100
Ondrej Zajicek <santiago@crfreenet.org> wrote:

> On Sat, Mar 08, 2008 at 08:19:50PM -0500, Andres Salomon wrote:
> > > assuming this is historical at this point, and manual options parsing can
> > > be removed from all fb drivers at this point, or is there another reason
> > > why manual parsing would be necessary?
> > > 
> > 
> > Could I get an answer from the fbdevel folks about this?  It looks like
> > the fb_get_options stuff is there for backwards compatibility.
> 
> I think so. I used fb_get_options() in arkfb and vt8623fb for mode option
> only. Universal notation module.option=value is much nicer than manual
> options parsing.  
> 


If that is indeed the case, here's a patch that might be useful.  I have
no idea what kind of timeframe would be proposed for removing
fb_get_options (or if it's even desirable to remove it at all given that
people are likely still using video=$foo syntax).



From dc3f29acc1f5fedf41e1705d1d820727c8fa1e89 Mon Sep 17 00:00:00 2001
From: Andres Salomon <dilinger@debian.org>
Date: Sun, 9 Mar 2008 13:29:21 -0400
Subject: [PATCH] fb: mark fb_get_options as deprecated, and remove from skeletonfb.c

fb_get_options is now marked as deprecated.  This also removes
the fb_get_options stuff from skeletonfb.c (so people don't use it
in new code), and it allows mode_options to be set via module_param.

Signed-off-by: Andres Salomon <dilinger@debian.org>
---
 drivers/video/skeletonfb.c |   40 ++++------------------------------------
 include/linux/fb.h         |    2 +-
 2 files changed, 5 insertions(+), 37 deletions(-)

diff --git a/drivers/video/skeletonfb.c b/drivers/video/skeletonfb.c
index 6232145..62cbc08 100644
--- a/drivers/video/skeletonfb.c
+++ b/drivers/video/skeletonfb.c
@@ -903,17 +903,6 @@ MODULE_DEVICE_TABLE(pci, xxxfb_id_table);
 
 int __init xxxfb_init(void)
 {
-	/*
-	 *  For kernel boot options (in 'video=xxxfb:<options>' format)
-	 */
-#ifndef MODULE
-	char *option = NULL;
-
-	if (fb_get_options("xxxfb", &option))
-		return -ENODEV;
-	xxxfb_setup(option);
-#endif
-
 	return pci_register_driver(&xxxfb_driver);
 }
 
@@ -974,34 +963,10 @@ static struct platform_device xxxfb_device = {
 	.name = "xxxfb",
 };
 
-#ifndef MODULE
-    /*
-     *  Setup
-     */
-
-/*
- * Only necessary if your driver takes special options,
- * otherwise we fall back on the generic fb_setup().
- */
-int __init xxxfb_setup(char *options)
-{
-    /* Parse user speficied options (`video=xxxfb:') */
-}
-#endif /* MODULE */
-
 static int __init xxxfb_init(void)
 {
 	int ret;
-	/*
-	 *  For kernel boot options (in 'video=xxxfb:<options>' format)
-	 */
-#ifndef MODULE
-	char *option = NULL;
-
-	if (fb_get_options("xxxfb", &option))
-		return -ENODEV;
-	xxxfb_setup(option);
-#endif
+
 	ret = driver_register(&xxxfb_driver);
 
 	if (!ret) {
@@ -1031,3 +996,6 @@ module_init(xxxfb_init);
 module_exit(xxxfb_remove);
 
 MODULE_LICENSE("GPL");
+module_param(mode_option, charp, 0);
+MODULE_PARM_DESC(mode_option, "Specify resolution as"
+		" \"<xres>x<yres>[-<bpp>][@<refresh>]\"");
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 58c57a3..c46009b 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -942,7 +942,7 @@ extern void fb_pad_aligned_buffer(u8 *dst, u32 d_pitch, u8 *src, u32 s_pitch, u3
 extern void fb_set_suspend(struct fb_info *info, int state);
 extern int fb_get_color_depth(struct fb_var_screeninfo *var,
 			      struct fb_fix_screeninfo *fix);
-extern int fb_get_options(char *name, char **option);
+extern int __deprecated fb_get_options(char *name, char **option);
 extern int fb_new_modelist(struct fb_info *info);
 
 extern struct fb_info *registered_fb[FB_MAX];
-- 
1.5.3.7




-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/4] gxfb: Replace FBSIZE config option with a kernel argument

[Geert Uytterhoeven]

On Sun, 9 Mar 2008, Andres Salomon wrote:
> On Sun, 9 Mar 2008 23:00:25 +0100
> Ondrej Zajicek <santiago@crfreenet.org> wrote:
> 
> > On Sat, Mar 08, 2008 at 08:19:50PM -0500, Andres Salomon wrote:
> > > > assuming this is historical at this point, and manual options parsing can
> > > > be removed from all fb drivers at this point, or is there another reason
> > > > why manual parsing would be necessary?
> > > > 
> > > 
> > > Could I get an answer from the fbdevel folks about this?  It looks like
> > > the fb_get_options stuff is there for backwards compatibility.
> > 
> > I think so. I used fb_get_options() in arkfb and vt8623fb for mode option
> > only. Universal notation module.option=value is much nicer than manual
> > options parsing.  

This `module.option=value' is a quite recent introduction? I first saw it
mentioned earlier this year...

> If that is indeed the case, here's a patch that might be useful.  I have
> no idea what kind of timeframe would be proposed for removing
> fb_get_options (or if it's even desirable to remove it at all given that
> people are likely still using video=$foo syntax).

Probably it's way too early for that...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/4] gxfb: Replace FBSIZE config option with a kernel argument

[Andres Salomon]

On Wed, 27 Feb 2008 16:31:05 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Sat, 23 Feb 2008 01:10:45 -0500
> Andres Salomon <dilinger@queued.net> wrote:
> 
> > @@ -425,7 +424,10 @@ static int __init gxfb_setup(char *options)
> >  		if (!*opt)
> >  			continue;
> >  
> > -		mode_option = opt;
> > +		if (!strncmp(opt, "fbsize:", 7))
> > +			fbsize = simple_strtoul(opt+7, NULL, 0);
> > +		else
> > +			mode_option = opt;
> >  	}
> 
> The above shouldn't be necessary.
> 
> And it should have been documented in Documentation/kernel-parameters.txt.
> 
> And "fbsize=N" would be a lot more conventional than "fbsize:N"
> 
> I suspect that the formulation you have here will not permit "fbsize:128k",
> whereas "fbsize=128k" or "gxfb.fbsize=128k" should work.  Needs checking.
> 
> >  	return 0;
> > @@ -456,5 +458,8 @@ module_exit(gxfb_cleanup);
> >  module_param(mode_option, charp, 0);
> >  MODULE_PARM_DESC(mode_option, "video mode (<x>x<y>[-<bpp>][@<refr>])");
> >  
> > +module_param(fbsize, int, 0);
> > +MODULE_PARM_DESC(fbsize, "video memory size");
> > +
> 
> Because the module_param() already makes fbsize available on the kernel
> boot command line via gxfb.fbsize=42 (or something similar).
> 
> 


Thanks, here's an updated patch:




Use a command line option (vram) rather than hardcoding the vram size.
LxFB already does this; it's useful for machines that can't query the
BIOS for fb size.  This patch originated from David Woodhouse, was
modified by Jordan Crouse, and was then modified further by me.

This also adds some gxfb documentation in Documentation/fb.

Signed-off-by: Andres Salomon <dilinger@debian.org>
---
 Documentation/fb/gxfb.txt        |   51 ++++++++++++++++++++++++++++++++++++++
 drivers/video/geode/Kconfig      |   20 ---------------
 drivers/video/geode/display_gx.c |    7 -----
 drivers/video/geode/gxfb_core.c  |   14 ++++++----
 4 files changed, 59 insertions(+), 33 deletions(-)
 create mode 100644 Documentation/fb/gxfb.txt

diff --git a/Documentation/fb/gxfb.txt b/Documentation/fb/gxfb.txt
new file mode 100644
index 0000000..2d3dbaf
--- /dev/null
+++ b/Documentation/fb/gxfb.txt
@@ -0,0 +1,51 @@
+[This file is cloned from VesaFB/aty128fb]
+
+What is gxfb?
+=================
+
+This is a graphics framebuffer driver for AMD Geode GX2 based processors.
+
+Advantages:
+
+ * No need to use AMD's VSA code (or other VESA emulation layer) in the
+   BIOS.
+ * It provides a nice large console (128 cols + 48 lines with 1024x768)
+   without using tiny, unreadable fonts.
+ * You can run XF68_FBDev on top of /dev/fb0
+ * Most important: boot logo :-)
+
+Disadvantages:
+
+ * graphic mode is slower than text mode... 
+
+
+How to use it?
+==============
+
+Switching modes is done using  gxfb.mode_option=<resolution>... boot
+parameter or using `fbset' program.
+
+See Documentation/fb/modedb.txt for more information on modedb
+resolutions.
+
+
+X11
+===
+
+XF68_FBDev should generally work fine, but it is non-accelerated.
+
+
+Configuration
+=============
+
+You can pass kernel command line options to gxfb with gxfb.<option>.
+For example, gxfb.mode_option=800x600@75.
+Accepted options:
+
+mode_option	- specify the video mode.  Of the form
+		  <x>x<y>[-<bpp>][@<refresh>]
+vram		- size of video ram (normally auto-detected)
+
+
+--
+Andres Salomon <dilinger@debian.org>
diff --git a/drivers/video/geode/Kconfig b/drivers/video/geode/Kconfig
index 7608429..c5d8ba4 100644
--- a/drivers/video/geode/Kconfig
+++ b/drivers/video/geode/Kconfig
@@ -38,26 +38,6 @@ config FB_GEODE_GX
 
 	  If unsure, say N.
 
-config FB_GEODE_GX_SET_FBSIZE
-	bool "Manually specify the Geode GX framebuffer size"
-	depends on FB_GEODE_GX
-	default n
-	---help---
-	  If you want to manually specify the size of your GX framebuffer,
-	  say Y here, otherwise say N to dynamically probe it.
-
-	  Say N unless you know what you are doing.
-
-config FB_GEODE_GX_FBSIZE
-	hex "Size of the GX framebuffer, in bytes"
-	depends on FB_GEODE_GX_SET_FBSIZE
-	default "0x1600000"
-	---help---
-	  Specify the size of the GX framebuffer.  Normally, you will
-	  want this to be MB aligned.  Common values are 0x80000 (8MB)
-	  and 0x1600000 (16MB).  Don't change this unless you know what
-	  you are doing
-
 config FB_GEODE_GX1
 	tristate "AMD Geode GX1 framebuffer support (EXPERIMENTAL)"
 	depends on FB && FB_GEODE && EXPERIMENTAL
diff --git a/drivers/video/geode/display_gx.c b/drivers/video/geode/display_gx.c
index 0f16e4b..8cd7572 100644
--- a/drivers/video/geode/display_gx.c
+++ b/drivers/video/geode/display_gx.c
@@ -21,12 +21,6 @@
 #include "geodefb.h"
 #include "display_gx.h"
 
-#ifdef CONFIG_FB_GEODE_GX_SET_FBSIZE
-unsigned int gx_frame_buffer_size(void)
-{
-	return CONFIG_FB_GEODE_GX_FBSIZE;
-}
-#else
 unsigned int gx_frame_buffer_size(void)
 {
 	unsigned int val;
@@ -41,7 +35,6 @@ unsigned int gx_frame_buffer_size(void)
 	val = (unsigned int)(inw(0xAC1E)) & 0xFFl;
 	return (val << 19);
 }
-#endif
 
 int gx_line_delta(int xres, int bpp)
 {
diff --git a/drivers/video/geode/gxfb_core.c b/drivers/video/geode/gxfb_core.c
index cf841ef..a3e4a34 100644
--- a/drivers/video/geode/gxfb_core.c
+++ b/drivers/video/geode/gxfb_core.c
@@ -36,6 +36,7 @@
 #include "video_gx.h"
 
 static char *mode_option;
+static int vram;
 
 /* Modes relevant to the GX (taken from modedb.c) */
 static const struct fb_videomode gx_modedb[] __initdata = {
@@ -207,7 +208,6 @@ static int gxfb_blank(int blank_mode, struct fb_info *info)
 static int __init gxfb_map_video_memory(struct fb_info *info, struct pci_dev *dev)
 {
 	struct geodefb_par *par = info->par;
-	int fb_len;
 	int ret;
 
 	ret = pci_enable_device(dev);
@@ -232,21 +232,20 @@ static int __init gxfb_map_video_memory(struct fb_info *info, struct pci_dev *de
 	ret = pci_request_region(dev, 0, "gxfb (framebuffer)");
 	if (ret < 0)
 		return ret;
-	if ((fb_len = gx_frame_buffer_size()) < 0)
-		return -ENOMEM;
+
 	info->fix.smem_start = pci_resource_start(dev, 0);
-	info->fix.smem_len = fb_len;
+	info->fix.smem_len = vram ? vram : gx_frame_buffer_size();
 	info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
 	if (!info->screen_base)
 		return -ENOMEM;
 
-	/* Set the 16MB aligned base address of the graphics memory region
+	/* Set the 16MiB aligned base address of the graphics memory region
 	 * in the display controller */
 
 	writel(info->fix.smem_start & 0xFF000000,
 			par->dc_regs + DC_GLIU0_MEM_OFFSET);
 
-	dev_info(&dev->dev, "%d Kibyte of video memory at 0x%lx\n",
+	dev_info(&dev->dev, "%d KiB of video memory at 0x%lx\n",
 		 info->fix.smem_len / 1024, info->fix.smem_start);
 
 	return 0;
@@ -456,5 +455,8 @@ module_exit(gxfb_cleanup);
 module_param(mode_option, charp, 0);
 MODULE_PARM_DESC(mode_option, "video mode (<x>x<y>[-<bpp>][@<refr>])");
 
+module_param(vram, int, 0);
+MODULE_PARM_DESC(vram, "video memory size");
+
 MODULE_DESCRIPTION("Framebuffer driver for the AMD Geode GX");
 MODULE_LICENSE("GPL");
-- 
1.5.3.7

.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/