Skip to contentSkip to conversation

fbdev: Add framebuffer and display update module support for pnx4008

PATCH In review Antonino A. Daplas 2 participants 11 messages Jun 30, 2006 – Jul 1, 2006 Framebuffer layer

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 fbdev: Add framebuffer and display update module support for pnx4008

From: Vitaly Wool <[email protected]>

Add support for Display Update Module and RGB framebuffer device on Philips PNX4008 ARM board.

---

 drivers/video/Kconfig            |   15 +
 drivers/video/Makefile           |    2 
 drivers/video/pnx4008/Makefile   |    7 
 drivers/video/pnx4008/dum.h      |  211 +++++++++
 drivers/video/pnx4008/fbcommon.h |   43 ++
 drivers/video/pnx4008/pnxrgbfb.c |  214 +++++++++
 drivers/video/pnx4008/sdum.c     |  872 ++++++++++++++++++++++++++++++++++++++
 drivers/video/pnx4008/sdum.h     |  139 ++++++
 8 files changed, 1503 insertions(+), 0 deletions(-)
drivers/video/Kconfig +15 -0
index 49234d1..985dbe6 100644
@@ -1557,6 +1557,21 @@ config FB_S3C2410_DEBUG
 	  Turn on debugging messages. Note that you can set/unset at run time
 	  through sysfs
 
+config FB_PNX4008_DUM
+	tristate "Display Update Module support on Philips PNX4008 board"
+	depends on FB && ARCH_PNX4008
+	---help---
+	  Say Y here to enable support for PNX4008 Display Update Module (DUM)
+
+config FB_PNX4008_DUM_RGB
+	tristate "RGB Framebuffer support on Philips PNX4008 board"
+	depends on FB_PNX4008_DUM
+	select FB_CFB_FILLRECT
+	select FB_CFB_COPYAREA
+	select FB_CFB_IMAGEBLIT
+	---help---
+	  Say Y here to enable support for PNX4008 RGB Framebuffer
+
 config FB_VIRTUAL
 	tristate "Virtual Frame Buffer support (ONLY FOR TESTING!)"
 	depends on FB
drivers/video/Makefile +2 -0
index c335e9b..6283d01 100644
@@ -94,6 +94,8 @@ obj-$(CONFIG_FB_TX3912)		  += tx3912fb.o
 obj-$(CONFIG_FB_S1D13XXX)	  += s1d13xxxfb.o
 obj-$(CONFIG_FB_IMX)              += imxfb.o
 obj-$(CONFIG_FB_S3C2410)	  += s3c2410fb.o
+obj-$(CONFIG_FB_PNX4008_DUM)	  += pnx4008/
+obj-$(CONFIG_FB_PNX4008_DUM_RGB)  += pnx4008/
 
 # Platform or fallback drivers go here
 obj-$(CONFIG_FB_VESA)             += vesafb.o
drivers/video/pnx4008/Makefile new file +7 -0
new file mode 100644
index 0000000..636aacc
@@ -0,0 +1,7 @@
+#
+# Makefile for the new PNX4008 framebuffer device driver
+#
+
+obj-$(CONFIG_FB_PNX4008_DUM) += sdum.o
+obj-$(CONFIG_FB_PNX4008_DUM_RGB) += pnxrgbfb.o
+
drivers/video/pnx4008/dum.h new file +211 -0

211 lines not on this page · raw message

drivers/video/pnx4008/fbcommon.h new file +43 -0

43 lines not on this page · raw message

drivers/video/pnx4008/pnxrgbfb.c new file +214 -0

214 lines not on this page · raw message

drivers/video/pnx4008/sdum.c new file +872 -0

872 lines not on this page · raw message

drivers/video/pnx4008/sdum.h new file +139 -0

139 lines not on this page · raw message

Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

#1 lore raw
Quoted 1 line · On Fri, 30 Jun 2006 20:00:23 +0800 "Antonino A. Daplas" <[email protected]>…

