2006-11-14 19:41:43

by James Simmons

[permalink] [raw]
Subject: ACPI output/lcd/auxdisplay mess


I have noticed a bunch of patches coming in dealing with the same problem. How to handle displays
i.e LCD, CRT etc. We have a lcd class in video/backlight which no one is using. Because no one was
aware of it auxdisplay was created instead. Then we have the acpi code which wants a output class
to handle powering down the display device. Plus the acpi layer does something very similar to the
framebuffer with getting edids and data relating to the display device. We really should place all
this handling in one standard place. To do this I need to know what all of you require.


2006-11-14 22:26:20

by Miguel Ojeda

[permalink] [raw]
Subject: Re: ACPI output/lcd/auxdisplay mess

On 11/14/06, James Simmons <[email protected]> wrote:
>
> I have noticed a bunch of patches coming in dealing with the same problem. How to handle displays
> i.e LCD, CRT etc. We have a lcd class in video/backlight which no one is using. Because no one was
> aware of it auxdisplay was created instead. Then we have the acpi code which wants a output class

Well, we were aware of video/backlight/* (read below). Anyway,
auxdisplay doesn't create a class; it did in first versions, but right
now it behaves just like a framebuffer, no classes in the playground
(maybe you read a old version?).

> to handle powering down the display device. Plus the acpi layer does something very similar to the
> framebuffer with getting edids and data relating to the display device. We really should place all
> this handling in one standard place. To do this I need to know what all of you require.
>

auxdisplay was discussed and created because lcd/backlight mean a primary LCD:

"Backlight & LCD device support"
Enable this to be able to choose the drivers for controlling the
backlight and the LCD panel on some platforms, for example on PDAs.

However, auxdisplay means "auxiliary display device drivers", not _the
display_. In such folder we can put every
auxiliar-optional-secundary-rare display (not just LCDs, framebuffers,
...) who has special requirements (like parport wiring, fixed refresh
rate, different properties...). Also, things like "set_contrast",
"max_constrast", "set_power"... didn't seem very appropriate.

There was a long discussion about this stuff and someone pointed out
the existence of video/backlight/*, so, well, we were aware of such
it. I think people prefered drivers/auxdisplay.

As people wish,

Miguel Ojeda

--
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm

2006-11-15 00:54:56

by James Simmons

[permalink] [raw]
Subject: Re: ACPI output/lcd/auxdisplay mess


> Well, we were aware of video/backlight/* (read below). Anyway,
> auxdisplay doesn't create a class; it did in first versions, but right
> now it behaves just like a framebuffer, no classes in the playground
> (maybe you read a old version?).
...
> However, auxdisplay means "auxiliary display device drivers", not _the
> display_. In such folder we can put every
> auxiliar-optional-secundary-rare display (not just LCDs, framebuffers,
> ...) who has special requirements (like parport wiring, fixed refresh
> rate, different properties...). Also, things like "set_contrast",
> "max_constrast", "set_power"... didn't seem very appropriate.

Is it a framebuffer device ? The framebuffer layer is abstracted to work
with such devices.

2006-11-16 08:45:07

by Miguel Ojeda

[permalink] [raw]
Subject: Re: ACPI output/lcd/auxdisplay mess

On 11/15/06, James Simmons <[email protected]> wrote:
>
> > Well, we were aware of video/backlight/* (read below). Anyway,
> > auxdisplay doesn't create a class; it did in first versions, but right
> > now it behaves just like a framebuffer, no classes in the playground
> > (maybe you read a old version?).
> ...
> > However, auxdisplay means "auxiliary display device drivers", not _the
> > display_. In such folder we can put every
> > auxiliar-optional-secundary-rare display (not just LCDs, framebuffers,
> > ...) who has special requirements (like parport wiring, fixed refresh
> > rate, different properties...). Also, things like "set_contrast",
> > "max_constrast", "set_power"... didn't seem very appropriate.
>
> Is it a framebuffer device ? The framebuffer layer is abstracted to work
> with such devices.
>

cfag12864bcfb is a "fbdev" (actually, it is a "fb wrapper" for
cfag12864b, so it behaves like a framebuffer, although it is not an
usual framebuffer. f.e. it has asynchronous refresh rate, a mmaped
page to appear to be a fb...).

Still, it is not the front panel lcd of any specific device like PDA,
so people that expects only their primary video/ displays may be
confused if it appears at such section. So we decided to go away from
video/. Maybe we can change the description, as right now it only
refers to front panel lcds.

--
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm

2006-11-16 15:38:43

by James Simmons

[permalink] [raw]
Subject: Re: ACPI output/lcd/auxdisplay mess


> > Is it a framebuffer device ? The framebuffer layer is abstracted to work
> > with such devices.
> >
>
> cfag12864bcfb is a "fbdev" (actually, it is a "fb wrapper" for
> cfag12864b, so it behaves like a framebuffer, although it is not an
> usual framebuffer. f.e. it has asynchronous refresh rate, a mmaped
> page to appear to be a fb...).

BTW to use it as a fb you need to set the FILLRECT etc. See Kconfig in the
drivers/video directory and look at one of the graphic card examples.

> Still, it is not the front panel lcd of any specific device like PDA,
> so people that expects only their primary video/ displays may be
> confused if it appears at such section. So we decided to go away from
> video/. Maybe we can change the description, as right now it only
> refers to front panel lcds.

Neither is a monitor for a PC desktop. That is why we have ddc. If I
take a desktop with more than one video card and swap the lcd monitors
the lcd monitor data remains the same. As soon as the display device is
attached to the graphics card the graphics card will then communicate
with the monitor to retrieve data. For example if the mode of the
graphics card is set to 1900x1080 which is supported by the current
monitor. Then we swap it for a CRT that supports only 1280x1024 then in
that case when the graphics card probes the CRT it will change the
resolution to the maximum that is supported by the CRT.
Currently the fbdev layer handles all this with struct fb_monspecs. Now
I know that structure doesn't cover everything. Nor does it handle
multiple displays attached to one piece of hardware. These where things I
was hoping to fix. Now that there are display devices that can handle
there own power management I have no problem having another sysfs device
to handle it. A representation that is more generic than lcd in the
backlight directory. Like the output device suggested by Yu. Of course I'm
not fond of that name. Display would be better.

2006-11-16 21:48:23

by Miguel Ojeda

[permalink] [raw]
Subject: Re: ACPI output/lcd/auxdisplay mess

On 11/16/06, James Simmons <[email protected]> wrote:
>
> >
> > cfag12864bcfb is a "fbdev" (actually, it is a "fb wrapper" for
> > cfag12864b, so it behaves like a framebuffer, although it is not an
> > usual framebuffer. f.e. it has asynchronous refresh rate, a mmaped
> > page to appear to be a fb...).
>
> BTW to use it as a fb you need to set the FILLRECT etc. See Kconfig in the
> drivers/video directory and look at one of the graphic card examples.

I see, thanks you. I will take care of that.

>
> > Still, it is not the front panel lcd of any specific device like PDA,
> > so people that expects only their primary video/ displays may be
> > confused if it appears at such section. So we decided to go away from
> > video/. Maybe we can change the description, as right now it only
> > refers to front panel lcds.
>
> Neither is a monitor for a PC desktop. That is why we have ddc. If I
> take a desktop with more than one video card and swap the lcd monitors
> the lcd monitor data remains the same. As soon as the display device is
> attached to the graphics card the graphics card will then communicate
> with the monitor to retrieve data. For example if the mode of the
> graphics card is set to 1900x1080 which is supported by the current
> monitor. Then we swap it for a CRT that supports only 1280x1024 then in
> that case when the graphics card probes the CRT it will change the
> resolution to the maximum that is supported by the CRT.
> Currently the fbdev layer handles all this with struct fb_monspecs. Now
> I know that structure doesn't cover everything. Nor does it handle
> multiple displays attached to one piece of hardware. These where things I
> was hoping to fix. Now that there are display devices that can handle
> there own power management I have no problem having another sysfs device
> to handle it. A representation that is more generic than lcd in the
> backlight directory. Like the output device suggested by Yu. Of course I'm
> not fond of that name. Display would be better.
>

Yep, indeed it would be better to have both generic place and sysfs
device to put all these generic displays, however, I think they
shouldn't be at video/* (maybe video/something/* or outside). I mean,
we can do something like:

1. First, we move auxiliary-special displays (including cfag12864b,
[*]"Arc Monochrome LCD board" and stuff like that: they aren't really
primary video displays) to another folder outside video/, maybe
"drivers/display/" ("drivers/auxdisplay/"...).
2. Then we create a sysfs device, called "display" ("auxdisplay"...)
for all of them, which must be generic as you suggested, not just lcds
or "front panels".

So finally we will have usual-primary fbdevs and video drivers at
"drivers/video/*", and all the other stuff at "drivers/display/*" or
some similar place.

Is that a good suggestion?

[*] I saw fbdevs like "Arc Monochorme LCD board support" that can be
put there the same way like auxdisplay/cfag12864b, as they look pretty
similar.

--
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm

2006-12-05 18:03:45

by James Simmons

[permalink] [raw]
Subject: Display class


This is the pass at a display class to meet the needs of the output class
of Mr. Yu for acpi. Also this class could in time replace the lcd class
located in the backlight directory since a lcd is a type of display.
The final hope is that the purpose auxdisplay could fall under this
catergory.

P.S
I know the edid parsing would have to be pulled out of the fbdev layer.


diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/acpi/video.c fbdev-2.6/drivers/acpi/video.c
--- linus-2.6/drivers/acpi/video.c 2006-11-07 05:38:34.000000000 -0500
+++ fbdev-2.6/drivers/acpi/video.c 2006-12-04 10:40:48.000000000 -0500
@@ -30,6 +30,9 @@
#include <linux/list.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
+#if defined(CONFIG_DISPLAY_DDC)
+#include <linux/display.h>
+#endif

#include <asm/uaccess.h>

@@ -154,6 +157,20 @@
int *levels;
};

+#if defined(CONFIG_DISPLAY_DDC)
+static int
+acpi_display_get_power(struct display_device *dev)
+{
+ return 0;
+}
+
+static int
+acpi_display_set_power(struct display_device *dev)
+{
+ return 0;
+}
+#endif
+
struct acpi_video_device {
unsigned long device_id;
struct acpi_video_device_flags flags;
@@ -162,6 +179,10 @@
struct acpi_video_bus *video;
struct acpi_device *dev;
struct acpi_video_device_brightness *brightness;
+#if defined(CONFIG_DISPLAY_DDC)
+ struct display_properties acpi_display_properties;
+ struct display_device *display;
+#endif
};

/* bus */
@@ -399,6 +420,27 @@
return status;
}

+#if defined(CONFIG_DISPLAY_DDC)
+static void*
+acpi_display_get_EDID(struct acpi_video_device *dev)
+{
+ union acpi_object *edid = NULL;
+ void *data = NULL;
+ int status;
+
+ status = acpi_video_device_EDID(dev, &edid, 128);
+ if (ACPI_FAILURE(status))
+ status = acpi_video_device_EDID(dev, &edid, 256);
+
+ if (ACPI_SUCCESS(status)) {
+ if (edid && edid->type == ACPI_TYPE_BUFFER) {
+ data = edid->buffer.pointer;
+ }
+ }
+ return data;
+}
+#endif
+
/* bus */

static int
@@ -1255,7 +1297,6 @@
int status;
struct acpi_video_device *data;

-
if (!device || !video)
return -EINVAL;

