OLPC: gxfb/lxfb: add DCON panel modes to framebuffer drivers

PATCH In review Andres Salomon 4 participants 7 messages Apr 14, 2008 – Apr 18, 2008 Framebuffer layer

Share & tools

Andres Salomon

author

2008-04-14 07:51 UTC #0 lore raw

PATCH 3/3 OLPC: gxfb/lxfb: add DCON panel modes to framebuffer drivers

Since there's no way to autodetect panel modes, we're forced to hardcode them in the driver and add a big fat #ifdef. The OLPC DCON needs a specific mode line (at 1200x900). This adds it to both gxfb and lxfb.

Signed-off-by Andres Salomon <[email protected]>

---
 drivers/video/geode/gxfb_core.c |   35 ++++++++++++++++++++++++++++++++++-
 drivers/video/geode/lxfb_core.c |   35 +++++++++++++++++++++++++++++++----
 2 files changed, 65 insertions(+), 5 deletions(-)

drivers/video/geode/gxfb_core.c +34 -1

index 988c8e4..3e76e06 100644
@@ -108,6 +108,35 @@ static const struct fb_videomode gx_modedb[] __initdata = {
 	  FB_VMODE_NONINTERLACED, FB_MODE_IS_VESA },
 };
 
+#ifdef CONFIG_OLPC
+#include <asm/olpc.h>
+
+static const struct fb_videomode gx_dcon_modedb[] __initdata = {
+	/* The only mode the DCON has is 1200x900 */
+	{ NULL, 50, 1200, 900, 17460, 24, 8, 4, 5, 8, 3,
+	  FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
+	  FB_VMODE_NONINTERLACED, 0 }
+};
+
+static void get_modedb(struct fb_videomode **modedb, unsigned int *size)
+{
+	if (olpc_has_dcon()) {
+		*modedb = (struct fb_videomode *) gx_dcon_modedb;
+		*size = ARRAY_SIZE(gx_dcon_modedb);
+	} else {
+		*modedb = (struct fb_videomode *) gx_modedb;
+		*size = ARRAY_SIZE(gx_modedb);
+	}
+}
+
+#else
+static void get_modedb(struct fb_videomode **modedb, unsigned int *size)
+{
+	*modedb = (struct fb_videomode *) gx_modedb;
+	*size = ARRAY_SIZE(gx_modedb);
+}
+#endif
+
 static int gxfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 {
 	if (var->xres > 1600 || var->yres > 1200)

@@ -356,6 +385,9 @@ static int __init gxfb_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	int ret;
 	unsigned long val;
 
+	struct fb_videomode *modedb_ptr;
+	unsigned int modedb_size;
+
 	info = gxfb_init_fbinfo(&pdev->dev);
 	if (!info)
 		return -ENOMEM;

@@ -375,8 +407,9 @@ static int __init gxfb_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	else
 		par->enable_crt = 1;
 
+	get_modedb(&modedb_ptr, &modedb_size);
 	ret = fb_find_mode(&info->var, info, mode_option,
-			   gx_modedb, ARRAY_SIZE(gx_modedb), NULL, 16);
+			   modedb_ptr, modedb_size, NULL, 16);
 	if (ret == 0 || ret == 4) {
 		dev_err(&pdev->dev, "could not find valid video mode\n");
 		ret = -EINVAL;

drivers/video/geode/lxfb_core.c +31 -4

index e290981..28fa3c4 100644
@@ -217,6 +217,35 @@ static const struct fb_videomode geode_modedb[] __initdata = {
 	  0, FB_VMODE_NONINTERLACED, 0 },
 };
 
