[PATCH] Xilinx framebuffer device driver

[PATCH] Xilinx framebuffer device driver

[Andrei Konovalov]

[-- Attachment #1: Type: text/plain, Size: 575 bytes --]

Add support for the video controller IP block included into Xilinx ML300 and
ML403 reference designs.

Signed-off-by: Andrei Konovalov <akonovalov@ru.mvista.com>
---

This patch relies on the "Patchset to establish sanity in Xilinx Virtex support" by Gran Likely to have
the frame buffer device registered on the platform bus. Without this patchset one needs to fill in
the struct platform_device and make sure platform_device_register() is called elsewhere.

Reviews and comments are welcome.

Would be nice to get this driver into mainline for the 2.6.22.

Thanks,
Andrei


[-- Attachment #2: ppc32-xilinx_fb.patch --]
[-- Type: text/x-patch, Size: 11377 bytes --]

Add support for the video controller IP block included into Xilinx ML300 and
ML403 reference designs.

 drivers/video/Kconfig    |   15 +
 drivers/video/Makefile   |    1 
 drivers/video/xilinxfb.c |  358 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 374 insertions(+)

Index: linux-2.6.20/drivers/video/Kconfig
===================================================================
--- linux-2.6.20.orig/drivers/video/Kconfig
+++ linux-2.6.20/drivers/video/Kconfig
@@ -1633,6 +1633,21 @@ config FB_PS3_DEFAULT_SIZE_M
 	  The default value can be overridden on the kernel command line
 	  using the "ps3fb" option (e.g. "ps3fb=9M");
 
+config FB_XILINX
+	tristate "Xilinx frame buffer support"
+	depends on FB && XILINX_VIRTEX
+	select FB_CFB_FILLRECT
+	select FB_CFB_COPYAREA
+	select FB_CFB_IMAGEBLIT
+	---help---
+	  Include support for the Xilinx ML300/ML403 reference design
+	  framebuffer. ML300 carries a 640*480 LCD display on the board,
+	  ML403 uses a standard DB15 VGA connector.
+
+config FB_XILINX_ROTATE
+	bool "Rotate display"
+	depends on FB_XILINX
+
 config FB_VIRTUAL
 	tristate "Virtual Frame Buffer support (ONLY FOR TESTING!)"
 	depends on FB
Index: linux-2.6.20/drivers/video/Makefile
===================================================================
--- linux-2.6.20.orig/drivers/video/Makefile
+++ linux-2.6.20/drivers/video/Makefile
@@ -99,6 +99,7 @@ obj-$(CONFIG_FB_PNX4008_DUM_RGB)  += pnx
 obj-$(CONFIG_FB_IBM_GXT4500)	  += gxt4500.o
 obj-$(CONFIG_FB_PS3)		  += ps3fb.o
 obj-$(CONFIG_FB_SM501)            += sm501fb.o
+obj-$(CONFIG_FB_XILINX)           += xilinxfb.o
 
 # Platform or fallback drivers go here
 obj-$(CONFIG_FB_VESA)             += vesafb.o
Index: linux-2.6.20/drivers/video/xilinxfb.c
===================================================================
--- /dev/null
+++ linux-2.6.20/drivers/video/xilinxfb.c
@@ -0,0 +1,358 @@
+/*
+ * xilinxfb.c
+ *
+ * Xilinx TFT LCD frame buffer driver
+ *
+ * Author: MontaVista Software, Inc.
+ *         source@mvista.com
+ *
+ * 2002-2007 (c) MontaVista Software, Inc.  This file is licensed under the
+ * terms of the GNU General Public License version 2.  This program is licensed
+ * "as is" without any warranty of any kind, whether express or implied.
+ */
+
+/*
+ * This driver was based on au1100fb.c by MontaVista rewritten for 2.6
+ * by Embedded Alley Solutions <source@embeddedalley.com>, which in turn
+ * was based on skeletonfb.c, Skeleton for a frame buffer device by
+ * Geert Uytterhoeven.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/version.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/mm.h>
+#include <linux/fb.h>
+#include <linux/init.h>
+#include <linux/dma-mapping.h>
+#include <linux/platform_device.h>
+
+#include <asm/io.h>
+
+#define DRIVER_NAME		"xilinxfb"
+#define DRIVER_DESCRIPTION	"Xilinx TFT LCD frame buffer driver"
+
+/*
+ * The interface to the framebuffer is nice and simple.  There are two
+ * control registers.  The first tells the LCD interface where in memory
+ * the frame buffer is (only the 11 most significant bits are used, so
+ * don't start thinking about scrolling).  The second allows the LCD to
+ * be turned on or off as well as rotated 180 degrees.
+ */
+#define NUM_REGS	2
+#define REG_FB_ADDR	0
+#define REG_CTRL	1
+#define REG_CTRL_ENABLE	 0x0001
+#define REG_CTRL_ROTATE	 0x0002
+#if defined(CONFIG_FB_XILINX_ROTATE)
+#define REG_CTRL_DEFAULT (REG_CTRL_ENABLE | REG_CTRL_ROTATE)
+#else
+#define REG_CTRL_DEFAULT (REG_CTRL_ENABLE)
+#endif				/* CONFIG_FB_XILINX_ROTATE */
+
+/*
+ * The hardware only handles a single mode: 640x480 24 bit true
+ * color. Each pixel gets a word (32 bits) of memory.  Within each word,
+ * the 8 most significant bits are ignored, the next 8 bits are the red
+ * level, the next 8 bits are the green level and the 8 least
+ * significant bits are the blue level.  Each row of the LCD uses 1024
+ * words, but only the first 640 pixels are displayed with the other 384
+ * words being ignored.  There are 480 rows.
+ */
+#define BYTES_PER_PIXEL	4
+#define BITS_PER_PIXEL	(BYTES_PER_PIXEL * 8)
+#define XRES		640
+#define YRES		480
+#define XRES_VIRTUAL	1024
+#define YRES_VIRTUAL	YRES
+#define LINE_LENGTH	(XRES_VIRTUAL * BYTES_PER_PIXEL)
+#define FB_SIZE		(YRES_VIRTUAL * LINE_LENGTH)
+
+#define PALETTE_ENTRIES_NO	16	/* passed to fb_alloc_cmap() */
+
+/*
+ * Here are the default fb_fix_screeninfo and fb_var_screeninfo structures
+ */
+static struct fb_fix_screeninfo xilinx_fb_fix __initdata = {
+	.id =		"Xilinx",
+	.type =		FB_TYPE_PACKED_PIXELS,
+	.visual =	FB_VISUAL_TRUECOLOR,
+	.smem_len =	FB_SIZE,
+	.line_length =	LINE_LENGTH,
+	.accel =	FB_ACCEL_NONE
+};
+
+static struct fb_var_screeninfo xilinx_fb_var __initdata = {
+	.xres =			XRES,
+	.yres =			YRES,
+	.xres_virtual =		XRES_VIRTUAL,
+	.yres_virtual =		YRES_VIRTUAL,
+
+	.bits_per_pixel =	BITS_PER_PIXEL,
+
+	.red =		{ 16, 8, 0 },
+	.green =	{ 8, 8, 0 },
+	.blue =		{ 0, 8, 0 },
+	.transp =	{ 0, 0, 0 },
+
+	.activate =	FB_ACTIVATE_NOW,
+	.height = 	99,	/* in mm of NEC NL6448BC20-08 on ML300 */
+	.width =	132	/* in mm of NEC NL6448BC20-08 on ML300 */
+};
+
+struct xilinxfb_drvdata {
+
+	struct fb_info	info;		/* FB driver info record */
+
+	unsigned long	regs_phys;	/* phys. address of the control registers */
+	u32 		*regs;		/* virt. address of the control registers */
+
+	unsigned char	*fb_virt;	/* virt. address of the frame buffer */
+	dma_addr_t	fb_phys;	/* phys. address of the frame buffer */
+
+	u32		pseudo_palette[16];	/* Fake palette of 16 colors */
+};
+
+#define to_xilinxfb_drvdata(_info) \
+	container_of(_info, struct xilinxfb_drvdata, info)
+
+static int
+xilinx_fb_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned blue,
+	unsigned transp, struct fb_info *fbi)
+{
+	u32 *palette = fbi->pseudo_palette;
+
+	if (regno >= PALETTE_ENTRIES_NO)
+		return -EINVAL;
+
+	if (fbi->var.grayscale) {
+		/* Convert color to grayscale.
+		 * grayscale = 0.30*R + 0.59*G + 0.11*B */
+		red = green = blue =
+			(red * 77 + green * 151 + blue * 28 + 127) >> 8;
+	}
+
+	/* fbi->fix.visual is always FB_VISUAL_TRUECOLOR */
+
+	/* We only handle 8 bits of each color. */
+	red >>= 8;
+	green >>= 8;
+	blue >>= 8;
+	palette[regno] = (red << 16) | (green << 8) | blue;
+
+	return 0;
+}
+
+static int
+xilinx_fb_blank(int blank_mode, struct fb_info *fbi)
+{
+	struct xilinxfb_drvdata *drvdata = to_xilinxfb_drvdata(fbi);
+
+	switch (blank_mode) {
+	case VESA_NO_BLANKING:
+		/* turn on panel */
+		out_be32(drvdata->regs + REG_CTRL, REG_CTRL_DEFAULT);
+		break;
+
+	case VESA_VSYNC_SUSPEND:
+	case VESA_HSYNC_SUSPEND:
+	case VESA_POWERDOWN:
+		/* turn off panel */
+		out_be32(drvdata->regs + REG_CTRL, 0);
+	default:
+		break;
+
+	}
+	return 0; /* success */
+}
+
+static int
+xilinx_fb_pan_display(struct fb_var_screeninfo *var, struct fb_info *fbi)
+{
+	if (var->xoffset != 0 || var->yoffset != 0)
+		return -EINVAL;
+
+	return 0;
+}
+
+static struct fb_ops xilinxfb_ops =
+{
+	.owner			= THIS_MODULE,
+	.fb_setcolreg		= xilinx_fb_setcolreg,
+	.fb_blank		= xilinx_fb_blank,
+	.fb_pan_display		= xilinx_fb_pan_display,
+	.fb_fillrect		= cfb_fillrect,
+	.fb_copyarea		= cfb_copyarea,
+	.fb_imageblit		= cfb_imageblit,
+};
+
+/* === The device driver === */
+
+static int
+xilinxfb_drv_probe(struct device *dev)
+{
+	struct xilinxfb_drvdata *drvdata;
+	struct resource *regs_res;
+	int retval;
+
+	if (!dev)
+		return -EINVAL;
+
+	drvdata = kmalloc(sizeof(struct xilinxfb_drvdata), GFP_KERNEL);
+	if (!drvdata) {
+		printk(KERN_ERR "Couldn't allocate device private record\n");
+		return -ENOMEM;
+	}
+	memset((void*)drvdata, 0, sizeof(struct xilinxfb_drvdata));
+	dev_set_drvdata(dev, (void *)drvdata);
+
+	/* Map the control registers in */
+	regs_res = platform_get_resource(to_platform_device(dev),
+			IORESOURCE_IO, 0);
+	if (!regs_res || (regs_res->end - regs_res->start + 1 < 8)) {
+		printk(KERN_ERR "Couldn't get registers resource\n");
+		retval = -EFAULT;
+		goto failed1;
+	}
+
+	if (!request_mem_region(regs_res->start, 8, DRIVER_NAME)) {
+		printk(KERN_ERR "Couldn't lock memory region at 0x%08lX\n",
+			regs_res->start);
+		retval = -EBUSY;
+		goto failed1;
+	}
+
+	drvdata->regs_phys = regs_res->start;
+	drvdata->regs = (u32 *) ioremap(regs_res->start, 8);
+
+	/* Allocate the framebuffer memory */
+	drvdata->fb_virt = dma_alloc_coherent(dev, PAGE_ALIGN(FB_SIZE),
+				&drvdata->fb_phys, GFP_KERNEL);
+	if (!drvdata->fb_virt) {
+		printk(KERN_ERR "Could not allocate frame buffer memory\n");
+		retval = -ENOMEM;
+		goto failed2;
+	}
+
+	/* Clear (turn to black) the framebuffer */
+	memset((void *) drvdata->fb_virt, 0, FB_SIZE);
+
+	/* Tell the hardware where the frame buffer is */
+	out_be32(drvdata->regs + REG_FB_ADDR, drvdata->fb_phys);
+
+	/* Turn on the display */
+	out_be32(drvdata->regs + REG_CTRL, REG_CTRL_DEFAULT);
+
+	/* Fill struct fb_info */
+	drvdata->info.screen_base = drvdata->fb_virt;
+	drvdata->info.fbops = &xilinxfb_ops;
+	drvdata->info.fix = xilinx_fb_fix;
+	drvdata->info.fix.smem_start = drvdata->fb_phys;
+	drvdata->info.pseudo_palette = drvdata->pseudo_palette;
+
+	if (fb_alloc_cmap(&drvdata->info.cmap, PALETTE_ENTRIES_NO, 0) < 0) {
+		printk(KERN_ERR "Fail to allocate colormap (%d entries)\n",
+			PALETTE_ENTRIES_NO);
+		retval = -EFAULT;
+		goto failed3;
+	}
+
+	drvdata->info.flags = FBINFO_DEFAULT;
+	drvdata->info.var = xilinx_fb_var;
+
+	/* Register new frame buffer */
+	if (register_framebuffer(&drvdata->info) < 0) {
+		printk(KERN_ERR "Could not register frame buffer\n");
+		retval = -EINVAL;
+		goto failed4;
+	}
+
+	return 0;	/* success */
+
+failed4:
+	fb_dealloc_cmap(&drvdata->info.cmap);
+
+failed3:
+	dma_free_coherent(dev, PAGE_ALIGN(FB_SIZE), drvdata->fb_virt,
+		drvdata->fb_phys);
+
+	/* Turn off the display */
+	out_be32(drvdata->regs + REG_CTRL, 0);
+	iounmap(drvdata->regs);
+
+failed2:
+	release_mem_region(regs_res->start, 8);
+
+failed1:
+	kfree(drvdata);
+	dev_set_drvdata(dev, NULL);
+
+	return retval;
+}
+
+static int
+xilinxfb_drv_remove(struct device *dev)
+{
+	struct xilinxfb_drvdata *drvdata;
+
+	if (!dev)
+		return -ENODEV;
+
+	drvdata = (struct xilinxfb_drvdata *) dev_get_drvdata(dev);
+
+#if !defined(CONFIG_FRAMEBUFFER_CONSOLE) && defined(CONFIG_LOGO)
+	xilinx_fb_blank(VESA_POWERDOWN, &drvdata->info);
+#endif
+
+	unregister_framebuffer(&drvdata->info);
+
+	fb_dealloc_cmap(&drvdata->info.cmap);
+
+	dma_free_coherent(dev, PAGE_ALIGN(FB_SIZE), drvdata->fb_virt,
+		drvdata->fb_phys);
+
+	/* Turn off the display */
+	out_be32(drvdata->regs + REG_CTRL, 0);
+	iounmap(drvdata->regs);
+
+	release_mem_region(drvdata->regs_phys, 8);
+
+	kfree(drvdata);
+	dev_set_drvdata(dev, NULL);
+
+	return 0;
+}
+
+
+static struct device_driver xilinxfb_driver = {
+	.name		= DRIVER_NAME,
+	.bus		= &platform_bus_type,
+
+	.probe		= xilinxfb_drv_probe,
+	.remove		= xilinxfb_drv_remove
+};
+
+static int __init
+xilinxfb_init(void)
+{
+	/*
+	 * No kernel boot options used,
+	 * so we just need to register the driver
+	 */
+	return driver_register(&xilinxfb_driver);
+}
+
+static void __exit
+xilinxfb_cleanup(void)
+{
+	driver_unregister(&xilinxfb_driver);
+}
+
+module_init(xilinxfb_init);
+module_exit(xilinxfb_cleanup);
+
+MODULE_AUTHOR("MontaVista Software, Inc. <source@mvista.com>");
+MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
+MODULE_LICENSE("GPL");

Re: [PATCH] Xilinx framebuffer device driver

[Peter Mendham]

Andrei Konovalov wrote:
> Add support for the video controller IP block included into Xilinx 
> ML300 and
> ML403 reference designs.
>
> Signed-off-by: Andrei Konovalov <akonovalov@ru.mvista.com>
> ---
>
> This patch relies on the "Patchset to establish sanity in Xilinx 
> Virtex support" by Gran Likely to have
> the frame buffer device registered on the platform bus. Without this 
> patchset one needs to fill in
> the struct platform_device and make sure platform_device_register() is 
> called elsewhere.
>
> Reviews and comments are welcome.
Thanks, that's great.  I've built it into my latest kernel and as soon 
as I can get the hardware to work I'll test it out.  Watch this space :)
-- Peter


-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
MailScanner thanks transtec Computers for their support.

Re: [PATCH] Xilinx framebuffer device driver

[Grant Likely]

On 4/24/07, Andrei Konovalov <akonovalov@ru.mvista.com> wrote:
> Add support for the video controller IP block included into Xilinx ML300 and
> ML403 reference designs.
>
> Signed-off-by: Andrei Konovalov <akonovalov@ru.mvista.com>
> ---
>
> This patch relies on the "Patchset to establish sanity in Xilinx Virtex support" by Gran Likely to have
> the frame buffer device registered on the platform bus. Without this patchset one needs to fill in
> the struct platform_device and make sure platform_device_register() is called elsewhere.
>
> Reviews and comments are welcome.
>
> Would be nice to get this driver into mainline for the 2.6.22.

Quick comment on first perusal:  The driver uses the out_be32 macro
directly for accessing registers, which doesn't work if the FB block
is configured for DCR access (like the ML403 reference design).  There
will need to be a property in the platform device binding to determine
how to access registers.

Cheers,
g.

-- 
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH] Xilinx framebuffer device driver

