Skip to contentSkip to conversation

fbdev: Clean up framebuffer initialization

RFC PATCH In review Antonino A. Daplas 4 participants 9 messages Sep 4, 2004 – Sep 5, 2004

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

RFC PATCH 4/5 fbdev: Clean up framebuffer initialization

This patch probably deserves discussion among developers.

Currently, the framebuffer system is initialized in a roundabout manner. First, drivers/char/mem.c calls fbmem_init(). fbmem_init() will then iterate over an array of individual drivers' xxxfb_init(), then each driver registers its presence back to fbmem. During console_init(), drivers/char/vt.c will call fb_console_init(). fbcon will check for registered drivers, and if any are present, will call take_over_console() in drivers/char/vt.c.

This patch changes the initialization sequence so it proceeds in this
manner: Each driver has its own module_init(). Each driver calls register_framebuffer() in fbmem.c. fbmem.c will then notify fbcon of the driver registration. Upon notification, fbcon calls take_over_console() in vt.c.

The following are the changes brought about by this patch:

1. Each subsystem (fbcon, fbmem, xxxfb) will have their own module_init. Thus, explicit calls to each subsystem's init functions are eliminated.

2. The struct fb_drivers array in fbmem.c can be removed. This slashes around 400 lines in fbmem.c

3. Parsing of kernel boot options were done by fbmem.c calling each driver's xxxfb_setup() function. Because this is not possible with this patch, drivers can choose to either:
a. have their own __setup() routine
b. call fb_get_options("xxxfb") and pass the return value to
xxxfb_setup(). This is to maintain compatibility with the
'video=xxxfb:<options>' semantics.

4. Getting a framebuffer console will occur a bit late during the boot process since the initialization sequence will depend upon the link order. So, 'video/' is moved up in drivers/Makefile, shortly after 'pci/'

5. Because driver initialization will be dependent on the link order, hardware that depends on other subsystems (agpgart, usb, serial, etc) may choose to initialize after the subsystems they depend on.

---

 drivers/Makefile              |    3
 drivers/char/mem.c            |    6
 drivers/char/vt.c             |    6
 drivers/video/Makefile        |   10
 drivers/video/console/fbcon.c |    3
 drivers/video/fbmem.c         |  471 ++++--------------------------------------
 include/linux/fb.h            |    1
 7 files changed, 57 insertions(+), 443 deletions(-)
drivers/char/mem.c +0 -6
diff -uprN linux-2.6.9-rc1-mm3-orig/drivers/char/mem.c linux-2.6.9-rc1-mm3/drivers/char/mem.c
@@ -31,9 +31,6 @@
 # include <linux/efi.h>
 #endif
 
-#ifdef CONFIG_FB
-extern void fbmem_init(void);
-#endif
 #if defined(CONFIG_S390_TAPE) && defined(CONFIG_S390_TAPE_CHAR)
 extern void tapechar_init(void);
 #endif
@@ -685,9 +682,6 @@ static int __init chr_dev_init(void)
 				S_IFCHR | devlist[i].mode, devlist[i].name);
 	}
 	
-#if defined (CONFIG_FB)
-	fbmem_init();
-#endif
 	return 0;
 }
 
drivers/char/vt.c +0 -6
diff -uprN linux-2.6.9-rc1-mm3-orig/drivers/char/vt.c linux-2.6.9-rc1-mm3/drivers/char/vt.c
@@ -136,9 +136,6 @@ extern void prom_con_init(void);
 #ifdef CONFIG_MDA_CONSOLE
 extern int mda_console_init(void);
 #endif
-#ifdef CONFIG_FRAMEBUFFER_CONSOLE
-extern int fb_console_init(void);
-#endif
 
 struct vc vc_cons [MAX_NR_CONSOLES];
 
@@ -2672,9 +2669,6 @@ int __init vty_init(void)
 #ifdef CONFIG_MDA_CONSOLE
 	mda_console_init();
 #endif
-#ifdef CONFIG_FRAMEBUFFER_CONSOLE
-	fb_console_init();
-#endif	
 	return 0;
 }
 
drivers/Makefile +2 -1
diff -uprN linux-2.6.9-rc1-mm3-orig/drivers/Makefile linux-2.6.9-rc1-mm3/drivers/Makefile
@@ -7,6 +7,7 @@
 
 obj-$(CONFIG_PCI)		+= pci/
 obj-$(CONFIG_PARISC)		+= parisc/
