[GEODE] Add framebuffer support for the AMD Geode LX

I sent this out a few weeks ago, but I didn't hear anything back - so I'll
give it another shot now that .22 is in the can.  This is a basic driver
adding support for the AMD Geode LX framebuffer.  More advanced features,
such as acceleration and power management will come along later after they
have had a chance to soak a little bit more in the OLPC tree.

Jordan

On Mon, Jul 09, 2007 at 11:32:11AM -0600, Jordan Crouse wrote:

+const 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,
+	  FB_VMODE_NONINTERLACED, 0 },
+	/* 640x400-70 */
+	{ NULL, 70, 640, 400, 39770, 40, 8, 28, 5, 96, 2,
+	  FB_SYNC_HOR_HIGH_ACT,
+	  FB_VMODE_NONINTERLACED, 0 },
+	/* 640x480-70 */

This looks like standard timings, is there any reason why not
use modes from modedb.c?

+static int lxfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
+{
+	if (var->xres > 1920 || var->yres > 1440)
+		return -EINVAL;
+
+	if (var->bits_per_pixel == 32) {
+		var->red.offset   = 16; var->red.length   = 8;
+		var->green.offset =  8; var->green.length = 8;
+		var->blue.offset  =  0; var->blue.length  = 8;
+	} else if (var->bits_per_pixel == 16) {
+		var->red.offset   = 11; var->red.length   = 5;
+		var->green.offset =  5; var->green.length = 6;
+		var->blue.offset  =  0; var->blue.length  = 5;
+	} else if (var->bits_per_pixel == 8) {
+		var->red.offset   = 0; var->red.length   = 8;
+		var->green.offset = 0; var->green.length = 8;
+		var->blue.offset  = 0; var->blue.length  = 8;
+	} else
+		return -EINVAL;

Clear var->{color}.msb_right ?

If don't support panning, it should ensure that
xres_virtual == xres and yres_virtual == yres

+static int __init lxfb_map_video_memory(struct fb_info *info,
+					struct pci_dev *dev)
+{
+	struct lxfb_par *par = info->par;
+	int ret;
+
+	ret = pci_enable_device(dev);
+
+	if (ret)
+		return ret;
+
+	ret = pci_request_region(dev, 0, "lxfb-framebuffer");
+
+	if (ret)
+		return ret;
+
+	ret = pci_request_region(dev, 1, "lxfb-gp");
+
+	if (ret)
+		return ret;
+

...

Free allocated regions in a case of a error?

+
+	ret = -ENOMEM;
+
+	if (info->screen_base == NULL)
+		return ret;
+
+	par->gp_regs = ioremap(pci_resource_start(dev, 1),
+				pci_resource_len(dev, 1));

pci_iomap(dev, 1, 0) ?

+static struct fb_info * __init lxfb_init_fbinfo(struct device *dev)
+{
+	struct lxfb_par *par;
+	struct fb_info *info;
+
+	/* Alloc enough space for the pseudo palette. */
+	info = framebuffer_alloc(sizeof(struct lxfb_par) + sizeof(u32) * 16,
+				 dev);

What about adding u32 pseudo_palette[16] at the end of struct lxfb_par
instead of that trick?

+module_param(mode_option, charp, 0);
+MODULE_PARM_DESC(mode_option, "video mode (<x>x<y>[-<bpp>][@<refr>])");
+
+module_param(fbsize, int, 0);
+MODULE_PARM_DESC(fbsize, "video memory size");

What about adding module params for noclear, nopanel and nocrt ?

+static void lx_set_clock(struct fb_info *info)
+{
+	unsigned int diff, min, best = 0;
+	unsigned int freq, i;
+
+	freq = (unsigned int) (0x3b9aca00 / info->var.pixclock);

Here using 0x3b9aca00 instead of 1000000000 is obscuring.

-- 
Elen sila lumenn' omentielvo

Ondrej 'SanTiago' Zajicek (email: santiago@crfreenet.org, jabber: santiago@njs.netlab.cz)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."

On Tue, 2007-07-10 at 19:50 +0200, Ondrej Zajicek wrote:

On Mon, Jul 09, 2007 at 11:32:11AM -0600, Jordan Crouse wrote:

If don't support panning, it should ensure that
xres_virtual == xres and yres_virtual == yres

There is no rule for this, but the above suggestion is preferred.

Tony


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/