[Arnd Bergmann]

PiAroKCgoKCgoC5hY3RpdmF0ZSA9oKCgoKBGQl9BQ1RJVkFURV9OT1csCj4gK6CgoKCgoKAuaGVp
Z2h0ID0goKCgoKCgOTksoKCgoKAvKiBpbiBtbSBvZiBORUMgTkw2NDQ4QkMyMC0wOCBvbiBNTDMw
MCAqLwo+ICugoKCgoKCgLndpZHRoID2goKCgoKCgoDEzMqCgoKCgLyogaW4gbW0gb2YgTkVDIE5M
NjQ0OEJDMjAtMDggb24gTUwzMDAgKi8KPiArfTsKClRoZSBzaXplIGxvb2tzIHZlcnkgc3BlY2lm
aWMgdG8gYSBwYXJ0aWN1bGFyIGRpc3BsYXkgaGFyZHdhcmUuIEkgZ3Vlc3MKd2hlbiB0aGUgZHJp
dmVyIGdldHMgY29udmVydGVkIHRvIGFuIG9mX3BsYXRmb3JtX2RyaXZlciwgdGhpcyBzaG91bGQg
Y29tZQpmcm9tIHRoZSBkZXZpY2UgdHJlZS4KClVudGlsIHRoZW4sIG1heWJlIGEgY29uZmlnIG9w
dGlvbiwgd2l0aCBhIHJlYXNvbmFibGUgZGVmYXVsdCB3b3VsZCBiZQpyaWdodC4KCj4gKwo+ICtz
dHJ1Y3QgeGlsaW54ZmJfZHJ2ZGF0YSB7Cj4gKwo+ICugoKCgoKCgc3RydWN0IGZiX2luZm+goGlu
Zm87oKCgoKCgoKCgoKAvKiBGQiBkcml2ZXIgaW5mbyByZWNvcmQgKi8KPiArCj4gK6CgoKCgoKB1
bnNpZ25lZCBsb25noKCgcmVnc19waHlzO6CgoKCgoC8qIHBoeXMuIGFkZHJlc3Mgb2YgdGhlIGNv
bnRyb2wgcmVnaXN0ZXJzICovCj4gK6CgoKCgoKB1MzIgoKCgoKCgoKCgoKCgKnJlZ3M7oKCgoKCg
oKCgoC8qIHZpcnQuIGFkZHJlc3Mgb2YgdGhlIGNvbnRyb2wgcmVnaXN0ZXJzICovCj4gKwo+ICug
oKCgoKCgdW5zaWduZWQgY2hhcqCgoCpmYl92aXJ0O6CgoKCgoKAvKiB2aXJ0LiBhZGRyZXNzIG9m
IHRoZSBmcmFtZSBidWZmZXIgKi8KClRoZSB2aXJ0dWFsIGFkZHJlc3NlcyBzaG91bGQgYmUgbWFy
a2VkIGFzIF9faW9tZW0sIHNvIHlvdSBjYW4gY2hlY2sgdGhlIGNvcnJlY3QKdXNhZ2Ugd2l0aCBz
cGFyc2UuCgpOb3Qgc3VyZSBhYm91dCByZWdzX3BoeXMuIENhbiB0aGlzIGJlIGJleW9uZCB0aGUg
MzIgYml0IGxpbWl0IG9uIGFueSBtYWNoaW5lPwoKPiArCj4gK6CgoKCgoKBkcnZkYXRhID0ga21h
bGxvYyhzaXplb2Yoc3RydWN0IHhpbGlueGZiX2RydmRhdGEpLCBHRlBfS0VSTkVMKTsKPiAroKCg
oKCgoGlmICghZHJ2ZGF0YSkgewo+ICugoKCgoKCgoKCgoKCgoKBwcmludGsoS0VSTl9FUlIgIkNv
dWxkbid0IGFsbG9jYXRlIGRldmljZSBwcml2YXRlIHJlY29yZFxuIik7Cj4gK6CgoKCgoKCgoKCg
oKCgoHJldHVybiAtRU5PTUVNOwo+ICugoKCgoKCgfQo+ICugoKCgoKCgbWVtc2V0KCh2b2lkKilk
cnZkYXRhLCAwLCBzaXplb2Yoc3RydWN0IHhpbGlueGZiX2RydmRhdGEpKTsKClVzZQoJZHJ2ZGF0
YSA9IGt6YWxsb2Moc2l6ZW9mICgqZHJ2ZGF0YSksIEdGUF9LRVJORUwpOwoKPiAroKCgoKCgoGRy
dmRhdGEtPnJlZ3NfcGh5cyA9IHJlZ3NfcmVzLT5zdGFydDsKPiAroKCgoKCgoGRydmRhdGEtPnJl
Z3MgPSAodTMyICopIGlvcmVtYXAocmVnc19yZXMtPnN0YXJ0LCA4KTsKCnUzMiBfX2lvbWVtKgoK
CglBcm5kIDw+PAo=