+obj-y				+= video/
 obj-$(CONFIG_ACPI_BOOT)		+= acpi/
 # PnP must come after ACPI since it will eventually need to check if acpi
 # was used and do nothing if so
@@ -29,7 +30,7 @@ obj-$(CONFIG_FC4)		+= fc4/
 obj-$(CONFIG_SCSI)		+= scsi/
 obj-$(CONFIG_FUSION)		+= message/
 obj-$(CONFIG_IEEE1394)		+= ieee1394/
-obj-y				+= cdrom/ video/
+obj-y				+= cdrom/
 obj-$(CONFIG_MTD)		+= mtd/
 obj-$(CONFIG_PCMCIA)		+= pcmcia/
 obj-$(CONFIG_DIO)		+= dio/
drivers/video/console/fbcon.c +2 -1
diff -uprN linux-2.6.9-rc1-mm3-orig/drivers/video/console/fbcon.c linux-2.6.9-rc1-mm3/drivers/video/console/fbcon.c
@@ -2860,6 +2860,8 @@ int __init fb_console_init(void)
 	return 0;
 }
 
+module_init(fb_console_init);
+
 #ifdef MODULE
 
 void __exit fb_console_exit(void)
@@ -2870,7 +2872,6 @@ void __exit fb_console_exit(void)
 	give_up_console(&fb_con);
 }	
 
-module_init(fb_console_init);
 module_exit(fb_console_exit);
 
 #endif
drivers/video/fbmem.c +47 -424

526 lines not on this page · raw message

drivers/video/Makefile +5 -5
diff -uprN linux-2.6.9-rc1-mm3-orig/drivers/video/Makefile linux-2.6.9-rc1-mm3/drivers/video/Makefile
@@ -13,6 +13,7 @@ ifeq ($(CONFIG_FB),y)
 obj-$(CONFIG_PPC)                 += macmodes.o
 endif
 
+obj-$(CONFIG_FB_OF)               += offb.o cfbfillrect.o cfbimgblt.o cfbcopyarea.o
 obj-$(CONFIG_FB_ARMCLCD)	  += amba-clcd.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o
 obj-$(CONFIG_FB_ACORN)            += acornfb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o
 obj-$(CONFIG_FB_AMIGA)            += amifb.o c2p.o
@@ -40,16 +41,12 @@ obj-$(CONFIG_FB_3DFX)             += cfb
 endif
 obj-$(CONFIG_FB_MAC)              += macfb.o macmodes.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o 
 obj-$(CONFIG_FB_HP300)            += hpfb.o cfbfillrect.o cfbimgblt.o
-obj-$(CONFIG_FB_OF)               += offb.o cfbfillrect.o cfbimgblt.o cfbcopyarea.o
 obj-$(CONFIG_FB_IMSTT)            += imsttfb.o cfbimgblt.o
 obj-$(CONFIG_FB_RETINAZ3)         += retz3fb.o
 obj-$(CONFIG_FB_CIRRUS)		  += cirrusfb.o cfbfillrect.o cfbimgblt.o cfbcopyarea.o
 obj-$(CONFIG_FB_TRIDENT)	  += tridentfb.o cfbfillrect.o cfbimgblt.o cfbcopyarea.o
 obj-$(CONFIG_FB_S3TRIO)           += S3triofb.o
 obj-$(CONFIG_FB_TGA)              += tgafb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o 
-obj-$(CONFIG_FB_VESA)             += vesafb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o 
-obj-$(CONFIG_FB_VGA16)            += vga16fb.o cfbfillrect.o cfbcopyarea.o \
-	                             cfbimgblt.o vgastate.o 
 obj-$(CONFIG_FB_VIRGE)            += virgefb.o
 obj-$(CONFIG_FB_G364)             += g364fb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o
 obj-$(CONFIG_FB_FM2)              += fm2fb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o
@@ -71,7 +68,6 @@ obj-$(CONFIG_FB_I810)             += i81
 obj-$(CONFIG_FB_SUN3)             += sun3fb.o
 obj-$(CONFIG_FB_HGA)              += hgafb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o 
 obj-$(CONFIG_FB_SA1100)           += sa1100fb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o
