(Preliminary) Geode framebuffer driver.

Hi,

Attached is a patch (against 2.6.10) for a framebuffer driver for the Geode family of processors. It does not use AMD's horrible Durango API.
It is, however, rather feature incomplete at the moment.

* Only Geode GX1 (with CS5530 companion chip) is supported.
* Tested resolutions: 640x480 to 1280x1024.
* Supported bit depths: 8 bit palette, 16 bit RGB 5-6-5.
* CRT output only.
* No accelerated features.
* Compression is not enabled.

(Flat panel support and compression will likely be added next week, and acceleration after that.)

Could this patch be considered for inclusion, please?

David Vrabel

David Vrabel, Design Engineer

Arcom, Clifton Road           Tel: +44 (0)1223 411200 ext. 3233
Cambridge CB1 7EA, UK         Web: http://www.arcom.com/

Index: linux-2.6-i386/drivers/video/Kconfig
===================================================================
@@ -939,6 +939,16 @@
 	  similar boards, 3DLabs Permedia3 Create!, Appian Jeronimo 2000
 	  and maybe other boards.
 
+config FB_GEODE
+	bool "AMD Geode family framebuffer support (EXPERIMENTAL)"
+	default n
+	depends on FB && EXPERIMENTAL
+	---help---
+	  Say 'Y' here to allow you to select framebuffer drivers for
+	  the AMD Geode family of processors.
+
+source "drivers/video/geode/Kconfig
+
 config FB_SBUS
 	bool "SBUS and UPA framebuffers"
 	depends on FB && (SPARC32 || SPARC64)

Index: linux-2.6-i386/drivers/video/Makefile
===================================================================
@@ -94,6 +94,7 @@
 obj-$(CONFIG_FB_CIRRUS)		  += cirrusfb.o cfbfillrect.o cfbimgblt.o cfbcopyarea.o
 obj-$(CONFIG_FB_ASILIANT)	  += asiliantfb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o
 obj-$(CONFIG_FB_PXA)		  += pxafb.o cfbimgblt.o cfbcopyarea.o cfbfillrect.o
+obj-$(CONFIG_FB_GEODE)		  += geode/
 
 # Platform or fallback drivers go here
 obj-$(CONFIG_FB_VESA)             += vesafb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o

Index: linux-2.6-i386/drivers/video/geode/Kconfig
===================================================================
@@ -0,0 +1,20 @@
+#
+# Geode family framebuffer configuration
+#
+
+config FB_GEODE_GX1
+	tristate "AMD Geode GX1 framebuffer support (EXPERIMENTAL)"
+	default n
+	depends on FB_GEODE && EXPERIMENTAL
+	---help---
+	  Framebuffer driver for the display controller integrated
+	  into the AMD Geode GX1 processor.
+
+	  Display output is currently limited to CRT output only.
+
+	  This driver is also available as a module ( = code which can be
+	  inserted and removed from the running kernel whenever you want). The
+	  module will be called gx1fb. If you want to compile it as a module,
+	  say M here and read <file:Documentation/modules.txt>.
+
+	  If unsure, say N.

Index: linux-2.6-i386/drivers/video/geode/Makefile
===================================================================
@@ -0,0 +1,5 @@
+# Makefile for the Geode family framebuffer drivers
+
+obj-$(CONFIG_FB_GEODE_GX1) += gx1fb.o ../cfbimgblt.o ../cfbcopyarea.o ../cfbfillrect.o
+
+gx1fb-objs   		:= gx1fb_core.o display_gx1.o video_cs5530.o

Index: linux-2.6-i386/drivers/video/geode/geodefb.h
===================================================================
@@ -0,0 +1,36 @@
+/*
+ * drivers/video/geode/geodefb
+ *   -- Geode framebuffer driver
+ *
+ * Copyright (C) 2005 Arcom Control Systems Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#ifndef __GEODEFB_H__
+#define __GEODEFB_H__
+
+struct geodefb_info;
+
+struct geode_dc_ops {
+	void (*set_mode)(struct geodefb_info *);
+	void (*set_palette_reg)(struct geodefb_info *, unsigned, unsigned, unsigned, unsigned);
+};
+
+struct geode_vid_ops {
+	void (*set_dclk)(struct geodefb_info *);
+	void (*configure_display)(struct geodefb_info *);
+};
+
+struct geodefb_info {
+	struct fb_info fb;
+	struct pci_dev *vid_dev;
+	void __iomem *dc_regs;
+	void __iomem *vid_regs;
+	struct geode_dc_ops  *dc_ops;
+	struct geode_vid_ops *vid_ops;
+};
+
+#endif /* !__GEODEFB_H__ */

Yes.