@@ -1263,12 +1304,10 @@
acpi_evaluate_integer(device->handle, "_ADR", NULL, &device_id);
if (ACPI_SUCCESS(status)) {

- data = kmalloc(sizeof(struct acpi_video_device), GFP_KERNEL);
+ data = kzalloc(sizeof(struct acpi_video_device), GFP_KERNEL);
if (!data)
return -ENOMEM;

- memset(data, 0, sizeof(struct acpi_video_device));
-
strcpy(acpi_device_name(device), ACPI_VIDEO_DEVICE_NAME);
strcpy(acpi_device_class(device), ACPI_VIDEO_CLASS);
acpi_driver_data(device) = data;
@@ -1295,6 +1334,13 @@
acpi_video_device_bind(video, data);
acpi_video_device_find_cap(data);

+#ifdef CONFIG_DISPLAY_DDC
+ data->acpi_display_properties.get_power = acpi_display_get_power;
+ data->acpi_display_properties.set_power = acpi_display_set_power;
+ data->acpi_display_properties.probe = probe_edid;
+ data->display = display_device_register(NULL, acpi_display_get_EDID(data),
+ &data->acpi_display_properties);
+#endif
status = acpi_install_notify_handler(device->handle,
ACPI_DEVICE_NOTIFY,
acpi_video_device_notify,
diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/display-ddc.c fbdev-2.6/drivers/video/display/display-ddc.c
--- linus-2.6/drivers/video/display/display-ddc.c 1969-12-31 19:00:00.000000000 -0500
+++ fbdev-2.6/drivers/video/display/display-ddc.c 2006-12-03 05:27:00.000000000 -0500
@@ -0,0 +1,45 @@
+/*
+ * display-ddc.c - EDID paring code
+ *
+ * Copyright (C) 2006 James Simmons <[email protected]>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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/display.h>
+#include <linux/err.h>
+#include <linux/ctype.h>
+
+#include <linux/fb.h>
+
+int probe_edid(struct display_device *dev, void *data)
+{
+ struct fb_monspecs spec;
+ ssize_t size = 45;
+
+ dev->name = kzalloc(size, GFP_KERNEL);
+ fb_edid_to_monspecs((unsigned char *) data, &spec);
+ strcpy(dev->name, spec.manufacturer);
+ return snprintf(dev->name, size, "%s %s %s\n", spec.manufacturer, spec.monitor, spec.ascii);
+}
+EXPORT_SYMBOL(probe_edid);
+
+MODULE_DESCRIPTION("Display Hardware handling");
+MODULE_AUTHOR("James Simmons <[email protected]>");
+MODULE_LICENSE("GPL");
diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/display-sysfs.c fbdev-2.6/drivers/video/display/display-sysfs.c
--- linus-2.6/drivers/video/display/display-sysfs.c 1969-12-31 19:00:00.000000000 -0500
+++ fbdev-2.6/drivers/video/display/display-sysfs.c 2006-12-04 07:48:34.000000000 -0500
@@ -0,0 +1,164 @@
+/*
+ * display.c - Display output driver
+ *
+ * Copyright (C) 2006 James Simmons <[email protected]>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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/display.h>
+#include <linux/err.h>
+#include <linux/ctype.h>
+
+static ssize_t display_show_name(struct class_device *dev, char *buf)
+{
+ struct display_device *dsp = to_display_device(dev);
+ return snprintf(buf, PAGE_SIZE, "%s\n", dsp->name);
+}
+
+static ssize_t display_show_power(struct class_device *dev, char *buf)
+{
+ struct display_device *dsp = to_display_device(dev);
+ ssize_t ret = -ENXIO;
+
+ mutex_lock(&dsp->lock);
+ if (likely(dsp->driver->get_power))
+ ret = sprintf(buf,"%.8x\n", dsp->driver->get_power(dsp));
+ mutex_unlock(&dsp->lock);
+ return ret;
+}
+
+static ssize_t display_store_power(struct class_device *dev,
+ const char *buf,size_t count)
+{
+ struct display_device *dsp = to_display_device(dev);
+ size_t size;
+ char *endp;
+ int power;
+
+ power = simple_strtoul(buf, &endp, 0);
+ size = endp - buf;
+ if (*endp && isspace(*endp))
+ size++;
+ if (size != count)
+ return -EINVAL;
+
+ mutex_lock(&dsp->lock);
+ if (likely(dsp->driver->set_power)) {
+ dsp->request_state = power;
+ dsp->driver->set_power(dsp);
+ }
+ mutex_unlock(&dsp->lock);
+ return count;
+}
+
+static void display_class_release(struct class_device *dev)
+{
+ struct display_device *dsp = to_display_device(dev);
+ kfree(dsp);
+}
+
+static struct class_device_attribute display_attributes[] = {
+ __ATTR(name, S_IRUGO, display_show_name, NULL),
+ __ATTR(power, S_IRUGO | S_IWUSR, display_show_power, display_store_power),
+};
+
+static struct class display_class = {
+ .name = "display",
+ .release = display_class_release,
+ .class_dev_attrs = display_attributes,
+};
+
+static int index;
+
+struct display_device *display_device_register(struct display_driver *driver,
+ struct device *dev, void *devdata)
+{
+ struct display_device *new_dev;
+ int ret = -ENOMEM, i;
+
+ if (unlikely(!driver))
+ return ERR_PTR(-EINVAL);
+
+ new_dev = kzalloc(sizeof(struct display_device), GFP_KERNEL);
+ if (likely(new_dev)) {
+ driver->probe(new_dev, devdata);
+
+ mutex_init(&new_dev->lock);
+ new_dev->driver = driver;
+ new_dev->class_dev.class = &display_class;
+ new_dev->class_dev.dev = dev;
+ sprintf(new_dev->class_dev.class_id, "display%d", index++);
+ class_set_devdata(&new_dev->class_dev, devdata);
+ ret = class_device_register(&new_dev->class_dev);
+
+ if (likely(ret)) {
+ for (i = 0; i < ARRAY_SIZE(display_attributes); i++) {
+ ret = class_device_create_file(&new_dev->class_dev,
+ &display_attributes[i]);
+ if (unlikely(ret)) {
+ while (--i >= 0)
+ class_device_remove_file(&new_dev->class_dev,
+ &display_attributes[i]);
+ class_device_unregister(&new_dev->class_dev);
+ }
+ }
+ }
+ }
+ if (unlikely(ret)) {
+ printk("failed to register display device\n");
+ kfree(new_dev);
+ new_dev = ERR_PTR(ret);
+ }
+ return new_dev;
+}
+EXPORT_SYMBOL(display_device_register);
+
+void display_device_unregister(struct display_device *dev)
+{
+ int i;
+
+ if (!dev)
+ return;
+ mutex_lock(&dev->lock);
+ dev->driver = NULL;
+ for (i = 0; i < ARRAY_SIZE(display_attributes); i++)
+ class_device_remove_file(&dev->class_dev,
+ &display_attributes[i]);
+ mutex_unlock(&dev->lock);
+ class_device_unregister(&dev->class_dev);
+}
+EXPORT_SYMBOL(display_device_unregister);
+
+static void __exit display_class_exit(void)
+{
+ class_unregister(&display_class);
+}
+
+static int __init display_class_init(void)
+{
+ return class_register(&display_class);
+}
+
+postcore_initcall(display_class_init);
+module_exit(display_class_exit);
+
+MODULE_DESCRIPTION("Display Hardware handling");
+MODULE_AUTHOR("James Simmons <[email protected]>");
+MODULE_LICENSE("GPL");
diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/Kconfig fbdev-2.6/drivers/video/display/Kconfig
--- linus-2.6/drivers/video/display/Kconfig 1969-12-31 19:00:00.000000000 -0500
+++ fbdev-2.6/drivers/video/display/Kconfig 2006-12-04 11:41:03.000000000 -0500
@@ -0,0 +1,29 @@
+#
+# Display drivers configuration
+#
+
+menu "Display device support"
+
+config DISPLAY_SUPPORT
+ tristate "Display panel/monitor support"
+ ---help---
+ This framework adds support for low-level control of a display.
+ This includes support for power.
+
+ Enable this to be able to choose the drivers for controlling the
+ physical display panel/monitor on some platforms. This not only
+ covers LCD displays for PDAs but also other types of displays
+ such as CRT, TVout etc.
+
+ To have support for your specific display panel you will have to
+ select the proper drivers which depend on this option.
+
+config DISPLAY_DDC
+ tristate "Enable EDID parsing"
+ ---help---
+ Enable EDID parsing
+
+comment "Display hardware drivers"
+ depends on DISPLAY_SUPPORT
+
+endmenu
diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/Makefile fbdev-2.6/drivers/video/display/Makefile
--- linus-2.6/drivers/video/display/Makefile 1969-12-31 19:00:00.000000000 -0500
+++ fbdev-2.6/drivers/video/display/Makefile 2006-12-03 05:02:07.000000000 -0500
@@ -0,0 +1,7 @@
+# Display drivers
+
+display-objs := display-sysfs.o
+
+obj-$(CONFIG_DISPLAY_SUPPORT) += display.o
+
+obj-$(CONFIG_DISPLAY_DDC) += display-ddc.o
diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/Kconfig fbdev-2.6/drivers/video/Kconfig
--- linus-2.6/drivers/video/Kconfig 2006-11-30 05:06:13.000000000 -0500
+++ fbdev-2.6/drivers/video/Kconfig 2006-12-04 10:40:48.000000000 -0500
@@ -4,21 +4,6 @@

menu "Graphics support"

-config FIRMWARE_EDID
- bool "Enable firmware EDID"
- default y
- ---help---
- This enables access to the EDID transferred from the firmware.
- On the i386, this is from the Video BIOS. Enable this if DDC/I2C
- transfers do not work for your driver and if you are using
- nvidiafb, i810fb or savagefb.
-
- In general, choosing Y for this option is safe. If you
- experience extremely long delays while booting before you get
- something on your display, try setting this to N. Matrox cards in
- combination with certain motherboards and monitors are known to
- suffer from this problem.
-
config FB
tristate "Support for frame buffer devices"
---help---
@@ -53,6 +38,22 @@
(e.g. an accelerated X server) and that are not frame buffer
device-aware may cause unexpected results. If unsure, say N.

+config FIRMWARE_EDID
+ bool "Enable firmware EDID"
+ depends on FB
+ default n
+ ---help---
+ This enables access to the EDID transferred from the firmware.
+ On the i386, this is from the Video BIOS. Enable this if DDC/I2C
+ transfers do not work for your driver and if you are using
+ nvidiafb, i810fb or savagefb.
+
+ In general, choosing Y for this option is safe. If you
+ experience extremely long delays while booting before you get
+ something on your display, try setting this to N. Matrox cards in
+ combination with certain motherboards and monitors are known to
+ suffer from this problem.
+
config FB_DDC
tristate
depends on FB && I2C && I2C_ALGOBIT
@@ -126,6 +127,9 @@
This is particularly important to one driver, matroxfb. If
unsure, say N.

+comment "Frambuffer hardware drivers"
+ depends on FB
+
config FB_CIRRUS
tristate "Cirrus Logic support"
depends on FB && (ZORRO || PCI)
@@ -1641,6 +1647,7 @@
endif

if SYSFS
+ source "drivers/video/display/Kconfig"
source "drivers/video/backlight/Kconfig"
endif

diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/Makefile fbdev-2.6/drivers/video/Makefile
--- linus-2.6/drivers/video/Makefile 2006-11-07 05:38:36.000000000 -0500
+++ fbdev-2.6/drivers/video/Makefile 2006-12-04 10:40:48.000000000 -0500
@@ -12,7 +12,7 @@

obj-$(CONFIG_VT) += console/
obj-$(CONFIG_LOGO) += logo/
-obj-$(CONFIG_SYSFS) += backlight/
+obj-$(CONFIG_SYSFS) += backlight/ display/

obj-$(CONFIG_FB_CFB_FILLRECT) += cfbfillrect.o
obj-$(CONFIG_FB_CFB_COPYAREA) += cfbcopyarea.o
diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/include/linux/display.h fbdev-2.6/include/linux/display.h
--- linus-2.6/include/linux/display.h 1969-12-31 19:00:00.000000000 -0500
+++ fbdev-2.6/include/linux/display.h 2006-12-04 07:42:29.000000000 -0500
@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2006 James Simmons <[email protected]>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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 _LINUX_DISPLAY_H
+#define _LINUX_DISPLAY_H
+
+#include <linux/device.h>
+
+struct display_device;
+
+/* This structure defines all the properties of a Display. */
+struct display_driver {
+ int (*set_power)(struct display_device *);
+ int (*get_power)(struct display_device *);
+ int (*probe)(struct display_device *, void *);
+ int (*remove)(struct display_device *);
+};
+
+struct display_device {
+ struct module *owner; /* Owner module */
+ char *name;
+ struct mutex lock;
+ int request_state;
+ struct display_driver *driver;
+ struct class_device class_dev; /* The class device structure */
+};
+
+extern struct display_device *display_device_register(struct display_driver *driver,
+ struct device *dev, void *devdata);
+extern void display_device_unregister(struct display_device *dev);
+
+extern int probe_edid(struct display_device *dev, void *devdata);
+
+#define to_display_device(obj) container_of(obj, struct display_device, class_dev)
+
+#endif
diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/MAINTAINERS fbdev-2.6/MAINTAINERS
--- linus-2.6/MAINTAINERS 2006-12-04 04:11:38.000000000 -0500
+++ fbdev-2.6/MAINTAINERS 2006-12-04 10:40:48.000000000 -0500
@@ -911,6 +923,12 @@
L: [email protected]
S: Maintained

+DISPLAY SUBSYSTEM
+P: James Simmons
+M: [email protected]
+L: [email protected]
+S: Maintained
+
DISTRIBUTED LOCK MANAGER
P: Patrick Caulfield
M: [email protected]

2006-12-06 01:13:52

by Randy Dunlap

[permalink] [raw]
Subject: Re: Display class

On Tue, 5 Dec 2006 18:03:39 +0000 (GMT) James Simmons wrote:

> This is the pass at a display class to meet the needs of the output class
> of Mr. Yu for acpi. Also this class could in time replace the lcd class
> located in the backlight directory since a lcd is a type of display.
> The final hope is that the purpose auxdisplay could fall under this
> catergory.
>
> P.S
> I know the edid parsing would have to be pulled out of the fbdev layer.

Hi,

Where is "struct display_properties" defined?

CC [M] drivers/acpi/video.o
drivers/acpi/video.c:183: error: field 'acpi_display_properties' has incomplete type
make[2]: *** [drivers/acpi/video.o] Error 1

Lots of style issues (see below).

> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/acpi/video.c fbdev-2.6/drivers/acpi/video.c
> --- linus-2.6/drivers/acpi/video.c 2006-11-07 05:38:34.000000000 -0500
> +++ fbdev-2.6/drivers/acpi/video.c 2006-12-04 10:40:48.000000000 -0500
> @@ -30,6 +30,9 @@
> #include <linux/list.h>
> #include <linux/proc_fs.h>
> #include <linux/seq_file.h>
> +#if defined(CONFIG_DISPLAY_DDC)
> +#include <linux/display.h>
> +#endif
>
> #include <asm/uaccess.h>
>
> @@ -154,6 +157,20 @@
> int *levels;
> };
>
> +#if defined(CONFIG_DISPLAY_DDC)
> +static int
> +acpi_display_get_power(struct display_device *dev)

Combine those 2 lines into 1 line.

> +{
> + return 0;
> +}
> +
> +static int
> +acpi_display_set_power(struct display_device *dev)

Ditto.

> +{
> + return 0;
> +}
> +#endif
> +
> struct acpi_video_device {
> unsigned long device_id;
> struct acpi_video_device_flags flags;
> @@ -162,6 +179,10 @@
> struct acpi_video_bus *video;
> struct acpi_device *dev;
> struct acpi_video_device_brightness *brightness;
> +#if defined(CONFIG_DISPLAY_DDC)
> + struct display_properties acpi_display_properties;
> + struct display_device *display;
> +#endif
> };
>
> /* bus */
> @@ -399,6 +420,27 @@
> return status;
> }
>
> +#if defined(CONFIG_DISPLAY_DDC)
> +static void*
> +acpi_display_get_EDID(struct acpi_video_device *dev)

Combine and place '*' immediately before the function name,
not immediately after "void".

> +{
> + union acpi_object *edid = NULL;
> + void *data = NULL;
> + int status;
> +
> + status = acpi_video_device_EDID(dev, &edid, 128);
> + if (ACPI_FAILURE(status))
> + status = acpi_video_device_EDID(dev, &edid, 256);
> +
> + if (ACPI_SUCCESS(status)) {
> + if (edid && edid->type == ACPI_TYPE_BUFFER) {
> + data = edid->buffer.pointer;

No braces for one-statement "blocks".

> + }
> + }
> + return data;
> +}
> +#endif
> +
> /* bus */
>
> static int

> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/display-ddc.c fbdev-2.6/drivers/video/display/display-ddc.c
> --- linus-2.6/drivers/video/display/display-ddc.c 1969-12-31 19:00:00.000000000 -0500
> +++ fbdev-2.6/drivers/video/display/display-ddc.c 2006-12-03 05:27:00.000000000 -0500
> @@ -0,0 +1,45 @@
> +/*
> + * display-ddc.c - EDID paring code

typo: parsing

> +#include <linux/module.h>
> +#include <linux/display.h>
> +#include <linux/err.h>
> +#include <linux/ctype.h>
> +
> +#include <linux/fb.h>
> +
> +int probe_edid(struct display_device *dev, void *data)
> +{
> + struct fb_monspecs spec;
> + ssize_t size = 45;

Where does the magic # 45 come from?
Can you use a #define or sizeof(X) for it instead?

> +
> + dev->name = kzalloc(size, GFP_KERNEL);
> + fb_edid_to_monspecs((unsigned char *) data, &spec);
> + strcpy(dev->name, spec.manufacturer);
> + return snprintf(dev->name, size, "%s %s %s\n", spec.manufacturer, spec.monitor, spec.ascii);

Try to limit source lines to < 80 columns.

> +}
> +EXPORT_SYMBOL(probe_edid);
> +
> +MODULE_DESCRIPTION("Display Hardware handling");
> +MODULE_AUTHOR("James Simmons <[email protected]>");
> +MODULE_LICENSE("GPL");

> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/display-sysfs.c fbdev-2.6/drivers/video/display/display-sysfs.c
> --- linus-2.6/drivers/video/display/display-sysfs.c 1969-12-31 19:00:00.000000000 -0500
> +++ fbdev-2.6/drivers/video/display/display-sysfs.c 2006-12-04 07:48:34.000000000 -0500
> @@ -0,0 +1,164 @@
> +#include <linux/module.h>
> +#include <linux/display.h>
> +#include <linux/err.h>
> +#include <linux/ctype.h>
> +
> +struct display_device *display_device_register(struct display_driver *driver,
> + struct device *dev, void *devdata)
> +{
> + struct display_device *new_dev;
> + int ret = -ENOMEM, i;
> +
> + if (unlikely(!driver))
> + return ERR_PTR(-EINVAL);
> +
> + new_dev = kzalloc(sizeof(struct display_device), GFP_KERNEL);
> + if (likely(new_dev)) {
> + driver->probe(new_dev, devdata);
> +
> + mutex_init(&new_dev->lock);
> + new_dev->driver = driver;
> + new_dev->class_dev.class = &display_class;
> + new_dev->class_dev.dev = dev;
> + sprintf(new_dev->class_dev.class_id, "display%d", index++);
> + class_set_devdata(&new_dev->class_dev, devdata);
> + ret = class_device_register(&new_dev->class_dev);
> +
> + if (likely(ret)) {
> + for (i = 0; i < ARRAY_SIZE(display_attributes); i++) {
> + ret = class_device_create_file(&new_dev->class_dev,
> + &display_attributes[i]);
> + if (unlikely(ret)) {
> + while (--i >= 0)
> + class_device_remove_file(&new_dev->class_dev,
> + &display_attributes[i]);
> + class_device_unregister(&new_dev->class_dev);

Several lines > 80 columns there.

> + }
> + }
> + }
> + }
> + if (unlikely(ret)) {
> + printk("failed to register display device\n");

* KERN_ERR or some such printk log level

> + kfree(new_dev);
> + new_dev = ERR_PTR(ret);
> + }
> + return new_dev;
> +}
> +EXPORT_SYMBOL(display_device_register);
> +
> +void display_device_unregister(struct display_device *dev)
> +{
> + int i;
> +
> + if (!dev)
> + return;
> + mutex_lock(&dev->lock);
> + dev->driver = NULL;
> + for (i = 0; i < ARRAY_SIZE(display_attributes); i++)
> + class_device_remove_file(&dev->class_dev,
> + &display_attributes[i]);
> + mutex_unlock(&dev->lock);
> + class_device_unregister(&dev->class_dev);
> +}
> +EXPORT_SYMBOL(display_device_unregister);
> +
> +static void __exit display_class_exit(void)
> +{
> + class_unregister(&display_class);
> +}
> +
> +static int __init display_class_init(void)
> +{
> + return class_register(&display_class);
> +}
> +
> +postcore_initcall(display_class_init);
> +module_exit(display_class_exit);
> +
> +MODULE_DESCRIPTION("Display Hardware handling");
> +MODULE_AUTHOR("James Simmons <[email protected]>");
> +MODULE_LICENSE("GPL");

> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/Kconfig fbdev-2.6/drivers/video/display/Kconfig
> --- linus-2.6/drivers/video/display/Kconfig 1969-12-31 19:00:00.000000000 -0500
> +++ fbdev-2.6/drivers/video/display/Kconfig 2006-12-04 11:41:03.000000000 -0500
> @@ -0,0 +1,29 @@
> +#
> +# Display drivers configuration
> +#
> +
> +menu "Display device support"
> +
> +config DISPLAY_SUPPORT
> + tristate "Display panel/monitor support"
> + ---help---
> + This framework adds support for low-level control of a display.
> + This includes support for power.
> +
> + Enable this to be able to choose the drivers for controlling the
> + physical display panel/monitor on some platforms. This not only
> + covers LCD displays for PDAs but also other types of displays
> + such as CRT, TVout etc.
> +
> + To have support for your specific display panel you will have to
> + select the proper drivers which depend on this option.
> +
> +config DISPLAY_DDC
> + tristate "Enable EDID parsing"
> + ---help---
> + Enable EDID parsing

"Enable" makes it sound like a boolean, not a tristate....

> +
> +comment "Display hardware drivers"
> + depends on DISPLAY_SUPPORT
> +
> +endmenu
> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/Makefile fbdev-2.6/drivers/video/display/Makefile
> --- linus-2.6/drivers/video/display/Makefile 1969-12-31 19:00:00.000000000 -0500
> +++ fbdev-2.6/drivers/video/display/Makefile 2006-12-03 05:02:07.000000000 -0500
> @@ -0,0 +1,7 @@
> +# Display drivers
> +
> +display-objs := display-sysfs.o
> +
> +obj-$(CONFIG_DISPLAY_SUPPORT) += display.o
> +
> +obj-$(CONFIG_DISPLAY_DDC) += display-ddc.o
> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/Kconfig fbdev-2.6/drivers/video/Kconfig
> --- linus-2.6/drivers/video/Kconfig 2006-11-30 05:06:13.000000000 -0500
> +++ fbdev-2.6/drivers/video/Kconfig 2006-12-04 10:40:48.000000000 -0500
> @@ -4,21 +4,6 @@
>
> menu "Graphics support"
>
> -config FIRMWARE_EDID
> - bool "Enable firmware EDID"
> - default y
> - ---help---
> - This enables access to the EDID transferred from the firmware.
> - On the i386, this is from the Video BIOS. Enable this if DDC/I2C
> - transfers do not work for your driver and if you are using
> - nvidiafb, i810fb or savagefb.
> -
> - In general, choosing Y for this option is safe. If you
> - experience extremely long delays while booting before you get
> - something on your display, try setting this to N. Matrox cards in
> - combination with certain motherboards and monitors are known to
> - suffer from this problem.
> -
> config FB
> tristate "Support for frame buffer devices"
> ---help---
> @@ -53,6 +38,22 @@
> (e.g. an accelerated X server) and that are not frame buffer
> device-aware may cause unexpected results. If unsure, say N.
>
> +config FIRMWARE_EDID
> + bool "Enable firmware EDID"
> + depends on FB
> + default n
> + ---help---
> + This enables access to the EDID transferred from the firmware.
> + On the i386, this is from the Video BIOS. Enable this if DDC/I2C
> + transfers do not work for your driver and if you are using
> + nvidiafb, i810fb or savagefb.
> +

All of the help text (above & below) should be indented like the
first line above is: one tab + 2 spaces.

> + In general, choosing Y for this option is safe. If you
> + experience extremely long delays while booting before you get
> + something on your display, try setting this to N. Matrox cards in
> + combination with certain motherboards and monitors are known to
> + suffer from this problem.
> +
> config FB_DDC
> tristate
> depends on FB && I2C && I2C_ALGOBIT

---
~Randy

2006-12-06 13:19:30

by Miguel Ojeda

[permalink] [raw]
Subject: Re: Display class

On 12/5/06, James Simmons <[email protected]> wrote:
>
> This is the pass at a display class to meet the needs of the output class
> of Mr. Yu for acpi. Also this class could in time replace the lcd class
> located in the backlight directory since a lcd is a type of display.
> The final hope is that the purpose auxdisplay could fall under this
> catergory.
>
> P.S
> I know the edid parsing would have to be pulled out of the fbdev layer.
>

Few comments about EXPORT_SYMBOL_GPL:

>
> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/acpi/video.c fbdev-2.6/drivers/acpi/video.c
> --- linus-2.6/drivers/acpi/video.c 2006-11-07 05:38:34.000000000 -0500
> +++ fbdev-2.6/drivers/acpi/video.c 2006-12-04 10:40:48.000000000 -0500
> @@ -30,6 +30,9 @@
> #include <linux/list.h>
> #include <linux/proc_fs.h>
> #include <linux/seq_file.h>
> +#if defined(CONFIG_DISPLAY_DDC)
> +#include <linux/display.h>
> +#endif
>
> #include <asm/uaccess.h>
>
> @@ -154,6 +157,20 @@
> int *levels;
> };
>
> +#if defined(CONFIG_DISPLAY_DDC)
> +static int
> +acpi_display_get_power(struct display_device *dev)
> +{
> + return 0;
> +}
> +
> +static int
> +acpi_display_set_power(struct display_device *dev)
> +{
> + return 0;
> +}
> +#endif
> +
> struct acpi_video_device {
> unsigned long device_id;
> struct acpi_video_device_flags flags;
> @@ -162,6 +179,10 @@
> struct acpi_video_bus *video;
> struct acpi_device *dev;
> struct acpi_video_device_brightness *brightness;
> +#if defined(CONFIG_DISPLAY_DDC)
> + struct display_properties acpi_display_properties;
> + struct display_device *display;
> +#endif
> };
>
> /* bus */
> @@ -399,6 +420,27 @@
> return status;
> }
>
> +#if defined(CONFIG_DISPLAY_DDC)
> +static void*
> +acpi_display_get_EDID(struct acpi_video_device *dev)
> +{
> + union acpi_object *edid = NULL;
> + void *data = NULL;
> + int status;
> +
> + status = acpi_video_device_EDID(dev, &edid, 128);
> + if (ACPI_FAILURE(status))
> + status = acpi_video_device_EDID(dev, &edid, 256);
> +
> + if (ACPI_SUCCESS(status)) {
> + if (edid && edid->type == ACPI_TYPE_BUFFER) {
> + data = edid->buffer.pointer;
> + }
> + }
> + return data;
> +}
> +#endif
> +
> /* bus */
>
> static int
> @@ -1255,7 +1297,6 @@
> int status;
> struct acpi_video_device *data;
>
> -
> if (!device || !video)
> return -EINVAL;
>
> @@ -1263,12 +1304,10 @@
> acpi_evaluate_integer(device->handle, "_ADR", NULL, &device_id);
> if (ACPI_SUCCESS(status)) {
>
> - data = kmalloc(sizeof(struct acpi_video_device), GFP_KERNEL);
> + data = kzalloc(sizeof(struct acpi_video_device), GFP_KERNEL);
> if (!data)
> return -ENOMEM;
>
> - memset(data, 0, sizeof(struct acpi_video_device));
> -
> strcpy(acpi_device_name(device), ACPI_VIDEO_DEVICE_NAME);
> strcpy(acpi_device_class(device), ACPI_VIDEO_CLASS);
> acpi_driver_data(device) = data;
> @@ -1295,6 +1334,13 @@
> acpi_video_device_bind(video, data);
> acpi_video_device_find_cap(data);
>
> +#ifdef CONFIG_DISPLAY_DDC
> + data->acpi_display_properties.get_power = acpi_display_get_power;
> + data->acpi_display_properties.set_power = acpi_display_set_power;
> + data->acpi_display_properties.probe = probe_edid;
> + data->display = display_device_register(NULL, acpi_display_get_EDID(data),
> + &data->acpi_display_properties);
> +#endif
> status = acpi_install_notify_handler(device->handle,
> ACPI_DEVICE_NOTIFY,
> acpi_video_device_notify,
> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/display-ddc.c fbdev-2.6/drivers/video/display/display-ddc.c
> --- linus-2.6/drivers/video/display/display-ddc.c 1969-12-31 19:00:00.000000000 -0500
> +++ fbdev-2.6/drivers/video/display/display-ddc.c 2006-12-03 05:27:00.000000000 -0500
> @@ -0,0 +1,45 @@
> +/*
> + * display-ddc.c - EDID paring code
> + *
> + * Copyright (C) 2006 James Simmons <[email protected]>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * 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/display.h>
> +#include <linux/err.h>
> +#include <linux/ctype.h>
> +
> +#include <linux/fb.h>
> +
> +int probe_edid(struct display_device *dev, void *data)
> +{
> + struct fb_monspecs spec;
> + ssize_t size = 45;
> +
> + dev->name = kzalloc(size, GFP_KERNEL);
> + fb_edid_to_monspecs((unsigned char *) data, &spec);
> + strcpy(dev->name, spec.manufacturer);
> + return snprintf(dev->name, size, "%s %s %s\n", spec.manufacturer, spec.monitor, spec.ascii);
> +}
> +EXPORT_SYMBOL(probe_edid);

Shouldn't be EXPORT_SYMBOL_GPL?

> +
> +MODULE_DESCRIPTION("Display Hardware handling");
> +MODULE_AUTHOR("James Simmons <[email protected]>");
> +MODULE_LICENSE("GPL");
> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/display-sysfs.c fbdev-2.6/drivers/video/display/display-sysfs.c
> --- linus-2.6/drivers/video/display/display-sysfs.c 1969-12-31 19:00:00.000000000 -0500
> +++ fbdev-2.6/drivers/video/display/display-sysfs.c 2006-12-04 07:48:34.000000000 -0500
> @@ -0,0 +1,164 @@
> +/*
> + * display.c - Display output driver
> + *
> + * Copyright (C) 2006 James Simmons <[email protected]>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * 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/display.h>
> +#include <linux/err.h>
> +#include <linux/ctype.h>
> +
> +static ssize_t display_show_name(struct class_device *dev, char *buf)
> +{
> + struct display_device *dsp = to_display_device(dev);
> + return snprintf(buf, PAGE_SIZE, "%s\n", dsp->name);
> +}
> +
> +static ssize_t display_show_power(struct class_device *dev, char *buf)
> +{
> + struct display_device *dsp = to_display_device(dev);
> + ssize_t ret = -ENXIO;
> +
> + mutex_lock(&dsp->lock);
> + if (likely(dsp->driver->get_power))
> + ret = sprintf(buf,"%.8x\n", dsp->driver->get_power(dsp));
> + mutex_unlock(&dsp->lock);
> + return ret;
> +}
> +
> +static ssize_t display_store_power(struct class_device *dev,
> + const char *buf,size_t count)
> +{
> + struct display_device *dsp = to_display_device(dev);
> + size_t size;
> + char *endp;
> + int power;
> +
> + power = simple_strtoul(buf, &endp, 0);
> + size = endp - buf;
> + if (*endp && isspace(*endp))
> + size++;
> + if (size != count)
> + return -EINVAL;
> +
> + mutex_lock(&dsp->lock);
> + if (likely(dsp->driver->set_power)) {
> + dsp->request_state = power;
> + dsp->driver->set_power(dsp);
> + }
> + mutex_unlock(&dsp->lock);
> + return count;
> +}
> +
> +static void display_class_release(struct class_device *dev)
> +{
> + struct display_device *dsp = to_display_device(dev);
> + kfree(dsp);
> +}
> +
> +static struct class_device_attribute display_attributes[] = {
> + __ATTR(name, S_IRUGO, display_show_name, NULL),
> + __ATTR(power, S_IRUGO | S_IWUSR, display_show_power, display_store_power),
> +};
> +
> +static struct class display_class = {
> + .name = "display",
> + .release = display_class_release,
> + .class_dev_attrs = display_attributes,
> +};
> +
> +static int index;
> +
> +struct display_device *display_device_register(struct display_driver *driver,
> + struct device *dev, void *devdata)
> +{
> + struct display_device *new_dev;
> + int ret = -ENOMEM, i;
> +
> + if (unlikely(!driver))
> + return ERR_PTR(-EINVAL);
> +
> + new_dev = kzalloc(sizeof(struct display_device), GFP_KERNEL);
> + if (likely(new_dev)) {
> + driver->probe(new_dev, devdata);
> +
> + mutex_init(&new_dev->lock);
> + new_dev->driver = driver;
> + new_dev->class_dev.class = &display_class;
> + new_dev->class_dev.dev = dev;
> + sprintf(new_dev->class_dev.class_id, "display%d", index++);
> + class_set_devdata(&new_dev->class_dev, devdata);
> + ret = class_device_register(&new_dev->class_dev);
> +
> + if (likely(ret)) {
> + for (i = 0; i < ARRAY_SIZE(display_attributes); i++) {
> + ret = class_device_create_file(&new_dev->class_dev,
> + &display_attributes[i]);
> + if (unlikely(ret)) {
> + while (--i >= 0)
> + class_device_remove_file(&new_dev->class_dev,
> + &display_attributes[i]);
> + class_device_unregister(&new_dev->class_dev);
> + }
> + }
> + }
> + }
> + if (unlikely(ret)) {
> + printk("failed to register display device\n");
> + kfree(new_dev);
> + new_dev = ERR_PTR(ret);
> + }
> + return new_dev;
> +}
> +EXPORT_SYMBOL(display_device_register);

Shouldn't be EXPORT_SYMBOL_GPL?

> +
> +void display_device_unregister(struct display_device *dev)
> +{
> + int i;
> +
> + if (!dev)
> + return;
> + mutex_lock(&dev->lock);
> + dev->driver = NULL;
> + for (i = 0; i < ARRAY_SIZE(display_attributes); i++)
> + class_device_remove_file(&dev->class_dev,
> + &display_attributes[i]);
> + mutex_unlock(&dev->lock);
> + class_device_unregister(&dev->class_dev);
> +}
> +EXPORT_SYMBOL(display_device_unregister);

Shouldn't be EXPORT_SYMBOL_GPL?

> +
> +static void __exit display_class_exit(void)
> +{
> + class_unregister(&display_class);
> +}
> +
> +static int __init display_class_init(void)
> +{
> + return class_register(&display_class);
> +}
> +
> +postcore_initcall(display_class_init);
> +module_exit(display_class_exit);
> +
> +MODULE_DESCRIPTION("Display Hardware handling");
> +MODULE_AUTHOR("James Simmons <[email protected]>");
> +MODULE_LICENSE("GPL");
> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/Kconfig fbdev-2.6/drivers/video/display/Kconfig
> --- linus-2.6/drivers/video/display/Kconfig 1969-12-31 19:00:00.000000000 -0500
> +++ fbdev-2.6/drivers/video/display/Kconfig 2006-12-04 11:41:03.000000000 -0500
> @@ -0,0 +1,29 @@
> +#
> +# Display drivers configuration
> +#
> +
> +menu "Display device support"
> +
> +config DISPLAY_SUPPORT
> + tristate "Display panel/monitor support"
> + ---help---
> + This framework adds support for low-level control of a display.
> + This includes support for power.
> +
> + Enable this to be able to choose the drivers for controlling the
> + physical display panel/monitor on some platforms. This not only
> + covers LCD displays for PDAs but also other types of displays
> + such as CRT, TVout etc.
> +
> + To have support for your specific display panel you will have to
> + select the proper drivers which depend on this option.
> +
> +config DISPLAY_DDC
> + tristate "Enable EDID parsing"
> + ---help---
> + Enable EDID parsing
> +
> +comment "Display hardware drivers"
> + depends on DISPLAY_SUPPORT
> +
> +endmenu
> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/Makefile fbdev-2.6/drivers/video/display/Makefile
> --- linus-2.6/drivers/video/display/Makefile 1969-12-31 19:00:00.000000000 -0500
> +++ fbdev-2.6/drivers/video/display/Makefile 2006-12-03 05:02:07.000000000 -0500
> @@ -0,0 +1,7 @@
> +# Display drivers
> +
> +display-objs := display-sysfs.o
> +
> +obj-$(CONFIG_DISPLAY_SUPPORT) += display.o
> +
> +obj-$(CONFIG_DISPLAY_DDC) += display-ddc.o
> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/Kconfig fbdev-2.6/drivers/video/Kconfig
> --- linus-2.6/drivers/video/Kconfig 2006-11-30 05:06:13.000000000 -0500
> +++ fbdev-2.6/drivers/video/Kconfig 2006-12-04 10:40:48.000000000 -0500
> @@ -4,21 +4,6 @@
>
> menu "Graphics support"
>
> -config FIRMWARE_EDID
> - bool "Enable firmware EDID"
> - default y
> - ---help---
> - This enables access to the EDID transferred from the firmware.
> - On the i386, this is from the Video BIOS. Enable this if DDC/I2C
> - transfers do not work for your driver and if you are using
> - nvidiafb, i810fb or savagefb.
> -
> - In general, choosing Y for this option is safe. If you
> - experience extremely long delays while booting before you get
> - something on your display, try setting this to N. Matrox cards in
> - combination with certain motherboards and monitors are known to
> - suffer from this problem.
> -
> config FB
> tristate "Support for frame buffer devices"
> ---help---
> @@ -53,6 +38,22 @@
> (e.g. an accelerated X server) and that are not frame buffer
> device-aware may cause unexpected results. If unsure, say N.
>
> +config FIRMWARE_EDID
> + bool "Enable firmware EDID"
> + depends on FB
> + default n
> + ---help---
> + This enables access to the EDID transferred from the firmware.
> + On the i386, this is from the Video BIOS. Enable this if DDC/I2C
> + transfers do not work for your driver and if you are using
> + nvidiafb, i810fb or savagefb.
> +
> + In general, choosing Y for this option is safe. If you
> + experience extremely long delays while booting before you get
> + something on your display, try setting this to N. Matrox cards in
> + combination with certain motherboards and monitors are known to
> + suffer from this problem.
> +
> config FB_DDC
> tristate
> depends on FB && I2C && I2C_ALGOBIT
> @@ -126,6 +127,9 @@
> This is particularly important to one driver, matroxfb. If
> unsure, say N.
>
> +comment "Frambuffer hardware drivers"
> + depends on FB
> +
> config FB_CIRRUS
> tristate "Cirrus Logic support"
> depends on FB && (ZORRO || PCI)
> @@ -1641,6 +1647,7 @@
> endif
>
> if SYSFS
> + source "drivers/video/display/Kconfig"
> source "drivers/video/backlight/Kconfig"
> endif
>
> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/Makefile fbdev-2.6/drivers/video/Makefile
> --- linus-2.6/drivers/video/Makefile 2006-11-07 05:38:36.000000000 -0500
> +++ fbdev-2.6/drivers/video/Makefile 2006-12-04 10:40:48.000000000 -0500
> @@ -12,7 +12,7 @@
>
> obj-$(CONFIG_VT) += console/
> obj-$(CONFIG_LOGO) += logo/
> -obj-$(CONFIG_SYSFS) += backlight/
> +obj-$(CONFIG_SYSFS) += backlight/ display/
>
> obj-$(CONFIG_FB_CFB_FILLRECT) += cfbfillrect.o
> obj-$(CONFIG_FB_CFB_COPYAREA) += cfbcopyarea.o
> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/include/linux/display.h fbdev-2.6/include/linux/display.h
> --- linus-2.6/include/linux/display.h 1969-12-31 19:00:00.000000000 -0500
> +++ fbdev-2.6/include/linux/display.h 2006-12-04 07:42:29.000000000 -0500
> @@ -0,0 +1,55 @@
> +/*
> + * Copyright (C) 2006 James Simmons <[email protected]>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * 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 _LINUX_DISPLAY_H
> +#define _LINUX_DISPLAY_H
> +
> +#include <linux/device.h>
> +
> +struct display_device;
> +
> +/* This structure defines all the properties of a Display. */
> +struct display_driver {
> + int (*set_power)(struct display_device *);
> + int (*get_power)(struct display_device *);
> + int (*probe)(struct display_device *, void *);
> + int (*remove)(struct display_device *);
> +};
> +
> +struct display_device {
> + struct module *owner; /* Owner module */
> + char *name;
> + struct mutex lock;
> + int request_state;
> + struct display_driver *driver;
> + struct class_device class_dev; /* The class device structure */
> +};
> +
> +extern struct display_device *display_device_register(struct display_driver *driver,
> + struct device *dev, void *devdata);
> +extern void display_device_unregister(struct display_device *dev);
> +
> +extern int probe_edid(struct display_device *dev, void *devdata);
> +
> +#define to_display_device(obj) container_of(obj, struct display_device, class_dev)
> +
> +#endif
> diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/MAINTAINERS fbdev-2.6/MAINTAINERS
> --- linus-2.6/MAINTAINERS 2006-12-04 04:11:38.000000000 -0500
> +++ fbdev-2.6/MAINTAINERS 2006-12-04 10:40:48.000000000 -0500
> @@ -911,6 +923,12 @@
> L: [email protected]
> S: Maintained
>
> +DISPLAY SUBSYSTEM
> +P: James Simmons
> +M: [email protected]
> +L: [email protected]
> +S: Maintained
> +
> DISTRIBUTED LOCK MANAGER
> P: Patrick Caulfield
> M: [email protected]
>


--
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm

2006-12-06 15:10:55

by James Simmons

[permalink] [raw]
Subject: Re: Display class


> > of Mr. Yu for acpi. Also this class could in time replace the lcd class
> > located in the backlight directory since a lcd is a type of display.
> > The final hope is that the purpose auxdisplay could fall under this
> > catergory.
> >
> > P.S
> > I know the edid parsing would have to be pulled out of the fbdev layer.

That patch was rought draft for feedback. I applied your comments. This
patch actually works. It includes my backlight fix as well.

diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/acpi/video.c fbdev-2.6/drivers/acpi/video.c
--- linus-2.6/drivers/acpi/video.c 2006-11-07 05:38:34.000000000 -0500
+++ fbdev-2.6/drivers/acpi/video.c 2006-12-06 05:01:22.000000000 -0500
@@ -30,6 +30,7 @@
#include <linux/list.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
+#include <linux/display.h>

#include <asm/uaccess.h>

@@ -154,6 +155,19 @@
int *levels;
};

+
+#if defined(CONFIG_DISPLAY_DDC) || defined(CONFIG_DISPLAY_DDC_MODULE)
+static int acpi_display_get_power(struct display_device *dev)
+{
+ return 0;
+}
+
+static int acpi_display_set_power(struct display_device *dev)
+{
+ return 0;
+}
+#endif
+
struct acpi_video_device {
unsigned long device_id;
struct acpi_video_device_flags flags;
@@ -162,6 +176,10 @@
struct acpi_video_bus *video;
struct acpi_device *dev;
struct acpi_video_device_brightness *brightness;
+#if defined(CONFIG_DISPLAY_DDC) || defined(CONFIG_DISPLAY_DDC_MODULE)
+ struct display_driver acpi_display_driver;
+ struct display_device *display;
+#endif
};

/* bus */
@@ -399,6 +417,24 @@
return status;
}

+#if defined(CONFIG_DISPLAY_DDC) || defined(CONFIG_DISPLAY_DDC_MODULE)
+static void *acpi_display_get_EDID(struct acpi_video_device *dev)
+{
+ union acpi_object *edid = NULL;
+ void *data = NULL;
+ int status;
+
+ status = acpi_video_device_EDID(dev, &edid, 128);
+ if (ACPI_FAILURE(status))
+ status = acpi_video_device_EDID(dev, &edid, 256);
+
+ if (ACPI_SUCCESS(status))
+ if (edid && edid->type == ACPI_TYPE_BUFFER)
+ data = edid->buffer.pointer;
+ return data;
+}
+#endif
+
/* bus */