+static struct fb_var_screeninfo rgbfb_var __initdata = {

Is that right? We later do

+ info->var = rgbfb_var;

but rgbfb_var is about to be unloaded from memory. I suspect this'll crash if the driver is statically linked into the kernel.

Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

#2 lore raw
Quoted 12 lines · Andrew Morton
Quoted 1 line · On Fri, 30 Jun 2006 20:00:23 +0800 "Antonino A. Daplas" <[email protected]>…

+static struct fb_var_screeninfo rgbfb_var __initdata = {

Is that right? We later do

+ info->var = rgbfb_var;

but rgbfb_var is about to be unloaded from memory. I suspect this'll crash if the driver is statically linked into the kernel.

Should be okay, as the assignment is done during init only.

But it should be __devinitdata, not __initdata (or vice versa).

Tony

Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

#3 lore raw
Quoted 8 lines · On Sat, 01 Jul 2006 13:02:19 +0800 "Antonino A. Daplas" <[email protected]>…
Quoted 7 lines · Andrew Morton
Quoted 1 line · On Fri, 30 Jun 2006 20:00:23 +0800 "Antonino A. Daplas" <[email protected]>…

+static struct fb_var_screeninfo rgbfb_var __initdata = {

Is that right? We later do

Quoted 7 lines
Quoted 5 lines

+ info->var = rgbfb_var;

but rgbfb_var is about to be unloaded from memory. I suspect this'll crash if the driver is statically linked into the kernel.

Should be okay, as the assignment is done during init only.

But we're setting info->var to point at memory which will be freed up by free_initmem().

So if anyone _uses_ info->var after free_initmem() they'll reference random memory or they'll oops.

Looking at, say, drivers/video/vfb.c:vfb_probe(), I see

	info->var = vfb_default;

vfb_default is __init and `info' has a lifetime beyond the execution of this function. It looks wrong.

Bear in mind that the __init and __initdata segments do not get unloaded if the code is compiled as a module (yet). So the problems only become apparent when the driver is linked into vmlinux.

Also, we'll now get section mismatch warnings from the kbuild tools. But they won't help us in this case because vfb_probe() is __init.

Quoted 1 line

But it should be __devinitdata, not __initdata (or vice versa).

Yes, the driver seems a bit confused about that also.

Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

#4 lore raw
Quoted 15 lines · Andrew Morton
Quoted 12 lines · On Sat, 01 Jul 2006 13:02:19 +0800 "Antonino A. Daplas" <[email protected]>…
Quoted 11 lines · Andrew Morton
Quoted 1 line · On Fri, 30 Jun 2006 20:00:23 +0800 "Antonino A. Daplas" <[email protected]>…

+static struct fb_var_screeninfo rgbfb_var __initdata = {

Is that right? We later do

+ info->var = rgbfb_var;

but rgbfb_var is about to be unloaded from memory. I suspect this'll crash if the driver is statically linked into the kernel.

Quoted 7 lines
Quoted 1 line

Should be okay, as the assignment is done during init only.

But we're setting info->var to point at memory which will be freed up by free_initmem().

So if anyone _uses_ info->var after free_initmem() they'll reference random memory or they'll oops.

No, rgbfb_var is not a pointer, so the contents or rgbfb_var are copied to info->var.

Tony

Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

#5 lore raw
Quoted 18 lines · On Sat, 01 Jul 2006 13:18:02 +0800 "Antonino A. Daplas" <[email protected]>…
Quoted 17 lines · Andrew Morton
Quoted 13 lines · On Sat, 01 Jul 2006 13:02:19 +0800 "Antonino A. Daplas" <[email protected]>…
Quoted 11 lines · Andrew Morton
Quoted 1 line · On Fri, 30 Jun 2006 20:00:23 +0800 "Antonino A. Daplas" <[email protected]>…

+static struct fb_var_screeninfo rgbfb_var __initdata = {

Is that right? We later do

+ info->var = rgbfb_var;

but rgbfb_var is about to be unloaded from memory. I suspect this'll crash if the driver is statically linked into the kernel.

Should be okay, as the assignment is done during init only.

Quoted 9 lines
Quoted 5 lines

But we're setting info->var to point at memory which will be freed up by free_initmem().

So if anyone _uses_ info->var after free_initmem() they'll reference random memory or they'll oops.

No, rgbfb_var is not a pointer, so the contents or rgbfb_var are copied to info->var.

oh, ok. How unusual. I assume things inside fb_var_screeninfo.var get altered at runtime?

Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

#6 lore raw
Quoted 23 lines · Andrew Morton
Quoted 20 lines · On Sat, 01 Jul 2006 13:18:02 +0800 "Antonino A. Daplas" <[email protected]>…
Quoted 19 lines · Andrew Morton
Quoted 5 lines · On Sat, 01 Jul 2006 13:02:19 +0800 "Antonino A. Daplas" <[email protected]>…
Quoted 4 lines · Andrew Morton
Quoted 1 line · On Fri, 30 Jun 2006 20:00:23 +0800 "Antonino A. Daplas" <[email protected]>…

+static struct fb_var_screeninfo rgbfb_var __initdata = {

Quoted 8 lines
Quoted 7 lines

Is that right? We later do

+ info->var = rgbfb_var;

but rgbfb_var is about to be unloaded from memory. I suspect this'll crash if the driver is statically linked into the kernel.

Should be okay, as the assignment is done during init only.

But we're setting info->var to point at memory which will be freed up by free_initmem().

Quoted 10 lines
Quoted 5 lines
Quoted 2 lines

So if anyone _uses_ info->var after free_initmem() they'll reference random memory or they'll oops.

No, rgbfb_var is not a pointer, so the contents or rgbfb_var are copied to info->var.

oh, ok. How unusual. I assume things inside fb_var_screeninfo.var get altered at runtime?

For drivers that do allow changing of video modes at startup, yes. For drivers that don't, such as this one, they just set a known, minimal working mode which is copied to info->var.

Tony

Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

#7 lore raw
In reply to Antonino A. Daplas

drivers/video/pnx4008/sdum.c line 318

Quoted 3 lines · On Fri, 30 Jun 2006 20:00:23 +0800 "Antonino A. Daplas" <[email protected]>…
drivers/video/pnx4008/sdum.c new file +3 -0
+	if ((ret = put_cmd_string(cmds_c)) != 0) {
+		return ret;
+	}

Guys, please avoid the assign-then-test trick. Kernel style is super-simple style. The above should be coded as:

	ret = put_cmd_string(cmds_c);
	if (ret != 0)
		return ret;

Nice and easy.

Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

#8 lore raw
Quoted 10 lines · Andrew Morton

drivers/video/pnx4008/sdum.c line 318

Quoted 3 lines · On Fri, 30 Jun 2006 20:00:23 +0800 "Antonino A. Daplas" <[email protected]>…
drivers/video/pnx4008/sdum.c new file +3 -0
+	if ((ret = put_cmd_string(cmds_c)) != 0) {
+		return ret;
+	}

Guys, please avoid the assign-then-test trick. Kernel style is super-simple style. The above should be coded as:

I'm kinda lax when it comes to this style, but if you insist :-)

Quoted 7 lines
	ret = put_cmd_string(cmds_c);
	if (ret != 0)
		return ret;

Nice and easy.

Tony

Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

#9 lore raw
In reply to Antonino A. Daplas

+EXPORT_SYMBOL(pnx4008_get_fb_addresses); +EXPORT_SYMBOL(pnx4008_alloc_dum_channel); +EXPORT_SYMBOL(pnx4008_free_dum_channel); +EXPORT_SYMBOL(pnx4008_put_dum_channel_uf); +EXPORT_SYMBOL(pnx4008_set_dum_channel_sync); +EXPORT_SYMBOL(pnx4008_set_dum_channel_dirty_detect); +EXPORT_SYMBOL(pnx4008_get_dum_channel_uf); +EXPORT_SYMBOL(pnx4008_get_dum_channel_config); +EXPORT_SYMBOL(pnx4008_force_update_dum_channel); +EXPORT_SYMBOL(pnx4008_sdum_mmap); +EXPORT_SYMBOL(pnx4008_set_dum_exit_notification);

Are these all needed??

Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

#10 lore raw

drivers/video/pnx4008/sdum.c line 714

Quoted 12 lines · Andrew Morton
drivers/video/pnx4008/sdum.c new file +11 -0
+EXPORT_SYMBOL(pnx4008_get_fb_addresses);
+EXPORT_SYMBOL(pnx4008_alloc_dum_channel);
+EXPORT_SYMBOL(pnx4008_free_dum_channel);
+EXPORT_SYMBOL(pnx4008_put_dum_channel_uf);
+EXPORT_SYMBOL(pnx4008_set_dum_channel_sync);
+EXPORT_SYMBOL(pnx4008_set_dum_channel_dirty_detect);
+EXPORT_SYMBOL(pnx4008_get_dum_channel_uf);
+EXPORT_SYMBOL(pnx4008_get_dum_channel_config);
+EXPORT_SYMBOL(pnx4008_force_update_dum_channel);
+EXPORT_SYMBOL(pnx4008_sdum_mmap);
+EXPORT_SYMBOL(pnx4008_set_dum_exit_notification);

The unused exports are enclosed with #if 0/#endif. That was before EXPORT_SYMBOL_UNUSED went in.

The reason why I asked him if sdum and pnxrgbfb need to be separate modules. But the intent was to write another driver that will also use the sdum module.

Tony

Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642