Re: [PATCH] Xilinx framebuffer device driver

[Andrei Konovalov]

Grant Likely wrote:
> On 4/24/07, Andrei Konovalov <akonovalov@ru.mvista.com> wrote:
>> Add support for the video controller IP block included into Xilinx 
>> ML300 and
>> ML403 reference designs.
>>
>> Signed-off-by: Andrei Konovalov <akonovalov@ru.mvista.com>
>> ---
>>
>> This patch relies on the "Patchset to establish sanity in Xilinx 
>> Virtex support" by Gran Likely to have
>> the frame buffer device registered on the platform bus. Without this 
>> patchset one needs to fill in
>> the struct platform_device and make sure platform_device_register() is 
>> called elsewhere.
>>
>> Reviews and comments are welcome.
>>
>> Would be nice to get this driver into mainline for the 2.6.22.
> 
> Quick comment on first perusal:  The driver uses the out_be32 macro
> directly for accessing registers, which doesn't work if the FB block
> is configured for DCR access (like the ML403 reference design).

Yes, that's true.
But (at least) the EDK 8.1 reference design for ML403 has also opb2dcr bridge.
That's why I've tested the patch without problems on ML403 (in addition
to ML300).

I'll add DCR access, but not sure if I need a separate bitstream to
test DCR access (could I access the DCR registers directly in presence
of the bridge or not).