static int
@@ -1255,7 +1291,6 @@
int status;
struct acpi_video_device *data;

-
if (!device || !video)
return -EINVAL;

@@ -1263,12 +1298,10 @@
acpi_evaluate_integer(device->handle, "_ADR", NULL, &device_id);
if (ACPI_SUCCESS(status)) {

- data = kmalloc(sizeof(struct acpi_video_device), GFP_KERNEL);
+ data = kzalloc(sizeof(struct acpi_video_device), GFP_KERNEL);
if (!data)
return -ENOMEM;

- memset(data, 0, sizeof(struct acpi_video_device));
-
strcpy(acpi_device_name(device), ACPI_VIDEO_DEVICE_NAME);
strcpy(acpi_device_class(device), ACPI_VIDEO_CLASS);
acpi_driver_data(device) = data;
@@ -1295,6 +1328,13 @@
acpi_video_device_bind(video, data);
acpi_video_device_find_cap(data);

+#if defined(CONFIG_DISPLAY_DDC) || defined(CONFIG_DISPLAY_DDC_MODULE)
+ data->acpi_display_driver.get_power = acpi_display_get_power;
+ data->acpi_display_driver.set_power = acpi_display_set_power;
+ data->acpi_display_driver.probe = probe_edid;
+ data->display = display_device_register(&data->acpi_display_driver,
+ NULL, acpi_display_get_EDID(data));
+#endif
status = acpi_install_notify_handler(device->handle,
ACPI_DEVICE_NOTIFY,
acpi_video_device_notify,
diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/macintosh/Kconfig fbdev-2.6/drivers/macintosh/Kconfig
--- linus-2.6/drivers/macintosh/Kconfig 2006-12-05 05:07:18.000000000 -0500
+++ fbdev-2.6/drivers/macintosh/Kconfig 2006-12-06 05:01:22.000000000 -0500
@@ -123,7 +123,7 @@
config PMAC_BACKLIGHT
bool "Backlight control for LCD screens"
depends on ADB_PMU && FB = y && (BROKEN || !PPC64)
- select FB_BACKLIGHT
+ select BACKLIGHT_CLASS_DEVICE
help
Say Y here to enable Macintosh specific extensions of the generic
backlight code. With this enabled, the brightness keys on older
diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/display-ddc.c fbdev-2.6/drivers/video/display/display-ddc.c
--- linus-2.6/drivers/video/display/display-ddc.c 1969-12-31 19:00:00.000000000 -0500
+++ fbdev-2.6/drivers/video/display/display-ddc.c 2006-12-06 04:31:24.000000000 -0500
@@ -0,0 +1,49 @@
+/*
+ * display-ddc.c - EDID parsing code
+ *
+ * Copyright (C) 2006 James Simmons <[email protected]>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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/display.h>
+#include <linux/err.h>
+#include <linux/ctype.h>
+
+#include <linux/fb.h>
+
+int probe_edid(struct display_device *dev, void *data)
+{
+ struct fb_monspecs spec;
+ ssize_t size;
+
+ if (!data)
+ return 0;
+ size = sizeof(spec.manufacturer) + sizeof(spec.monitor) + sizeof(spec.ascii);
+ dev->name = kzalloc(size, GFP_KERNEL);
+ fb_edid_to_monspecs((unsigned char *) data, &spec);
+ strcpy(dev->name, spec.manufacturer);
+ return snprintf(dev->name, size, "%s %s %s\n", spec.manufacturer,
+ spec.monitor, spec.ascii);
+}
+EXPORT_SYMBOL(probe_edid);
+
+MODULE_DESCRIPTION("Display Hardware handling");
+MODULE_AUTHOR("James Simmons <[email protected]>");
+MODULE_LICENSE("GPL");
diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/display-sysfs.c fbdev-2.6/drivers/video/display/display-sysfs.c
--- linus-2.6/drivers/video/display/display-sysfs.c 1969-12-31 19:00:00.000000000 -0500
+++ fbdev-2.6/drivers/video/display/display-sysfs.c 2006-12-06 04:32:15.000000000 -0500
@@ -0,0 +1,161 @@
+/*
+ * display.c - Display output driver
+ *
+ * Copyright (C) 2006 James Simmons <[email protected]>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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/display.h>
+#include <linux/err.h>
+#include <linux/ctype.h>
+
+static ssize_t display_show_name(struct class_device *dev, char *buf)
+{
+ struct display_device *dsp = to_display_device(dev);
+ return snprintf(buf, PAGE_SIZE, "%s\n", dsp->name);
+}
+
+static ssize_t display_show_power(struct class_device *dev, char *buf)
+{
+ struct display_device *dsp = to_display_device(dev);
+ ssize_t ret = -ENXIO;
+
+ mutex_lock(&dsp->lock);
+ if (likely(dsp->driver->get_power))
+ ret = sprintf(buf,"%.8x\n", dsp->driver->get_power(dsp));
+ mutex_unlock(&dsp->lock);
+ return ret;
+}
+
+static ssize_t display_store_power(struct class_device *dev,
+ const char *buf,size_t count)
+{
+ struct display_device *dsp = to_display_device(dev);
+ size_t size;
+ char *endp;
+ int power;
+
+ power = simple_strtoul(buf, &endp, 0);
+ size = endp - buf;
+ if (*endp && isspace(*endp))
+ size++;
+ if (size != count)
+ return -EINVAL;
+
+ mutex_lock(&dsp->lock);
+ if (likely(dsp->driver->set_power)) {
+ dsp->request_state = power;
+ dsp->driver->set_power(dsp);
+ }
+ mutex_unlock(&dsp->lock);
+ return count;
+}
+
+static void display_class_release(struct class_device *dev)
+{
+ struct display_device *dsp = to_display_device(dev);
+ kfree(dsp);
+}
+
+static struct class_device_attribute display_attributes[] = {
+ __ATTR(name, S_IRUGO, display_show_name, NULL),
+ __ATTR(power, S_IRUGO | S_IWUSR, display_show_power, display_store_power),
+};
+
+static struct class display_class = {
+ .name = "display",
+ .release = display_class_release,
+ .class_dev_attrs = display_attributes,
+};
+
+static int index;
+
+struct display_device *display_device_register(struct display_driver *driver,
+ struct device *dev, void *devdata)
+{
+ struct display_device *new_dev;
+ int ret = -ENOMEM, i;
+
+ if (unlikely(!driver))
+ return ERR_PTR(-EINVAL);
+
+ new_dev = kzalloc(sizeof(struct display_device), GFP_KERNEL);
+ if (likely(new_dev) && unlikely(driver->probe(new_dev, devdata))) {
+ mutex_init(&new_dev->lock);
+ new_dev->driver = driver;
+ new_dev->class_dev.class = &display_class;
+ new_dev->class_dev.dev = dev;
+ sprintf(new_dev->class_dev.class_id, "display%d", index++);
+ class_set_devdata(&new_dev->class_dev, devdata);
+ ret = class_device_register(&new_dev->class_dev);
+
+ if (likely(ret)) {
+ for (i = 0; i < ARRAY_SIZE(display_attributes); i++) {
+ ret = class_device_create_file(&new_dev->class_dev,
+ &display_attributes[i]);
+ if (unlikely(ret)) {
+ while (--i >= 0)
+ class_device_remove_file(&new_dev->class_dev,
+ &display_attributes[i]);
+ class_device_unregister(&new_dev->class_dev);
+ }
+ }
+ }
+ }
+ if (unlikely(ret)) {
+ kfree(new_dev);
+ new_dev = ERR_PTR(ret);
+ }
+ return new_dev;
+}
+EXPORT_SYMBOL(display_device_register);
+
+void display_device_unregister(struct display_device *dev)
+{
+ int i;
+
+ if (!dev)
+ return;
+ mutex_lock(&dev->lock);
+ dev->driver = NULL;
+ for (i = 0; i < ARRAY_SIZE(display_attributes); i++)
+ class_device_remove_file(&dev->class_dev,
+ &display_attributes[i]);
+ mutex_unlock(&dev->lock);
+ class_device_unregister(&dev->class_dev);
+}
+EXPORT_SYMBOL(display_device_unregister);
+
+static void __exit display_class_exit(void)
+{
+ class_unregister(&display_class);
+}
+
+static int __init display_class_init(void)
+{
+ return class_register(&display_class);
+}
+
+postcore_initcall(display_class_init);
+module_exit(display_class_exit);
+
+MODULE_DESCRIPTION("Display Hardware handling");
+MODULE_AUTHOR("James Simmons <[email protected]>");
+MODULE_LICENSE("GPL");
diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/Kconfig fbdev-2.6/drivers/video/display/Kconfig
--- linus-2.6/drivers/video/display/Kconfig 1969-12-31 19:00:00.000000000 -0500
+++ fbdev-2.6/drivers/video/display/Kconfig 2006-12-06 04:50:40.000000000 -0500
@@ -0,0 +1,29 @@
+#
+# Display drivers configuration
+#
+
+menu "Display device support"
+
+config DISPLAY_SUPPORT
+ tristate "Display panel/monitor support"
+ ---help---
+ This framework adds support for low-level control of a display.
+ This includes support for power.
+
+ Enable this to be able to choose the drivers for controlling the
+ physical display panel/monitor on some platforms. This not only
+ covers LCD displays for PDAs but also other types of displays
+ such as CRT, TVout etc.
+
+ To have support for your specific display panel you will have to
+ select the proper drivers which depend on this option.
+
+config DISPLAY_DDC
+ tristate "Enable EDID parsing"
+ ---help---
+ Enable EDID parsing
+
+comment "Display hardware drivers"
+ depends on DISPLAY_SUPPORT
+
+endmenu
diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/display/Makefile fbdev-2.6/drivers/video/display/Makefile
--- linus-2.6/drivers/video/display/Makefile 1969-12-31 19:00:00.000000000 -0500
+++ fbdev-2.6/drivers/video/display/Makefile 2006-12-06 04:54:08.000000000 -0500
@@ -0,0 +1,7 @@
+# Display drivers
+
+display-objs := display-sysfs.o
+
+obj-$(CONFIG_DISPLAY_SUPPORT) += display.o
+
+obj-$(CONFIG_DISPLAY_DDC) += display-ddc.o
diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/Kconfig fbdev-2.6/drivers/video/Kconfig
--- linus-2.6/drivers/video/Kconfig 2006-11-30 05:06:13.000000000 -0500
+++ fbdev-2.6/drivers/video/Kconfig 2006-12-06 05:01:22.000000000 -0500
@@ -4,20 +4,10 @@

menu "Graphics support"

-config FIRMWARE_EDID
- bool "Enable firmware EDID"
- default y
- ---help---
- This enables access to the EDID transferred from the firmware.
- On the i386, this is from the Video BIOS. Enable this if DDC/I2C
- transfers do not work for your driver and if you are using
- nvidiafb, i810fb or savagefb.
-
- In general, choosing Y for this option is safe. If you
- experience extremely long delays while booting before you get
- something on your display, try setting this to N. Matrox cards in
- combination with certain motherboards and monitors are known to
- suffer from this problem.
+if SYSFS
+ source "drivers/video/backlight/Kconfig"
+ source "drivers/video/display/Kconfig"
+endif

config FB
tristate "Support for frame buffer devices"
@@ -53,6 +43,22 @@
(e.g. an accelerated X server) and that are not frame buffer
device-aware may cause unexpected results. If unsure, say N.

+config FIRMWARE_EDID
+ bool "Enable firmware EDID"
+ depends on FB
+ default n
+ ---help---
+ This enables access to the EDID transferred from the firmware.
+ On the i386, this is from the Video BIOS. Enable this if DDC/I2C
+ transfers do not work for your driver and if you are using
+ nvidiafb, i810fb or savagefb.
+
+ In general, choosing Y for this option is safe. If you
+ experience extremely long delays while booting before you get
+ something on your display, try setting this to N. Matrox cards in
+ combination with certain motherboards and monitors are known to
+ suffer from this problem.
+
config FB_DDC
tristate
depends on FB && I2C && I2C_ALGOBIT
@@ -90,13 +96,6 @@
depends on FB
default n

-config FB_BACKLIGHT
- bool
- depends on FB
- select BACKLIGHT_LCD_SUPPORT
- select BACKLIGHT_CLASS_DEVICE
- default n
-
config FB_MODE_HELPERS
bool "Enable Video Mode Handling Helpers"
depends on FB
@@ -126,6 +125,9 @@
This is particularly important to one driver, matroxfb. If
unsure, say N.

+comment "Frambuffer hardware drivers"
+ depends on FB
+
config FB_CIRRUS
tristate "Cirrus Logic support"
depends on FB && (ZORRO || PCI)
@@ -551,6 +553,7 @@
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
+ select VIDEO_SELECT
help
This is the frame buffer device driver for generic VESA 2.0
compliant graphic cards. The older VESA 1.2 cards are not supported.
@@ -728,8 +731,7 @@

config FB_NVIDIA_BACKLIGHT
bool "Support for backlight control"
- depends on FB_NVIDIA && PMAC_BACKLIGHT
- select FB_BACKLIGHT
+ depends on FB_NVIDIA && BACKLIGHT_CLASS_DEVICE
default y
help
Say Y here if you want to control the backlight of your display.
@@ -775,8 +777,7 @@

config FB_RIVA_BACKLIGHT
bool "Support for backlight control"
- depends on FB_RIVA && PMAC_BACKLIGHT
- select FB_BACKLIGHT
+ depends on FB_RIVA && BACKLIGHT_CLASS_DEVICE
default y
help
Say Y here if you want to control the backlight of your display.
@@ -1049,8 +1050,7 @@

config FB_RADEON_BACKLIGHT
bool "Support for backlight control"
- depends on FB_RADEON && PMAC_BACKLIGHT
- select FB_BACKLIGHT
+ depends on FB_RADEON && BACKLIGHT_CLASS_DEVICE
default y
help
Say Y here if you want to control the backlight of your display.
@@ -1081,8 +1081,7 @@

config FB_ATY128_BACKLIGHT
bool "Support for backlight control"
- depends on FB_ATY128 && PMAC_BACKLIGHT
- select FB_BACKLIGHT
+ depends on FB_ATY128 && BACKLIGHT_CLASS_DEVICE
default y
help
Say Y here if you want to control the backlight of your display.
@@ -1131,8 +1130,7 @@

config FB_ATY_BACKLIGHT
bool "Support for backlight control"
- depends on FB_ATY && PMAC_BACKLIGHT
- select FB_BACKLIGHT
+ depends on FB_ATY && BACKLIGHT_CLASS_DEVICE
default y
help
Say Y here if you want to control the backlight of your display.
@@ -1632,6 +1630,7 @@
the vfb_enable=1 option.

If unsure, say N.
+
if VT
source "drivers/video/console/Kconfig"
endif
@@ -1640,9 +1639,5 @@
source "drivers/video/logo/Kconfig"
endif

-if SYSFS
- source "drivers/video/backlight/Kconfig"
-endif
-
endmenu

diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/drivers/video/Makefile fbdev-2.6/drivers/video/Makefile
--- linus-2.6/drivers/video/Makefile 2006-11-07 05:38:36.000000000 -0500
+++ fbdev-2.6/drivers/video/Makefile 2006-12-06 05:01:22.000000000 -0500
@@ -12,7 +12,7 @@

obj-$(CONFIG_VT) += console/
obj-$(CONFIG_LOGO) += logo/
-obj-$(CONFIG_SYSFS) += backlight/
+obj-$(CONFIG_SYSFS) += backlight/ display/

obj-$(CONFIG_FB_CFB_FILLRECT) += cfbfillrect.o
obj-$(CONFIG_FB_CFB_COPYAREA) += cfbcopyarea.o
diff -urN -X fbdev-2.6/Documentation/dontdiff linus-2.6/include/linux/display.h fbdev-2.6/include/linux/display.h
--- linus-2.6/include/linux/display.h 1969-12-31 19:00:00.000000000 -0500
+++ fbdev-2.6/include/linux/display.h 2006-12-05 14:43:54.000000000 -0500
@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2006 James Simmons <[email protected]>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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 _LINUX_DISPLAY_H
+#define _LINUX_DISPLAY_H
+
+#include <linux/device.h>
+
+struct display_device;
+
+/* This structure defines all the properties of a Display. */
+struct display_driver {
+ int (*set_power)(struct display_device *);
+ int (*get_power)(struct display_device *);
+ int (*probe)(struct display_device *, void *);
+ int (*remove)(struct display_device *);
+};
+
+struct display_device {
+ struct module *owner; /* Owner module */
+ char *name;
+ struct mutex lock;
+ int request_state;
+ struct display_driver *driver;
+ struct class_device class_dev; /* The class device structure */
+};
+
+extern struct display_device *display_device_register(struct display_driver *driver,
+ struct device *dev, void *devdata);
+extern void display_device_unregister(struct display_device *dev);
+
+extern int probe_edid(struct display_device *dev, void *devdata);
+
+#define to_display_device(obj) container_of(obj, struct display_device, class_dev)
+
+#endif

2006-12-06 18:14:28

by Randy Dunlap

[permalink] [raw]
Subject: Re: Display class

On Wed, 6 Dec 2006 15:10:44 +0000 (GMT) James Simmons wrote:

>
> > > of Mr. Yu for acpi. Also this class could in time replace the lcd class
> > > located in the backlight directory since a lcd is a type of display.
> > > The final hope is that the purpose auxdisplay could fall under this
> > > catergory.
> > >
> > > P.S
> > > I know the edid parsing would have to be pulled out of the fbdev layer.
>
> That patch was rought draft for feedback. I applied your comments. This
> patch actually works. It includes my backlight fix as well.

Glad to hear it. I had to make the following changes
in order for it to build.
However, I still have build errors for aty.

---
From: Randy Dunlap <[email protected]>

Replace CONFIG_FB_BACKLIGHT with CONFIG_BACKLIGHT_CLASS_DEVICE
in include/linux/fb.h and drivers/video/fbsysfs.c
to match Kconfig changes.

Signed-off-by: Randy Dunlap <[email protected]>
---
drivers/video/fbsysfs.c | 8 ++++----
include/linux/fb.h | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)

--- linux-2.6.19-git7.orig/include/linux/fb.h
+++ linux-2.6.19-git7/include/linux/fb.h
@@ -367,7 +367,7 @@ struct fb_cursor {
struct fb_image image; /* Cursor image */
};

-#ifdef CONFIG_FB_BACKLIGHT
+#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
/* Settings for the generic backlight code */
#define FB_BACKLIGHT_LEVELS 128
#define FB_BACKLIGHT_MAX 0xFF
@@ -759,7 +759,7 @@ struct fb_info {
struct list_head modelist; /* mode list */
struct fb_videomode *mode; /* current mode */

-#ifdef CONFIG_FB_BACKLIGHT
+#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
/* Lock ordering:
* bl_mutex (protects bl_dev and bl_curve)
* bl_dev->sem (backlight class)
--- linux-2.6.19-git7.orig/drivers/video/fbsysfs.c
+++ linux-2.6.19-git7/drivers/video/fbsysfs.c
@@ -58,7 +58,7 @@ struct fb_info *framebuffer_alloc(size_t

info->device = dev;

-#ifdef CONFIG_FB_BACKLIGHT
+#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
mutex_init(&info->bl_mutex);
#endif

@@ -411,7 +411,7 @@ static ssize_t show_fbstate(struct devic
return snprintf(buf, PAGE_SIZE, "%d\n", fb_info->state);
}

-#ifdef CONFIG_FB_BACKLIGHT
+#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
static ssize_t store_bl_curve(struct device *device,
struct device_attribute *attr,
const char *buf, size_t count)
@@ -500,7 +500,7 @@ static struct device_attribute device_at
__ATTR(stride, S_IRUGO, show_stride, NULL),
__ATTR(rotate, S_IRUGO|S_IWUSR, show_rotate, store_rotate),
__ATTR(state, S_IRUGO|S_IWUSR, show_fbstate, store_fbstate),
-#ifdef CONFIG_FB_BACKLIGHT
+#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
__ATTR(bl_curve, S_IRUGO|S_IWUSR, show_bl_curve, store_bl_curve),
#endif
};
@@ -541,7 +541,7 @@ void fb_cleanup_device(struct fb_info *f
}
}

-#ifdef CONFIG_FB_BACKLIGHT
+#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
/* This function generates a linear backlight curve
*
* 0: off

2006-12-06 18:24:14

by James Simmons

[permalink] [raw]
Subject: Re: Display class


> > That patch was rought draft for feedback. I applied your comments. This
> > patch actually works. It includes my backlight fix as well.
>
> Glad to hear it. I had to make the following changes
> in order for it to build.
> However, I still have build errors for aty.

Ug. I see another problem. I had backlight completly compiled as a
module! Thus it hid these compile errors. So we need also a
CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE check as well. Can sysfs handle this
well or would it be better the the backlight class be a boolean instead?


> ---
> From: Randy Dunlap <[email protected]>
>
> Replace CONFIG_FB_BACKLIGHT with CONFIG_BACKLIGHT_CLASS_DEVICE
> in include/linux/fb.h and drivers/video/fbsysfs.c
> to match Kconfig changes.
>
> Signed-off-by: Randy Dunlap <[email protected]>
> ---
> drivers/video/fbsysfs.c | 8 ++++----
> include/linux/fb.h | 4 ++--
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> --- linux-2.6.19-git7.orig/include/linux/fb.h
> +++ linux-2.6.19-git7/include/linux/fb.h
> @@ -367,7 +367,7 @@ struct fb_cursor {
> struct fb_image image; /* Cursor image */
> };
>
> -#ifdef CONFIG_FB_BACKLIGHT
> +#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
> /* Settings for the generic backlight code */
> #define FB_BACKLIGHT_LEVELS 128
> #define FB_BACKLIGHT_MAX 0xFF
> @@ -759,7 +759,7 @@ struct fb_info {
> struct list_head modelist; /* mode list */
> struct fb_videomode *mode; /* current mode */
>
> -#ifdef CONFIG_FB_BACKLIGHT
> +#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
> /* Lock ordering:
> * bl_mutex (protects bl_dev and bl_curve)
> * bl_dev->sem (backlight class)
> --- linux-2.6.19-git7.orig/drivers/video/fbsysfs.c
> +++ linux-2.6.19-git7/drivers/video/fbsysfs.c
> @@ -58,7 +58,7 @@ struct fb_info *framebuffer_alloc(size_t
>
> info->device = dev;
>
> -#ifdef CONFIG_FB_BACKLIGHT
> +#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
> mutex_init(&info->bl_mutex);
> #endif
>
> @@ -411,7 +411,7 @@ static ssize_t show_fbstate(struct devic
> return snprintf(buf, PAGE_SIZE, "%d\n", fb_info->state);
> }
>
> -#ifdef CONFIG_FB_BACKLIGHT
> +#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
> static ssize_t store_bl_curve(struct device *device,
> struct device_attribute *attr,
> const char *buf, size_t count)
> @@ -500,7 +500,7 @@ static struct device_attribute device_at
> __ATTR(stride, S_IRUGO, show_stride, NULL),
> __ATTR(rotate, S_IRUGO|S_IWUSR, show_rotate, store_rotate),
> __ATTR(state, S_IRUGO|S_IWUSR, show_fbstate, store_fbstate),
> -#ifdef CONFIG_FB_BACKLIGHT
> +#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
> __ATTR(bl_curve, S_IRUGO|S_IWUSR, show_bl_curve, store_bl_curve),
> #endif
> };
> @@ -541,7 +541,7 @@ void fb_cleanup_device(struct fb_info *f
> }
> }
>
> -#ifdef CONFIG_FB_BACKLIGHT
> +#ifdef CONFIG_BACKLIGHT_CLASS_DEVICE
> /* This function generates a linear backlight curve
> *
> * 0: off
>

2006-12-06 18:44:54

by Miguel Ojeda

[permalink] [raw]
Subject: Re: Display class

Ok, here is the patch (against git7+displayclass) which moves auxdisplay/*
to video/display/* and start using the display class.

It is just a draft, but there isn't much code changed from -mm2.

- I would remove "struct device *dev, void *devdata" of display_device_register()
Are they neccesary for other display drivers? I have to pass NULL right now.

- I would add a paramtere ("char *name") to display_device_register() so we
set the name when registering. Right now I have to set my name after inited,
and this is a Linux module and not a person borning, right? ;)

- I would add a read/writeable attr called "rate" for set/unset the refresh rate
of a display.

- I was going to maintain the drivers/auxdisplay/* tree.
Are you going to maintain the driver? I think so, just for being sure.

P.S.

When I was working at 2.6.19-rc6-mm2 it worked all fine, but now
I have copied it to git7 I'm getting some weird segmentation faults
(oops) when at cfag12864bfb_init, at mutex_lock() in
display_device_unregister module... I think unrelated (?), but I will
look for some mistake I made.

miguelojeda-draft-drivers-video-display-add-support.patch
Signed-off-by: Miguel Ojeda Sandonis <[email protected]>
---
diff --git a/CREDITS b/CREDITS
index d088008..b55312a 100644
--- a/CREDITS
+++ b/CREDITS
@@ -2562,6 +2562,16 @@ S: Subiaco, 6008
S: Perth, Western Australia
S: Australia

+N: Miguel Ojeda Sandonis
+E: [email protected]
+W: http://maxextreme.googlepages.com/
+D: Author: ks0108 LCD Controller driver
+D: Author: cfag12864b LCD driver
+D: Author: cfag12864bfb LCD framebuffer driver
+S: C/ Mieses 20, 9-B
+S: Valladolid 47009
+S: Spain
+
N: Greg Page
E: [email protected]
D: IPX development and support
diff --git a/Documentation/display/cfag12864b b/Documentation/display/cfag12864b
new file mode 100644
index 0000000..8b6c01d
--- /dev/null
+++ b/Documentation/display/cfag12864b
@@ -0,0 +1,105 @@
+ ===================================
+ cfag12864b LCD Driver Documentation
+ ===================================
+
+License: GPLv2
+Author & Maintainer: Miguel Ojeda Sandonis <[email protected]>
+Date: 2006-10-31
+
+
+
+--------
+0. INDEX
+--------
+
+ 1. DRIVER INFORMATION
+ 2. DEVICE INFORMATION
+ 3. WIRING
+ 4. USERSPACE 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
+Data size: 1 byte each address
+Memory size: 2 * 8 * 64 * 1 = 1024 bytes = 1 Kbyte
+
+
+---------
+3. WIRING
+---------
+
+The cfag12864b LCD Series don't have official wiring.
+
+The common wiring is done to the parallel port as shown:
+
+Parallel Port cfag12864b
+
+ Name Pin# Pin# Name
+
+Strobe ( 1)------------------------------(17) Enable
+Data 0 ( 2)------------------------------( 4) Data 0
+Data 1 ( 3)------------------------------( 5) Data 1
+Data 2 ( 4)------------------------------( 6) Data 2
+Data 3 ( 5)------------------------------( 7) Data 3
+Data 4 ( 6)------------------------------( 8) Data 4
+Data 5 ( 7)------------------------------( 9) Data 5
+Data 6 ( 8)------------------------------(10) Data 6
+Data 7 ( 9)------------------------------(11) Data 7
+ (10) [+5v]---( 1) Vdd
+ (11) [GND]---( 2) Ground
+ (12) [+5v]---(14) Reset
+ (13) [GND]---(15) Read / Write
+ Line (14)------------------------------(13) Controller Select 1
+ (15)
+ Init (16)------------------------------(12) Controller Select 2
+Select (17)------------------------------(16) Data / Instruction
+Ground (18)---[GND] [+5v]---(19) LED +
+Ground (19)---[GND]
+Ground (20)---[GND] E A Values:
+Ground (21)---[GND] [GND]---[P1]---(18) Vee ? R = Resistor = 22 ohm
+Ground (22)---[GND] | ? P1 = Preset = 10 Kohm
+Ground (23)---[GND] ---- S ------( 3) V0 ? P2 = Preset = 1 Kohm
+Ground (24)---[GND] | |
+Ground (25)---[GND] [GND]---[P2]---[R]---(20) LED -
+
+
+------------------------
+4. USERSPACE PROGRAMMING
+------------------------
+
+The cfag12864bfb describes a framebuffer device (/dev/fbX).
+
+It has a size of 1024 bytes = 1 Kbyte.
+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 use the framebuffer as a file: fopen, fwrite, fclose...
+Although the LCD won't get updated until the next refresh time arrives.
+
+Also, you can mmap the framebuffer: open & mmap, munmap & close...
+which is the best option for most uses.
+
+Check Documentation/auxdisplay/cfag12864b-example.c
+for a real working userspace complete program with usage examples.
diff --git a/Documentation/display/cfag12864b-example.c b/Documentation/display/cfag12864b-example.c
new file mode 100644
index 0000000..7bfac35
--- /dev/null
+++ b/Documentation/display/cfag12864b-example.c
@@ -0,0 +1,282 @@
+/*
+ * Filename: cfag12864b-example.c
+ * Version: 0.1.0
+ * Description: cfag12864b LCD userspace example program
+ * License: GPLv2
+ *
+ * Author: Copyright (C) Miguel Ojeda Sandonis <[email protected]>
+ * Date: 2006-10-31
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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
+ *
+ */
+
+/*
+ * ------------------------
+ * start of cfag12864b code
+ * ------------------------
+ */
+
+#include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+
+#define CFAG12864B_WIDTH (128)
+#define CFAG12864B_HEIGHT (64)
+#define CFAG12864B_SIZE (128 * 64 / 8)
+#define CFAG12864B_BPB (8)
+#define CFAG12864B_ADDRESS(x, y) ((y) * CFAG12864B_WIDTH / \
+ CFAG12864B_BPB + (x) / CFAG12864B_BPB)
+#define CFAG12864B_BIT(n) (((unsigned char) 1) << (n))
+
+#undef CFAG12864B_DOCHECK
+#ifdef CFAG12864B_DOCHECK
+ #define CFAG12864B_CHECK(x, y) ((x) < CFAG12864B_WIDTH && \
+ (y) < CFAG12864B_HEIGHT)
+#else
+ #define CFAG12864B_CHECK(x, y) (1)
+#endif
+
+int cfag12864b_fd;
+unsigned char * cfag12864b_mem;
+unsigned char cfag12864b_buffer[CFAG12864B_SIZE];
+
+/*
+ * init a cfag12864b framebuffer device
+ *
+ * No error: return = 0
+ * Unable to open: return = -1
+ * Unable to mmap: return = -2
+ */
+int cfag12864b_init(char *path)
+{
+ cfag12864b_fd = open(path, O_RDWR);
+ if (cfag12864b_fd == -1)
+ return -1;
+
+ cfag12864b_mem = mmap(0, CFAG12864B_SIZE, PROT_READ | PROT_WRITE,
+ MAP_SHARED, cfag12864b_fd, 0);
+ if (cfag12864b_mem == MAP_FAILED) {
+ close(cfag12864b_fd);
+ return -2;
+ }
+
+ return 0;
+}
+
+/*
+ * exit a cfag12864b framebuffer device
+ */
+void cfag12864b_exit(void)
+{
+ munmap(cfag12864b_mem, CFAG12864B_SIZE);
+ close(cfag12864b_fd);
+}
+
+/*
+ * set (x, y) pixel
+ */
+void cfag12864b_set(unsigned char x, unsigned char y)
+{
+ if (CFAG12864B_CHECK(x, y))
+ cfag12864b_buffer[CFAG12864B_ADDRESS(x, y)] |=
+ CFAG12864B_BIT(x % CFAG12864B_BPB);
+}
+
+/*
+ * unset (x, y) pixel
+ */
+void cfag12864b_unset(unsigned char x, unsigned char y)
+{
+ if (CFAG12864B_CHECK(x, y))
+ cfag12864b_buffer[CFAG12864B_ADDRESS(x, y)] &=
+ ~CFAG12864B_BIT(x % CFAG12864B_BPB);
+}
+
+/*
+ * is set (x, y) pixel?
+ *
+ * Pixel off: return = 0
+ * Pixel on: return = 1
+ */
+unsigned char cfag12864b_isset(unsigned char x, unsigned char y)
+{
+ if (CFAG12864B_CHECK(x, y))
+ if (cfag12864b_buffer[CFAG12864B_ADDRESS(x, y)] &
+ CFAG12864B_BIT(x % CFAG12864B_BPB))
+ return 1;
+
+ return 0;
+}
+
+/*
+ * not (x, y) pixel
+ */
+void cfag12864b_not(unsigned char x, unsigned char y)
+{
+ if (cfag12864b_isset(x, y))
+ cfag12864b_unset(x, y);
+ else
+ cfag12864b_set(x, y);
+}
+
+/*
+ * fill (set all pixels)
+ */
+void cfag12864b_fill(void)
+{
+ unsigned short i;
+
+ for (i = 0; i < CFAG12864B_SIZE; i++)
+ cfag12864b_buffer[i] = 0xFF;
+}
+
+/*
+ * clear (unset all pixels)
+ */
+void cfag12864b_clear(void)
+{
+ unsigned short i;
+
+ for (i = 0; i < CFAG12864B_SIZE; i++)
+ cfag12864b_buffer[i] = 0;
+}
+
+/*
+ * format a [128*64] matrix
+ *
+ * Pixel off: src[i] = 0
+ * Pixel on: src[i] > 0
+ */
+void cfag12864b_format(unsigned char * matrix)
+{
+ unsigned char i, j, n;
+
+ for (i = 0; i < CFAG12864B_HEIGHT; i++)
+ for (j = 0; j < CFAG12864B_WIDTH / CFAG12864B_BPB; j++) {
+ cfag12864b_buffer[i * CFAG12864B_WIDTH / CFAG12864B_BPB +
+ j] = 0;
+ for (n = 0; n < CFAG12864B_BPB; n++)
+ if (matrix[i * CFAG12864B_WIDTH +
+ j * CFAG12864B_BPB + n])
+ cfag12864b_buffer[i * CFAG12864B_WIDTH /
+ CFAG12864B_BPB + j] |=
+ CFAG12864B_BIT(n);
+ }
+}
+
+/*
+ * blit buffer to lcd
+ */
+void cfag12864b_blit(void)
+{
+ memcpy(cfag12864b_mem, cfag12864b_buffer, CFAG12864B_SIZE);
+}
+
+/*
+ * ----------------------
+ * end of cfag12864b code
+ * ----------------------
+ */
+
+#include <stdio.h>
+#include <string.h>
+
+#define EXAMPLES 6
+
+void example(unsigned char n)
+{
+ unsigned short i, j;
+ unsigned char matrix[CFAG12864B_WIDTH * CFAG12864B_HEIGHT];
+
+ if (n > EXAMPLES)
+ return;
+
+ printf("Example %i/%i - ", n, EXAMPLES);
+
+ switch (n) {
+ case 1:
+ printf("Draw points setting bits");
+ cfag12864b_clear();
+ for (i = 0; i < CFAG12864B_WIDTH; i += 2)
+ for (j = 0; j < CFAG12864B_HEIGHT; j += 2)
+ cfag12864b_set(i, j);
+ break;
+
+ case 2:
+ printf("Clear the LCD");
+ cfag12864b_clear();
+ break;
+
+ case 3:
+ printf("Draw rows formatting a [128*64] matrix");
+ memset(matrix, 0, CFAG12864B_WIDTH * CFAG12864B_HEIGHT);
+ for (i = 0; i < CFAG12864B_WIDTH; i++)
+ for (j = 0; j < CFAG12864B_HEIGHT; j += 2)
+ matrix[j * CFAG12864B_WIDTH + i] = 1;
+ cfag12864b_format(matrix);
+ break;
+
+ case 4:
+ printf("Fill the lcd");
+ cfag12864b_fill();
+ break;
+
+ case 5:
+ printf("Draw columns unsetting bits");
+ for (i = 0; i < CFAG12864B_WIDTH; i += 2)
+ for (j = 0; j < CFAG12864B_HEIGHT; j++)
+ cfag12864b_unset(i, j);
+ break;
+
+ case 6:
+ printf("Do negative not-ing all bits");
+ for (i = 0; i < CFAG12864B_WIDTH; i++)
+ for (j = 0; j < CFAG12864B_HEIGHT; j ++)
+ cfag12864b_not(i, j);
+ break;
+ }
+
+ puts(" - [Press Enter]");
+}
+
+int main(int argc, char *argv[])
+{
+ unsigned char n;
+
+ if (argc != 2) {
+ printf(
+ "Sintax: %s fbdev\n"
+ "Usually: /dev/fb0, /dev/fb1...\n", argv[0]);
+ return -1;
+ }
+
+ if (cfag12864b_init(argv[1])) {
+ printf("Can't init %s fbdev\n", argv[1]);
+ return -2;
+ }
+
+ for (n = 1; n <= EXAMPLES; n++) {
+ example(n);
+ cfag12864b_blit();
+ while (getchar() != '\n');
+ }
+
+ cfag12864b_exit();
+
+ return 0;
+}
diff --git a/Documentation/display/ks0108 b/Documentation/display/ks0108
new file mode 100644
index 0000000..a006f79
--- /dev/null
+++ b/Documentation/display/ks0108
@@ -0,0 +1,55 @@
+ ==========================================
+ ks0108 LCD Controller Driver Documentation
+ ==========================================
+
+License: GPLv2
+Author & Maintainer: Miguel Ojeda Sandonis <[email protected]>
+Date: 2006-10-31
+
+
+
+--------
+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
+Data size: 1 byte each address
+Memory size: 8 * 64 * 1 = 512 bytes
+
+
+---------
+3. WIRING
+---------
+
+The driver supports data parallel port wiring.
+
+If you aren't building LCD related hardware, you should check
+your LCD specific wiring information in the same folder.
+
+For example, check Documentation/display/cfag12864b.
diff --git a/MAINTAINERS b/MAINTAINERS
index 5dff268..e31e0bf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -659,6 +659,20 @@ L: [email protected]
L: [email protected]
S: Maintained