However, I would prefer that instead of:

struct geodefb_info {
    	struct fb_info fb;
	...

use struct fb_info.par for all driver private data and use framebuffer_release()/framebuffer_alloc() to de/allocate the fb_info structure.

You can also make the check_var() function less strict. You can attempt to round up values instead of returning -EINVAL.

Tony

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Done this now. An updated patch is attached.

This also includes a number of other changes:

- now works when built into the kernel
- flat panel support (panel=<x>x<y> option). (Only one 640x480 panel has been tested though.)
- blank function for display blanking/powersaving.
- crt=<n> option to enable/disable the CRT output.

David Vrabel

David Vrabel, Design Engineer

Arcom, Clifton Road           Tel: +44 (0)1223 411200 ext. 3233
Cambridge CB1 7EA, UK         Web: http://www.arcom.com/

Index: linux-2.6-i386/drivers/video/Kconfig
===================================================================
@@ -1143,6 +1143,8 @@
 	  similar boards, 3DLabs Permedia3 Create!, Appian Jeronimo 2000
 	  and maybe other boards.
 
+source "drivers/video/geode/Kconfig"
+
 config FB_SBUS
 	bool "SBUS and UPA framebuffers"
 	depends on FB && (SPARC32 || SPARC64)

Index: linux-2.6-i386/drivers/video/Makefile
===================================================================
@@ -84,6 +84,7 @@
 obj-$(CONFIG_FB_CIRRUS)		  += cirrusfb.o
 obj-$(CONFIG_FB_ASILIANT)	  += asiliantfb.o
 obj-$(CONFIG_FB_PXA)		  += pxafb.o
+obj-$(CONFIG_FB_GEODE)		  += geode/
 
 # Platform or fallback drivers go here
 obj-$(CONFIG_FB_VESA)             += vesafb.o

Index: linux-2.6-i386/drivers/video/geode/Kconfig
===================================================================
@@ -0,0 +1,29 @@
+#
+# Geode family framebuffer configuration
+#
+config FB_GEODE
+	bool "AMD Geode family framebuffer support (EXPERIMENTAL)"
+	default n
+	depends on FB && EXPERIMENTAL
+	---help---
+	  Say 'Y' here to allow you to select framebuffer drivers for
+	  the AMD Geode family of processors.
+
+config FB_GEODE_GX1
+	tristate "AMD Geode GX1 framebuffer support (EXPERIMENTAL)"
+	default n
+	depends on FB_GEODE && EXPERIMENTAL
+	select FB_CFB_FILLRECT
+	select FB_CFB_COPYAREA
+	select FB_CFB_IMAGEBLIT
+	select FB_SOFT_CURSOR
+	---help---
+	  Framebuffer driver for the display controller integrated into the
+	  AMD Geode GX1 processor.
+
+	  This driver is also available as a module ( = code which can be
+	  inserted and removed from the running kernel whenever you want). The
+	  module will be called gx1fb. If you want to compile it as a module,
+	  say M here and read <file:Documentation/modules.txt>.
+
+	  If unsure, say N.

Index: linux-2.6-i386/drivers/video/geode/Makefile
===================================================================
@@ -0,0 +1,5 @@
+# Makefile for the Geode family framebuffer drivers
+
+obj-$(CONFIG_FB_GEODE_GX1) += gx1fb.o
+
+gx1fb-objs   		:= gx1fb_core.o display_gx1.o video_cs5530.o

Index: linux-2.6-i386/drivers/video/geode/geodefb.h
===================================================================
@@ -0,0 +1,39 @@
+/*
+ * drivers/video/geode/geodefb.h
+ *   -- Geode framebuffer driver
+ *
+ * Copyright (C) 2005 Arcom Control Systems Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#ifndef __GEODEFB_H__
+#define __GEODEFB_H__
+
+struct geodefb_info;
+
+struct geode_dc_ops {
+	void (*set_mode)(struct fb_info *);
+	void (*set_palette_reg)(struct fb_info *, unsigned, unsigned, unsigned, unsigned);
+};
+
+struct geode_vid_ops {
+	void (*set_dclk)(struct fb_info *);
+	void (*configure_display)(struct fb_info *);
+	int  (*blank_display)(const struct fb_info *, int blank_mode);
+};
+
+struct geodefb_par {
+	int enable_crt;
+	int panel_x; /* dimensions of an attached flat panel, non-zero => enable panel */
+	int panel_y;
+	struct pci_dev *vid_dev;
+	void __iomem *dc_regs;
+	void __iomem *vid_regs;
+	struct geode_dc_ops  *dc_ops;
+	struct geode_vid_ops *vid_ops;
+};
+
+#endif /* !__GEODEFB_H__ */