> There will need to be a property in the platform device binding to determine
> how to access registers.

OK.
The only problem is that there is no indication in the xparameters.h of how the
registers should be accessed (via DCR or opb).
Let's suppose XPAR_TFT_0_USE_DCR would be added by EDK (like XPAR_XINTC_USE_DCR
is used for the interrupt controller).

> Cheers,
> g.
> 

Thanks,
Andrei

Re: [PATCH] Xilinx framebuffer device driver

[Andrei Konovalov]

Hi Arnd,

Arnd Bergmann wrote:
>> +       .activate =     FB_ACTIVATE_NOW,
>> +       .height =       99,     /* in mm of NEC NL6448BC20-08 on ML300 */
>> +       .width =        132     /* in mm of NEC NL6448BC20-08 on ML300 */
>> +};
> 
> The size looks very specific to a particular display hardware. I guess
> when the driver gets converted to an of_platform_driver, this should come
> from the device tree.
> 
> Until then, maybe a config option, with a reasonable default would be
> right.

I could add these to platform data.
Together with the "use DCR access" option.
Say,

--------------------------------------------------------
struct xilinxfb_platform_data {
	u32 use_dcr;
	u32 screen_height_mm;
	u32 screen_width_mm;
};

static struct xilinxfb_platform_data xilinxfb_pdata = {
#if defined(XPAR_TFT_0_USE_DCR) && (XPAR_TFT_0_USE_DCR != 0)
	.use_dcr = 1;
#else
	.use_dcr = 0;
#endif
	.screen_height_mm = CONFIG_FB_XILINX_SCR_HEIGHT;
	.screen_width_mm = CONFIG_FB_XILINX_SCR_WIDTH;
};