+CFAG12864B LCD DRIVER
+P: Miguel Ojeda Sandonis
+M: [email protected]
+W: http://maxextreme.googlepages.com/display.htm
+L: [email protected]
+S: Maintained
+
+CFAG12864BFB LCD FRAMEBUFFER DRIVER
+P: Miguel Ojeda Sandonis
+M: [email protected]
+W: http://maxextreme.googlepages.com/display.htm
+L: [email protected]
+S: Maintained
+
COMMON INTERNET FILE SYSTEM (CIFS)
P: Steve French
M: [email protected]
@@ -1752,6 +1766,13 @@ M: [email protected]
L: [email protected]
S: Maintained

+KS0108 LCD CONTROLLER DRIVER
+P: Miguel Ojeda Sandonis
+M: [email protected]
+W: http://maxextreme.googlepages.com/display.htm
+L: [email protected]
+S: Maintained
+
LAPB module
L: [email protected]
S: Orphan
diff --git a/drivers/video/display/Kconfig b/drivers/video/display/Kconfig
index db2dae4..0c655a5 100644
--- a/drivers/video/display/Kconfig
+++ b/drivers/video/display/Kconfig
@@ -26,4 +26,102 @@ config DISPLAY_DDC
comment "Display hardware drivers"
depends on DISPLAY_SUPPORT