+#ifdef CONFIG_OLPC
+#include <asm/olpc.h>
+
+const struct fb_videomode olpc_dcon_modedb[] __initdata = {
+	/* The only mode the DCON has is 1200x900 */
+	{ NULL, 50, 1200, 900, 17460, 24, 8, 4, 5, 8, 3,
+	  FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
+	  FB_VMODE_NONINTERLACED, 0 }
+};
+
+static void get_modedb(struct fb_videomode **modedb, unsigned int *size)
+{
+	if (olpc_has_dcon()) {
+		*modedb = (struct fb_videomode *) olpc_dcon_modedb;
+		*size = ARRAY_SIZE(olpc_dcon_modedb);
+	} else {
+		*modedb = (struct fb_videomode *) geode_modedb;
+		*size = ARRAY_SIZE(geode_modedb);
+	}
+}
+
+#else
+static void get_modedb(struct fb_videomode **modedb, unsigned int *size)
+{
+	*modedb = (struct fb_videomode *) geode_modedb;
+	*size = ARRAY_SIZE(geode_modedb);
+}
+#endif
+
 static int lxfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 {
 	if (var->xres > 1920 || var->yres > 1440)

@@ -480,7 +509,7 @@ static int __init lxfb_probe(struct pci_dev *pdev,
 	int ret;
 
 	struct fb_videomode *modedb_ptr;
-	int modedb_size;
+	unsigned int modedb_size;
 
 	info = lxfb_init_fbinfo(&pdev->dev);

@@ -505,9 +534,7 @@ static int __init lxfb_probe(struct pci_dev *pdev,
 
 	/* Set up the mode database */
 
-	modedb_ptr = (struct fb_videomode *) geode_modedb;
-	modedb_size = ARRAY_SIZE(geode_modedb);
-
+	get_modedb(&modedb_ptr, &modedb_size);
 	ret = fb_find_mode(&info->var, info, mode_option,
 			   modedb_ptr, modedb_size, NULL, 16);

Signature · 12 lines

1.5.4.4



-- 
Need a kernel or Debian developer?  Contact me, I'm looking for contracts.

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Arnd Bergmann

2008-04-14 08:33 UTC #1 lore raw

Quoted 5 lines · Andres Salomon

Since there's no way to autodetect panel modes, we're forced to hardcode them in the driver and add a big fat #ifdef. The OLPC DCON needs a specific mode line (at 1200x900). This adds it to both gxfb and lxfb.
Signed-off-by Andres Salomon <[email protected]>

Since the XO is based on Open Firmware, shouldn't the panel size really be a property in the device tree that can be read by this driver?

	Arnd <><

Jordan Crouse

2008-04-14 15:12 UTC #2 lore raw

Quoted 9 lines · Arnd Bergmann

Quoted 5 lines · Andres Salomon
Since there's no way to autodetect panel modes, we're forced to hardcode them in the driver and add a big fat #ifdef. The OLPC DCON needs a specific mode line (at 1200x900). This adds it to both gxfb and lxfb.
Signed-off-by Andres Salomon <[email protected]>
Since the XO is based on Open Firmware, shouldn't the panel size really be a property in the device tree that can be read by this driver?

Andre's description was slightly misleading. We could probably detect the panel mode, but there isn't any reason to since the panel timings are well known and won't change. While OFW detection would be good computer science fu, it would be a wasted effort since its so easy to hard code them into the table.

Jordan

Signature · 3 lines

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

Andres Salomon

author

2008-04-14 15:31 UTC #3 lore raw

Quoted 11 lines · On Mon, 14 Apr 2008 09:18:56 -0600 Jordan Crouse <[email protected]> wrote:

Quoted 9 lines · Arnd Bergmann
Quoted 5 lines · Andres Salomon
Since there's no way to autodetect panel modes, we're forced to hardcode them in the driver and add a big fat #ifdef. The OLPC DCON needs a specific mode line (at 1200x900). This adds it to both gxfb and lxfb.
Signed-off-by Andres Salomon <[email protected]>
Since the XO is based on Open Firmware, shouldn't the panel size really be a property in the device tree that can be read by this driver?

Quoted 8 lines

Andre's description was slightly misleading. We could probably detect the panel mode, but there isn't any reason to since the panel timings are well known and won't change. While OFW detection would be good computer science fu, it would be a wasted effort since its so easy to hard code them into the table.
Jordan

Right, that's what I get for sending this stuff out at 4am. To be clear, the issue that we have to hard code ways to access panel timing info on a per-platform and/or per-bios basis. There's no standard way to fetch it across all Geodes (that I'm aware of). Even the VSA doesn't really help us here.

Signature · 1 line

Need a kernel or Debian developer?  Contact me, I'm looking for contracts.

Andrew Morton

2008-04-15 01:21 UTC #4 lore raw

In reply to Andres Salomon

Review

Quoted 9 lines · On Mon, 14 Apr 2008 03:53:02 -0400 Andres Salomon <[email protected]> wrote:

Since there's no way to autodetect panel modes, we're forced to hardcode them in the driver and add a big fat #ifdef. The OLPC DCON needs a specific mode line (at 1200x900). This adds it to both gxfb and lxfb.
...
(quoted diff) +2 -0
+static const struct fb_videomode gx_dcon_modedb[] __initdata = {
+const struct fb_videomode olpc_dcon_modedb[] __initdata = {

include/linux/init.h:38 points out that const' and __initdata' should not be mixed.

I forget which architecture/compiler versions explode when we do this. But it's not x86, iirc.

Still, it's poor form and, err, I don't immediately recall which tag we _should_ use, and the comments in there are unhelpful. It might be __initconst, but that has exactly zero users. Help.

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Andres Salomon

author

2008-04-15 02:37 UTC #5 lore raw

Quoted 11 lines · On Mon, 14 Apr 2008 18:21:12 -0700 Andrew Morton <[email protected]>…

Review
Quoted 9 lines · On Mon, 14 Apr 2008 03:53:02 -0400 Andres Salomon <[email protected]> wrote:
Since there's no way to autodetect panel modes, we're forced to hardcode them in the driver and add a big fat #ifdef. The OLPC DCON needs a specific mode line (at 1200x900). This adds it to both gxfb and lxfb.
...
(quoted diff) +2 -0
+static const struct fb_videomode gx_dcon_modedb[] __initdata = {
+const struct fb_videomode olpc_dcon_modedb[] __initdata = {

Hm, that one should be static as well. Will fix in the following patch.

Quoted 11 lines

include/linux/init.h:38 points out that const' and __initdata' should not be mixed.
I forget which architecture/compiler versions explode when we do this. But it's not x86, iirc.
Still, it's poor form and, err, I don't immediately recall which tag we _should_ use, and the comments in there are unhelpful. It might be __initconst, but that has exactly zero users. Help.

My guess would be __initconst or __devinitconst? That would put it in .{init,devinit}.rodata rather than .{init,devinit}.data.

Alternatively, I could just remove the 'const'.

Documentation for this stuff would be awesome. :/

Signature · 1 line

Need a kernel or Debian developer?  Contact me, I'm looking for contracts.

Andres Salomon

author

2008-04-18 04:05 UTC #6 lore raw

PATCH 1/1 lxfb/gxfb: fix up __init and __initdata fields for modedb-related stuff

Quoted 30 lines · On Mon, 14 Apr 2008 22:39:15 -0400 Andres Salomon <[email protected]> wrote:

Quoted 11 lines · On Mon, 14 Apr 2008 18:21:12 -0700 Andrew Morton <[email protected]>…
Review
Quoted 9 lines · On Mon, 14 Apr 2008 03:53:02 -0400 Andres Salomon <[email protected]> wrote:
Since there's no way to autodetect panel modes, we're forced to hardcode them in the driver and add a big fat #ifdef. The OLPC DCON needs a specific mode line (at 1200x900). This adds it to both gxfb and lxfb.
...
(quoted diff) +2 -0
+static const struct fb_videomode gx_dcon_modedb[] __initdata = {
+const struct fb_videomode olpc_dcon_modedb[] __initdata = {
Hm, that one should be static as well. Will fix in the following patch.
Quoted 11 lines
include/linux/init.h:38 points out that const' and __initdata' should not be mixed.
I forget which architecture/compiler versions explode when we do this. But it's not x86, iirc.
Still, it's poor form and, err, I don't immediately recall which tag we _should_ use, and the comments in there are unhelpful. It might be __initconst, but that has exactly zero users. Help.

Quoted 9 lines

My guess would be __initconst or __devinitconst? That would put it in .{init,devinit}.rodata rather than .{init,devinit}.data.
Alternatively, I could just remove the 'const'.
Documentation for this stuff would be awesome. :/

Hm, no answer on that. Might as well just remove the const. Can you please add the patch below?

A couple things were wrong here:
- a struct in lxfb should've been static.
- 'const' and '__initdata' don't mix, so the consts have been dropped
- get_modedb() in both drivers should've been marked w/ __init

Signed-off-by Andres Salomon <[email protected]>

---
 drivers/video/geode/gxfb_core.c |    8 ++++----
 drivers/video/geode/lxfb_core.c |    8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

drivers/video/geode/gxfb_core.c +4 -4

index 3e76e06..6b341b5 100644
@@ -41,7 +41,7 @@ static int vram;
 static int vt_switch;
 
 /* Modes relevant to the GX (taken from modedb.c) */
-static const struct fb_videomode gx_modedb[] __initdata = {
+static struct fb_videomode gx_modedb[] __initdata = {
 	/* 640x480-60 VESA */
 	{ NULL, 60, 640, 480, 39682,  48, 16, 33, 10, 96, 2,
 	  0, FB_VMODE_NONINTERLACED, FB_MODE_IS_VESA },

@@ -111,14 +111,14 @@ static const struct fb_videomode gx_modedb[] __initdata = {
 #ifdef CONFIG_OLPC
 #include <asm/olpc.h>
 
-static const struct fb_videomode gx_dcon_modedb[] __initdata = {
+static struct fb_videomode gx_dcon_modedb[] __initdata = {
 	/* The only mode the DCON has is 1200x900 */
 	{ NULL, 50, 1200, 900, 17460, 24, 8, 4, 5, 8, 3,
 	  FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
 	  FB_VMODE_NONINTERLACED, 0 }
 };
 
-static void get_modedb(struct fb_videomode **modedb, unsigned int *size)
+static void __init get_modedb(struct fb_videomode **modedb, unsigned int *size)
 {
 	if (olpc_has_dcon()) {
 		*modedb = (struct fb_videomode *) gx_dcon_modedb;

@@ -130,7 +130,7 @@ static void get_modedb(struct fb_videomode **modedb, unsigned int *size)
 }
 
 #else
-static void get_modedb(struct fb_videomode **modedb, unsigned int *size)
+static void __init get_modedb(struct fb_videomode **modedb, unsigned int *size)
 {
 	*modedb = (struct fb_videomode *) gx_modedb;
 	*size = ARRAY_SIZE(gx_modedb);

drivers/video/geode/lxfb_core.c +4 -4

index 28fa3c4..ae69feb 100644
@@ -36,7 +36,7 @@ static int vt_switch;
  * we try to make it something sane - 640x480-60 is sane
  */
 
-static const struct fb_videomode geode_modedb[] __initdata = {
+static struct fb_videomode geode_modedb[] __initdata = {
 	/* 640x480-60 */
 	{ NULL, 60, 640, 480, 39682, 48, 8, 25, 2, 88, 2,
 	  FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,

@@ -220,14 +220,14 @@ static const struct fb_videomode geode_modedb[] __initdata = {
 #ifdef CONFIG_OLPC
 #include <asm/olpc.h>
 
-const struct fb_videomode olpc_dcon_modedb[] __initdata = {
+static struct fb_videomode olpc_dcon_modedb[] __initdata = {
 	/* The only mode the DCON has is 1200x900 */
 	{ NULL, 50, 1200, 900, 17460, 24, 8, 4, 5, 8, 3,
 	  FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
 	  FB_VMODE_NONINTERLACED, 0 }
 };
 
-static void get_modedb(struct fb_videomode **modedb, unsigned int *size)
+static void __init get_modedb(struct fb_videomode **modedb, unsigned int *size)
 {
 	if (olpc_has_dcon()) {
 		*modedb = (struct fb_videomode *) olpc_dcon_modedb;

@@ -239,7 +239,7 @@ static void get_modedb(struct fb_videomode **modedb, unsigned int *size)
 }
 
 #else
-static void get_modedb(struct fb_videomode **modedb, unsigned int *size)
+static void __init get_modedb(struct fb_videomode **modedb, unsigned int *size)
 {
 	*modedb = (struct fb_videomode *) geode_modedb;
 	*size = ARRAY_SIZE(geode_modedb);

Signature · 6 lines

1.5.4.4



-- 
Need a kernel or Debian developer?  Contact me, I'm looking for contracts.