...

/* ML300/403 reference design framebuffer */
#if defined(XPAR_TFT_0_BASEADDR)
	{
		.name		= "xilinxfb",
		.id		= 0,
		.dev.platform_data = &xilinxfb_pdata,
		.num_resources	= 1,
		.resource = (struct resource[]) {
			{
				.start	= XPAR_TFT_0_BASEADDR,
				.end	= XPAR_TFT_0_BASEADDR+7,
				.flags	= IORESOURCE_IO,
			},
		},
	},
#endif
--------------------------------------------------------

Does it look OK?

>> +
>> +struct xilinxfb_drvdata {
>> +
>> +       struct fb_info  info;           /* FB driver info record */
>> +
>> +       unsigned long   regs_phys;      /* phys. address of the control registers */
>> +       u32             *regs;          /* virt. address of the control registers */
>> +
>> +       unsigned char   *fb_virt;       /* virt. address of the frame buffer */
> 
> The virtual addresses should be marked as __iomem, so you can check the correct
> usage with sparse.

OK. Thanks.

> Not sure about regs_phys. Can this be beyond the 32 bit limit on any machine?

The driver is for Xilinx FPGAs only (the video controller is the "soft core" IP
block).
My personal feeling is that someday in the future Virtex FPGAs would carry ppc440 "hard" cores.
But all current FPGAs have only ppc405 "hard core" CPU blocks inside (0, 1 or several per chip).
Another option is to use Microblaze "soft core" CPU which is 32-bit too.
I doubt someone would connect a Virtex FPGA to a "standalone" CPU to use
the FPGA as video controller (this is not impossible though).

I.e. for the moment
   u32	regs_phys;
should be OK.

Other options could be
   dma_addr_t	regs_phys;
or
   resource_size_t	regs_phys;
which are u32 or u64 depending on the architecture.
But both don't sound like intended specifically for phys. addresses
(not necessary DMA-able).

What would you recommend?

>> +
>> +       drvdata = kmalloc(sizeof(struct xilinxfb_drvdata), GFP_KERNEL);
>> +       if (!drvdata) {
>> +               printk(KERN_ERR "Couldn't allocate device private record\n");
>> +               return -ENOMEM;
>> +       }
>> +       memset((void*)drvdata, 0, sizeof(struct xilinxfb_drvdata));
> 
> Use
> 	drvdata = kzalloc(sizeof (*drvdata), GFP_KERNEL);

Oops.. Sure I will.

>> +       drvdata->regs_phys = regs_res->start;
>> +       drvdata->regs = (u32 *) ioremap(regs_res->start, 8);
> 
> u32 __iomem*

OK

> 	Arnd <><

Thanks,
Andrei

Re: [PATCH] Xilinx framebuffer device driver

[Andrei Konovalov]

Grant,

Andrei Konovalov wrote:
> I could add these to platform data.
> Together with the "use DCR access" option.
> Say,
> 
> --------------------------------------------------------
> struct xilinxfb_platform_data {
>     u32 use_dcr;
>     u32 screen_height_mm;
>     u32 screen_width_mm;
> };
> 
> static struct xilinxfb_platform_data xilinxfb_pdata = {
> #if defined(XPAR_TFT_0_USE_DCR) && (XPAR_TFT_0_USE_DCR != 0)
>     .use_dcr = 1;
> #else
>     .use_dcr = 0;
> #endif

IOW I am trying to avoid conditional compilation like:

#if defined(XPAR_TFT_0_USE_DCR) && (XPAR_TFT_0_USE_DCR != 0)
#define xilinxfb_out_be32(addr, mask)     mtdcr((addr), (mask))
#else
#define xilinxfb_out_be32(addr, mask)     out_be32((addr), (mask))
#endif

- as this would make the driver to include xparameters.h which would
be an issue when moving to the OF device tree.

Or "use DCR" could be a Kconfig option for the driver.


Thanks,
Andrei

Re: [PATCH] Xilinx framebuffer device driver

[Grant Likely]

On 4/25/07, Andrei Konovalov <akonovalov@ru.mvista.com> wrote:
> Grant,
>
> > static struct xilinxfb_platform_data xilinxfb_pdata = {
> > #if defined(XPAR_TFT_0_USE_DCR) && (XPAR_TFT_0_USE_DCR != 0)
> >     .use_dcr = 1;
> > #else
> >     .use_dcr = 0;
> > #endif
>
> IOW I am trying to avoid conditional compilation like:

I agree 100%.  All the device options should be specified dynamically.
 Heck, it should be possible to build a single kernel and boot it on
just about any design.

>
> #if defined(XPAR_TFT_0_USE_DCR) && (XPAR_TFT_0_USE_DCR != 0)
> #define xilinxfb_out_be32(addr, mask)     mtdcr((addr), (mask))
> #else
> #define xilinxfb_out_be32(addr, mask)     out_be32((addr), (mask))
> #endif
>
> - as this would make the driver to include xparameters.h which would
> be an issue when moving to the OF device tree.

Which is a bad thing.  I believe that the Xilinx folks would like to
use the device tree for the microblaze target also; but there is a
fair bit of work that need to be done before that is feasable.

>
> Or "use DCR" could be a Kconfig option for the driver.

My preference would be for it to be a runtime thing.

-- 
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195