+config KS0108
+ tristate "KS0108 LCD Controller"
+ depends on PARPORT_PC
+ depends on DISPLAY_SUPPORT
+ 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 X86
+ 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 Documentation/auxdisplay/cfag12864b
+
+ 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 --git a/drivers/video/display/Makefile b/drivers/video/display/Makefile
index d210f52..fc9f7dd 100644
--- a/drivers/video/display/Makefile
+++ b/drivers/video/display/Makefile
@@ -5,3 +5,6 @@ display-objs := display-sysfs.o
obj-$(CONFIG_DISPLAY_SUPPORT) += display.o

obj-$(CONFIG_DISPLAY_DDC) += display-ddc.o
+
+obj-$(CONFIG_KS0108) += ks0108.o
+obj-$(CONFIG_CFAG12864B) += cfag12864b.o cfag12864bfb.o
diff --git a/drivers/video/display/cfag12864b.c b/drivers/video/display/cfag12864b.c
new file mode 100644
index 0000000..1a25df9
--- /dev/null
+++ b/drivers/video/display/cfag12864b.c
@@ -0,0 +1,404 @@
+/*
+ * 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-31
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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/mutex.h>
+#include <linux/uaccess.h>
+#include <linux/vmalloc.h>
+#include <linux/workqueue.h>
+#include <linux/ks0108.h>
+#include <linux/cfag12864b.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)");
+
+unsigned int cfag12864b_get_rate(void)
+{
+ return cfag12864b_rate;
+}
+
+/*
+ * 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 controller receives commands/data.
+ *
+ * CS2 = Second ks0108 controller
+ * If high, the second ks0108 controller 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
+ */
+
+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
+ */
+
+unsigned char *cfag12864b_buffer;
+static unsigned char *cfag12864b_cache;
+static DEFINE_MUTEX(cfag12864b_mutex);
+static unsigned char cfag12864b_updating;
+static void cfag12864b_update(void *arg);
+static struct workqueue_struct *cfag12864b_workqueue;
+static DECLARE_WORK(cfag12864b_work, cfag12864b_update, NULL);
+
+static void cfag12864b_queue(void)
+{
+ queue_delayed_work(cfag12864b_workqueue, &cfag12864b_work,
+ HZ / cfag12864b_rate);
+}
+
+void cfag12864b_power_on(void)
+{
+ mutex_lock(&cfag12864b_mutex);
+
+ cfag12864b_on();
+
+ mutex_unlock(&cfag12864b_mutex);
+}
+
+void cfag12864b_power_off(void)
+{
+ mutex_lock(&cfag12864b_mutex);
+
+ cfag12864b_off();
+
+ mutex_unlock(&cfag12864b_mutex);
+}
+
+unsigned char cfag12864b_enable(void)
+{
+ unsigned char ret;
+
+ mutex_lock(&cfag12864b_mutex);
+
+ if (!cfag12864b_updating) {
+ cfag12864b_updating = 1;
+ cfag12864b_queue();
+ ret = 0;
+ } else
+ ret = 1;
+
+ mutex_unlock(&cfag12864b_mutex);
+
+ return ret;
+}
+
+void cfag12864b_disable(void)
+{
+ mutex_lock(&cfag12864b_mutex);
+
+ if (cfag12864b_updating) {
+ cfag12864b_updating = 0;
+ cancel_delayed_work(&cfag12864b_work);
+ flush_workqueue(cfag12864b_workqueue);
+ cfag12864b_off();
+ }
+
+ mutex_unlock(&cfag12864b_mutex);
+}
+
+unsigned char cfag12864b_is_enabled(void)
+{
+ return 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)
+ cfag12864b_queue();
+}
+
+/*
+ * cfag12864b Exported Symbols
+ */
+
+EXPORT_SYMBOL_GPL(cfag12864b_buffer);
+EXPORT_SYMBOL_GPL(cfag12864b_get_rate);
+EXPORT_SYMBOL_GPL(cfag12864b_power_on);
+EXPORT_SYMBOL_GPL(cfag12864b_power_off);
+EXPORT_SYMBOL_GPL(cfag12864b_enable);
+EXPORT_SYMBOL_GPL(cfag12864b_disable);
+EXPORT_SYMBOL_GPL(cfag12864b_is_enabled);
+
+/*
+ * Module Init & Exit
+ */
+
+static int __init cfag12864b_init(void)
+{
+ int ret = -EINVAL;
+
+ if (PAGE_SIZE < CFAG12864B_SIZE) {
+ printk(KERN_ERR CFAG12864B_NAME ": ERROR: "
+ "page size (%i) < cfag12864b size (%i)\n",
+ (unsigned int)PAGE_SIZE, CFAG12864B_SIZE);
+ ret = -ENOMEM;
+ goto none;
+ }
+
+ cfag12864b_buffer = (unsigned char *) __get_free_page(GFP_KERNEL);
+ if (cfag12864b_buffer == NULL) {
+ printk(KERN_ERR CFAG12864B_NAME ": ERROR: "
+ "can't get a free page\n");
+ 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;
+ }
+
+ cfag12864b_workqueue = create_singlethread_workqueue(CFAG12864B_NAME);
+ if (cfag12864b_workqueue == NULL)
+ goto cachealloced;
+
+ memset(cfag12864b_buffer, 0, CFAG12864B_SIZE);
+
+ cfag12864b_clear();
+ cfag12864b_on();
+
+ return 0;
+
+cachealloced:
+ kfree(cfag12864b_cache);
+
+bufferalloced:
+ free_page((unsigned long) cfag12864b_buffer);
+
+none:
+ return ret;
+}
+
+static void __exit cfag12864b_exit(void)
+{
+ cfag12864b_disable();
+ cfag12864b_off();
+ destroy_workqueue(cfag12864b_workqueue);
+ kfree(cfag12864b_cache);
+ free_page((unsigned long) 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 --git a/drivers/video/display/cfag12864bfb.c b/drivers/video/display/cfag12864bfb.c
new file mode 100644
index 0000000..e51655f
--- /dev/null
+++ b/drivers/video/display/cfag12864bfb.c
@@ -0,0 +1,205 @@
+/*
+ * 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-31
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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/delay.h>
+#include <linux/display.h>
+#include <linux/errno.h>
+#include <linux/fb.h>
+#include <linux/mm.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/uaccess.h>
+#include <linux/cfag12864b.h>
+
+#define CFAG12864BFB_NAME "cfag12864bfb"
+
+static struct fb_info *fb;
+
+static char cfag12864bfb_name[] = CFAG12864BFB_NAME;
+static int cfag12864bfb_power;
+
+static struct fb_fix_screeninfo cfag12864bfb_fix __initdata = {
+ .id = CFAG12864BFB_NAME,
+ .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 int cfag12864bfb_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+ return vm_insert_page(vma, vma->vm_start,
+ virt_to_page(cfag12864b_buffer));
+}
+
+static struct fb_ops cfag12864bfb_ops = {
+ .owner = THIS_MODULE,
+ .fb_fillrect = cfb_fillrect,
+ .fb_copyarea = cfb_copyarea,
+ .fb_imageblit = cfb_imageblit,
+ .fb_mmap = cfag12864bfb_mmap,
+};
+
+static int cfag12864bfb_set_power(struct display_device *device)
+{
+ if (device->request_state > 0) {
+ cfag12864bfb_power = 1;
+ cfag12864b_power_on();
+ } else {
+ cfag12864bfb_power = 0;
+ cfag12864b_power_off();
+ }
+
+ return 0;
+}
+
+static int cfag12864bfb_get_power(struct display_device *device)
+{
+ return cfag12864bfb_power;
+}
+
+static int cfag12864bfb_probe(struct display_device *device, void *data)
+{
+ return 0;
+}
+
+static int cfag12864bfb_remove(struct display_device *device)
+{
+ return 0;
+}
+
+static struct display_driver cfag12864bfb_driver = {
+ .set_power = cfag12864bfb_set_power,
+ .get_power = cfag12864bfb_get_power,
+ .probe = cfag12864bfb_probe,
+ .remove = cfag12864bfb_remove,
+};
+
+static struct display_device *cfag12864bfb_device;
+
+static int __init cfag12864bfb_init(void)
+{
+ int ret = -EINVAL;
+
+ if (cfag12864b_enable()) {
+ printk(KERN_ERR CFAG12864BFB_NAME ": ERROR: "
+ "can't enable cfag12864b refreshing (being used)\n");
+ ret = -ENODEV;
+ goto none;
+ }
+
+ fb = framebuffer_alloc(0, NULL);
+ if (!fb) {
+ printk(KERN_ERR CFAG12864BFB_NAME ": ERROR: "
+ "can't alloc framebuffer\n");
+ ret = -ENODEV;
+ goto enabled;
+ }
+
+ fb->screen_base = (char __iomem *) cfag12864b_buffer;
+ fb->screen_size = CFAG12864B_SIZE;
+ fb->fbops = &cfag12864bfb_ops;
+ fb->fix = cfag12864bfb_fix;
+ fb->var = cfag12864bfb_var;
+ fb->pseudo_palette = NULL;
+ fb->par = NULL;
+ fb->flags = FBINFO_FLAG_DEFAULT;
+
+ if (register_framebuffer(fb) < 0) {
+ printk(KERN_ERR CFAG12864BFB_NAME ": ERROR: "
+ "can't register framebuffer\n");
+ ret = -ENODEV;
+ goto fballoced;
+ }
+
+ cfag12864bfb_device = display_device_register(&cfag12864bfb_driver,
+ NULL, NULL);
+ if (!cfag12864bfb_device) {
+ printk(KERN_ERR CFAG12864BFB_NAME ": ERROR: "
+ "can't register display device\n");
+ ret = -ENODEV;
+ goto fbregistered;
+ }
+
+ cfag12864bfb_device->name = cfag12864bfb_name;
+ cfag12864bfb_device->owner = THIS_MODULE;
+ cfag12864bfb_device->request_state = cfag12864bfb_power = 1;
+
+ cfag12864b_power_on();
+
+ printk(KERN_INFO "fb%d: %s framebuffer device\n", fb->node,
+ fb->fix.id);
+
+ return 0;
+
+fbregistered:
+ unregister_framebuffer(fb);
+
+fballoced:
+ framebuffer_release(fb);
+
+enabled:
+ cfag12864b_disable();
+
+none:
+ return ret;
+}
+
+static void __exit cfag12864bfb_exit(void)
+{
+ display_device_unregister(cfag12864bfb_device);
+ unregister_framebuffer(fb);
+ framebuffer_release(fb);
+ cfag12864b_disable();
+}
+
+module_init(cfag12864bfb_init);
+module_exit(cfag12864bfb_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Miguel Ojeda Sandonis <[email protected]>");
+MODULE_DESCRIPTION("cfag12864bfb LCD framebuffer driver");
diff --git a/drivers/video/display/ks0108.c b/drivers/video/display/ks0108.c
new file mode 100644
index 0000000..a637575
--- /dev/null
+++ b/drivers/video/display/ks0108.c
@@ -0,0 +1,166 @@
+/*
+ * 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-31
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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/delay.h>
+#include <linux/fs.h>
+#include <linux/io.h>
+#include <linux/parport.h>
+#include <linux/uaccess.h>
+#include <linux/ks0108.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 Commands (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 using 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 --git a/include/linux/cfag12864b.h b/include/linux/cfag12864b.h
new file mode 100644
index 0000000..5174670
--- /dev/null
+++ b/include/linux/cfag12864b.h
@@ -0,0 +1,91 @@
+/*
+ * Filename: cfag12864b.h
+ * Version: 0.1.0
+ * Description: cfag12864b LCD driver header
+ * License: GPLv2
+ *
+ * Author: Copyright (C) Miguel Ojeda Sandonis <[email protected]>
+ * Date: 2006-12-06
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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))
+
+/*
+ * The driver will blit this buffer to the LCD
+ *
+ * Its size is CFAG12864B_SIZE.
+ */
+extern unsigned char * cfag12864b_buffer;
+
+/*
+ * Get the refresh rate of the LCD
+ *
+ * Returns the refresh rate (hertzs).
+ */
+extern unsigned int cfag12864b_get_rate(void);
+
+/*
+ * Power on the LCD
+ *
+ * Screen memory will be showed
+ */
+extern void cfag12864b_power_on(void);
+
+/*
+ * Power off the LCD
+ *
+ * Screen memory won't be showed
+ */
+extern void cfag12864b_power_off(void);
+
+/*
+ * Enable refreshing
+ *
+ * Returns 0 if successful (anyone was using it),
+ * or != 0 if failed (someone is using it).
+ */
+extern unsigned char cfag12864b_enable(void);
+
+/*
+ * Disable refreshing
+ *
+ * You should call this only when you finish using the LCD.
+ */
+extern void cfag12864b_disable(void);
+
+/*
+ * Is enabled refreshing? (is anyone using the module?)
+ *
+ * Returns 0 if refreshing is not enabled (anyone is using it),
+ * or != 0 if refreshing is enabled (someone is using it).
+ *
+ * Useful for buffer read-only modules.
+ */
+extern unsigned char cfag12864b_is_enabled(void);
+
+#endif /* _CFAG12864B_H_ */
+
diff --git a/include/linux/ks0108.h b/include/linux/ks0108.h
new file mode 100644
index 0000000..aca57f1
--- /dev/null
+++ b/include/linux/ks0108.h
@@ -0,0 +1,46 @@
+/*
+ * 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-12-06
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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_
+
+/* Write a byte to the data port */
+extern void ks0108_writedata(unsigned char byte);
+
+/* Write a byte to the control port */
+extern void ks0108_writecontrol(unsigned char byte);
+
+/* Set the controller's current display state (0..1) */
+extern void ks0108_displaystate(unsigned char state);
+
+/* Set the controller's current startline (0..63) */
+extern void ks0108_startline(unsigned char startline);
+
+/* Set the controller's current address (0..63) */
+extern void ks0108_address(unsigned char address);
+
+/* Set the controller's current page (0..7) */
+extern void ks0108_page(unsigned char page);
+
+#endif /* _KS0108_H_ */

2006-12-06 18:57:06

by Randy Dunlap

[permalink] [raw]
Subject: Re: Display class

On Wed, 6 Dec 2006 18:24:08 +0000 (GMT) James Simmons wrote:

>
> > > That patch was rought draft for feedback. I applied your comments. This
> > > patch actually works. It includes my backlight fix as well.
> >
> > Glad to hear it. I had to make the following changes
> > in order for it to build.
> > However, I still have build errors for aty.
>
> Ug. I see another problem. I had backlight completly compiled as a
> module! Thus it hid these compile errors. So we need also a
> CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE check as well. Can sysfs handle this
> well or would it be better the the backlight class be a boolean instead?

SCSI works as a module and it uses sysfs.
See drivers/scsi/scsi_sysfs.c.
Does that answer your question? I wasn't quite sure what
the question was.

Next question, based on:
drivers/built-in.o: In function `probe_edid':
(.text.probe_edid+0x42): undefined reference to `fb_edid_to_monspecs'

Should backlight and/or display support depend on
CONFIG_FB? Right now they don't, so the above can happen...

---
~Randy

2006-12-06 19:06:27

by Miguel Ojeda

[permalink] [raw]
Subject: Re: Display class

On 12/6/06, Miguel Ojeda Sandonis <[email protected]> wrote:
> Ok, here is the patch (against git7+displayclass) which moves auxdisplay/*
> to video/display/* and start using the display class.
>
> It is just a draft, but there isn't much code changed from -mm2.
>
> - I would remove "struct device *dev, void *devdata" of display_device_register()
> Are they neccesary for other display drivers? I have to pass NULL right now.
>
> - I would add a paramtere ("char *name") to display_device_register() so we
> set the name when registering. Right now I have to set my name after inited,
> and this is a Linux module and not a person borning, right? ;)
>
> - I would add a read/writeable attr called "rate" for set/unset the refresh rate
> of a display.
>
> - I was going to maintain the drivers/auxdisplay/* tree.
> Are you going to maintain the driver? I think so, just for being sure.

I meant display driver (display/*).

>
> P.S.
>
> When I was working at 2.6.19-rc6-mm2 it worked all fine, but now
> I have copied it to git7 I'm getting some weird segmentation faults
> (oops) when at cfag12864bfb_init, at mutex_lock() in
> display_device_unregister module... I think unrelated (?), but I will
>

I meant display_device_unregister function of display-sysfs.c

--

The changes have been made in driver/video/display/cfag12864bfb.c, so
please focus the review on it.

Thanks

2006-12-06 19:13:56

by James Simmons

[permalink] [raw]
Subject: Re: Display class


> > > > That patch was rought draft for feedback. I applied your comments. This
> > > > patch actually works. It includes my backlight fix as well.
> > >
> > > Glad to hear it. I had to make the following changes
> > > in order for it to build.
> > > However, I still have build errors for aty.
> >
> > Ug. I see another problem. I had backlight completly compiled as a
> > module! Thus it hid these compile errors. So we need also a
> > CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE check as well. Can sysfs handle this
> > well or would it be better the the backlight class be a boolean instead?
>
> SCSI works as a module and it uses sysfs.
> See drivers/scsi/scsi_sysfs.c.
> Does that answer your question? I wasn't quite sure what
> the question was.

I'm scratching my head on how to configure a modular driver and
two modular sysfs classes.

> Next question, based on:
> drivers/built-in.o: In function `probe_edid':
> (.text.probe_edid+0x42): undefined reference to `fb_edid_to_monspecs'
>
> Should backlight and/or display support depend on
> CONFIG_FB? Right now they don't, so the above can happen...

I already sent a patch to Andrew to make backlight/lcd work independent of
CONFIG_FB. Display is still in the alpha stage. In time it will work
independent of CONFIG_FB.

2006-12-06 20:28:22

by Randy Dunlap

[permalink] [raw]
Subject: Re: Display class

On Wed, 6 Dec 2006 15:10:44 +0000 (GMT) James Simmons wrote:

>
> > > of Mr. Yu for acpi. Also this class could in time replace the lcd class
> > > located in the backlight directory since a lcd is a type of display.
> > > The final hope is that the purpose auxdisplay could fall under this
> > > catergory.
> > >
> > > P.S
> > > I know the edid parsing would have to be pulled out of the fbdev layer.
>
> That patch was rought draft for feedback. I applied your comments. This
> patch actually works. It includes my backlight fix as well.

BTW, this patch version contains trailing whitespace
which should be cleaned up:

Warning: trailing whitespace in lines 33,158 of drivers/acpi/video.c
Warning: trailing whitespace in line 10 of drivers/video/display/Kconfig
Warning: trailing whitespace in line 41 of include/linux/display.h
Warning: trailing whitespace in lines 49,1053,1084,1133 of drivers/video/Kconfig
Warning: trailing whitespace in line 3 of drivers/video/display/Makefile

---
~Randy

2006-12-07 15:20:31

by James Simmons

[permalink] [raw]
Subject: Re: Display class


> - I would remove "struct device *dev, void *devdata" of display_device_register()
> Are they neccesary for other display drivers? I have to pass NULL right now.

Yes. Passing in a struct device allows you a link between the device and
the class. If you pass in the device for the parport a link to the parport
device would exist in you displayX directory. The devdata is used by the
probe function to get data about the monitor if it is not NULL.

In the case of most desktop monitors they have EDID blocks. You can
retrieve them (devdata) and it gets parsed thus you have detail data
about the monitor. In your case it can be null.

> - I would add a paramtere ("char *name") to display_device_register() so we
> set the name when registering. Right now I have to set my name after inited,
> and this is a Linux module and not a person borning, right? ;)

The probe function gets this for you. In your case you would have a probe
method that would just fill in the name of the LCD. For me using the ACPI
video driver I get the name for my monitor

LEN 15"XGA 200nit

Which is the manufacturer - monitor id - ascii block.

> - I would add a read/writeable attr called "rate" for set/unset the refresh rate
> of a display.

I suggest creating a group for your driver. See device.h for
group_attributes.

> - I was going to maintain the drivers/auxdisplay/* tree.
> Are you going to maintain the driver? I think so, just for being sure.

Yes. I need it for the ACPI video and fbdev layer. Remember its in the
early stages yet.

> P.S.
>
> When I was working at 2.6.19-rc6-mm2 it worked all fine, but now
> I have copied it to git7 I'm getting some weird segmentation faults
> (oops) when at cfag12864bfb_init, at mutex_lock() in
> display_device_unregister module... I think unrelated (?), but I will
> look for some mistake I made.

Did you solve the problem?

2006-12-07 16:21:15

by Miguel Ojeda

[permalink] [raw]
Subject: Re: Display class

On 12/7/06, James Simmons <[email protected]> wrote:
>
> > - I would remove "struct device *dev, void *devdata" of display_device_register()
> > Are they neccesary for other display drivers? I have to pass NULL right now.
>
> Yes. Passing in a struct device allows you a link between the device and
> the class. If you pass in the device for the parport a link to the parport
> device would exist in you displayX directory. The devdata is used by the
> probe function to get data about the monitor if it is not NULL.
>
> In the case of most desktop monitors they have EDID blocks. You can
> retrieve them (devdata) and it gets parsed thus you have detail data
> about the monitor. In your case it can be null.
>
> > - I would add a paramtere ("char *name") to display_device_register() so we
> > set the name when registering. Right now I have to set my name after inited,
> > and this is a Linux module and not a person borning, right? ;)
>
> The probe function gets this for you. In your case you would have a probe
> method that would just fill in the name of the LCD. For me using the ACPI
> video driver I get the name for my monitor
>
> LEN 15"XGA 200nit
>
> Which is the manufacturer - monitor id - ascii block.
>
> > - I would add a read/writeable attr called "rate" for set/unset the refresh rate
> > of a display.
>
> I suggest creating a group for your driver. See device.h for
> group_attributes.
>
> > - I was going to maintain the drivers/auxdisplay/* tree.
> > Are you going to maintain the driver? I think so, just for being sure.
>
> Yes. I need it for the ACPI video and fbdev layer. Remember its in the
> early stages yet.
>
> > P.S.
> >
> > When I was working at 2.6.19-rc6-mm2 it worked all fine, but now
> > I have copied it to git7 I'm getting some weird segmentation faults
> > (oops) when at cfag12864bfb_init, at mutex_lock() in
> > display_device_unregister module... I think unrelated (?), but I will
> > look for some mistake I made.
>
> Did you solve the problem?
>

I didn't look further, but I will be able to try again in some hours.
Anyway, the problem is probably at cfag12864bfb.c (as it was almost
the only file I modified from -mm2); but I can't tell where the
problem is as I tried just a few times.

Please review cfag12864bfb_init/_exit to check if your code/class is
intended to be used that way, I will focus on the segfaults.

Greets!

--
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm

2006-12-08 11:58:15

by Miguel Ojeda

[permalink] [raw]
Subject: Re: Display class

On 12/7/06, James Simmons <[email protected]> wrote:
>
> > P.S.
> >
> > When I was working at 2.6.19-rc6-mm2 it worked all fine, but now
> > I have copied it to git7 I'm getting some weird segmentation faults
> > (oops) when at cfag12864bfb_init, at mutex_lock() in
> > display_device_unregister module... I think unrelated (?), but I will
> > look for some mistake I made.
>
> Did you solve the problem?
>
>

Ok, found it. It was at cfag12864bfb_init:

cfag12864bfb_device = display_device_register(&cfag12864bfb_driver,
NULL, NULL);
if (!cfag12864bfb_device) {
printk(KERN_ERR CFAG12864BFB_NAME ": ERROR: "
"can't register display device\n");
ret = -ENODEV;
goto fbregistered;
}

/*cfag12864bfb_device->name = cfag12864bfb_name;
cfag12864bfb_device->request_state = 1;*/

cfag12864bfb_power = 1;
cfag12864b_power_on();

The commented code. I have exchange it to pass the name through probe
as you suggested, still, it brokes when rmmoding it at
display_device_unregister:

root@morfeo:/usr/src/git/linux-2.6.19-git12# dmesg
[ 1593.770774] BUG: unable to handle kernel paging request at virtual
address ff fffffc
[ 1593.770780] printing eip:
[ 1593.770782] c02e3f3f
[ 1593.770784] *pde = 00003067
[ 1593.770787] Oops: 0002 [#1]
[ 1593.770788] PREEMPT SMP
[ 1593.770791] Modules linked in: cfag12864bfb display cfag12864b
ks0108 ppdev n ls_utf8 ntfs ipv6 md_mod af_packet tsdev snd_intel8x0
snd_ac97_codec snd_ac97_bu s snd_pcm_oss snd_mixer_oss sk98lin psmouse
snd_pcm snd_timer snd soundcore parp ort_pc parport skge floppy
snd_page_alloc serio_raw pcspkr rtc intel_agp agpgart evdev dm_mirror
dm_mod ide_generic ide_disk ide_cd cdrom piix generic vga16fb v
gastate
[ 1593.770829] CPU: 1
[ 1593.770829] EIP: 0060:[<c02e3f3f>] Not tainted VLI
[ 1593.770831] EFLAGS: 00010282 (2.6.19-git12 #1)
[ 1593.770838] EIP is at mutex_lock+0x0/0xb
[ 1593.770841] eax: fffffffc ebx: fffffff4 ecx: 00000002 edx: f9955f00
[ 1593.770844] esi: 00000000 edi: fffffffc ebp: e2ae6000 esp: e2ae7f48
[ 1593.770847] ds: 007b es: 007b ss: 0068
[ 1593.770850] Process rmmod (pid: 13348, ti=e2ae6000 task=dfd6f030
task.ti=e2ae 6000)
[ 1593.770852] Stack: f9952114 f9955e00 00000000 00000003 f9955066
c01374a1 6761 6663 36383231
[ 1593.770860] 62666234 b7eec000 f7d68780 c014f67b ffffffff
b7eed000 f7d6 8784 e2616aac
[ 1593.770868] b7eed000 e2616ab8 e2616aac f7d68780 00d687b4
f9955e00 0000 0880 e2ae7fa8
[ 1593.770877] Call Trace:
[ 1593.770879] [<f9952114>] display_device_unregister+0x13/0x51 [display]
[ 1593.770888] [<f9955066>] cfag12864bfb_exit+0xa/0x23 [cfag12864bfb]
[ 1593.770893] [<c01374a1>] sys_delete_module+0x11d/0x189
[ 1593.770902] [<c014f67b>] do_munmap+0x177/0x1d0
[ 1593.770913] [<c0102d76>] sysenter_past_esp+0x5f/0x85
[ 1593.770922] [<c02e0033>] atm_dev_ioctl+0x35f/0x655
[ 1593.770929] =======================
[ 1593.770931] Code: c0 8d 54 24 08 8d 4c 24 1c 89 4c 24 1c 89 4c 24
20 8b 4c 24 38 89 0c 24 89 e9 8b 44 24 04 e8 3f ff ff ff 83 c4 24 5b
5e 5f 5d c3 <f0> ff 08 79 05 e8 1b 01 00 00 c3 f0 ff 00 7f 05 e8 34
00 00 00
[ 1593.770973] EIP: [<c02e3f3f>] mutex_lock+0x0/0xb SS:ESP 0068:e2ae7f48
[ 1593.770979]

--
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm

2006-12-30 03:32:32

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: Display class

Hi,

On Tuesday 05 December 2006 13:03, James Simmons wrote:
> +int probe_edid(struct display_device *dev, void *data)
> +{
> +???????struct fb_monspecs spec;
> +???????ssize_t size = 45;

const ssize_t size = 45?

> +
> +???????dev->name = kzalloc(size, GFP_KERNEL);

Why do you need kzalloc here?

> +???????fb_edid_to_monspecs((unsigned char *) data, &spec);
> +???????strcpy(dev->name, spec.manufacturer);

You seem to be overwriting dev->name in the very next line?

> +???????return snprintf(dev->name, size, "%s %s %s\n", spec.manufacturer, spec.monitor, spec.ascii);
>

Is result of snprintf interesting to the callers?

--
Dmitry

2007-01-13 22:41:15

by James Simmons

[permalink] [raw]
Subject: Re: Display class


> Hi,
>
> On Tuesday 05 December 2006 13:03, James Simmons wrote:
> > +int probe_edid(struct display_device *dev, void *data)
> > +{
> > +???????struct fb_monspecs spec;
> > +???????ssize_t size = 45;

That code was only for testing. I do have new core code. Andrew could
you merge this patch as it is against the -mm tree.

This new class provides a way common interface for various types of
displays such as LCD, CRT, LVDS etc. It is a expansion of the lcd
class to include other types of displays.

diff -urN linux-2.6.20-rc3/drivers/video/display/display-sysfs.c linux-2.6.20-rc3-display/drivers/video/display/display-sysfs.c
--- linux-2.6.20-rc3/drivers/video/display/display-sysfs.c 1969-12-31 19:00:00.000000000 -0500
+++ linux-2.6.20-rc3-display/drivers/video/display/display-sysfs.c 2007-01-13 16:22:54.000000000 -0500
@@ -0,0 +1,208 @@
+/*
+ * display.c - Display output driver
+ *
+ * Copyright (C) 2007 James Simmons <[email protected]>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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/display.h>
+#include <linux/err.h>
+#include <linux/ctype.h>
+
+static ssize_t display_show_name(struct class_device *cdev, char *buf)
+{
+ struct display_device *dsp = to_display_device(cdev);
+ return snprintf(buf, PAGE_SIZE, "%s\n", dsp->name);
+}
+
+static ssize_t display_show_type(struct class_device *cdev, char *buf)
+{
+ struct display_device *dsp = to_display_device(cdev);
+ return snprintf(buf, PAGE_SIZE, "%s\n", dsp->driver->type);
+}
+
+static ssize_t display_show_power(struct class_device *cdev, char *buf)
+{
+ struct display_device *dsp = to_display_device(cdev);
+ ssize_t ret = -ENXIO;
+
+ mutex_lock(&dsp->lock);
+ if (likely(dsp->driver->get_power))
+ ret = sprintf(buf,"%.8x\n", dsp->driver->get_power(dsp));
+ mutex_unlock(&dsp->lock);
+ return ret;
+}
+
+static ssize_t display_store_power(struct class_device *cdev,
+ const char *buf, size_t count)
+{
+ struct display_device *dsp = to_display_device(cdev);
+ ssize_t size;
+ char *endp;
+ int power;
+
+ power = simple_strtoul(buf, &endp, 0);
+ size = endp - buf;
+ if (*endp && isspace(*endp))
+ size++;
+ if (size != count)
+ return -EINVAL;
+
+ mutex_lock(&dsp->lock);
+ if (likely(dsp->driver->set_power)) {
+ dsp->request_state = power;
+ dsp->driver->set_power(dsp);
+ }
+ mutex_unlock(&dsp->lock);
+ return count;
+}
+
+static ssize_t display_show_contrast(struct class_device *cdev, char *buf)
+{
+ struct display_device *dsp = to_display_device(cdev);
+ ssize_t rc = -ENXIO;
+
+ mutex_lock(&dsp->lock);
+ if (likely(dsp->driver) && dsp->driver->get_contrast)
+ rc = sprintf(buf, "%d\n", dsp->driver->get_contrast(dsp));
+ mutex_unlock(&dsp->lock);
+ return rc;
+}
+
+static ssize_t display_store_contrast(struct class_device *cdev, const char *buf, size_t count)
+{
+
+ struct display_device *dsp = to_display_device(cdev);
+ ssize_t ret = -EINVAL, size;
+ int contrast;
+ char *endp;
+
+ contrast = simple_strtoul(buf, &endp, 0);
+ size = endp - buf;
+
+ if (*endp && isspace(*endp))
+ size++;
+
+ if (size != count)
+ return ret;
+
+ mutex_lock(&dsp->lock);
+ if (likely(dsp->driver && dsp->driver->set_contrast)) {
+ pr_debug("display: set contrast to %d\n", contrast);
+ dsp->driver->set_contrast(dsp, contrast);
+ ret = count;
+ }
+ mutex_unlock(&dsp->lock);
+ return ret;
+}
+
+static ssize_t display_show_max_contrast(struct class_device *cdev, char *buf)
+{
+ struct display_device *dsp = to_display_device(cdev);
+ ssize_t rc = -ENXIO;
+
+ mutex_lock(&dsp->lock);
+ if (likely(dsp->driver))
+ rc = sprintf(buf, "%d\n", dsp->driver->max_contrast);
+ mutex_unlock(&dsp->lock);
+ return rc;
+}
+
+static void display_class_release(struct class_device *dev)
+{
+ struct display_device *dsp = to_display_device(dev);
+ kfree(dsp);
+}
+
+static struct class_device_attribute display_attributes[] = {
+ __ATTR(name, S_IRUGO, display_show_name, NULL),
+ __ATTR(type, S_IRUGO, display_show_type, NULL),
+ __ATTR(power, S_IRUGO | S_IWUSR, display_show_power, display_store_power),
+ __ATTR(contrast, S_IRUGO | S_IWUSR, display_show_contrast, display_store_contrast),
+ __ATTR(max_contrast, S_IRUGO, display_show_max_contrast, NULL),
+};
+
+static struct class display_class = {
+ .name = "display",
+ .release = display_class_release,
+ .class_dev_attrs = display_attributes,
+};
+
+static int index;
+
+struct display_device *display_device_register(struct display_driver *driver,
+ struct device *dev, void *devdata)
+{
+ struct display_device *new_dev;
+ int ret = -ENOMEM;
+
+ if (unlikely(!driver))
+ return ERR_PTR(-EINVAL);
+
+ new_dev = kzalloc(sizeof(struct display_device), GFP_KERNEL);
+ if (likely(new_dev) && unlikely(driver->probe(new_dev, devdata))) {
+ mutex_init(&new_dev->lock);
+
+ new_dev->class_dev.groups = &driver->type_prop;
+ new_dev->class_dev.class = &display_class;
+ new_dev->class_dev.dev = dev;
+ new_dev->driver = driver;
+
+ sprintf(new_dev->class_dev.class_id, "display%d", index++);
+ class_set_devdata(&new_dev->class_dev, devdata);
+ ret = class_device_register(&new_dev->class_dev);
+ }
+ if (unlikely(ret)) {
+ kfree(new_dev);
+ new_dev = ERR_PTR(ret);
+ }
+ return new_dev;
+}
+EXPORT_SYMBOL(display_device_register);
+
+void display_device_unregister(struct display_device *dev)
+{
+ if (!dev)
+ return;
+ mutex_lock(&dev->lock);
+ class_device_unregister(&dev->class_dev);
+ dev->driver = NULL;
+ index--;
+ mutex_unlock(&dev->lock);
+ kfree(dev);
+}
+EXPORT_SYMBOL(display_device_unregister);
+
+static void __exit display_class_exit(void)
+{
+ class_unregister(&display_class);
+}
+
+static int __init display_class_init(void)
+{
+ return class_register(&display_class);
+}
+
+postcore_initcall(display_class_init);
+module_exit(display_class_exit);
+
+MODULE_DESCRIPTION("Display Hardware handling");
+MODULE_AUTHOR("James Simmons <[email protected]>");
+MODULE_LICENSE("GPL");
diff -urN linux-2.6.20-rc3/drivers/video/display/Kconfig linux-2.6.20-rc3-display/drivers/video/display/Kconfig
--- linux-2.6.20-rc3/drivers/video/display/Kconfig 1969-12-31 19:00:00.000000000 -0500
+++ linux-2.6.20-rc3-display/drivers/video/display/Kconfig 2007-01-13 16:00:51.000000000 -0500
@@ -0,0 +1,24 @@
+#
+# Display drivers configuration
+#
+
+menu "Display device support"
+
+config DISPLAY_SUPPORT
+ tristate "Display panel/monitor support"
+ ---help---
+ This framework adds support for low-level control of a display.
+ This includes support for power.
+
+ Enable this to be able to choose the drivers for controlling the
+ physical display panel/monitor on some platforms. This not only
+ covers LCD displays for PDAs but also other types of displays
+ such as CRT, TVout etc.
+
+ To have support for your specific display panel you will have to
+ select the proper drivers which depend on this option.
+
+comment "Display hardware drivers"
+ depends on DISPLAY_SUPPORT
+
+endmenu
diff -urN linux-2.6.20-rc3/drivers/video/display/Makefile linux-2.6.20-rc3-display/drivers/video/display/Makefile
--- linux-2.6.20-rc3/drivers/video/display/Makefile 1969-12-31 19:00:00.000000000 -0500
+++ linux-2.6.20-rc3-display/drivers/video/display/Makefile 2007-01-13 16:01:03.000000000 -0500
@@ -0,0 +1,6 @@
+# Display drivers
+
+display-objs := display-sysfs.o
+
+obj-$(CONFIG_DISPLAY_SUPPORT) += display.o
+
diff -urN linux-2.6.20-rc3/drivers/video/Kconfig linux-2.6.20-rc3-display/drivers/video/Kconfig
--- linux-2.6.20-rc3/drivers/video/Kconfig 2007-01-08 14:00:26.000000000 -0500
+++ linux-2.6.20-rc3-display/drivers/video/Kconfig 2007-01-13 09:23:03.000000000 -0500
@@ -6,6 +6,7 @@

if SYSFS
source "drivers/video/backlight/Kconfig"
+ source "drivers/video/display/Kconfig"
endif

config FB
diff -urN linux-2.6.20-rc3/drivers/video/Makefile linux-2.6.20-rc3-display/drivers/video/Makefile
--- linux-2.6.20-rc3/drivers/video/Makefile 2007-01-08 14:00:26.000000000 -0500
+++ linux-2.6.20-rc3-display/drivers/video/Makefile 2007-01-13 09:22:40.000000000 -0500
@@ -12,7 +12,7 @@

obj-$(CONFIG_VT) += console/
obj-$(CONFIG_LOGO) += logo/
-obj-$(CONFIG_SYSFS) += backlight/
+obj-$(CONFIG_SYSFS) += backlight/ display/

obj-$(CONFIG_FB_CFB_FILLRECT) += cfbfillrect.o
obj-$(CONFIG_FB_CFB_COPYAREA) += cfbcopyarea.o
diff -urN linux-2.6.20-rc3/include/linux/display.h linux-2.6.20-rc3-display/include/linux/display.h
--- linux-2.6.20-rc3/include/linux/display.h 1969-12-31 19:00:00.000000000 -0500
+++ linux-2.6.20-rc3-display/include/linux/display.h 2007-01-13 16:23:51.000000000 -0500
@@ -0,0 +1,60 @@
+/*
+ * Copyright (C) 2006 James Simmons <[email protected]>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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 _LINUX_DISPLAY_H
+#define _LINUX_DISPLAY_H
+
+#include <linux/device.h>
+
+struct display_device;
+
+/* This structure defines all the properties of a Display. */
+struct display_driver {
+ int (*set_power)(struct display_device *);
+ int (*get_power)(struct display_device *);
+ int (*set_contrast)(struct display_device *, unsigned int);
+ int (*get_contrast)(struct display_device *);
+ int (*probe)(struct display_device *, void *);
+ int (*remove)(struct display_device *);
+ struct attribute_group *type_prop;
+ int max_contrast;
+ char type[16];
+};
+
+struct display_device {
+ struct module *owner; /* Owner module */
+ char *name;
+ struct mutex lock;
+ int request_state;
+ struct display_driver *driver;
+ struct class_device class_dev; /* The class device structure */
+};
+
+extern struct display_device *display_device_register(struct display_driver *driver,
+ struct device *dev, void *devdata);
+extern void display_device_unregister(struct display_device *dev);
+
+extern int display_probe_edid(struct display_device *dev, void *devdata);
+
+#define to_display_device(obj) container_of(obj, struct display_device, class_dev)
+
+#endif

2007-01-13 22:48:04

by James Simmons

[permalink] [raw]
Subject: Re: [Linux-fbdev-devel] Display class


Andrew please apply this patch.

Signed-off-by: James Simmons <[email protected]>

>
> > Hi,
> >
> > On Tuesday 05 December 2006 13:03, James Simmons wrote:
> > > +int probe_edid(struct display_device *dev, void *data)
> > > +{
> > > +???????struct fb_monspecs spec;
> > > +???????ssize_t size = 45;
>
> That code was only for testing. I do have new core code. Andrew could
> you merge this patch as it is against the -mm tree.
>
> This new class provides a way common interface for various types of
> displays such as LCD, CRT, LVDS etc. It is a expansion of the lcd
> class to include other types of displays.
>
> diff -urN linux-2.6.20-rc3/drivers/video/display/display-sysfs.c linux-2.6.20-rc3-display/drivers/video/display/display-sysfs.c
> --- linux-2.6.20-rc3/drivers/video/display/display-sysfs.c 1969-12-31 19:00:00.000000000 -0500
> +++ linux-2.6.20-rc3-display/drivers/video/display/display-sysfs.c 2007-01-13 16:22:54.000000000 -0500
> @@ -0,0 +1,208 @@
> +/*
> + * display.c - Display output driver
> + *
> + * Copyright (C) 2007 James Simmons <[email protected]>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * 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/display.h>
> +#include <linux/err.h>
> +#include <linux/ctype.h>
> +
> +static ssize_t display_show_name(struct class_device *cdev, char *buf)
> +{
> + struct display_device *dsp = to_display_device(cdev);
> + return snprintf(buf, PAGE_SIZE, "%s\n", dsp->name);
> +}
> +
> +static ssize_t display_show_type(struct class_device *cdev, char *buf)
> +{
> + struct display_device *dsp = to_display_device(cdev);
> + return snprintf(buf, PAGE_SIZE, "%s\n", dsp->driver->type);
> +}
> +
> +static ssize_t display_show_power(struct class_device *cdev, char *buf)
> +{
> + struct display_device *dsp = to_display_device(cdev);
> + ssize_t ret = -ENXIO;
> +
> + mutex_lock(&dsp->lock);
> + if (likely(dsp->driver->get_power))
> + ret = sprintf(buf,"%.8x\n", dsp->driver->get_power(dsp));
> + mutex_unlock(&dsp->lock);
> + return ret;
> +}
> +
> +static ssize_t display_store_power(struct class_device *cdev,
> + const char *buf, size_t count)
> +{
> + struct display_device *dsp = to_display_device(cdev);
> + ssize_t size;
> + char *endp;
> + int power;
> +
> + power = simple_strtoul(buf, &endp, 0);
> + size = endp - buf;
> + if (*endp && isspace(*endp))
> + size++;
> + if (size != count)
> + return -EINVAL;
> +
> + mutex_lock(&dsp->lock);
> + if (likely(dsp->driver->set_power)) {
> + dsp->request_state = power;
> + dsp->driver->set_power(dsp);
> + }
> + mutex_unlock(&dsp->lock);
> + return count;
> +}
> +
> +static ssize_t display_show_contrast(struct class_device *cdev, char *buf)
> +{
> + struct display_device *dsp = to_display_device(cdev);
> + ssize_t rc = -ENXIO;
> +
> + mutex_lock(&dsp->lock);
> + if (likely(dsp->driver) && dsp->driver->get_contrast)
> + rc = sprintf(buf, "%d\n", dsp->driver->get_contrast(dsp));
> + mutex_unlock(&dsp->lock);
> + return rc;
> +}
> +
> +static ssize_t display_store_contrast(struct class_device *cdev, const char *buf, size_t count)
> +{
> +
> + struct display_device *dsp = to_display_device(cdev);
> + ssize_t ret = -EINVAL, size;
> + int contrast;
> + char *endp;
> +
> + contrast = simple_strtoul(buf, &endp, 0);
> + size = endp - buf;
> +
> + if (*endp && isspace(*endp))
> + size++;
> +
> + if (size != count)
> + return ret;
> +
> + mutex_lock(&dsp->lock);
> + if (likely(dsp->driver && dsp->driver->set_contrast)) {
> + pr_debug("display: set contrast to %d\n", contrast);
> + dsp->driver->set_contrast(dsp, contrast);
> + ret = count;
> + }
> + mutex_unlock(&dsp->lock);
> + return ret;
> +}
> +
> +static ssize_t display_show_max_contrast(struct class_device *cdev, char *buf)
> +{
> + struct display_device *dsp = to_display_device(cdev);
> + ssize_t rc = -ENXIO;
> +
> + mutex_lock(&dsp->lock);
> + if (likely(dsp->driver))
> + rc = sprintf(buf, "%d\n", dsp->driver->max_contrast);
> + mutex_unlock(&dsp->lock);
> + return rc;
> +}
> +
> +static void display_class_release(struct class_device *dev)
> +{
> + struct display_device *dsp = to_display_device(dev);
> + kfree(dsp);
> +}
> +
> +static struct class_device_attribute display_attributes[] = {
> + __ATTR(name, S_IRUGO, display_show_name, NULL),
> + __ATTR(type, S_IRUGO, display_show_type, NULL),
> + __ATTR(power, S_IRUGO | S_IWUSR, display_show_power, display_store_power),
> + __ATTR(contrast, S_IRUGO | S_IWUSR, display_show_contrast, display_store_contrast),
> + __ATTR(max_contrast, S_IRUGO, display_show_max_contrast, NULL),
> +};
> +
> +static struct class display_class = {
> + .name = "display",
> + .release = display_class_release,
> + .class_dev_attrs = display_attributes,
> +};
> +
> +static int index;
> +
> +struct display_device *display_device_register(struct display_driver *driver,
> + struct device *dev, void *devdata)
> +{
> + struct display_device *new_dev;
> + int ret = -ENOMEM;
> +
> + if (unlikely(!driver))
> + return ERR_PTR(-EINVAL);
> +
> + new_dev = kzalloc(sizeof(struct display_device), GFP_KERNEL);
> + if (likely(new_dev) && unlikely(driver->probe(new_dev, devdata))) {
> + mutex_init(&new_dev->lock);
> +
> + new_dev->class_dev.groups = &driver->type_prop;
> + new_dev->class_dev.class = &display_class;
> + new_dev->class_dev.dev = dev;
> + new_dev->driver = driver;
> +
> + sprintf(new_dev->class_dev.class_id, "display%d", index++);
> + class_set_devdata(&new_dev->class_dev, devdata);
> + ret = class_device_register(&new_dev->class_dev);
> + }
> + if (unlikely(ret)) {
> + kfree(new_dev);
> + new_dev = ERR_PTR(ret);
> + }
> + return new_dev;
> +}
> +EXPORT_SYMBOL(display_device_register);
> +
> +void display_device_unregister(struct display_device *dev)
> +{
> + if (!dev)
> + return;
> + mutex_lock(&dev->lock);
> + class_device_unregister(&dev->class_dev);
> + dev->driver = NULL;
> + index--;
> + mutex_unlock(&dev->lock);
> + kfree(dev);
> +}
> +EXPORT_SYMBOL(display_device_unregister);
> +
> +static void __exit display_class_exit(void)
> +{
> + class_unregister(&display_class);
> +}
> +
> +static int __init display_class_init(void)
> +{
> + return class_register(&display_class);
> +}
> +
> +postcore_initcall(display_class_init);
> +module_exit(display_class_exit);
> +
> +MODULE_DESCRIPTION("Display Hardware handling");
> +MODULE_AUTHOR("James Simmons <[email protected]>");
> +MODULE_LICENSE("GPL");
> diff -urN linux-2.6.20-rc3/drivers/video/display/Kconfig linux-2.6.20-rc3-display/drivers/video/display/Kconfig
> --- linux-2.6.20-rc3/drivers/video/display/Kconfig 1969-12-31 19:00:00.000000000 -0500
> +++ linux-2.6.20-rc3-display/drivers/video/display/Kconfig 2007-01-13 16:00:51.000000000 -0500
> @@ -0,0 +1,24 @@
> +#
> +# Display drivers configuration
> +#
> +
> +menu "Display device support"
> +
> +config DISPLAY_SUPPORT
> + tristate "Display panel/monitor support"
> + ---help---
> + This framework adds support for low-level control of a display.
> + This includes support for power.
> +
> + Enable this to be able to choose the drivers for controlling the
> + physical display panel/monitor on some platforms. This not only
> + covers LCD displays for PDAs but also other types of displays
> + such as CRT, TVout etc.
> +
> + To have support for your specific display panel you will have to
> + select the proper drivers which depend on this option.
> +
> +comment "Display hardware drivers"
> + depends on DISPLAY_SUPPORT
> +
> +endmenu
> diff -urN linux-2.6.20-rc3/drivers/video/display/Makefile linux-2.6.20-rc3-display/drivers/video/display/Makefile
> --- linux-2.6.20-rc3/drivers/video/display/Makefile 1969-12-31 19:00:00.000000000 -0500
> +++ linux-2.6.20-rc3-display/drivers/video/display/Makefile 2007-01-13 16:01:03.000000000 -0500
> @@ -0,0 +1,6 @@
> +# Display drivers
> +
> +display-objs := display-sysfs.o
> +
> +obj-$(CONFIG_DISPLAY_SUPPORT) += display.o
> +
> diff -urN linux-2.6.20-rc3/drivers/video/Kconfig linux-2.6.20-rc3-display/drivers/video/Kconfig
> --- linux-2.6.20-rc3/drivers/video/Kconfig 2007-01-08 14:00:26.000000000 -0500
> +++ linux-2.6.20-rc3-display/drivers/video/Kconfig 2007-01-13 09:23:03.000000000 -0500
> @@ -6,6 +6,7 @@
>
> if SYSFS
> source "drivers/video/backlight/Kconfig"
> + source "drivers/video/display/Kconfig"
> endif
>
> config FB
> diff -urN linux-2.6.20-rc3/drivers/video/Makefile linux-2.6.20-rc3-display/drivers/video/Makefile
> --- linux-2.6.20-rc3/drivers/video/Makefile 2007-01-08 14:00:26.000000000 -0500
> +++ linux-2.6.20-rc3-display/drivers/video/Makefile 2007-01-13 09:22:40.000000000 -0500
> @@ -12,7 +12,7 @@
>
> obj-$(CONFIG_VT) += console/
> obj-$(CONFIG_LOGO) += logo/
> -obj-$(CONFIG_SYSFS) += backlight/
> +obj-$(CONFIG_SYSFS) += backlight/ display/
>
> obj-$(CONFIG_FB_CFB_FILLRECT) += cfbfillrect.o
> obj-$(CONFIG_FB_CFB_COPYAREA) += cfbcopyarea.o
> diff -urN linux-2.6.20-rc3/include/linux/display.h linux-2.6.20-rc3-display/include/linux/display.h
> --- linux-2.6.20-rc3/include/linux/display.h 1969-12-31 19:00:00.000000000 -0500
> +++ linux-2.6.20-rc3-display/include/linux/display.h 2007-01-13 16:23:51.000000000 -0500
> @@ -0,0 +1,60 @@
> +/*
> + * Copyright (C) 2006 James Simmons <[email protected]>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * 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 _LINUX_DISPLAY_H
> +#define _LINUX_DISPLAY_H
> +
> +#include <linux/device.h>
> +
> +struct display_device;
> +
> +/* This structure defines all the properties of a Display. */
> +struct display_driver {
> + int (*set_power)(struct display_device *);
> + int (*get_power)(struct display_device *);
> + int (*set_contrast)(struct display_device *, unsigned int);
> + int (*get_contrast)(struct display_device *);
> + int (*probe)(struct display_device *, void *);
> + int (*remove)(struct display_device *);
> + struct attribute_group *type_prop;
> + int max_contrast;
> + char type[16];
> +};
> +
> +struct display_device {
> + struct module *owner; /* Owner module */
> + char *name;
> + struct mutex lock;
> + int request_state;
> + struct display_driver *driver;
> + struct class_device class_dev; /* The class device structure */
> +};
> +
> +extern struct display_device *display_device_register(struct display_driver *driver,
> + struct device *dev, void *devdata);
> +extern void display_device_unregister(struct display_device *dev);
> +
> +extern int display_probe_edid(struct display_device *dev, void *devdata);
> +
> +#define to_display_device(obj) container_of(obj, struct display_device, class_dev)
> +
> +#endif
>

2007-01-14 08:05:23

by Greg KH

[permalink] [raw]
Subject: Re: Display class

On Sat, Jan 13, 2007 at 10:40:55PM +0000, James Simmons wrote:
>
> > Hi,
> >
> > On Tuesday 05 December 2006 13:03, James Simmons wrote:
> > > +int probe_edid(struct display_device *dev, void *data)
> > > +{
> > > +???????struct fb_monspecs spec;
> > > +???????ssize_t size = 45;
>
> That code was only for testing. I do have new core code. Andrew could
> you merge this patch as it is against the -mm tree.
>
> This new class provides a way common interface for various types of
> displays such as LCD, CRT, LVDS etc. It is a expansion of the lcd
> class to include other types of displays.

Any chance you can change this to use "struct device" instead of "struct
class_device" so I don't have to convert it myself in the next few weeks
if it ever gets merged? :)

The functionality will be the same, if not, please let me know what
problems you have with the change.

thanks,

greg k-h

2007-01-14 08:05:39

by Greg KH

[permalink] [raw]
Subject: Re: Display class

On Sat, Jan 13, 2007 at 10:40:55PM +0000, James Simmons wrote:
>
> > Hi,
> >
> > On Tuesday 05 December 2006 13:03, James Simmons wrote:
> > > +int probe_edid(struct display_device *dev, void *data)
> > > +{
> > > +???????struct fb_monspecs spec;
> > > +???????ssize_t size = 45;
>
> That code was only for testing. I do have new core code. Andrew could
> you merge this patch as it is against the -mm tree.
>
> This new class provides a way common interface for various types of
> displays such as LCD, CRT, LVDS etc. It is a expansion of the lcd
> class to include other types of displays.

Have you worked with the DRM developers who also need to tie into this
CRT class somehow?

thanks,

greg k-h