-obj-$(CONFIG_FB_VIRTUAL)          += vfb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o 
 obj-$(CONFIG_FB_HIT)              += hitfb.o cfbfillrect.o cfbimgblt.o
 obj-$(CONFIG_FB_EPSON1355)	  += epson1355fb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o
 obj-$(CONFIG_FB_PVR2)             += pvr2fb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o
@@ -94,3 +90,7 @@ obj-$(CONFIG_FB_TCX)               += tc
 obj-$(CONFIG_FB_LEO)               += leo.o sbuslib.o cfbimgblt.o cfbcopyarea.o \
 				      cfbfillrect.o
 obj-$(CONFIG_FB_PXA)		   += pxafb.o cfbimgblt.o cfbcopyarea.o cfbfillrect.o
+obj-$(CONFIG_FB_VGA16)            += vga16fb.o cfbfillrect.o cfbcopyarea.o \
+	                             cfbimgblt.o vgastate.o
+obj-$(CONFIG_FB_VESA)             += vesafb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o
+obj-$(CONFIG_FB_VIRTUAL)          += vfb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o
include/linux/fb.h +1 -0
diff -uprN linux-2.6.9-rc1-mm3-orig/include/linux/fb.h linux-2.6.9-rc1-mm3/include/linux/fb.h
@@ -708,6 +708,7 @@ extern void fb_sysmove_buf_aligned(struc
 extern void fb_load_cursor_image(struct fb_info *);
 extern void fb_set_suspend(struct fb_info *info, int state);
 extern int fb_get_color_depth(struct fb_info *info);
+extern char* fb_get_options(char *name);
 
 extern struct fb_info *registered_fb[FB_MAX];
 extern int num_registered_fb;

-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click

#1 lore raw
Quoted 36 lines · Antonino A. Daplas

This patch probably deserves discussion among developers.

Currently, the framebuffer system is initialized in a roundabout manner. First, drivers/char/mem.c calls fbmem_init(). fbmem_init() will then iterate over an array of individual drivers' xxxfb_init(), then each driver registers its presence back to fbmem. During console_init(), drivers/char/vt.c will call fb_console_init(). fbcon will check for registered drivers, and if any are present, will call take_over_console() in drivers/char/vt.c.

This patch changes the initialization sequence so it proceeds in this
manner: Each driver has its own module_init(). Each driver calls register_framebuffer() in fbmem.c. fbmem.c will then notify fbcon of the driver registration. Upon notification, fbcon calls take_over_console() in vt.c.

The following are the changes brought about by this patch:

1. Each subsystem (fbcon, fbmem, xxxfb) will have their own module_init. Thus, explicit calls to each subsystem's init functions are eliminated.

2. The struct fb_drivers array in fbmem.c can be removed. This slashes around 400 lines in fbmem.c

3. Parsing of kernel boot options were done by fbmem.c calling each driver's xxxfb_setup() function. Because this is not possible with this patch, drivers can choose to either:
a. have their own __setup() routine
b. call fb_get_options("xxxfb") and pass the return value to
xxxfb_setup(). This is to maintain compatibility with the
'video=xxxfb:<options>' semantics.

4. Getting a framebuffer console will occur a bit late during the boot process since the initialization sequence will depend upon the link order. So, 'video/' is moved up in drivers/Makefile, shortly after 'pci/'

Quoted 5 lines

5. Because driver initialization will be dependent on the link order, hardware that depends on other subsystems (agpgart, usb, serial, etc) may choose to initialize after the subsystems they depend on.

I don't really see a benefit but it's ok with me.

(Thanks for considering the "unified" nature of sisfb, by the way. Very considarate. Very much appreciated.)

I assume that you tested this stuff before posting it here.

Thomsd

Signature · 4 lines
Thomas Winischhofer
Vienna/Austria
thomas AT winischhofer DOT net          http://www.winischhofer.net/
twini AT xfree86 DOT org
#2 lore raw
Quoted 8 lines · Thomas Winischhofer
Quoted 5 lines

5. Because driver initialization will be dependent on the link order, hardware that depends on other subsystems (agpgart, usb, serial, etc) may choose to initialize after the subsystems they depend on.

I don't really see a benefit but it's ok with me.

Mainly cleanup, but also point #5. The i810fb, for instance, depends on agpgart, but agpgart gets initialized way after fbdev. The workaround is for i810fb to explicitly call intel_agp_init(). Besides the ugliness, forcibly initializing the agpgart subsystem out of sequence may cause problems.

With this change, in theory, I can move i810fb's link order so it gets initialized after agpgart.

Quoted 4 lines

(Thanks for considering the "unified" nature of sisfb, by the way. Very considarate. Very much appreciated.)

I assume that you tested this stuff before posting it here.

Yes, with hardware that I have. I did try to at least compile test what I can.

Tony

-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click

#3 lore raw
Quoted 6 lines · Antonino A. Daplas

Mainly cleanup, but also point #5. The i810fb, for instance, depends on agpgart, but agpgart gets initialized way after fbdev. The workaround is for i810fb to explicitly call intel_agp_init(). Besides the ugliness, forcibly initializing the agpgart subsystem out of sequence may cause problems.

Sorry if I interrupt without deeper knowledge but doesn't this sound very strange? Why is a subsystem initialized much later than the one which depends on it? Doesn't this look like a design error?

O. Wyss

Signature · 8 lines
See a huge pile of work at "http://wyodesktop.sourceforge.net/"


-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click
#4 lore raw
Quoted 9 lines · Otto Wyss
Quoted 5 lines · Antonino A. Daplas

Mainly cleanup, but also point #5. The i810fb, for instance, depends on agpgart, but agpgart gets initialized way after fbdev. The workaround is for i810fb to explicitly call intel_agp_init(). Besides the ugliness, forcibly initializing the agpgart subsystem out of sequence may cause problems.

Sorry if I interrupt without deeper knowledge but doesn't this sound very strange?

Yes, it was confusing the first time I noticed this.

Quoted 3 lines

Why is a subsystem initialized much later than the one which depends on it? Doesn't this look like a design error?

I don't know about the design error part, but this is due to explicit initialization.

The framebuffer system gets initialized quite early in the boot process. This is because fbmem_init() is called by drivers/char/mem.c:chr_dev_init(), then each of the fbdev's xxxfb_init(), in turn, are called by fbmem_init(). If you look at drivers/char/Makefile, you'll see that mem.o is the very first in the list, while agp is one of the last. For i810fb, it will always get initialized first before agp, and it doesn't have any choice on the matter.

Anyway, if you look at drivers/char/agp/intel-agp.c, you'll see this comment:

/* intel_agp_init() must not be declared static for explicit
early initialization to work (ie i810fb) */

In theory, with the patch, the above hack will not be needed. The explicit initializations of fbmem, fbcon, and xxxfb are removed. Each will have their own module_init(), meaning their initialization order will now depend on their link order (basically Makefile order). I can then choose to move i810fb to load after agp.

Tony

-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click

#5 lore raw
In reply to Antonino A. Daplas
Quoted 8 lines · Antonino A. Daplas

Currently, the framebuffer system is initialized in a roundabout manner. First, drivers/char/mem.c calls fbmem_init(). fbmem_init() will then iterate over an array of individual drivers' xxxfb_init(), then each driver registers its presence back to fbmem. During console_init(), drivers/char/vt.c will call fb_console_init(). fbcon will check for registered drivers, and if any are present, will call take_over_console() in drivers/char/vt.c.

Quoted 5 lines

This patch changes the initialization sequence so it proceeds in this
manner: Each driver has its own module_init(). Each driver calls register_framebuffer() in fbmem.c. fbmem.c will then notify fbcon of the driver registration. Upon notification, fbcon calls take_over_console() in vt.c.

My main concern with this change is that it will be no longer possible to change initialization order (and hence choose the primary display for systems with multiple graphics adapters) by specifying `video=xxxfb' on the kernel command line.

That's also the reason why this was never done before.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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 BEA Weblogic Workshop FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click

#6 lore raw
Quoted 15 lines · Geert Uytterhoeven
Quoted 8 lines · Antonino A. Daplas

Currently, the framebuffer system is initialized in a roundabout manner. First, drivers/char/mem.c calls fbmem_init(). fbmem_init() will then iterate over an array of individual drivers' xxxfb_init(), then each driver registers its presence back to fbmem. During console_init(), drivers/char/vt.c will call fb_console_init(). fbcon will check for registered drivers, and if any are present, will call take_over_console() in drivers/char/vt.c.

Quoted 5 lines

This patch changes the initialization sequence so it proceeds in this
manner: Each driver has its own module_init(). Each driver calls register_framebuffer() in fbmem.c. fbmem.c will then notify fbcon of the driver registration. Upon notification, fbcon calls take_over_console() in vt.c.

Quoted 5 lines

My main concern with this change is that it will be no longer possible to change initialization order (and hence choose the primary display for systems with multiple graphics adapters) by specifying `video=xxxfb' on the kernel command line.

I see your point. But, can we use "fbcon=" setup options to choose which fb gets mapped to what console? We already have fbcon=map:<option> so we can choose which becomes the primary display. Granted the "fbcon=" setup is currently broken, but if fixed, will that be a fair compromise?

Tony

#7 lore raw
Quoted 21 lines · Antonino A. Daplas
Quoted 15 lines · Geert Uytterhoeven
Quoted 8 lines · Antonino A. Daplas

Currently, the framebuffer system is initialized in a roundabout manner. First, drivers/char/mem.c calls fbmem_init(). fbmem_init() will then iterate over an array of individual drivers' xxxfb_init(), then each driver registers its presence back to fbmem. During console_init(), drivers/char/vt.c will call fb_console_init(). fbcon will check for registered drivers, and if any are present, will call take_over_console() in drivers/char/vt.c.

Quoted 5 lines

This patch changes the initialization sequence so it proceeds in this
manner: Each driver has its own module_init(). Each driver calls register_framebuffer() in fbmem.c. fbmem.c will then notify fbcon of the driver registration. Upon notification, fbcon calls take_over_console() in vt.c.

Quoted 4 lines

My main concern with this change is that it will be no longer possible to change initialization order (and hence choose the primary display for systems with multiple graphics adapters) by specifying `video=xxxfb' on the kernel command line.

Quoted 4 lines

I see your point. But, can we use "fbcon=" setup options to choose which fb gets mapped to what console? We already have fbcon=map:<option> so we can choose which becomes the primary display. Granted the "fbcon=" setup is currently broken, but if fixed, will that be a fair compromise?

Yes, sounds fine.

Just thinking of something else: right now it's possible to disable a fbdev by saying `video=xxxfb:off'. This can be useful if the fbdev driver doesn't work with your hardware. After you change, individual fbdev drivers will have to reimplement this theirselves in their __setup() functions.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

#8 lore raw
Quoted 22 lines · Geert Uytterhoeven
Quoted 21 lines · Antonino A. Daplas
Quoted 19 lines · Geert Uytterhoeven
Quoted 8 lines · Antonino A. Daplas

Currently, the framebuffer system is initialized in a roundabout manner. First, drivers/char/mem.c calls fbmem_init(). fbmem_init() will then iterate over an array of individual drivers' xxxfb_init(), then each driver registers its presence back to fbmem. During console_init(), drivers/char/vt.c will call fb_console_init(). fbcon will check for registered drivers, and if any are present, will call take_over_console() in drivers/char/vt.c.

Quoted 5 lines

This patch changes the initialization sequence so it proceeds in this
manner: Each driver has its own module_init(). Each driver calls register_framebuffer() in fbmem.c. fbmem.c will then notify fbcon of the driver registration. Upon notification, fbcon calls take_over_console() in vt.c.

My main concern with this change is that it will be no longer possible to change initialization order (and hence choose the primary display for systems with multiple graphics adapters) by specifying `video=xxxfb' on the kernel command line.

Quoted 6 lines
Quoted 4 lines

I see your point. But, can we use "fbcon=" setup options to choose which fb gets mapped to what console? We already have fbcon=map:<option> so we can choose which becomes the primary display. Granted the "fbcon=" setup is currently broken, but if fixed, will that be a fair compromise?

Yes, sounds fine.

Thanks.

Quoted 5 lines

Just thinking of something else: right now it's possible to disable a fbdev by saying `video=xxxfb:off'. This can be useful if the fbdev driver doesn't work with your hardware. After you change, individual fbdev drivers will have to reimplement this theirselves in their __setup() functions.

Yes. I hope this doesn't bite too deep. If it does, I think we can let fb_get_options() return an error when the "off" option is present. But I'm not going to implement it unless we get a lot of complaints.

Tony