A few minor comments:

+ /* Clear the frame buffer of garbage. */
+ memset(info->screen_base, 0, info->fix.smem_len);

Is the above really necessary? And if it is, memset_io is preferred.

+ info->screen_base = (unsigned char *)ioremap(info->fix.smem_start, info->fix.smem_len);

You also do not need the (unsigned char *) part. (sparse will complain.)

No need to resend the patch, just let me know what changes you want, if any.

Tony

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Note that this driver requires the FB_CFB_xxx and FB_SOFT_CURSOR kbuild option patches I posted earlier today.

It's also:

Thanks for looking this over.

Yes. Otherwise the screen will show garbage until whatever application you're using gets around to clearing the screen. (An embedded system won't necessarily have fbcon.)

Done.

Done.

I think I got that from skeletonfb.c (which is looking a bit out of date).

I've also fixed a warning and corrected some whitespace.

Here's a diff of these changes.

Index: linux-2.6-i386/drivers/video/geode/geodefb.h
===================================================================
--- linux-2.6-i386.orig/drivers/video/geode/geodefb.h 2005-02-15
10:33:59.000000000 +0000
+++ linux-2.6-i386/drivers/video/geode/geodefb.h 2005-02-15
13:49:54.782318435 +0000
@@ -22,7 +22,7 @@
struct geode_vid_ops {
void (*set_dclk)(struct fb_info *); void (*configure_display)(struct fb_info *);
- int (*blank_display)(const struct fb_info *, int blank_mode);
+ int (*blank_display)(struct fb_info *, int blank_mode);
};

struct geodefb_par {
Index: linux-2.6-i386/drivers/video/geode/gx1fb_core.c
===================================================================
--- linux-2.6-i386.orig/drivers/video/geode/gx1fb_core.c 2005-02-15
10:33:59.000000000 +0000
+++ linux-2.6-i386/drivers/video/geode/gx1fb_core.c 2005-02-15
13:53:56.244807541 +0000
@@ -139,7 +139,7 @@
return 0;
}

-static int gx1fb_blank(int blank_mode, const struct fb_info *info) +static int gx1fb_blank(int blank_mode, struct fb_info *info)
{
struct geodefb_par *par = info->par;

@@ -174,7 +174,7 @@
if ((fb_len = gx1_frame_buffer_size()) < 0)
return -ENOMEM;
info->fix.smem_len = fb_len;
- info->screen_base = (unsigned char *)ioremap(info->fix.smem_start,
info->fix.smem_len);
+ info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
if (!info->screen_base)
return -ENOMEM;

@@ -269,7 +269,7 @@
int __init gx1fb_init(void)
{
struct fb_info *info;
- struct geodefb_par *par;
+ struct geodefb_par *par;
int ret;

#ifndef MODULE
@@ -300,8 +300,8 @@
goto err;
}

- /* Clear the frame buffer of garbage. */
- memset(info->screen_base, 0, info->fix.smem_len);
+ /* Clear the frame buffer of garbage. */
+ memset_io(info->screen_base, 0, info->fix.smem_len);

gx1fb_check_var(&info->var, info);
gx1fb_set_par(info);
@@ -315,7 +315,7 @@

err:
if (info->screen_base)
- iounmap((void __iomem *)info->screen_base);
+ iounmap(info->screen_base);
if (par->vid_regs)
iounmap(par->vid_regs);
if (par->dc_regs)
@@ -330,7 +330,7 @@
static void __exit gx1fb_cleanup(void)
{
struct fb_info *info = gx1fb_info;
- struct geodefb_par *par = gx1fb_info->par;
+ struct geodefb_par *par = gx1fb_info->par;

unregister_framebuffer(info);

Index: linux-2.6-i386/drivers/video/geode/video_cs5530.c
===================================================================
--- linux-2.6-i386.orig/drivers/video/geode/video_cs5530.c 2005-02-15
10:33:59.000000000 +0000
+++ linux-2.6-i386/drivers/video/geode/video_cs5530.c 2005-02-15
13:50:10.660522983 +0000
@@ -135,7 +135,7 @@
writel(dcfg, par->vid_regs + CS5530_DISPLAY_CONFIG);
}

-static int cs5530_blank_display(const struct fb_info *info, int blank_mode) +static int cs5530_blank_display(struct fb_info *info, int blank_mode)
{
struct geodefb_par *par = info->par;
u32 dcfg;

David Vrabel, Design Engineer

Arcom, Clifton Road           Tel: +44 (0)1223 411200 ext. 3233
Cambridge CB1 7EA, UK         Web: http://www.arcom.com/


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Noted. I'll submit this for the mm tree, it is too late for 2.6.11.

Tony

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click