2006-10-13 00:32:24

by Miguel Ojeda

[permalink] [raw]
Subject: [PATCH 2.6.19-rc1 full] drivers: add LCD support

Andrew, here it is the complete patch again as you requested.

Paulo, I got the algorithm much faster, thanks you very much.
(<2% CPU at 20 Hz on P4 3Ghz :).
---
miguelojeda-2.6.19-rc1-add-LCD-support.patch

Adds support for auxiliary displays, the ks0108 LCD controller,
the cfag12864b LCD and adds a framebuffer device: cfag12864bfb.

Brief
-----

- Adds a "auxdisplay/" folder in "drivers/" for auxiliary display drivers.
- Adds support for the ks0108 LCD Controller as a device driver.
(uses parport interface)
- Adds support for the cfag12864b LCD as a device driver.
(uses ks0108 LCD Controller driver)
- Adds a framebuffer device called cfag12864bfb.
(uses cfag12864b LCD driver)
- Adds the usual Documentation, includes, Makefiles, Kconfigs, MAINTAINERS, CREDITS...
- Miguel Ojeda will maintain all the stuff above.

Patched files Index
-------------------

CREDITS | 10
Documentation/auxdisplay/cfag12864b | 98 +++++++
Documentation/auxdisplay/ks0108 | 59 ++++
MAINTAINERS | 24 +
drivers/Kconfig | 2
drivers/Makefile | 1
drivers/auxdisplay/Kconfig | 112 ++++++++
drivers/auxdisplay/Makefile | 6
drivers/auxdisplay/cfag12864b.c | 328 ++++++++++++++++++++++++++
drivers/auxdisplay/cfag12864bfb.c | 165 +++++++++++++
drivers/auxdisplay/ks0108.c | 167 +++++++++++++
include/linux/cfag12864b.h | 41 +++
include/linux/ks0108.h | 36 ++
13 files changed, 1049 insertions(+)

miguelojeda-2.6.19-rc1-add-LCD-support.patch
Signed-off-by: Miguel Ojeda Sandonis <[email protected]>
---
diff -uprN -X dontdiff linux-2.6.19-rc1-vanilla/CREDITS linux/CREDITS
--- linux-2.6.19-rc1-vanilla/CREDITS 2006-10-13 02:10:16.000000000 +0000
+++ linux/CREDITS 2006-10-12 12:26:23.000000000 +0000
@@ -2562,6 +2562,16 @@ S: Subiaco, 6008
S: Perth, Western Australia
S: Australia

