fix ps3fb glue allowing a modular build

PATCH NAK Al Viro 6 participants 15 messages Mar 14, 2007 – Mar 21, 2007

Subsystems (4)

PowerPC Framebuffer layer Console PS3 platform support

lore

Share & tools

lore

Al Viro

author

2007-03-14 09:17 UTC #0 lore raw

PATCH 13/13 fix ps3fb glue allowing a modular build

Signed-off-by Al Viro <[email protected]>

---
 arch/powerpc/platforms/ps3/htab.c |    2 ++
 drivers/video/Kconfig             |    2 +-
 include/asm-powerpc/ps3fb.h       |    5 -----
 3 files changed, 3 insertions(+), 6 deletions(-)

arch/powerpc/platforms/ps3/htab.c +2 -0

index e12e59f..67d6f58 100644
@@ -235,7 +235,9 @@ static void ps3_hpte_invalidate(unsigned long slot, unsigned long va,
 static void ps3_hpte_clear(void)
 {
 	/* Make sure to clean up the frame buffer device first */
+#ifdef CONFIG_PS3_FB
 	ps3fb_cleanup();
+#endif
 
 	lv1_unmap_htab(htab_addr);
 }

drivers/video/Kconfig +1 -1

index 7f5a598..ab43a32 100644
@@ -1615,7 +1615,7 @@ config FB_IBM_GXT4500
 	  adaptor, found on some IBM System P (pSeries) machines.
 
 config FB_PS3
-	bool "PS3 GPU framebuffer driver"
+	tristate "PS3 GPU framebuffer driver"
 	depends on FB && PS3_PS3AV
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA

include/asm-powerpc/ps3fb.h +0 -5

index ad81cf4..a447387 100644
@@ -43,13 +43,8 @@ struct ps3fb_ioctl_res {
 
 #ifdef __KERNEL__
 
-#ifdef CONFIG_FB_PS3
 extern void ps3fb_flip_ctl(int on);
 extern void ps3fb_cleanup(void);
-#else
-static inline void ps3fb_flip_ctl(int on) {}
-static inline void ps3fb_cleanup(void) {}
-#endif
 
 #endif /* __KERNEL__ */

Signature · 1 line

1.5.0-rc2.GIT

| drivers/built-in.o: In function ps3av_cmd_avb_param':ps3-linux-2.6.21-rc3/drivers/ps3/ps3av_cmd.c:855: undefined reference to .ps3fb_flip_ctl' | :ps3-linux-2.6.21-rc3/drivers/ps3/ps3av_cmd.c:869: undefined reference to `.ps3fb_flip_ctl'

Which could be fixed with something like:

drivers/ps3/ps3av.c +16 -0

@@ -868,6 +868,22 @@ int ps3av_dev_close(void)
 
 EXPORT_SYMBOL_GPL(ps3av_dev_close);
 
+void ps3av_register_flip_ctl(void (*flip_ctl)(int on))
+{
+	mutex_lock(&ps3av.mutex);
+	ps3av.flip_ctl = flip_ctl;
+	mutex_unlock(&ps3av.mutex);
+}
+EXPORT_SYMBOL_GPL(ps3av_register_flip_ctl);
+
+void ps3av_flip_ctl(int on)
+{
+	mutex_lock(&ps3av.mutex);
+	if (ps3av.flip_ctl)
+		ps3av.flip_ctl(on);
+	mutex_unlock(&ps3av.mutex);
+}
+
 static int ps3av_probe(struct ps3_vuart_port_device *dev)
 {
 	int res;

drivers/ps3/ps3av_cmd.c +2 -2

@@ -852,7 +852,7 @@ int ps3av_cmd_avb_param(struct ps3av_pkt
 {
 	int res;
 
-	ps3fb_flip_ctl(0);	/* flip off */
+	ps3av_flip_ctl(0);	/* flip off */
 
 	/* avb packet */
 	res = ps3av_do_pkt(PS3AV_CID_AVB_PARAM, send_len, sizeof(*avb),

@@ -866,7 +866,7 @@ int ps3av_cmd_avb_param(struct ps3av_pkt
 			 res);
 
       out:
-	ps3fb_flip_ctl(1);	/* flip on */
+	ps3av_flip_ctl(1);	/* flip on */
 	return res;
 }

include/asm-powerpc/ps3av.h +3 -0

@@ -660,6 +660,7 @@ struct ps3av {
 	u32 audio_port;
 	int ps3av_mode;
 	int ps3av_mode_old;
+	void (*flip_ctl)(int on);
 };
 
 /** command status **/

@@ -734,5 +735,7 @@ extern int ps3av_video_mute(int);
 extern int ps3av_audio_mute(int);
 extern int ps3av_dev_open(void);
 extern int ps3av_dev_close(void);
+extern void ps3av_register_flip_ctl(void (*func)(int on));
+extern void ps3av_flip_ctl(int on);
 
 #endif	/* _ASM_POWERPC_PS3AV_H_ */

and calling ps3av_register_flip_ctl() from ps3fb at the appropriate places.

Still, the module won't load, as ps3fb needs ps3fb_videomemory[] (do you know a better way to allocate a few mebibytes of physically contiguous memory?):

arch/powerpc/platforms/ps3/setup.c +2 -1

@@ -115,12 +115,13 @@ static void prealloc(struct ps3_prealloc
 	       p->address);
 }
 
-#ifdef CONFIG_FB_PS3
+#if defined(CONFIG_FB_PS3) || defined(CONFIG_FB_PS3_MODULE)
 struct ps3_prealloc ps3fb_videomemory = {
     .name = "ps3fb videomemory",
     .size = CONFIG_FB_PS3_DEFAULT_SIZE_M*1024*1024,
     .align = 1024*1024			/* the GPU requires 1 MiB alignment */
 };
+EXPORT_SYMBOL_GPL(ps3fb_videomemory);
 #define prealloc_ps3fb_videomemory()	prealloc(&ps3fb_videomemory)
 
 static int __init early_parse_ps3fb(char *p)

And finally, make sure CONFIG_LOGO=n, as there's a bug in the logo code: logos are __initdata but the logo code still tries to draw them for a modular fbdev. Originally (eons ago) this case was handled by the flag initmem_freed, which no longer exists.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- Sony Network and Software Technology Center Europe (NSCE) [email protected] ------- The Corporate Village, Da Vincilaan 7-D1 Voice +32-2-7008453 Fax +32-2-7008622 ---------------- B-1935 Zaventem, Belgium

Al Viro

author

2007-03-14 16:03 UTC #2 lore raw

Quoted 7 lines · Geert Uytterhoeven

Quoted 1 line · Al Viro
Signed-off-by Al Viro <[email protected]>
NAK
There are several problems with making it modular. I did try, cfr. the incomplete patchlets below.

[snip]

Quoted 4 lines

And finally, make sure CONFIG_LOGO=n, as there's a bug in the logo code: logos are __initdata but the logo code still tries to draw them for a modular fbdev. Originally (eons ago) this case was handled by the flag initmem_freed, which no longer exists.

Lovely... Consider that one withdrawn, please. FWIW, if we leave it bool (and flip_ctl mess alone would make a good reason for that) we probably want to have it depend on FB=y

Geert Uytterhoeven

2007-03-14 16:17 UTC #3 lore raw

Hi Al,

Quoted 9 lines · Al Viro

Quoted 4 lines · Geert Uytterhoeven
Quoted 1 line · Al Viro
Signed-off-by Al Viro <[email protected]>
NAK
Lovely... Consider that one withdrawn, please. FWIW, if we leave it bool (and flip_ctl mess alone would make a good reason for that) we probably want to have it depend on FB=y

What's the advantage of making FB_PS3 depend on FB=y vs. plain FB? In both cases it can be enabled if FB=y only, right? Or am I missing something?

Gr{oetje,eeting}s,

Geert

Al Viro

author

2007-03-14 17:07 UTC #4 lore raw

Quoted 7 lines · Geert Uytterhoeven

Hi Al,
Quoted 4 lines · Al Viro
Quoted 3 lines · Geert Uytterhoeven
Quoted 1 line · Al Viro
Signed-off-by Al Viro <[email protected]>

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
NAK
Lovely... Consider that one withdrawn, please. FWIW, if we leave it bool (and flip_ctl mess alone would make a good reason for that) we probably want to have it depend on FB=y
What's the advantage of making FB_PS3 depend on FB=y vs. plain FB? In both cases it can be enabled if FB=y only, right? Or am I missing something?

Nope. How can kconfig distinguish that from a boolean option in modular driver? bool *can* depend on tristate and be selected when tristate is set to m.

Geert Uytterhoeven

2007-03-14 17:23 UTC #5 lore raw

Quoted 12 lines · Al Viro

Quoted 11 lines · Geert Uytterhoeven
Quoted 9 lines · Al Viro
Quoted 4 lines · Geert Uytterhoeven
Quoted 1 line · Al Viro
Signed-off-by Al Viro <[email protected]>
NAK
Lovely... Consider that one withdrawn, please. FWIW, if we leave it bool (and flip_ctl mess alone would make a good reason for that) we probably want to have it depend on FB=y

Quoted 6 lines

Quoted 2 lines
What's the advantage of making FB_PS3 depend on FB=y vs. plain FB? In both cases it can be enabled if FB=y only, right? Or am I missing something?
Nope. How can kconfig distinguish that from a boolean option in modular driver? bool *can* depend on tristate and be selected when tristate is set to m.

Sorry, you're right. I tried it before sending the previous mail, but apparently I made a mistake. I tried again, and now it indeed allows me to select FB_PS3.

Gr{oetje,eeting}s,

Geert

Linus Torvalds

2007-03-14 17:31 UTC #6 lore raw

In reply to Al Viro

Quoted 4 lines · Al Viro

Nope. How can kconfig distinguish that from a boolean option in modular driver? bool *can* depend on tristate and be selected when tristate is set to m.

Btw, this is one of those things that easily causes problems.

In many ways it would be nice if we had two different kinds of "bool": one where "m" in the dependency chain means "y" is ok, and one where "m" means "n".

We used to have "dep_bool" and "dep_mbool" for this, a long time ago. It got dropped in the Kconfig language rewrite, and I think it was a mistake.

So I think it would be nice to re-introduce it. As it is, we have a number of Kconfig language constructs that are just unnecessarily hard to understand, because we end up having to add a "= y" or similar.

The rule *used* to be: "dep_mbool" was a boolean that was valid even for modules, while "dep_bool" was a boolean that was valid only for straigth "y", and a module would turn it off.

Maybe not "bool" vs "mbool", but it might be nice to have

bool FB_PS3
depends strictly on FB

ie a "depends strictly" refuses to upgrade a bool dependency from "m" to "y", while a regular depends allows it.

Or something.. The "depends strictly on X" thing would really be just a mental shorthand for "depends on (X)=y" (it's actually longer to type, but I think it's a bit more intuitive, thus "mental shortcut").

Linus

Geert Uytterhoeven

2007-03-14 17:45 UTC #7 lore raw

Quoted 6 lines · Linus Torvalds

Quoted 3 lines · Al Viro
Nope. How can kconfig distinguish that from a boolean option in modular driver? bool *can* depend on tristate and be selected when tristate is set to m.
Btw, this is one of those things that easily causes problems.

I just found another caveat: while select' is some super-override-it-all that ignores depends on non-existent symbols, it doesn't promote m' to y'. E.g. select FB_CFB_FILLRECT' from a bool still gives FB_CFB_FILLRECT=m if FB=m.

Anyway, here's a fix for the 3 problematic cases I found.

Subject: bool fbdevs must depend on FB = y

Frame buffer device drivers that cannot be built as modules must depend on `FB = y'. Correct the 3 remaining offenders.

Signed-off-by Geert Uytterhoeven <[email protected]>

drivers/video/Kconfig +3 -3

index 7f5a598..e4f0dd0 100644
@@ -1320,7 +1320,7 @@ config FB_AU1100
 
 config FB_AU1200
 	bool "Au1200 LCD Driver"
-	depends on FB && MIPS && SOC_AU1200
+	depends on (FB = y) && MIPS && SOC_AU1200
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT

@@ -1470,7 +1470,7 @@ config FB_G364
 
 config FB_68328
 	bool "Motorola 68328 native frame buffer support"
-	depends on FB && (M68328 || M68EZ328 || M68VZ328)
+	depends on (FB = y) && (M68328 || M68EZ328 || M68VZ328)
  	select FB_CFB_FILLRECT
  	select FB_CFB_COPYAREA
  	select FB_CFB_IMAGEBLIT

@@ -1616,7 +1616,7 @@ config FB_IBM_GXT4500
 
 config FB_PS3
 	bool "PS3 GPU framebuffer driver"
-	depends on FB && PS3_PS3AV
+	depends on (FB = y) && PS3_PS3AV
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT

Gr{oetje,eeting}s,

Geert

Al Viro

author

2007-03-14 17:59 UTC #8 lore raw

In reply to Linus Torvalds

Quoted 9 lines · Linus Torvalds

Quoted 3 lines
Nope. How can kconfig distinguish that from a boolean option in modular driver? bool *can* depend on tristate and be selected when tristate is set to m.
Btw, this is one of those things that easily causes problems.
In many ways it would be nice if we had two different kinds of "bool": one where "m" in the dependency chain means "y" is ok, and one where "m" means "n".

*nod*

Quoted 11 lines

Maybe not "bool" vs "mbool", but it might be nice to have
bool FB_PS3
depends strictly on FB
ie a "depends strictly" refuses to upgrade a bool dependency from "m" to "y", while a regular depends allows it.
Or something.. The "depends strictly on X" thing would really be just a mental shorthand for "depends on (X)=y" (it's actually longer to type, but I think it's a bit more intuitive, thus "mental shortcut").

There's a fun side question, though: what should allmodconfig do? FB=m, FB_PS3=n? Or FB=y, FB_PS3=y?

Geert Uytterhoeven

2007-03-14 18:09 UTC #9 lore raw

Quoted 9 lines · Al Viro

Quoted 8 lines · Linus Torvalds
Maybe not "bool" vs "mbool", but it might be nice to have
bool FB_PS3
depends strictly on FB
ie a "depends strictly" refuses to upgrade a bool dependency from "m" to "y", while a regular depends allows it.

Quoted 8 lines

Quoted 3 lines
Or something.. The "depends strictly on X" thing would really be just a mental shorthand for "depends on (X)=y" (it's actually longer to type, but I think it's a bit more intuitive, thus "mental shortcut").
There's a fun side question, though: what should allmodconfig do? FB=m, FB_PS3=n? Or FB=y, FB_PS3=y?
From `make help':

| New config selecting modules when possible

FB can be a module, so FB=m, FB_PS3=n.

It doesn't say anything about things that can't be modules :-)

But I agree the chances of getting a system that doesn't work increase...

Gr{oetje,eeting}s,

Geert

Al Viro

author

2007-03-14 18:25 UTC #10 lore raw

Quoted 17 lines · Geert Uytterhoeven

Quoted 15 lines · Al Viro
Quoted 11 lines · Linus Torvalds
Maybe not "bool" vs "mbool", but it might be nice to have
bool FB_PS3
depends strictly on FB
ie a "depends strictly" refuses to upgrade a bool dependency from "m" to "y", while a regular depends allows it.
Or something.. The "depends strictly on X" thing would really be just a mental shorthand for "depends on (X)=y" (it's actually longer to type, but I think it's a bit more intuitive, thus "mental shortcut").
There's a fun side question, though: what should allmodconfig do? FB=m, FB_PS3=n? Or FB=y, FB_PS3=y?

Quoted 8 lines

Quoted 1 line
From `make help':
| New config selecting modules when possible
FB can be a module, so FB=m, FB_PS3=n.
It doesn't say anything about things that can't be modules :-)
But I agree the chances of getting a system that doesn't work increase...

No, I realize what kind of behaviour we'll get if we go for dependency on FB=y. However, if we really introduce a new kconfig primitive, it might make sense to teach allmodconfig to deal with it.

Geert Uytterhoeven

2007-03-20 21:07 UTC #11 lore raw

kconfig `bool' (was: Re: [PATCH 13/13] fix ps3fb glue allowing a modular build)

In reply to Linus Torvalds

Quoted 27 lines · Linus Torvalds

Quoted 3 lines · Al Viro
Nope. How can kconfig distinguish that from a boolean option in modular driver? bool *can* depend on tristate and be selected when tristate is set to m.
Btw, this is one of those things that easily causes problems.
In many ways it would be nice if we had two different kinds of "bool": one where "m" in the dependency chain means "y" is ok, and one where "m" means "n".
We used to have "dep_bool" and "dep_mbool" for this, a long time ago. It got dropped in the Kconfig language rewrite, and I think it was a mistake.
So I think it would be nice to re-introduce it. As it is, we have a number of Kconfig language constructs that are just unnecessarily hard to understand, because we end up having to add a "= y" or similar.
The rule *used* to be: "dep_mbool" was a boolean that was valid even for modules, while "dep_bool" was a boolean that was valid only for straigth "y", and a module would turn it off.
Maybe not "bool" vs "mbool", but it might be nice to have
bool FB_PS3
depends strictly on FB

Quoted 6 lines

ie a "depends strictly" refuses to upgrade a bool dependency from "m" to "y", while a regular depends allows it.
Or something.. The "depends strictly on X" thing would really be just a mental shorthand for "depends on (X)=y" (it's actually longer to type, but I think it's a bit more intuitive, thus "mental shortcut").

I've been thinking about this a bit more...

Kconfig knows about the following types:
o bool
o tristate
o string
o hex
o int

However, from a semantical point of view, they can be subdivided in 2 classes:
1. driver/subsystem/library enablers (i.e. things that are used in a Makefile to decide whether to compile a unit or not):
o tristate (y=builtin, m=loadable, n=disabled)
o bool (y=builtin, n=disabled)
2. options (i.e. things that control some features, limits, or default values):
o bool (y=true, n=false)
o string (literal)
o hex (literal)
o int (literal)

The confusion arises from the 2 different semantics for bool': for the former, a depends on' obviously cannot be builtin' if the dependency is modular', while for the latter, it can be true' if the dependency is modular'.

It would be nice if Kconfig would make the distinction between the two types of bool' explicit. Anyone with a good replacement name for the first type of bool'? I can't find one.

Gr{oetje,eeting}s,

Geert

Jan Engelhardt

2007-03-20 21:20 UTC #12 lore raw

Re: kconfig `bool' (was: Re: [PATCH 13/13] fix ps3fb glue allowing a modular build)

Quoted 34 lines · Geert Uytterhoeven

Quoted 6 lines
Maybe not "bool" vs "mbool", but it might be nice to have
bool FB_PS3
depends strictly on FB
Quoted 6 lines
ie a "depends strictly" refuses to upgrade a bool dependency from "m" to "y", while a regular depends allows it.
Or something.. The "depends strictly on X" thing would really be just a mental shorthand for "depends on (X)=y" (it's actually longer to type, but I think it's a bit more intuitive, thus "mental shortcut").
I've been thinking about this a bit more...
Kconfig knows about the following types:
o bool
o tristate
o string
o hex
o int
However, from a semantical point of view, they can be subdivided in 2 classes:
1. driver/subsystem/library enablers (i.e. things that are used in a Makefile to decide whether to compile a unit or not):
o tristate (y=builtin, m=loadable, n=disabled)
o bool (y=builtin, n=disabled)
2. options (i.e. things that control some features, limits, or default values):
o bool (y=true, n=false)
o string (literal)
o hex (literal)
o int (literal)

Quoted 3 lines

The confusion arises from the 2 different semantics for bool': for the former, a depends on' obviously cannot be builtin' if the dependency is modular', while for the latter, it can be true' if the dependency is modular'.

I think it was once (is still?) possible to have something like

<M> Foo

      <*> Bar

which would mean: include bar.o into foo.ko. If one chose to

<M> Foo

      <M> Bar

you'd get foo.ko and bar.ko, with a modinfo dependency of course.

Jan

Roman Zippel

2007-03-21 12:00 UTC #13 lore raw

Re: kconfig `bool' (was: Re: [PATCH 13/13] fix ps3fb glue allowing a modular build)

In reply to Geert Uytterhoeven

Hi,

Quoted 7 lines · Geert Uytterhoeven

Quoted 6 lines · Linus Torvalds
In many ways it would be nice if we had two different kinds of "bool": one where "m" in the dependency chain means "y" is ok, and one where "m" means "n".
We used to have "dep_bool" and "dep_mbool" for this, a long time ago. It got dropped in the Kconfig language rewrite, and I think it was a mistake.

Well, it wasn't really dropped, just the syntax changed... There were only one or two rules which depended on the dep_bool syntax and it were special cases anyway.

Quoted 13 lines

Quoted 13 lines
So I think it would be nice to re-introduce it. As it is, we have a number of Kconfig language constructs that are just unnecessarily hard to understand, because we end up having to add a "= y" or similar.
The rule *used* to be: "dep_mbool" was a boolean that was valid even for modules, while "dep_bool" was a boolean that was valid only for straigth "y", and a module would turn it off.
Maybe not "bool" vs "mbool", but it might be nice to have
bool FB_PS3
depends strictly on FB

Quoted 6 lines

Quoted 6 lines
ie a "depends strictly" refuses to upgrade a bool dependency from "m" to "y", while a regular depends allows it.
Or something.. The "depends strictly on X" thing would really be just a mental shorthand for "depends on (X)=y" (it's actually longer to type, but I think it's a bit more intuitive, thus "mental shortcut").

I'm not sure this would really an improvement, for the casual reader X=y is IMO more readable if he knows the basic kconfig syntax, whereas he had to lookup first what "strictly" means. Also the potential user had to know about this syntax, but it's simply not needed often enough that it would be common and might be easily forgotten if one fixed the driver and changed the bool to tristate.

Quoted 21 lines

I've been thinking about this a bit more...
Kconfig knows about the following types:
o bool
o tristate
o string
o hex
o int
However, from a semantical point of view, they can be subdivided in 2 classes:
1. driver/subsystem/library enablers (i.e. things that are used in a Makefile to decide whether to compile a unit or not):
o tristate (y=builtin, m=loadable, n=disabled)
o bool (y=builtin, n=disabled)
2. options (i.e. things that control some features, limits, or default values):
o bool (y=true, n=false)
o string (literal)
o hex (literal)
o int (literal)

Quoted 3 lines

The confusion arises from the 2 different semantics for bool': for the former, a depends on' obviously cannot be builtin' if the dependency is modular', while for the latter, it can be true' if the dependency is modular'.

What I'm considering in this context is to introduce a syntax to specifically describe modules (e.g. to also generate the Makefile from it):

module FB_PS3

	bool ".."
	depends on FB

In this context it would be valid to add the requirement that the dependencies must be 'y'.

bye, Roman

Antonino A. Daplas

2007-03-15 01:16 UTC #14 lore raw

In reply to Geert Uytterhoeven

Quoted 9 lines · Geert Uytterhoeven

Quoted 1 line · Al Viro
Signed-off-by Al Viro <[email protected]>
And finally, make sure CONFIG_LOGO=n, as there's a bug in the logo code: logos are __initdata but the logo code still tries to draw them for a modular fbdev. Originally (eons ago) this case was handled by the flag initmem_freed, which no longer exists.

True, I tried to prevent the logo from being drawn if the driver is loaded first prior to fbcon, but the code will still draw the logo if the load order is reversed. Can you try this patch? It will only permit the drawing of the logo if both the driver and fbcon are compiled statically.

Tony

drivers/video/console/fbcon.c +17 -0

index bd131d4..12e8a3b 100644
@@ -107,7 +107,9 @@ static struct display fb_display[MAX_NR_
 
 static signed char con2fb_map[MAX_NR_CONSOLES];
 static signed char con2fb_map_boot[MAX_NR_CONSOLES];
+#ifndef MODULE
 static int logo_height;
+#endif
 static int logo_lines;
 /* logo_shown is an index to vc_cons when >= 0; otherwise follows FBCON_LOGO
    enums.  */

@@ -576,6 +578,13 @@ static int fbcon_takeover(int show_logo)
 	return err;
 }
 
+#ifdef MODULE
+static void fbcon_prepare_logo(struct vc_data *vc, struct fb_info *info,
+			       int cols, int rows, int new_cols, int new_rows)
+{
+	logo_shown = FBCON_LOGO_DONTSHOW;
+}
+#else
 static void fbcon_prepare_logo(struct vc_data *vc, struct fb_info *info,
 			       int cols, int rows, int new_cols, int new_rows)
 {

@@ -584,6 +593,11 @@ static void fbcon_prepare_logo(struct vc
 	int cnt, erase = vc->vc_video_erase_char, step;
 	unsigned short *save = NULL, *r, *q;
 
+	if (info->flags & FBINFO_MODULE) {
+		logo_shown = FBCON_LOGO_DONTSHOW;
+		goto done;
+	}
+
 	/*
 	 * remove underline attribute from erase character
 	 * if black and white framebuffer.

@@ -654,7 +668,10 @@ static void fbcon_prepare_logo(struct vc
 		logo_shown = FBCON_LOGO_DRAW;
 		vc->vc_top = logo_lines;
 	}
+
+done:
 }
+#endif /* MODULE */
 
 #ifdef CONFIG_FB_TILEBLITTING
 static void set_blitting_type(struct vc_data *vc, struct fb_info *info)

drivers/video/fbmem.c +4 -2

index 45f3839..08c292d 100644
@@ -418,7 +418,8 @@ int fb_prepare_logo(struct fb_info *info
 
 	memset(&fb_logo, 0, sizeof(struct logo_data));
 
-	if (info->flags & FBINFO_MISC_TILEBLITTING)
+	if (info->flags & FBINFO_MISC_TILEBLITTING ||
+	    info->flags & FBINFO_MODULE)
 		return 0;
 
 	if (info->fix.visual == FB_VISUAL_DIRECTCOLOR) {

@@ -483,7 +484,8 @@ int fb_show_logo(struct fb_info *info, i
 	struct fb_image image;
 
 	/* Return if the frame buffer is not mapped or suspended */
-	if (fb_logo.logo == NULL || info->state != FBINFO_STATE_RUNNING)
+	if (fb_logo.logo == NULL || info->state != FBINFO_STATE_RUNNING ||
+	    info->flags & FBINFO_MODULE)
 		return 0;
 
 	image.depth = 8;