* Re: [Bugme-new] [Bug 9847] New: i810fb: module parameter 'mode_option' inconsistent with other framebuffer modules
[not found] <bug-9847-10286@http.bugzilla.kernel.org/>
@ 2008-01-30 5:33 ` Andrew Morton
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2008-01-30 5:33 UTC (permalink / raw)
To: linux-fbdev-devel; +Cc: miki, bugme-daemon
On Tue, 29 Jan 2008 21:02:39 -0800 (PST) bugme-daemon@bugzilla.kernel.org wrote:
> http://bugzilla.kernel.org/show_bug.cgi?id=9847
>
> Summary: i810fb: module parameter 'mode_option' inconsistent with
> other framebuffer modules
> Product: Drivers
> Version: 2.5
> KernelVersion: 2.6.22
> Platform: All
> OS/Version: Linux
> Tree: Mainline
> Status: NEW
> Severity: normal
> Priority: P1
> Component: Console/Framebuffers
> AssignedTo: jsimmons@infradead.org
> ReportedBy: miki@dds.nl
>
>
> Latest working kernel version: -
> Earliest failing kernel version:
> Distribution: Debian Release: lenny/sid
> Hardware Environment: Intel PIII / i815M
> Software Environment:
> Problem Description: While all other framebuffer modules use the option
> "mode=..." to specify a modedb-style video mode, i810fb uses the non-standard
> "mode_option=..." This breaks the use of "video=i810fb:<videomode>" boot
> parameter, such that for instance "video=i810fb:1024x768" doesn't work.
> The boot scripts parse the "<videomode>" parameter internally and pass
> it to the correct module as 'mode=<videomode>'.
>
> -Alain
-------------------------------------------------------------------------
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/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Bugme-new] [Bug 9847] New: i810fb: module parameter 'mode_option' inconsistent with other framebuffer modules
@ 2008-02-01 7:01 krzysztof.h1
2008-02-01 7:32 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: krzysztof.h1 @ 2008-02-01 7:01 UTC (permalink / raw)
To: Andrew Morton, linux-fbdev-devel, miki; +Cc: jsimmons, adaplas
[-- Attachment #1: Type: TEXT/plain, Size: 3409 bytes --]
> On Tue, 29 Jan 2008 21:02:39 -0800 (PST) bugme-daemon@bugzilla.kernel.org
> wrote:
> > Problem Description: While all other framebuffer modules use the option
> > "mode=..." to specify a modedb-style video mode, i810fb uses the
> non-standard
> > "mode_option=..."
The attached patch should fix it.
--
From: Krzysztof Helt <krzysztof.h1@wp.pl>
This patch changes the option "mode_option" into "mode".
Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
---
If the patch below is mangled by my web based email client please use the attached one.
The patch was done against 2.6.24-git9.
diff -urp linux-ref/drivers/video/i810/i810_main.c linux-pc/drivers/video/i810/i810_main.c
--- linux-ref/drivers/video/i810/i810_main.c 2008-01-24 23:58:37.000000000 +0100
+++ linux-pc/drivers/video/i810/i810_main.c 2008-01-31 20:43:56.000000000 +0100
@@ -132,7 +132,7 @@ static struct pci_driver i810fb_driver =
.resume = i810fb_resume,
};
-static char *mode_option __devinitdata = NULL;
+static char *mode __devinitdata;
static int vram __devinitdata = 4;
static int bpp __devinitdata = 8;
static int mtrr __devinitdata;
@@ -1891,7 +1891,7 @@ i810_allocate_pci_resource(struct i810fb
static void __devinit i810fb_find_init_mode(struct fb_info *info)
{
- struct fb_videomode mode;
+ struct fb_videomode mode_new;
struct fb_var_screeninfo var;
struct fb_monspecs *specs = &info->monspecs;
int found = 0;
@@ -1902,7 +1902,7 @@ static void __devinit i810fb_find_init_m
#endif
INIT_LIST_HEAD(&info->modelist);
- memset(&mode, 0, sizeof(struct fb_videomode));
+ memset(&mode_new, 0, sizeof(struct fb_videomode));
var = info->var;
#ifdef CONFIG_FB_I810_I2C
i810_create_i2c_busses(par);
@@ -1928,23 +1928,23 @@ static void __devinit i810fb_find_init_m
if (xres && yres) {
if ((m = fb_find_best_mode(&var, &info->modelist))) {
- mode = *m;
+ mode_new = *m;
found = 1;
}
}
if (!found) {
m = fb_find_best_display(&info->monspecs, &info->modelist);
- mode = *m;
+ mode_new = *m;
found = 1;
}
- fb_videomode_to_var(&var, &mode);
+ fb_videomode_to_var(&var, &mode_new);
}
#endif
- if (mode_option)
- fb_find_mode(&var, info, mode_option, specs->modedb,
- specs->modedb_len, (found) ? &mode : NULL,
+ if (mode)
+ fb_find_mode(&var, info, mode, specs->modedb,
+ specs->modedb_len, (found) ? &mode_new : NULL,
info->var.bits_per_pixel);
info->var = var;
@@ -1998,7 +1998,7 @@ static int __devinit i810fb_setup(char *
else if (!strncmp(this_opt, "ddc3", 4))
ddc3 = 3;
else
- mode_option = this_opt;
+ mode = this_opt;
}
return 0;
}
@@ -2202,8 +2202,8 @@ MODULE_PARM_DESC(dcolor, "use DirectColo
" (default = 0 = TrueColor)");
module_param(ddc3, bool, 0);
MODULE_PARM_DESC(ddc3, "Probe DDC bus 3 (default = 0 = no)");
-module_param(mode_option, charp, 0);
-MODULE_PARM_DESC(mode_option, "Specify initial video mode");
+module_param(mode, charp, 0);
+MODULE_PARM_DESC(mode, "Specify initial video mode");
MODULE_AUTHOR("Tony A. Daplas");
MODULE_DESCRIPTION("Framebuffer device for the Intel 810/815 and"
----------------------------------------------------------------------
Lezac w wannie skorzystala z ...
Kliknij >>> http://link.interia.pl/f1ce9
[-- Attachment #2: i810-mode.diff --]
[-- Type: APPLICATION/octet-stream, Size: 2636 bytes --]
diff -urp linux-ref/drivers/video/i810/i810_main.c linux-pc/drivers/video/i810/i810_main.c
--- linux-ref/drivers/video/i810/i810_main.c 2008-01-24 23:58:37.000000000 +0100
+++ linux-pc/drivers/video/i810/i810_main.c 2008-01-31 20:43:56.000000000 +0100
@@ -132,7 +132,7 @@ static struct pci_driver i810fb_driver =
.resume = i810fb_resume,
};
-static char *mode_option __devinitdata = NULL;
+static char *mode __devinitdata;
static int vram __devinitdata = 4;
static int bpp __devinitdata = 8;
static int mtrr __devinitdata;
@@ -1891,7 +1891,7 @@ i810_allocate_pci_resource(struct i810fb
static void __devinit i810fb_find_init_mode(struct fb_info *info)
{
- struct fb_videomode mode;
+ struct fb_videomode mode_new;
struct fb_var_screeninfo var;
struct fb_monspecs *specs = &info->monspecs;
int found = 0;
@@ -1902,7 +1902,7 @@ static void __devinit i810fb_find_init_m
#endif
INIT_LIST_HEAD(&info->modelist);
- memset(&mode, 0, sizeof(struct fb_videomode));
+ memset(&mode_new, 0, sizeof(struct fb_videomode));
var = info->var;
#ifdef CONFIG_FB_I810_I2C
i810_create_i2c_busses(par);
@@ -1928,23 +1928,23 @@ static void __devinit i810fb_find_init_m
if (xres && yres) {
if ((m = fb_find_best_mode(&var, &info->modelist))) {
- mode = *m;
+ mode_new = *m;
found = 1;
}
}
if (!found) {
m = fb_find_best_display(&info->monspecs, &info->modelist);
- mode = *m;
+ mode_new = *m;
found = 1;
}
- fb_videomode_to_var(&var, &mode);
+ fb_videomode_to_var(&var, &mode_new);
}
#endif
- if (mode_option)
- fb_find_mode(&var, info, mode_option, specs->modedb,
- specs->modedb_len, (found) ? &mode : NULL,
+ if (mode)
+ fb_find_mode(&var, info, mode, specs->modedb,
+ specs->modedb_len, (found) ? &mode_new : NULL,
info->var.bits_per_pixel);
info->var = var;
@@ -1998,7 +1998,7 @@ static int __devinit i810fb_setup(char *
else if (!strncmp(this_opt, "ddc3", 4))
ddc3 = 3;
else
- mode_option = this_opt;
+ mode = this_opt;
}
return 0;
}
@@ -2202,8 +2202,8 @@ MODULE_PARM_DESC(dcolor, "use DirectColo
" (default = 0 = TrueColor)");
module_param(ddc3, bool, 0);
MODULE_PARM_DESC(ddc3, "Probe DDC bus 3 (default = 0 = no)");
-module_param(mode_option, charp, 0);
-MODULE_PARM_DESC(mode_option, "Specify initial video mode");
+module_param(mode, charp, 0);
+MODULE_PARM_DESC(mode, "Specify initial video mode");
MODULE_AUTHOR("Tony A. Daplas");
MODULE_DESCRIPTION("Framebuffer device for the Intel 810/815 and"
[-- Attachment #3: Type: text/plain, Size: 228 bytes --]
-------------------------------------------------------------------------
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/
[-- Attachment #4: Type: text/plain, Size: 182 bytes --]
_______________________________________________
Linux-fbdev-devel mailing list
Linux-fbdev-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Bugme-new] [Bug 9847] New: i810fb: module parameter 'mode_option' inconsistent with other framebuffer modules
2008-02-01 7:01 krzysztof.h1
@ 2008-02-01 7:32 ` Andrew Morton
2008-02-02 4:18 ` Alain Kalker
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2008-02-01 7:32 UTC (permalink / raw)
To: krzysztof.h1; +Cc: jsimmons, miki, linux-fbdev-devel, adaplas
On 01 Feb 2008 08:01:30 +0100 krzysztof.h1@poczta.fm wrote:
> > On Tue, 29 Jan 2008 21:02:39 -0800 (PST) bugme-daemon@bugzilla.kernel.org
> > wrote:
> > > Problem Description: While all other framebuffer modules use the option
> > > "mode=..." to specify a modedb-style video mode, i810fb uses the
> > non-standard
> > > "mode_option=..."
>
> The attached patch should fix it.
> --
>
> From: Krzysztof Helt <krzysztof.h1@wp.pl>
>
> This patch changes the option "mode_option" into "mode".
>
Thanks.
> -module_param(mode_option, charp, 0);
> -MODULE_PARM_DESC(mode_option, "Specify initial video mode");
> +module_param(mode, charp, 0);
> +MODULE_PARM_DESC(mode, "Specify initial video mode");
But can we retain the old option as well, to avoid breaking people's
existing setups?
-------------------------------------------------------------------------
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/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Bugme-new] [Bug 9847] New: i810fb: module parameter 'mode_option' inconsistent with other framebuffer modules
2008-02-01 7:32 ` Andrew Morton
@ 2008-02-02 4:18 ` Alain Kalker
2008-02-02 4:37 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Alain Kalker @ 2008-02-02 4:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: jsimmons, linux-fbdev-devel, adaplas
On Thu, 2008-01-31 at 23:32 -0800, Andrew Morton wrote:
> On 01 Feb 2008 08:01:30 +0100 krzysztof.h1@poczta.fm wrote:
>
> > > On Tue, 29 Jan 2008 21:02:39 -0800 (PST) bugme-daemon@bugzilla.kernel.org
> > > wrote:
> > > > Problem Description: While all other framebuffer modules use the option
> > > > "mode=..." to specify a modedb-style video mode, i810fb uses the
> > > non-standard
> > > > "mode_option=..."
> >
> > The attached patch should fix it.
> > --
> >
> > From: Krzysztof Helt <krzysztof.h1@wp.pl>
> >
> > This patch changes the option "mode_option" into "mode".
> >
>
> Thanks.
>
> > -module_param(mode_option, charp, 0);
> > -MODULE_PARM_DESC(mode_option, "Specify initial video mode");
> > +module_param(mode, charp, 0);
> > +MODULE_PARM_DESC(mode, "Specify initial video mode");
>
> But can we retain the old option as well, to avoid breaking people's
> existing setups?
Thanks for the quick reply and patch. In the meantime, I have found out
that the option inconsistency is far more widespread among framebuffer
modules. The documentation on video mode setting
(Documentation/fb/modedb.txt) states that the mode parameter should be
called 'mode_option', but the framebuffer modules seem to be split
exactly down the middle in using 'mode' versus 'mode_option'
On a Debian lenny/sid system running a 2.6.22 kernel I get this:
Framebuffer modules accepting a 'mode' parameter:
---[cut here]---
iki@miki:~$ find /lib/modules/`uname -r`/kernel -name '*fb.ko'|while
read ko; do sudo modinfo -F parm $ko|grep -q "^mode:" && echo $ko; done
/lib/modules/2.6.22-3-686/kernel/drivers/video/s3fb.ko
/lib/modules/2.6.22-3-686/kernel/drivers/video/sis/sisfb.ko
/lib/modules/2.6.22-3-686/kernel/drivers/video/arkfb.ko
/lib/modules/2.6.22-3-686/kernel/drivers/video/geode/gx1fb.ko
/lib/modules/2.6.22-3-686/kernel/drivers/video/vt8623fb.ko
/lib/modules/2.6.22-3-686/kernel/drivers/video/intelfb/intelfb.ko
/lib/modules/2.6.22-3-686/kernel/drivers/video/pm2fb.ko
/lib/modules/2.6.22-3-686/kernel/drivers/video/aty/atyfb.ko
---
Framebuffer modules accepting a 'mode_option' parameter:
miki@miki:~$ find /lib/modules/`uname -r`/kernel -name '*fb.ko'|while
read ko; do sudo modinfo -F parm $ko|grep -q "^mode_option:" && echo
$ko; done
/lib/modules/2.6.22-3-686/kernel/drivers/video/savage/savagefb.ko
/lib/modules/2.6.22-3-686/kernel/drivers/video/sstfb.ko
/lib/modules/2.6.22-3-686/kernel/drivers/video/neofb.ko
/lib/modules/2.6.22-3-686/kernel/drivers/video/geode/gxfb.ko
/lib/modules/2.6.22-3-686/kernel/drivers/video/i810/i810fb.ko
/lib/modules/2.6.22-3-686/kernel/drivers/video/nvidia/nvidiafb.ko
/lib/modules/2.6.22-3-686/kernel/drivers/video/aty/aty128fb.ko
/lib/modules/2.6.22-3-686/kernel/drivers/video/aty/radeonfb.ko
I assume the parameter was called 'mode_option' originally to indicate
that the parameter name itself is optional, "modprobe i810fb
mode=1024x768" is equivalent to "modprobe i810fb 1024x768", which at
least this module seems to support.
Should this be filed as a more general bug report?
-Alain
-------------------------------------------------------------------------
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/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Bugme-new] [Bug 9847] New: i810fb: module parameter 'mode_option' inconsistent with other framebuffer modules
2008-02-02 4:18 ` Alain Kalker
@ 2008-02-02 4:37 ` Andrew Morton
2008-02-02 5:27 ` Alain Kalker
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2008-02-02 4:37 UTC (permalink / raw)
To: Alain Kalker; +Cc: jsimmons, linux-fbdev-devel, adaplas
On Sat, 02 Feb 2008 05:18:10 +0100 Alain Kalker <miki@dds.nl> wrote:
> Should this be filed as a more general bug report?
It is unlikely that anyone would actually act upon such a thing, I'm afraid.
Maybe if we could come up with a single, centralised option parser
for all the drivers and then migrate individual drivers over to or
or something like that..
-------------------------------------------------------------------------
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/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Bugme-new] [Bug 9847] New: i810fb: module parameter 'mode_option' inconsistent with other framebuffer modules
2008-02-02 4:37 ` Andrew Morton
@ 2008-02-02 5:27 ` Alain Kalker
0 siblings, 0 replies; 6+ messages in thread
From: Alain Kalker @ 2008-02-02 5:27 UTC (permalink / raw)
To: Andrew Morton; +Cc: jsimmons, linux-fbdev-devel, adaplas
On Fri, 2008-02-01 at 20:37 -0800, Andrew Morton wrote:
> On Sat, 02 Feb 2008 05:18:10 +0100 Alain Kalker <miki@dds.nl> wrote:
>
> > Should this be filed as a more general bug report?
>
> It is unlikely that anyone would actually act upon such a thing, I'm afraid.
> Maybe if we could come up with a single, centralised option parser
> for all the drivers and then migrate individual drivers over to or
> or something like that..
The reason I asked is that the current situation presents a problem to
the boot scripts. Currently none of the modules support both options
simultaneously, and I'm not sure if every module supports the parameter
name being optional. The only sure ways to ensure that kernel boot
option "video=<framebuffer>:<videomode> works as intended, is either to
parse the modules' option list using modinfo (which may not be available
at boot) or to try each option name in turn using "modprobe
--first-time", which I find very kludgy.
-Alain
-------------------------------------------------------------------------
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/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-02-02 5:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <bug-9847-10286@http.bugzilla.kernel.org/>
2008-01-30 5:33 ` [Bugme-new] [Bug 9847] New: i810fb: module parameter 'mode_option' inconsistent with other framebuffer modules Andrew Morton
2008-02-01 7:01 krzysztof.h1
2008-02-01 7:32 ` Andrew Morton
2008-02-02 4:18 ` Alain Kalker
2008-02-02 4:37 ` Andrew Morton
2008-02-02 5:27 ` Alain Kalker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).