+N: Miguel Ojeda Sandonis
+E: [email protected]
+D: Author: Auxiliary LCD Controller driver (ks0108)
+D: Author: Auxiliary LCD driver (cfag12864b)
+D: Author: Auxiliary LCD framebuffer driver (cfag12864bfb)
+D: Maintainer: Auxiliary display drivers tree (drivers/auxdisplay/*)
+S: C/ Mieses 20, 9-B
+S: Valladolid 47009
+S: Spain
+
N: Greg Page
E: [email protected]
D: IPX development and support
diff -uprN -X dontdiff linux-2.6.19-rc1-vanilla/Documentation/auxdisplay/cfag12864b linux/Documentation/auxdisplay/cfag12864b
--- linux-2.6.19-rc1-vanilla/Documentation/auxdisplay/cfag12864b 1970-01-01 00:00:00.000000000 +0000
+++ linux/Documentation/auxdisplay/cfag12864b 2006-10-13 01:41:22.000000000 +0000
@@ -0,0 +1,98 @@
+ ===================================
+ cfag12864b LCD Driver Documentation
+ ===================================
+
+License: GPLv2
+Author & Maintainer: Miguel Ojeda Sandonis <[email protected]>
+Date: 2006-10-11
+
+
+
+--------
+0. INDEX
+--------
+
+ 1. DRIVER INFORMATION
+ 2. DEVICE INFORMATION
+ 3. WIRING
+ 4. USER-SPACE PROGRAMMING
+
+
+---------------------
+1. DRIVER INFORMATION
+---------------------
+
+This driver support one cfag12864b display at time.
+
+
+---------------------
+2. DEVICE INFORMATION
+---------------------
+
+Manufacturer: Crystalfontz
+Device Name: Crystalfontz 12864b LCD Series
+Device Code: cfag12864b
+Webpage: http://www.crystalfontz.com
+Device Webpage: http://www.crystalfontz.com/products/12864b/
+Type: LCD (Liquid Crystal Display)
+Width: 128
+Height: 64
+Colors: 2 (B/N)
+Controller: ks0108
+Controllers: 2
+Pages: 8 each controller
+Addresses: 64 each page
+
+
+
+---------
+3. WIRING
+---------
+
+The cfag12864b LCD Series don't have official wiring.
+
+The common wiring is done to the parallel port:
+
+http://www.skippari.net/lcd/sekalaista/crystalfontz_cfag12864B-TMI-V.png
+
+You can get help at Crystalfontz and LCDInfo forums.
+
+
+
+------------------------
+4. USERSPACE PROGRAMMING
+------------------------
+
+The cfag12864bfb describes a framebuffer driver (/dev/fbX).
+
+It has a size of 128 * 64 / 8 = 1024 bytes = 1 kB.
+Each bit represents one pixel. If the bit is high, the pixel will
+turn on. If the pixel is low, the pixel will turn off.
+
+You can mmap the framebuffer as usual.
+
+You can use a copy of this header in your userspace programs.
+
+---8<---
+/*
+ * Filename: cfag12864b.h
+ * Description: cfag12864b LCD Display Driver Header for user-space apps
+ *
+ * Author: Miguel Ojeda Sandonis <[email protected]>
+ * Date: 2006-10-11
+ */
+
+#ifndef _CFAG12864B_H_
+#define _CFAG12864B_H_
+
+#define CFAG12864B_WIDTH (128)
+#define CFAG12864B_HEIGHT (64)
+#define CFAG12864B_SIZE ((CFAG12864B_CONTROLLERS) * \
+ (CFAG12864B_PAGES) * \
+ (CFAG12864B_ADDRESSES))
+
+#endif // _CFAG12864B_H_
+---8<---
+
+
+EOF
diff -uprN -X dontdiff linux-2.6.19-rc1-vanilla/Documentation/auxdisplay/ks0108 linux/Documentation/auxdisplay/ks0108
--- linux-2.6.19-rc1-vanilla/Documentation/auxdisplay/ks0108 1970-01-01 00:00:00.000000000 +0000
+++ linux/Documentation/auxdisplay/ks0108 2006-10-05 13:46:44.000000000 +0000
@@ -0,0 +1,59 @@
+ ==========================================
+ ks0108 LCD Controller Driver Documentation
+ ==========================================
+
+License: GPLv2
+Author & Maintainer: Miguel Ojeda Sandonis <[email protected]>
+Date: 2006-10-04
+
+
+
+--------
+0. INDEX
+--------
+
+ 1. DRIVER INFORMATION
+ 2. DEVICE INFORMATION
+ 3. WIRING
+
+
+---------------------
+1. DRIVER INFORMATION
+---------------------
+
+This driver support the ks0108 LCD controller.
+
+
+---------------------
+2. DEVICE INFORMATION
+---------------------
+
+Manufacturer: Samsung
+Device Name: KS0108 LCD Controller
+Device Code: ks0108
+Webpage: -
+Device Webpage: -
+Type: LCD Controller (Liquid Crystal Display Controller)
+Width: 64
+Height: 64
+Colors: 2 (B/N)
+Pages: 8
+Addresses: 64 each page
+
+
+
+---------
+3. WIRING
+---------
+
+The driver supports data parallel port wiring.
+
+If you aren't creating a LCD related hardware, you should check
+your LCD specific wiring information in the same folder, not this one.
+
+
+Wiring example of a cfag12864b LCD which has two ks0108 controllers:
+
+http://www.skippari.net/lcd/sekalaista/crystalfontz_cfag12864B-TMI-V.png
+
+EOF
diff -uprN -X dontdiff linux-2.6.19-rc1-vanilla/drivers/auxdisplay/cfag12864b.c linux/drivers/auxdisplay/cfag12864b.c
--- linux-2.6.19-rc1-vanilla/drivers/auxdisplay/cfag12864b.c 1970-01-01 00:00:00.000000000 +0000
+++ linux/drivers/auxdisplay/cfag12864b.c 2006-10-13 02:18:59.000000000 +0000
@@ -0,0 +1,328 @@
+/*
+ * Filename: cfag12864b.c
+ * Version: 0.1.0
+ * Description: cfag12864b LCD driver
+ * License: GPLv2
+ * Depends: ks0108
+ *
+ * Author: Copyright (C) Miguel Ojeda Sandonis <[email protected]>
+ * Date: 2006-10-13
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/fs.h>
+#include <linux/cdev.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/jiffies.h>
+#include <linux/workqueue.h>
+#include <linux/vmalloc.h>
+#include <linux/ks0108.h>
+#include <linux/cfag12864b.h>
+#include <linux/uaccess.h>
+
+#define CFAG12864B_NAME "cfag12864b"
+
+/*
+ * Module Parameters
+ */
+
+static unsigned int cfag12864b_rate = CONFIG_CFAG12864B_RATE;
+module_param(cfag12864b_rate, uint, S_IRUGO);
+MODULE_PARM_DESC(cfag12864b_rate,
+ "Refresh rate (hertzs)");
+
+/*
+ * cfag12864b Commands
+ *
+ * E = Enable signal
+ * Everytime E switch from low to high,
+ * cfag12864b/ks0108 reads the command/data.
+ *
+ * CS1 = First ks0108controller.
+ * If high, the first ks0108 receives commands/data.
+ *
+ * CS2 = Second ks0108 controller
+ * If high, the second ks0108 receives commands/data.
+ *
+ * DI = Data/Instruction
+ * If low, cfag12864b will expect commands.
+ * If high, cfag12864b will expect data.
+ *
+ */
+
+#define bit(n) (((unsigned char)1)<<(n))
+
+#define CFAG12864B_BIT_E (0)
+#define CFAG12864B_BIT_CS1 (2)
+#define CFAG12864B_BIT_CS2 (1)
+#define CFAG12864B_BIT_DI (3)
+
+static unsigned char cfag12864b_state;
+
+static void cfag12864b_set(void)
+{
+ ks0108_writecontrol(cfag12864b_state);
+}
+
+static void cfag12864b_setbit(unsigned char state, unsigned char n)
+{
+ if (state)
+ cfag12864b_state |= bit(n);
+ else
+ cfag12864b_state &= ~bit(n);
+}
+
+static void cfag12864b_e(unsigned char state)
+{
+ cfag12864b_setbit(state, CFAG12864B_BIT_E);
+ cfag12864b_set();
+}
+
+static void cfag12864b_cs1(unsigned char state)
+{
+ cfag12864b_setbit(state, CFAG12864B_BIT_CS1);
+}
+
+static void cfag12864b_cs2(unsigned char state)
+{
+ cfag12864b_setbit(state, CFAG12864B_BIT_CS2);
+}
+
+static void cfag12864b_di(unsigned char state)
+{
+ cfag12864b_setbit(state, CFAG12864B_BIT_DI);
+}
+
+static void cfag12864b_setcontrollers(unsigned char first,
+ unsigned char second)
+{
+ if (first)
+ cfag12864b_cs1(0);
+ else
+ cfag12864b_cs1(1);
+
+ if (second)
+ cfag12864b_cs2(0);
+ else
+ cfag12864b_cs2(1);
+}
+
+static void cfag12864b_controller(unsigned char which)
+{
+ if (which == 0)
+ cfag12864b_setcontrollers(1, 0);
+ else if (which == 1)
+ cfag12864b_setcontrollers(0, 1);
+}
+
+static void cfag12864b_displaystate(unsigned char state)
+{
+ cfag12864b_di(0);
+ cfag12864b_e(1);
+ ks0108_displaystate(state);
+ cfag12864b_e(0);
+}
+
+static void cfag12864b_address(unsigned char address)
+{
+ cfag12864b_di(0);
+ cfag12864b_e(1);
+ ks0108_address(address);
+ cfag12864b_e(0);
+}
+
+static void cfag12864b_page(unsigned char page)
+{
+ cfag12864b_di(0);
+ cfag12864b_e(1);
+ ks0108_page(page);
+ cfag12864b_e(0);
+}
+
+static void cfag12864b_startline(unsigned char startline)
+{
+ cfag12864b_di(0);
+ cfag12864b_e(1);
+ ks0108_startline(startline);
+ cfag12864b_e(0);
+}
+
+static void cfag12864b_writebyte(unsigned char byte)
+{
+ cfag12864b_di(1);
+ cfag12864b_e(1);
+ ks0108_writedata(byte);
+ cfag12864b_e(0);
+}
+
+static void cfag12864b_nop(void)
+{
+ cfag12864b_startline(0);
+}
+
+/*
+ * cfag12864b Internal Commands
+ */
+
+unsigned char *cfag12864b_cache;
+unsigned char *cfag12864b_buffer;
+EXPORT_SYMBOL_GPL(cfag12864b_buffer);
+
+static void cfag12864b_on(void)
+{
+ cfag12864b_setcontrollers(1, 1);
+ cfag12864b_displaystate(1);
+}
+
+static void cfag12864b_off(void)
+{
+ cfag12864b_setcontrollers(1, 1);
+ cfag12864b_displaystate(0);
+}
+
+static void cfag12864b_clear(void)
+{
+ unsigned char i, j;
+
+ cfag12864b_setcontrollers(1, 1);
+ for (i = 0; i < CFAG12864B_PAGES; i++) {
+ cfag12864b_page(i);
+ cfag12864b_address(0);
+ for (j = 0; j < CFAG12864B_ADDRESSES; j++)
+ cfag12864b_writebyte(0);
+ }
+}
+
+/*
+ * Update work
+ */
+
+static void cfag12864b_update(void *arg);
+static struct workqueue_struct *cfag12864b_workqueue;
+DECLARE_WORK(cfag12864b_work, cfag12864b_update, NULL);
+
+static unsigned char cfag12864b_updating;
+
+static void cfag12864b_update(void *arg)
+{
+ unsigned char c;
+ unsigned short i, j, k, b;
+
+ if(memcmp(cfag12864b_cache, cfag12864b_buffer, CFAG12864B_SIZE)) {
+ for (i = 0; i < CFAG12864B_CONTROLLERS; i++) {
+ cfag12864b_controller(i);
+ cfag12864b_nop();
+ for (j = 0; j < CFAG12864B_PAGES; j++) {
+ cfag12864b_page(j);
+ cfag12864b_nop();
+ cfag12864b_address(0);
+ cfag12864b_nop();
+ for (k = 0; k < CFAG12864B_ADDRESSES; k++) {
+ for (c = 0, b = 0; b < 8; b++)
+ if (cfag12864b_buffer
+ [i * CFAG12864B_ADDRESSES / 8
+ + k / 8 + (j * 8 + b) *
+ CFAG12864B_WIDTH / 8]
+ & bit(k % 8))
+ c |= bit(b);
+ cfag12864b_writebyte(c);
+ }
+ }
+ }
+
+ memcpy(cfag12864b_cache, cfag12864b_buffer, CFAG12864B_SIZE);
+ }
+
+ if(cfag12864b_updating)
+ queue_delayed_work(cfag12864b_workqueue, &cfag12864b_work,
+ HZ / cfag12864b_rate);
+}
+
+/*
+ * Module Init & Exit
+ */
+
+static int __init cfag12864b_init(void)
+{
+ int ret = -EINVAL;
+
+ cfag12864b_buffer = vmalloc(sizeof(unsigned char) *
+ CFAG12864B_SIZE);
+ if (cfag12864b_buffer == NULL) {
+ printk(KERN_ERR CFAG12864B_NAME ": ERROR: "
+ "can't v-alloc buffer (%i bytes)\n",
+ CFAG12864B_SIZE);
+ ret = -ENOMEM;
+ goto none;
+ }
+
+ cfag12864b_cache = kmalloc(sizeof(unsigned char) *
+ CFAG12864B_SIZE, GFP_KERNEL);
+ if (cfag12864b_buffer == NULL) {
+ printk(KERN_ERR CFAG12864B_NAME ": ERROR: "
+ "can't alloc cache buffer (%i bytes)\n",
+ CFAG12864B_SIZE);
+ ret = -ENOMEM;
+ goto bufferalloced;
+ }
+
+ memset(cfag12864b_buffer, 0, CFAG12864B_SIZE);
+
+ cfag12864b_clear();
+ cfag12864b_on();
+
+ cfag12864b_workqueue = create_singlethread_workqueue(CFAG12864B_NAME);
+ if(cfag12864b_workqueue == NULL)
+ goto cachealloced;
+
+ cfag12864b_updating = 1;
+ cfag12864b_update(NULL);
+
+ return 0;
+
+cachealloced:
+ kfree(cfag12864b_cache);
+
+bufferalloced:
+ vfree(cfag12864b_buffer);
+
+none:
+ return ret;
+}
+
+static void __exit cfag12864b_exit(void)
+{
+ cfag12864b_updating = 0;
+ mdelay((1000 / cfag12864b_rate) * 2);
+ destroy_workqueue(cfag12864b_workqueue);
+
+ cfag12864b_off();
+
+ kfree(cfag12864b_cache);
+ vfree(cfag12864b_buffer);
+}
+
+module_init(cfag12864b_init);
+module_exit(cfag12864b_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Miguel Ojeda Sandonis <[email protected]>");
+MODULE_DESCRIPTION("cfag12864b LCD driver");
diff -uprN -X dontdiff linux-2.6.19-rc1-vanilla/drivers/auxdisplay/cfag12864bfb.c linux/drivers/auxdisplay/cfag12864bfb.c
--- linux-2.6.19-rc1-vanilla/drivers/auxdisplay/cfag12864bfb.c 1970-01-01 00:00:00.000000000 +0000
+++ linux/drivers/auxdisplay/cfag12864bfb.c 2006-10-13 02:19:04.000000000 +0000
@@ -0,0 +1,165 @@
+/*
+ * Filename: cfag12864bfb.c
+ * Version: 0.1.0
+ * Description: cfag12864b LCD framebuffer driver
+ * License: GPLv2
+ * Depends: cfag12864b
+ *
+ * Author: Copyright (C) Miguel Ojeda Sandonis <[email protected]>
+ * Date: 2006-10-13
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/mm.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/fb.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/cfag12864b.h>
+#include <asm/uaccess.h>
+
+#define CFAG12864BFB_NAME "cfag12864bfb"
+
+static struct fb_fix_screeninfo cfag12864bfb_fix __initdata = {
+ .id = "cfag12864b",
+ .type = FB_TYPE_PACKED_PIXELS,
+ .visual = FB_VISUAL_MONO10,
+ .xpanstep = 0,
+ .ypanstep = 0,
+ .ywrapstep = 0,
+ .line_length = CFAG12864B_WIDTH / 8,
+ .accel = FB_ACCEL_NONE,
+};
+
+static struct fb_var_screeninfo cfag12864bfb_var __initdata = {
+ .xres = CFAG12864B_WIDTH,
+ .yres = CFAG12864B_HEIGHT,
+ .xres_virtual = CFAG12864B_WIDTH,
+ .yres_virtual = CFAG12864B_HEIGHT,
+ .bits_per_pixel = 1,
+ .red = { 0, 1, 0 },
+ .green = { 0, 1, 0 },
+ .blue = { 0, 1, 0 },
+ .left_margin = 0,
+ .right_margin = 0,
+ .upper_margin = 0,
+ .lower_margin = 0,
+ .vmode = FB_VMODE_NONINTERLACED,
+};
+
+static struct fb_ops cfag12864bfb_ops = {
+ .owner = THIS_MODULE,
+ .fb_fillrect = cfb_fillrect,
+ .fb_copyarea = cfb_copyarea,
+ .fb_imageblit = cfb_imageblit,
+};
+
+static int __init cfag12864bfb_probe(struct platform_device *device)
+{
+ int ret = -EINVAL;
+ struct fb_info *info = framebuffer_alloc(0, &device->dev);
+
+ if (!info)
+ goto none;
+
+ info->screen_base = (char __iomem *)cfag12864b_buffer;
+ info->screen_size = CFAG12864B_SIZE;
+ info->fbops = &cfag12864bfb_ops;
+ info->fix = cfag12864bfb_fix;
+ info->var = cfag12864bfb_var;
+ info->pseudo_palette = NULL;
+ info->par = NULL;
+ info->flags = FBINFO_FLAG_DEFAULT;
+
+ if (register_framebuffer(info) < 0)
+ goto fballoced;
+
+ platform_set_drvdata(device, info);
+
+ printk(KERN_INFO "fb%d: %s frame buffer device\n", info->node,
+ info->fix.id);
+
+ return 0;
+
+fballoced:
+ framebuffer_release(info);
+
+none:
+ return ret;
+}
+
+static int cfag12864bfb_remove(struct platform_device *device)
+{
+ struct fb_info *info = platform_get_drvdata(device);
+
+ if (info) {
+ unregister_framebuffer(info);
+ framebuffer_release(info);
+ }
+
+ return 0;
+}
+
+static struct platform_driver cfag12864bfb_driver = {
+ .probe = cfag12864bfb_probe,
+ .remove = cfag12864bfb_remove,
+ .driver = {
+ .name = CFAG12864BFB_NAME,
+ },
+};
+
+static struct platform_device *cfag12864bfb_device;
+
+static int __init cfag12864bfb_init(void)
+{
+ int ret = platform_driver_register(&cfag12864bfb_driver);
+
+ if (!ret) {
+ cfag12864bfb_device =
+ platform_device_alloc(CFAG12864BFB_NAME, 0);
+
+ if (cfag12864bfb_device)
+ ret = platform_device_add(cfag12864bfb_device);
+ else
+ ret = -ENOMEM;
+
+ if (ret) {
+ platform_device_put(cfag12864bfb_device);
+ platform_driver_unregister(&cfag12864bfb_driver);
+ }
+ }
+
+ return ret;
+}
+
+static void __exit cfag12864bfb_exit(void)
+{
+ platform_device_unregister(cfag12864bfb_device);
+ platform_driver_unregister(&cfag12864bfb_driver);
+}
+
+module_init(cfag12864bfb_init);
+module_exit(cfag12864bfb_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Miguel Ojeda Sandonis <[email protected]>");
+MODULE_DESCRIPTION("cfag12864b LCD framebuffer driver");
diff -uprN -X dontdiff linux-2.6.19-rc1-vanilla/drivers/auxdisplay/Kconfig linux/drivers/auxdisplay/Kconfig
--- linux-2.6.19-rc1-vanilla/drivers/auxdisplay/Kconfig 1970-01-01 00:00:00.000000000 +0000
+++ linux/drivers/auxdisplay/Kconfig 2006-10-13 02:17:19.000000000 +0000
@@ -0,0 +1,112 @@
+#
+# For a description of the syntax of this configuration file,
+# see Documentation/kbuild/kconfig-language.txt.
+#
+# Auxiliary display drivers configuration.
+#
+
+menu "Auxiliary Display support"
+
+config KS0108
+ tristate "KS0108 LCD Controller"
+ select PARPORT
+ select PARPORT_PC
+ default n
+ ---help---
+ If you have a LCD controlled by one or more KS0108
+ controllers, say Y. You will need also another more specific
+ driver for your LCD.
+
+ Depends on Parallel Port support. If you say Y at
+ parport, you will be able to compile this as a module (M)
+ and built-in as well (Y).
+
+ To compile this as a module, choose M here:
+ the module will be called ks0108.
+
+ If unsure, say N.
+
+config KS0108_PORT
+ hex "Parallel port where the LCD is connected"
+ depends on KS0108
+ default 0x378
+ ---help---
+ The address of the parallel port where the LCD is connected.
+
+ The first standard parallel port address is 0x378.
+ The second standard parallel port address is 0x278.
+ The third standard parallel port address is 0x3BC.
+
+ You can specify a different address if you need.
+
+ If you don't know what I'm talking about, load the parport module,
+ and execute "dmesg" or "cat /proc/ioports". You can see there how
+ many parallel ports are present and which address each one has.
+
+ Usually you only need to use 0x378.
+
+ If you compile this as a module, you can still override this
+ using the module parameters.
+
+config KS0108_DELAY
+ int "Delay between each control writing (microseconds)"
+ depends on KS0108
+ default "2"
+ ---help---
+ Amount of time the ks0108 should wait between each control write
+ to the parallel port.
+
+ If your driver seems to miss random writings, increment this.
+
+ If you don't know what I'm talking about, ignore it.
+
+ If you compile this as a module, you can still override this
+ value using the module parameters.
+
+config CFAG12864B
+ tristate "CFAG12864B LCD"
+ depends on KS0108
+ default n
+ ---help---
+ If you have a Crystalfontz 128x64 2-color LCD, cfag12864b Series,
+ say Y. You also need the ks0108 LCD Controller driver.
+
+ For help about how to wire your LCD to the parallel port,
+ check this image:
+
+ http://www.skippari.net/lcd/sekalaista/crystalfontz_cfag12864B-TMI-V.png
+
+ Also, you can find help in Crystalfontz and LCDStudio forums.
+ Check Documentation/lcddisplay/cfag12864b for more information.
+
+ The LCD framebuffer driver can be attached to a console.
+ It will work fine. However, you can't attach it to the fbdev driver
+ of the xorg server.
+
+ To compile this as a module, choose M here:
+ the modules will be called cfag12864b and cfag12864bfb.
+
+ If unsure, say N.
+
+config CFAG12864B_RATE
+ int "Refresh rate (hertzs)"
+ depends on CFAG12864B
+ default "20"
+ ---help---
+ Refresh rate of the LCD.
+
+ As the LCD is not memory mapped, the driver has to make the work by
+ software. This means you should be careful setting this value higher.
+ If your CPUs are really slow or you feel the system is slowed down,
+ decrease the value.
+
+ Be careful modifying this value to a very high value:
+ You can freeze the computer, or the LCD maybe can't draw as fast as you
+ are requesting.
+
+ If you don't know what I'm talking about, ignore it.
+
+ If you compile this as a module, you can still override this
+ value using the module parameters.
+endmenu
+
diff -uprN -X dontdiff linux-2.6.19-rc1-vanilla/drivers/auxdisplay/ks0108.c linux/drivers/auxdisplay/ks0108.c
--- linux-2.6.19-rc1-vanilla/drivers/auxdisplay/ks0108.c 1970-01-01 00:00:00.000000000 +0000
+++ linux/drivers/auxdisplay/ks0108.c 2006-10-11 23:20:33.000000000 +0000
@@ -0,0 +1,167 @@
+/*
+ * Filename: ks0108.c
+ * Version: 0.1.0
+ * Description: ks0108 LCD Controller driver
+ * License: GPLv2
+ * Depends: parport
+ *
+ * Author: Copyright (C) Miguel Ojeda Sandonis <[email protected]>
+ * Date: 2006-10-04
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/fs.h>
+#include <linux/delay.h>
+#include <linux/parport.h>
+#include <linux/ks0108.h>
+#include <asm/io.h>
+#include <asm/uaccess.h>
+
+#define KS0108_NAME "ks0108"
+
+/*
+ * Module Parameters
+ */
+
+static unsigned int ks0108_port = CONFIG_KS0108_PORT;
+module_param(ks0108_port, uint, S_IRUGO);
+MODULE_PARM_DESC(ks0108_port, "Parallel port where the LCD is connected");
+
+static unsigned int ks0108_delay = CONFIG_KS0108_DELAY;
+module_param(ks0108_delay, uint, S_IRUGO);
+MODULE_PARM_DESC(ks0108_delay, "Delay between each control writing (microseconds)");
+
+/*
+ * Device
+ */
+
+static struct parport *ks0108_parport;
+static struct pardevice *ks0108_pardevice;
+
+/*
+ * ks0108 Exported cmds (don't lock)
+ *
+ * You _should_ lock in the top driver: This functions _should not_
+ * get race conditions in any way. Locking for each byte here would be
+ * so slow and useless.
+ *
+ * There are not bit definitions because they are not flags,
+ * just arbitrary combinations defined by the documentation for each
+ * function in the ks0108 LCD controller. If you want to know what means
+ * a specific combination, look at the function's name.
+ *
+ * The ks0108_writecontrol bits need to be reverted ^(0,1,3) because
+ * the parallel port also revert them with a "not" logic gate.
+ */
+
+#define bit(n) (((unsigned char)1)<<(n))
+
+void ks0108_writedata(unsigned char byte)
+{
+ parport_write_data(ks0108_parport, byte);
+}
+
+void ks0108_writecontrol(unsigned char byte)
+{
+ udelay(ks0108_delay);
+ parport_write_control(ks0108_parport, byte ^ (bit(0) | bit(1) | bit(3)));
+}
+
+void ks0108_displaystate(unsigned char state)
+{
+ ks0108_writedata((state ? bit(0) : 0) | bit(1) | bit(2) | bit(3) | bit(4) | bit(5));
+}
+
+void ks0108_startline(unsigned char startline)
+{
+ ks0108_writedata(min(startline,(unsigned char)63) | bit(6) | bit(7));
+}
+
+void ks0108_address(unsigned char address)
+{
+ ks0108_writedata(min(address,(unsigned char)63) | bit(6));
+}
+
+void ks0108_page(unsigned char page)
+{
+ ks0108_writedata(min(page,(unsigned char)7) | bit(3) | bit(4) | bit(5) | bit(7));
+}
+
+EXPORT_SYMBOL_GPL(ks0108_writedata);
+EXPORT_SYMBOL_GPL(ks0108_writecontrol);
+EXPORT_SYMBOL_GPL(ks0108_displaystate);
+EXPORT_SYMBOL_GPL(ks0108_startline);
+EXPORT_SYMBOL_GPL(ks0108_address);
+EXPORT_SYMBOL_GPL(ks0108_page);
+
+/*
+ * Module Init & Exit
+ */
+
+static int __init ks0108_init(void)
+{
+ int result;
+ int ret = -EINVAL;
+
+ ks0108_parport = parport_find_base(ks0108_port);
+ if (ks0108_parport == NULL) {
+ printk(KERN_ERR KS0108_NAME ": ERROR: "
+ "parport didn't find %i port\n", ks0108_port);
+ goto none;
+ }
+
+ ks0108_pardevice = parport_register_device(ks0108_parport, KS0108_NAME,
+ NULL, NULL, NULL, PARPORT_DEV_EXCL, NULL);
+ if (ks0108_pardevice == NULL) {
+ printk(KERN_ERR KS0108_NAME ": ERROR: "
+ "parport didn't register new device\n");
+ goto none;
+ }
+
+ result = parport_claim(ks0108_pardevice);
+ if (result != 0) {
+ printk(KERN_ERR KS0108_NAME ": ERROR: "
+ "can't claim %i parport, maybe in use\n", ks0108_port);
+ ret = result;
+ goto registered;
+ }
+
+ return 0;
+
+registered:
+ parport_unregister_device(ks0108_pardevice);
+
+none:
+ return ret;
+}
+
+static void __exit ks0108_exit(void)
+{
+ parport_release(ks0108_pardevice);
+ parport_unregister_device(ks0108_pardevice);
+}
+
+module_init(ks0108_init);
+module_exit(ks0108_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Miguel Ojeda Sandonis <[email protected]>");
+MODULE_DESCRIPTION("ks0108 LCD Controller driver");
+
diff -uprN -X dontdiff linux-2.6.19-rc1-vanilla/drivers/auxdisplay/Makefile linux/drivers/auxdisplay/Makefile
--- linux-2.6.19-rc1-vanilla/drivers/auxdisplay/Makefile 1970-01-01 00:00:00.000000000 +0000
+++ linux/drivers/auxdisplay/Makefile 2006-10-11 15:51:37.000000000 +0000
@@ -0,0 +1,6 @@
+#
+# Makefile for the kernel auxiliary displays device drivers.
+#
+
+obj-$(CONFIG_KS0108) += ks0108.o
+obj-$(CONFIG_CFAG12864B) += cfag12864b.o cfag12864bfb.o
diff -uprN -X dontdiff linux-2.6.19-rc1-vanilla/drivers/Kconfig linux/drivers/Kconfig
--- linux-2.6.19-rc1-vanilla/drivers/Kconfig 2006-10-13 02:10:22.000000000 +0000
+++ linux/drivers/Kconfig 2006-10-05 13:46:44.000000000 +0000
@@ -76,4 +76,6 @@ source "drivers/rtc/Kconfig"

source "drivers/dma/Kconfig"

+source "drivers/auxdisplay/Kconfig"
+
endmenu
diff -uprN -X dontdiff linux-2.6.19-rc1-vanilla/drivers/Makefile linux/drivers/Makefile
--- linux-2.6.19-rc1-vanilla/drivers/Makefile 2006-10-13 02:10:22.000000000 +0000
+++ linux/drivers/Makefile 2006-10-05 13:46:44.000000000 +0000
@@ -77,3 +77,4 @@ obj-$(CONFIG_CRYPTO) += crypto/
obj-$(CONFIG_SUPERH) += sh/
obj-$(CONFIG_GENERIC_TIME) += clocksource/
obj-$(CONFIG_DMA_ENGINE) += dma/
+obj-y += auxdisplay/
diff -uprN -X dontdiff linux-2.6.19-rc1-vanilla/include/linux/cfag12864b.h linux/include/linux/cfag12864b.h
--- linux-2.6.19-rc1-vanilla/include/linux/cfag12864b.h 1970-01-01 00:00:00.000000000 +0000
+++ linux/include/linux/cfag12864b.h 2006-10-12 15:14:09.000000000 +0000
@@ -0,0 +1,41 @@
+/*
+ * Filename: cfag12864b.h
+ * Version: 0.1.0
+ * Description: cfag12864b LCD driver header
+ * License: GPLv2
+ *
+ * Author: Copyright (C) Miguel Ojeda Sandonis <[email protected]>
+ * Date: 2006-10-12
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#ifndef _CFAG12864B_H_
+#define _CFAG12864B_H_
+
+#define CFAG12864B_WIDTH (128)
+#define CFAG12864B_HEIGHT (64)
+#define CFAG12864B_CONTROLLERS (2)
+#define CFAG12864B_PAGES (8)
+#define CFAG12864B_ADDRESSES (64)
+#define CFAG12864B_SIZE ((CFAG12864B_CONTROLLERS) * \
+ (CFAG12864B_PAGES) * \
+ (CFAG12864B_ADDRESSES))
+
+extern unsigned char * cfag12864b_buffer;
+
+#endif /* _CFAG12864B_H_ */
+
diff -uprN -X dontdiff linux-2.6.19-rc1-vanilla/include/linux/ks0108.h linux/include/linux/ks0108.h
--- linux-2.6.19-rc1-vanilla/include/linux/ks0108.h 1970-01-01 00:00:00.000000000 +0000
+++ linux/include/linux/ks0108.h 2006-10-05 13:46:44.000000000 +0000
@@ -0,0 +1,36 @@
+/*
+ * Filename: ks0108.h
+ * Version: 0.1.0
+ * Description: ks0108 LCD Controller driver header
+ * License: GPLv2
+ *
+ * Author: Copyright (C) Miguel Ojeda Sandonis <[email protected]>
+ * Date: 2006-10-04
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#ifndef _KS0108_H_
+#define _KS0108_H_
+
+extern void ks0108_writedata(unsigned char byte);
+extern void ks0108_writecontrol(unsigned char byte);
+extern void ks0108_displaystate(unsigned char state);
+extern void ks0108_startline(unsigned char startline);
+extern void ks0108_address(unsigned char address);
+extern void ks0108_page(unsigned char page);
+
+#endif /* _KS0108_H_ */
diff -uprN -X dontdiff linux-2.6.19-rc1-vanilla/MAINTAINERS linux/MAINTAINERS
--- linux-2.6.19-rc1-vanilla/MAINTAINERS 2006-10-13 02:10:17.000000000 +0000
+++ linux/MAINTAINERS 2006-10-12 12:24:08.000000000 +0000
@@ -441,6 +441,12 @@ W: http://people.redhat.com/sgrubb/audit
T: git kernel.org:/pub/scm/linux/kernel/git/dwmw2/audit-2.6.git
S: Maintained

+AUXILIARY DISPLAY DRIVERS
+P: Miguel Ojeda Sandonis
+M: [email protected]
+L: [email protected]
+S: Maintained
+
AVR32 ARCHITECTURE
P: Atmel AVR32 Support Team
M: [email protected]
@@ -646,6 +652,18 @@ L: [email protected]
L: [email protected]
S: Maintained

+CFAG12864B LCD DRIVER
+P: Miguel Ojeda Sandonis
+M: [email protected]
+L: [email protected]
+S: Maintained
+
+CFAG12864BFB LCD FRAMEBUFFER DRIVER
+P: Miguel Ojeda Sandonis
+M: [email protected]
+L: [email protected]
+S: Maintained
+
COMMON INTERNET FILE SYSTEM (CIFS)
P: Steve French
M: [email protected]
@@ -1736,6 +1754,12 @@ M: [email protected]
L: [email protected]
S: Maintained

+KS0108 LCD CONTROLLER DRIVER
+P: Miguel Ojeda Sandonis
+M: [email protected]
+L: [email protected]
+S: Maintained
+
LAPB module
L: [email protected]
S: Orphan


2006-10-13 12:43:30

by Paulo Marques

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

Miguel Ojeda Sandonis wrote:
> Andrew, here it is the complete patch again as you requested.
>
> Paulo, I got the algorithm much faster, thanks you very much.
> (<2% CPU at 20 Hz on P4 3Ghz :).
> ---
> miguelojeda-2.6.19-rc1-add-LCD-support.patch
>
> Adds support for auxiliary displays, the ks0108 LCD controller,
> the cfag12864b LCD and adds a framebuffer device: cfag12864bfb.
> [...]
> Signed-off-by: Miguel Ojeda Sandonis <[email protected]>

Acked-by: Paulo Marques <[email protected]>

--
Paulo Marques - http://www.grupopie.com

2006-10-18 14:55:38

by Franck Bui-Huu

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

Hi

Miguel Ojeda Sandonis wrote:
> Andrew, here it is the complete patch again as you requested.
>

sorry for coming lately, I just noticed your patch in -mm tree.

Did you took a look at drivers/video/arcfb.c driver ?
It seems to be a fb driver for ks108 lcd controller. A lot of code
is related to the platform though and the controller is driven
through GPIO but have you tried to split the code for sharing
controller specific code. Note, I don't say it's a good idea, it's
just a question that comes in mind.

Also you create driver/auxdisplay directory whereas drivers such
the one I mentioned previously is located in drivers/video. Why
not putting your driver in driver/video ?

Franck

2006-10-23 08:41:01

by Franck Bui-Huu

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

Franck Bui-Huu wrote:
> Hi
>
> Miguel Ojeda Sandonis wrote:
>> Andrew, here it is the complete patch again as you requested.
>>
>
> sorry for coming lately, I just noticed your patch in -mm tree.
>
> Did you took a look at drivers/video/arcfb.c driver ?
> It seems to be a fb driver for ks108 lcd controller. A lot of code
> is related to the platform though and the controller is driven
> through GPIO but have you tried to split the code for sharing
> controller specific code. Note, I don't say it's a good idea, it's
> just a question that comes in mind.
>
> Also you create driver/auxdisplay directory whereas drivers such
> the one I mentioned previously is located in drivers/video. Why
> not putting your driver in driver/video ?

Is this driver is already no more maintained ?

Thanks,

Franck

2006-10-23 12:56:58

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/23/06, Franck Bui-Huu <[email protected]> wrote:
> Franck Bui-Huu wrote:
> > Hi
> >
> >
> > sorry for coming lately, I just noticed your patch in -mm tree.
> >
> > Did you took a look at drivers/video/arcfb.c driver ?
> > It seems to be a fb driver for ks108 lcd controller. A lot of code
> > is related to the platform though and the controller is driven
> > through GPIO but have you tried to split the code for sharing
> > controller specific code. Note, I don't say it's a good idea, it's
> > just a question that comes in mind.
> >
> > Also you create driver/auxdisplay directory whereas drivers such
> > the one I mentioned previously is located in drivers/video. Why
> > not putting your driver in driver/video ?
>
> Is this driver is already no more maintained ?
>

The driver is waiting in the -mm tree (-mm2 right now) for being
included in the mainline kernel sometime in the future. If it is
included, I will maintain it as I coded it as it apears in the
MAINTAINERS file. Why are you so worried about it if I can ask? Do you
want some more features or something like that?

I missed the other two questions you wrote few days ago. About the
second one, that was discussed a lot in the past and the people
decided that (it wasn't my idea). About the first one, well, my ks0108
code is the one for the wiring of an auxiliary LCD, so if you read the
discussion you will find the people wanted to split video things and
other auxiliary displays, so I think it is better to split it.
(Anyway, I'm answering quickly, I haven't checked the code you talk
about, but I will anyway).

Thanks,
Miguel Ojeda

2006-10-23 15:35:32

by Franck Bui-Huu

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support


[ note I'm not familiar with lcds...just try to understand what you've
done ]

Miguel Ojeda wrote:
> The driver is waiting in the -mm tree (-mm2 right now) for being
> included in the mainline kernel sometime in the future. If it is
> included, I will maintain it as I coded it as it apears in the
> MAINTAINERS file. Why are you so worried about it if I can ask? Do you
> want some more features or something like that?
>
> I missed the other two questions you wrote few days ago. About the
> second one, that was discussed a lot in the past and the people

yeah I found the thread and indeed it has been discussed very deeply ;)

> decided that (it wasn't my idea). About the first one, well, my ks0108
> code is the one for the wiring of an auxiliary LCD, so if you read the
> discussion you will find the people wanted to split video things and
> other auxiliary displays, so I think it is better to split it.
> (Anyway, I'm answering quickly, I haven't checked the code you talk
> about, but I will anyway).
>

What I was worry about is that you actually wrote a frame buffer
driver, which are normally located in drivers/video, and put it in a
new directory drivers/auxdisplay. So now we have two places for frame
buffer drivers. It looks like, now some frame buffer drivers in
drivers/video should be moved in drivers/auxdisplay, shouldn't it ?

Maybe just a stupid idea but why not restructuring the thing like:

drivers
|-- display
| |-- video
| |-- aux
| |-- fbmem.c
| |-- ...

Another point: does the ks0108 controller is only used with the 'cfag'
display ? If not, suppose I'm using the same controller with another
lcd different from 'cfga'...Am I supposed to reuse your code in
cfag12864b.c ?

BTW, did you try to mmap your fbdev ? Does it work ?

Thanks
Franck

2006-10-23 16:05:41

by Franck Bui-Huu

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

Miguel Ojeda wrote:
> The driver is waiting in the -mm tree (-mm2 right now) for being
> included in the mainline kernel sometime in the future. If it is
> included, I will maintain it as I coded it as it apears in the
> MAINTAINERS file. Why are you so worried about it if I can ask? Do you
> want some more features or something like that?

Are you sure the patch you sent to Andrew is your latest patch
version ? For example I can't found any locks in the patch that
you normally added during the V6 patch version; auxlcddisplay.c
doesn't no exist...

Franck

2006-10-23 16:08:47

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/23/06, Franck Bui-Huu <[email protected]> wrote:
> Miguel Ojeda wrote:
> > The driver is waiting in the -mm tree (-mm2 right now) for being
> > included in the mainline kernel sometime in the future. If it is
> > included, I will maintain it as I coded it as it apears in the
> > MAINTAINERS file. Why are you so worried about it if I can ask? Do you
> > want some more features or something like that?
>
> Are you sure the patch you sent to Andrew is your latest patch
> version ? For example I can't found any locks in the patch that
> you normally added during the V6 patch version; auxlcddisplay.c
> doesn't no exist...
>
> Franck
>

Yes, we are sure. AFAIK there is no need to lock when it is a fbdev.
The older version were "alone" drivers: they needed to lock because
they used fops and they exported functions.

2006-10-23 16:21:35

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/23/06, Franck Bui-Huu <[email protected]> wrote:
>
> [ note I'm not familiar with lcds...just try to understand what you've
> done ]
>
> What I was worry about is that you actually wrote a frame buffer
> driver, which are normally located in drivers/video, and put it in a
> new directory drivers/auxdisplay. So now we have two places for frame
> buffer drivers. It looks like, now some frame buffer drivers in
> drivers/video should be moved in drivers/auxdisplay, shouldn't it ?
>
> Maybe just a stupid idea but why not restructuring the thing like:
>
> drivers
> |-- display
> | |-- video
> | |-- aux
> | |-- fbmem.c
> | |-- ...
>
>

Yes, I got your idea in your first message, and well, that was
discussed (however, not being a fbdev), and the people thought that
putting them together will, maybe, cause confusion; so having them out
from drivers/video should be better. Your idea, which merge them into
a "drivers/display" could be a good one, but I don't think people will
like to change such critical tree right now. Also, I'm not the one who
maintain such tree, so my opinion won't make changes about that ;)

>
> Another point: does the ks0108 controller is only used with the 'cfag'
> display ? If not, suppose I'm using the same controller with another
> lcd different from 'cfga'...Am I supposed to reuse your code in
> cfag12864b.c ?
>

No. You are supposed to _use_ the ks0108's exported functions: I split
the code into the ks0108 and the cfag12864b because I thought it was
the logical way, as the cfag12864b LCD just send the data through two
different ks0108 controllers. The same way, you can use the ks0108's
exported functions to create another whateverlcd.c which has one or
more ks0108 controllers.

>
> BTW, did you try to mmap your fbdev ? Does it work ?
>

Why it shouldn't? It doesn't work for you? AFAIK, it is a usual fbdev,
and you can map any fbdev as they are simple character file devices.

>
> Thanks
> Franck
>

Thanks
Miguel Ojeda

2006-10-23 16:51:18

by Franck Bui-Huu

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/23/06, Miguel Ojeda <[email protected]> wrote:
> On 10/23/06, Franck Bui-Huu <[email protected]> wrote:
> >
> > [ note I'm not familiar with lcds...just try to understand what you've
> > done ]
> >
> > What I was worry about is that you actually wrote a frame buffer
> > driver, which are normally located in drivers/video, and put it in a
> > new directory drivers/auxdisplay. So now we have two places for frame
> > buffer drivers. It looks like, now some frame buffer drivers in
> > drivers/video should be moved in drivers/auxdisplay, shouldn't it ?
> >
> > Maybe just a stupid idea but why not restructuring the thing like:
> >
> > drivers
> > |-- display
> > | |-- video
> > | |-- aux
> > | |-- fbmem.c
> > | |-- ...
> >
> >
>
> Yes, I got your idea in your first message, and well, that was
> discussed (however, not being a fbdev), and the people thought that
> putting them together will, maybe, cause confusion; so having them out
> from drivers/video should be better. Your idea, which merge them into
> a "drivers/display" could be a good one, but I don't think people will
> like to change such critical tree right now. Also, I'm not the one who
> maintain such tree, so my opinion won't make changes about that ;)
>

well, if it is the right thing to do, why not doing it. (note: I don't
claim it's the right thing to do though). Futhermore such change is
going to move a lot of files _but_ if after the move it compiles, then
it works...

> >
> > Another point: does the ks0108 controller is only used with the 'cfag'
> > display ? If not, suppose I'm using the same controller with another
> > lcd different from 'cfga'...Am I supposed to reuse your code in
> > cfag12864b.c ?
> >
>
> No. You are supposed to _use_ the ks0108's exported functions: I split
> the code into the ks0108 and the cfag12864b because I thought it was
> the logical way, as the cfag12864b LCD just send the data through two
> different ks0108 controllers. The same way, you can use the ks0108's
> exported functions to create another whateverlcd.c which has one or
> more ks0108 controllers.
>

So, to sum up, if I have a lcd named 'foo_lcd' which use 4 ks0108
controllers (256x128), and want to make your fb driver work on it, I
need to copy your cfag12864b.c file, rename it to foo_lcd256128.c and
just change the number of controllers, is that correct ?

> >
> > BTW, did you try to mmap your fbdev ? Does it work ?
> >
>
> Why it shouldn't? It doesn't work for you? AFAIK, it is a usual fbdev,
> and you can map any fbdev as they are simple character file devices.
>

well you actually haven't answered to the initial question... I
suspect it won't work since you haven't setup "info->fix.smem_start"
anywhere. But I may be wrong since I don't know fb code.

--
Franck

2006-10-23 17:15:12

by Franck Bui-Huu

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/23/06, Miguel Ojeda <[email protected]> wrote:
> Yes, we are sure. AFAIK there is no need to lock when it is a fbdev.
> The older version were "alone" drivers: they needed to lock because
> they used fops and they exported functions.
>

ok, so no other driver than fb could use 'cfag12864b_buffer'. Maybe
I'm missing something but why did you split your fb driver into
cfag12864b.c and cfag12864fb.c ?

BTW, 'cfag12864b_cache' could have been static...

Franck

2006-10-26 14:45:36

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/23/06, Franck Bui-Huu <[email protected]> wrote:
> On 10/23/06, Miguel Ojeda <[email protected]> wrote:
> > On 10/23/06, Franck Bui-Huu <[email protected]> wrote:
> > >
> > > [ note I'm not familiar with lcds...just try to understand what you've
> > > done ]
> > >
> > > What I was worry about is that you actually wrote a frame buffer
> > > driver, which are normally located in drivers/video, and put it in a
> > > new directory drivers/auxdisplay. So now we have two places for frame
> > > buffer drivers. It looks like, now some frame buffer drivers in
> > > drivers/video should be moved in drivers/auxdisplay, shouldn't it ?
> > >
> > > Maybe just a stupid idea but why not restructuring the thing like:
> > >
> > > drivers
> > > |-- display
> > > | |-- video
> > > | |-- aux
> > > | |-- fbmem.c
> > > | |-- ...
> > >
> > >
> >
> > Yes, I got your idea in your first message, and well, that was
> > discussed (however, not being a fbdev), and the people thought that
> > putting them together will, maybe, cause confusion; so having them out
> > from drivers/video should be better. Your idea, which merge them into
> > a "drivers/display" could be a good one, but I don't think people will
> > like to change such critical tree right now. Also, I'm not the one who
> > maintain such tree, so my opinion won't make changes about that ;)
> >
>
> well, if it is the right thing to do, why not doing it. (note: I don't
> claim it's the right thing to do though). Futhermore such change is
> going to move a lot of files _but_ if after the move it compiles, then
> it works...
>
> > >
> > > Another point: does the ks0108 controller is only used with the 'cfag'
> > > display ? If not, suppose I'm using the same controller with another
> > > lcd different from 'cfga'...Am I supposed to reuse your code in
> > > cfag12864b.c ?
> > >
> >
> > No. You are supposed to _use_ the ks0108's exported functions: I split
> > the code into the ks0108 and the cfag12864b because I thought it was
> > the logical way, as the cfag12864b LCD just send the data through two
> > different ks0108 controllers. The same way, you can use the ks0108's
> > exported functions to create another whateverlcd.c which has one or
> > more ks0108 controllers.
> >
>
> So, to sum up, if I have a lcd named 'foo_lcd' which use 4 ks0108
> controllers (256x128), and want to make your fb driver work on it, I
> need to copy your cfag12864b.c file, rename it to foo_lcd256128.c and
> just change the number of controllers, is that correct ?
>

No way. Each controller would have different wirings, pins, in-outs,
specifications (...) You will need to code an almost whole new fbdev
driver (althought maybe it will be so similar to cfag12864b so you
only need to make few little changes, but that is unsure).

>
> > >
> > > BTW, did you try to mmap your fbdev ? Does it work ?
> > >
> >
> > Why it shouldn't? It doesn't work for you? AFAIK, it is a usual fbdev,
> > and you can map any fbdev as they are simple character file devices.
> >
>
> well you actually haven't answered to the initial question... I
> suspect it won't work since you haven't setup "info->fix.smem_start"
> anywhere. But I may be wrong since I don't know fb code.
>

Well, you were right about mmaping, but you weren't about
"info->fix.smem_start". smem_start expects a physical address. RAM
addresses can't be mmapped as usual, so I have made some changes and
coded the nopage and mmap ops.

I'm sending the patch in a few moments. Thanks for pointing it: Now it
can be mmaped as well.

2006-10-26 14:55:38

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/23/06, Franck Bui-Huu <[email protected]> wrote:
> On 10/23/06, Miguel Ojeda <[email protected]> wrote:
> > Yes, we are sure. AFAIK there is no need to lock when it is a fbdev.
> > The older version were "alone" drivers: they needed to lock because
> > they used fops and they exported functions.
> >
>
> ok, so no other driver than fb could use 'cfag12864b_buffer'. Maybe
> I'm missing something but why did you split your fb driver into
> cfag12864b.c and cfag12864fb.c ?
>

To be clearer. And you are wrong: you can write other modules which
want to know what the LCD is showing, or use it; without worrying
about framebuffer things. They can read / write "cfag12864b_buffer" as
well as cfag12864bfb do. Why not?

The cfag12864b module is the real driver, which uses ks0108 module.
The cfag12864bfb is just the framebuffer device.

>
> BTW, 'cfag12864b_cache' could have been static...

Right, I saw that in the meantime I was adding the mmap feature. Thanks.

>
> Franck
>

2006-10-27 20:03:28

by Franck Bui-Huu

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/26/06, Miguel Ojeda <[email protected]> wrote:
> To be clearer. And you are wrong: you can write other modules which
> want to know what the LCD is showing, or use it; without worrying
> about framebuffer things. They can read / write "cfag12864b_buffer" as
> well as cfag12864bfb do. Why not?
>

Suppose I'm writing a user space application which uses your frame
buffer driver. I would naturaly mmap your device since it's the
easiest way to use a frame buffer. Now I want to display as fast as
possible a set of images. How am I sure that each image is sent to the
lcd ? For example, suppose the application just finished to copy image
A into the buffer, and now it starts to copy image B into the buffer
but the work queue has not been scheduled yet...

Futhermore I'm not sure it's a common use case for such device, is it
? I would say that the usual case for such LCD is to display an image
every now and then. If so do we really need to give the possibility to
mmap the device ? Is a simple synchrone write() enough ?

BTW how can the application retrieve the refresh rate from the driver ?

Franck

2006-10-27 20:08:49

by Franck Bui-Huu

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/26/06, Miguel Ojeda <[email protected]> wrote:
> No way. Each controller would have different wirings, pins, in-outs,
> specifications (...) You will need to code an almost whole new fbdev
> driver (althought maybe it will be so similar to cfag12864b so you
> only need to make few little changes, but that is unsure).
>

that's what I was trying to point out. I was wondering if you could
make your driver a little more generic so another lcd could use your
driver as is.

> Well, you were right about mmaping, but you weren't about
> "info->fix.smem_start". smem_start expects a physical address. RAM
> addresses can't be mmapped as usual

Sorry I don't understand your last sentence. Can you explain please ?

--
Franck

2006-10-27 20:25:08

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/27/06, Franck Bui-Huu <[email protected]> wrote:
> On 10/26/06, Miguel Ojeda <[email protected]> wrote:
> > To be clearer. And you are wrong: you can write other modules which
> > want to know what the LCD is showing, or use it; without worrying
> > about framebuffer things. They can read / write "cfag12864b_buffer" as
> > well as cfag12864bfb do. Why not?
> >
>
> Suppose I'm writing a user space application which uses your frame
> buffer driver. I would naturaly mmap your device since it's the
> easiest way to use a frame buffer. Now I want to display as fast as
> possible a set of images. How am I sure that each image is sent to the
> lcd ? For example, suppose the application just finished to copy image
> A into the buffer, and now it starts to copy image B into the buffer
> but the work queue has not been scheduled yet...
>

Refresh rate is fixed in this driver (the user can change it to other
value at Kconfig or at loading time as a module parameter).

An application should not refresh images so fast (the LCD controller
can handle it, but the readability is pretty bad). Having a refresh
rate like 10 Hz for example, the application can be sure all images
are displayed. Anyway, an animation of 10 Hz wouldn't be fine at this
kind of LCDs, so it is pointless which the refresh rate of the driver
is, as it is not useful to display images as fast as the driver
refresh the LCD.

(I don't know if I'm explaining myself...)

>
> Futhermore I'm not sure it's a common use case for such device, is it
> ? I would say that the usual case for such LCD is to display an image
> every now and then. If so do we really need to give the possibility to
> mmap the device ? Is a simple synchrone write() enough ?
>

Well, you can show a set of images, animations... althought it's not
so useful. Usual cases:
1. Display a static image / info.
2. Display refreshed info every X seconds (like % usage of CPU and so
on, the music you are listening to...)
3. Display, maybe, a animated graph (like the wave created by your
music), althought this one is not so usual, as it changes much more
quickly than [2]. Still, there are apps in other SOs that show such
kind of info.

Well, mmaping is the best option, as it is the easiest and the
fastest. Any use will be better using mmaping than doing synchrone
write(). Yes, many uses just need a write() call, but other uses would
need mmap.

> BTW how can the application retrieve the refresh rate from the driver ?
>

Hum, right now one way is:

$ cat /sys/module/cfag12864b/parameters/cfag12864b_rate

Yes, a more generic option would be better. Do the fbdevices have some
standard way to retrieve such kind of info (like the bits per pixel,
width, height...)? If not, which would be a good to retrieve the
refresh rate?

Anyway, a application which would like to use this LCD should know its
specs and the user knows he shouldn't change the refresh rate without
a good reason. If a user changes it to other value, he knows he can be
breaking the apps, the same way you can break your kernel adding
things you shouldn't; also the driver could stop working properly at
higher rates.

>
> Franck
>

2006-10-27 20:38:18

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/27/06, Franck Bui-Huu <[email protected]> wrote:
> On 10/26/06, Miguel Ojeda <[email protected]> wrote:
> > No way. Each controller would have different wirings, pins, in-outs,
> > specifications (...) You will need to code an almost whole new fbdev
> > driver (althought maybe it will be so similar to cfag12864b so you
> > only need to make few little changes, but that is unsure).
> >
>
> that's what I was trying to point out. I was wondering if you could
> make your driver a little more generic so another lcd could use your
> driver as is.
>

Hum... Others LCDs has different pinouts, different timings, different
timings... It would be a really hard work, and maybe the result
wouldn't be good.

I will think about it, but I think it would lead to a really complex
generic driver (something like the fbdev API). Also, we would need
many kinds of LCDs to know what can be a common factor between all of
them, what can't, etc.

Really, I coded 2 drivers. The generic driver is ks0108, which it can
be used with a lot more LCDs drivers. But the cfag12864b driver is
really specific, as all the LCDs are different (as different as the
user (wiring) and the builder (timings, pinout, commands...) wants).
In the other hand, LCD controllers like ks0108 are industry-standard.

> > Well, you were right about mmaping, but you weren't about
> > "info->fix.smem_start". smem_start expects a physical address. RAM
> > addresses can't be mmapped as usual
>
> Sorry I don't understand your last sentence. Can you explain please ?
>

Sorry, I meant: You can't mmap a RAM address using functions like the
usual remap_pfn_range (as such functions doesn't like physical RAM
addresses, they want I/O ports for example, like 0x378). So, you can't
use smem_start. You need to code your own mmap & nopage function. (It
is explained in LDD3 very well).

> --
> Franck
>

2006-10-30 08:43:17

by Franck Bui-Huu

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

Miguel Ojeda wrote:
>
> Sorry, I meant: You can't mmap a RAM address using functions like the
> usual remap_pfn_range (as such functions doesn't like physical RAM
> addresses, they want I/O ports for example, like 0x378). So, you can't
> use smem_start. You need to code your own mmap & nopage function. (It
> is explained in LDD3 very well).
>

well I must admit that I don't understand why... I suppose you refered
to that section in ldd3:

An interesting limitation of remap_pfn_range is that it gives
access only to reserved pages and physical addresses above the
top of physical memory.

I take a quick look at the implementation of remap_pfn_range() and
there's no such limitation I can see (fortunately).

Franck

2006-10-30 09:25:38

by Franck Bui-Huu

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

Miguel Ojeda wrote:
> Refresh rate is fixed in this driver (the user can change it to other
> value at Kconfig or at loading time as a module parameter).
>
> An application should not refresh images so fast (the LCD controller
> can handle it, but the readability is pretty bad). Having a refresh
> rate like 10 Hz for example, the application can be sure all images
> are displayed. Anyway, an animation of 10 Hz wouldn't be fine at this
> kind of LCDs, so it is pointless which the refresh rate of the driver
> is, as it is not useful to display images as fast as the driver
> refresh the LCD.
>

An application might want to display quickly a set of images, not for
doing animations but rather displaying 'fake' greyscale images.

> Well, mmaping is the best option, as it is the easiest and the
> fastest. Any use will be better using mmaping than doing synchrone
> write(). Yes, many uses just need a write() call, but other uses would
> need mmap.
>

That's true for devices which have a frame buffer. But it's not your
case. You _emulate_ this behaviour and I can see big drawbacks to
this design:

- You need to keep synchrone your buffer and the display every
100ms. It makes your CPU busy even if nothing has been written in
LCD for a while. Futhermore this kind of display is likely to run
on embedded system where CPU speed can be less than 100MHz.

- All accesses to the device depend on the previous behaviour whereas
write(), read() syscall could be synchrone and easier to use for
fast writing of image sets. Actually the refresh stuff is really
needed only if you mmap the device. And it seems not really used
for now since it was broken on your last patch.

- Your driver _is_ a frame buffer driver by design, I think you
don't need to seperate cfa12864cb.c and cfa12864cfb.c

So ok you code is very small with this design but IMHO it is not the
most polyvalent if you want to address all needs. Why not starting the
refresh stuff only if you mmap the device and keep the rest synch ?


> $ cat /sys/module/cfag12864b/parameters/cfag12864b_rate
>
> Yes, a more generic option would be better. Do the fbdevices have some
> standard way to retrieve such kind of info (like the bits per pixel,
> width, height...)? If not, which would be a good to retrieve the
> refresh rate?

maybe fb_ioctl() can return that. Or your driver can have its own
ioctl() method which could return such specific info.

Franck

2006-10-30 13:23:04

by Paulo Marques

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

Franck Bui-Huu wrote:
> Miguel Ojeda wrote:
>> [...]
>> Anyway, an animation of 10 Hz wouldn't be fine at this
>> kind of LCDs, so it is pointless which the refresh rate of the driver
>> is, as it is not useful to display images as fast as the driver
>> refresh the LCD.
>
> An application might want to display quickly a set of images, not for
> doing animations but rather displaying 'fake' greyscale images.

To do "fake" greyscale you would need to synchronize with the actual
refresh of the controller or you will have very ugly aliasing artifacts.

Since there is no hardware interface to know when the controller is
refreshing, I don't think this is one viable usage scenario.

>> Well, mmaping is the best option, as it is the easiest and the
>> fastest. Any use will be better using mmaping than doing synchrone
>> write(). Yes, many uses just need a write() call, but other uses would
>> need mmap.
>
> That's true for devices which have a frame buffer. But it's not your
> case. You _emulate_ this behaviour and I can see big drawbacks to
> this design:
>
> - You need to keep synchrone your buffer and the display every
> 100ms. It makes your CPU busy even if nothing has been written in
> LCD for a while. Futhermore this kind of display is likely to run
> on embedded system where CPU speed can be less than 100MHz.

It is not *that* bad. If nothing has changed, the driver only pays the
cost of a 1024 bytes memcmp. Even more, there are no current users for
anything other than a PC, so I think we shouldn't overdesign at this point.

> - All accesses to the device depend on the previous behaviour whereas
> write(), read() syscall could be synchrone and easier to use for
> fast writing of image sets. Actually the refresh stuff is really
> needed only if you mmap the device. And it seems not really used
> for now since it was broken on your last patch.

That is not entirely true. If a user-space application misbehaves and
starts writing faster than what can actually be seen, not having a fixed
refresh rate means that your CPU will be *very* busy writing garbage
that the user won't be able to see.

A fixed refresh rate is a maximum CPU usage tunable that prevents
applications (that are not aware that this is an external slow device)
to write more than what they should.

This can be improved, however.

We could have the concept of a "dirty" buffer. Any write or nopage call
would set the dirty flag and set a timer to refresh the display in one
refresh period from now.

When doing the actual refresh, the "dirty" flag would be cleared and the
buffer unmapped.

In this case, if nothing was ever written to the display, the CPU usage
would be _zero_ (as it should), and it would work nicely with dynticks
and such.

Anyway, I think that the driver is useful as is and fulfills its
original goals: drive an LCD connected to a parelel port on a PC.

Since the actual userspace interface supports the suggested
improvements, we can just merge now and improve it later to support
different usage scenarios as they appear (or even before they appear).

--
Paulo Marques - http://www.grupopie.com

"The face of a child can say it all, especially the
mouth part of the face."

2006-10-30 13:35:05

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/30/06, Franck Bui-Huu <[email protected]> wrote:
> Miguel Ojeda wrote:
> >
> > Sorry, I meant: You can't mmap a RAM address using functions like the
> > usual remap_pfn_range (as such functions doesn't like physical RAM
> > addresses, they want I/O ports for example, like 0x378). So, you can't
> > use smem_start. You need to code your own mmap & nopage function. (It
> > is explained in LDD3 very well).
> >
>
> well I must admit that I don't understand why... I suppose you refered
> to that section in ldd3:
>
> An interesting limitation of remap_pfn_range is that it gives
> access only to reserved pages and physical addresses above the
> top of physical memory.
>
> I take a quick look at the implementation of remap_pfn_range() and
> there's no such limitation I can see (fortunately).
>
> Franck
>

Read further ;-)

"Therefore, remap_pfn_range won't allow you to remap
conventional addresses, which include the ones you obtain by calling
get_free_pages"

Because of such limitation, they teach you to remap RAM in other ways.

2006-10-30 13:47:29

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/30/06, Franck Bui-Huu <[email protected]> wrote:
> Miguel Ojeda wrote:
> > Refresh rate is fixed in this driver (the user can change it to other
> > value at Kconfig or at loading time as a module parameter).
> >
> > An application should not refresh images so fast (the LCD controller
> > can handle it, but the readability is pretty bad). Having a refresh
> > rate like 10 Hz for example, the application can be sure all images
> > are displayed. Anyway, an animation of 10 Hz wouldn't be fine at this
> > kind of LCDs, so it is pointless which the refresh rate of the driver
> > is, as it is not useful to display images as fast as the driver
> > refresh the LCD.
> >
>
> An application might want to display quickly a set of images, not for
> doing animations but rather displaying 'fake' greyscale images.
>

And what is the problem?

> > Well, mmaping is the best option, as it is the easiest and the
> > fastest. Any use will be better using mmaping than doing synchrone
> > write(). Yes, many uses just need a write() call, but other uses would
> > need mmap.
> >
>
> That's true for devices which have a frame buffer. But it's not your
> case. You _emulate_ this behaviour and I can see big drawbacks to
> this design:
>
> - You need to keep synchrone your buffer and the display every
> 100ms. It makes your CPU busy even if nothing has been written in
> LCD for a while. Futhermore this kind of display is likely to run
> on embedded system where CPU speed can be less than 100MHz.
>

No. Paulo pointed that and I added a cache so the CPU is doesn't waste
much time. Less than 1% on a P4 3Ghz. The kernel do a lot more things,
not only refreshing the LCD.

The driver is not intended for an embedded system, as they usually
doesn't have a parallel port.

> - All accesses to the device depend on the previous behaviour whereas
> write(), read() syscall could be synchrone and easier to use for
> fast writing of image sets. Actually the refresh stuff is really
> needed only if you mmap the device. And it seems not really used
> for now since it was broken on your last patch.
>

I had the write() and read() syscalls coded. People like Pavel didn't
like such behaviour, and he told me to recode it to be a framebuffer.
Please read other threads before telling it is wrong when other people
thought the opposite.

> - Your driver _is_ a frame buffer driver by design, I think you
> don't need to seperate cfa12864cb.c and cfa12864cfb.c
>

Well, I think it's better to split it. I have already given reasons to you.

> So ok you code is very small with this design but IMHO it is not the
> most polyvalent if you want to address all needs. Why not starting the
> refresh stuff only if you mmap the device and keep the rest synch ?
>

Because mmaping is better than read() write() in any case. I know, it
waste a little more CPU, but get the point: If you only need to show
static images, the driver won't update the LCD (that is what really
waste the CPU time). If you really need performance, you can decrease
the refresh rate if you don't need it. But 20 Hz is useful for
everybody and doesn't waste a lot of time.

Also, if you want attach fbcon to the device, you need to refresh it
constantly to behave like a framebuffer device, so it is a
requirement.

>
> > $ cat /sys/module/cfag12864b/parameters/cfag12864b_rate
> >
> > Yes, a more generic option would be better. Do the fbdevices have some
> > standard way to retrieve such kind of info (like the bits per pixel,
> > width, height...)? If not, which would be a good to retrieve the
> > refresh rate?
>
> maybe fb_ioctl() can return that. Or your driver can have its own
> ioctl() method which could return such specific info.

No way. Please read previous threads. Greg KH doesn't want more ioctls
in any way. The first versions used it, and he told me to remove them.
Sorry.

>
> Franck
>

2006-10-30 14:11:25

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/30/06, Paulo Marques <[email protected]> wrote:
> It is not *that* bad. If nothing has changed, the driver only pays the
> cost of a 1024 bytes memcmp. Even more, there are no current users for
> anything other than a PC, so I think we shouldn't overdesign at this point.
>

I really can't imagine a smartphone with a PC parallel port ;-)

> > - All accesses to the device depend on the previous behaviour whereas
> > write(), read() syscall could be synchrone and easier to use for
> > fast writing of image sets. Actually the refresh stuff is really
> > needed only if you mmap the device. And it seems not really used
> > for now since it was broken on your last patch.
>
> That is not entirely true. If a user-space application misbehaves and
> starts writing faster than what can actually be seen, not having a fixed
> refresh rate means that your CPU will be *very* busy writing garbage
> that the user won't be able to see.
>
> A fixed refresh rate is a maximum CPU usage tunable that prevents
> applications (that are not aware that this is an external slow device)
> to write more than what they should.
>
> This can be improved, however.
>
> We could have the concept of a "dirty" buffer. Any write or nopage call
> would set the dirty flag and set a timer to refresh the display in one
> refresh period from now.
>
> When doing the actual refresh, the "dirty" flag would be cleared and the
> buffer unmapped.
>
> In this case, if nothing was ever written to the display, the CPU usage
> would be _zero_ (as it should), and it would work nicely with dynticks
> and such.
>

Hum, interesting. But what happens if...

1. For example, the userspace app maps the fb (nopage is called, dirty
flag set, timer created), and then the app doesn't use the fb. The
timer will refresh the LCD, without anything new to refresh, so we are
wasting CPU time. After refreshing, the driver clears the dirty flag
and unmap the buffer.

2. After some seconds, the userspace app decides to write the buffer.
As it is not mmaped anymore, it will simply get a segmentation fault,
right? AFAIK nopage() is not called again because we have destroyed
the mmapping information, and Linux can't know what are the nopage()
ops to call to.

3. We mmap the device, timer is created, we start writing to the
buffer, the timer calls our refreshing function but we didn't finish
filling the buffer with our information: The LCD will have only some
of the screen.

And so on, am I right?

Thanks,
Miguel Ojeda

2006-10-30 15:21:56

by Paulo Marques

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

Miguel Ojeda wrote:
>[...]
>> In this case, if nothing was ever written to the display, the CPU usage
>> would be _zero_ (as it should), and it would work nicely with dynticks
>> and such.
>
> Hum, interesting. But what happens if...
>
> 1. For example, the userspace app maps the fb (nopage is called, dirty
> flag set, timer created), and then the app doesn't use the fb. The
> timer will refresh the LCD, without anything new to refresh, so we are
> wasting CPU time. After refreshing, the driver clears the dirty flag
> and unmap the buffer.
>
> 2. After some seconds, the userspace app decides to write the buffer.
> As it is not mmaped anymore, it will simply get a segmentation fault,
> right? AFAIK nopage() is not called again because we have destroyed
> the mmapping information, and Linux can't know what are the nopage()
> ops to call to.

No, this is not the sequence I thought of at all. I don't remember the
exact API functions you need to call (I have to read LDD3 again ;), but
if the hardware supports it, there must be a way. The plan is something
like:

- at mmap time you return a pointer to something that is not actually
mapped, and do nothing else

- when userspace actually writes to that area, you get a page fault,
and nopage is called. At this point you map the page, and set the dirty
state. All other writes from userspace until the timer completes are
done without faulting.

- when the timer completes, you unmap the page so that the next access
will generate a fault again

As I said, I don't remember the exact details, but this should be doable.

--
Paulo Marques - http://www.grupopie.com

"The face of a child can say it all, especially the
mouth part of the face."

2006-10-30 17:32:40

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/30/06, Paulo Marques <[email protected]> wrote:
>
> No, this is not the sequence I thought of at all. I don't remember the
> exact API functions you need to call (I have to read LDD3 again ;), but
> if the hardware supports it, there must be a way. The plan is something
> like:
>
> - at mmap time you return a pointer to something that is not actually
> mapped, and do nothing else
>
> - when userspace actually writes to that area, you get a page fault,
> and nopage is called. At this point you map the page, and set the dirty
> state. All other writes from userspace until the timer completes are
> done without faulting.
>
> - when the timer completes, you unmap the page so that the next access
> will generate a fault again
>
> As I said, I don't remember the exact details, but this should be doable.
>

Yes, I get the idea, you mean to "unmap" the page but don't remove the
vma so a page fault is raised and nopage() op must be called again.
May it decrease performance? (Linux /you must take care of a page
fault calling nopage() each time you write/refresh the LCD, then
actually use it).

Miguel Ojeda

2006-10-30 20:45:31

by Paulo Marques

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

Miguel Ojeda wrote:
>[...]
> Yes, I get the idea, you mean to "unmap" the page but don't remove the
> vma so a page fault is raised and nopage() op must be called again.
> May it decrease performance? (Linux /you must take care of a page
> fault calling nopage() each time you write/refresh the LCD, then
> actually use it).

Well, it's a trade-off: you pay a little extra when you write to the
display, but you pay _zero_ when you leave the display alone. And this
zero is an important concept for some applications like dynticks.

If you want to achieve deep sleep states on a laptop you might want to
be able to let it sleep for extended periods. Having a thread that wakes
up every 50 ms even when there is nothing to do is not so good in this
situation.

If your refresh rate is 20Hz you pay *at most* 20 minor faults per
second if the application is writing a lot to the display. That doesn't
sound so bad.

Even more, the application still as the option of using seek/write
instead of using mmap to avoid the page faults, but if you're writing a
lot, it might be better to take just one page fault and then write at
full speed than to pay the price of a seek/write every time.

I didn't include the locking in the description, though. If you're going
this way, you need to make sure that some of this operations are atomic
so that you don't mark the display as clean just as the userspace app is
dirtying it, leaving the physical display inconsistent until the next
refresh.

--
Paulo Marques - http://www.grupopie.com

"The face of a child can say it all, especially the
mouth part of the face."

2006-10-31 08:10:40

by Franck Bui-Huu

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

Paulo Marques wrote:
> Franck Bui-Huu wrote:
>> Miguel Ojeda wrote:
>>> [...]
>>> Anyway, an animation of 10 Hz wouldn't be fine at this
>>> kind of LCDs, so it is pointless which the refresh rate of the driver
>>> is, as it is not useful to display images as fast as the driver
>>> refresh the LCD.
>>
>> An application might want to display quickly a set of images, not for
>> doing animations but rather displaying 'fake' greyscale images.
>
> To do "fake" greyscale you would need to synchronize with the actual
> refresh of the controller or you will have very ugly aliasing artifacts.
>
> Since there is no hardware interface to know when the controller is
> refreshing, I don't think this is one viable usage scenario.
>

eh ?? Did you read my email before ? That was the point I was trying
to raise... and starting the refresh stuff _only_ when the device is
mmaped seems to me a good trade off.

Aynywas it seems that the discusion about the design is closed and
won't lead to interesting things...

bye
Franck

2006-10-31 14:12:34

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

On 10/31/06, Franck Bui-Huu <[email protected]> wrote:
> Paulo Marques wrote:
>
> starting the refresh stuff _only_ when the device is
> mmaped seems to me a good trade off.
>

Well, if someone writes to /dev/fbX the LCD won't get updated. We
would need also to start/stop at the write/release fbfops as well. Let
me check.

2006-10-31 14:54:49

by Paulo Marques

[permalink] [raw]
Subject: Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

Franck Bui-Huu wrote:
> Paulo Marques wrote:
>> Franck Bui-Huu wrote:
>>> An application might want to display quickly a set of images, not for
>>> doing animations but rather displaying 'fake' greyscale images.
>>
>> To do "fake" greyscale you would need to synchronize with the actual
>> refresh of the controller or you will have very ugly aliasing artifacts.
>>
>> Since there is no hardware interface to know when the controller is
>> refreshing, I don't think this is one viable usage scenario.
>
> eh ?? Did you read my email before ? That was the point I was trying
> to raise... and starting the refresh stuff _only_ when the device is
> mmaped seems to me a good trade off.

I think we are violently agreeing about the optimal way of doing things.

But maybe I didn't explain my point about the "fake" greyscale in the
best way, though.

There are two distinct "refresh"'s involved here: one is when the driver
writes its software buffer into the display internal memory using the
parallel port interface.

The other is when the actual display controller refresh that goes
through all the common lines, etc., using the values on its internal
memory to update the segment voltages.

The problem is that there is no way to know about the internal refresh
that the controller does. So if you update its memory very frequently to
try to produce a "fake" greyscale image, your updates will alias with
the refresh rate of the actual display controller and you will see all
sorts of strange effects on the display.

> Aynywas it seems that the discusion about the design is closed and
> won't lead to interesting things...

Only because we're mostly in agreement about what should be done ;)

But if you have other interesting things to suggest, I haven't seen
Miguel reject any suggestions by other developers, yet (very much on the
contrary, to be honest).

--
Paulo Marques
Software Development Department - Grupo PIE, S.A.
Phone: +351 252 290600, Fax: +351 252 290601
Web: http://www.grupopie.com

"The face of a child can say it all, especially the
mouth part of the face."