Re: [PATCH] display class
How does this look? The new and improved display class. Signed-Off: James Simmons <[EMAIL PROTECTED]> 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.0 -0500 +++ fbdev-2.6/drivers/video/display/display-sysfs.c 2007-02-28 10:09:47.0 -0500 @@ -0,0 +1,217 @@ +/* + * display-sysfs.c - Display output driver sysfs interface + * + * 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 +#include +#include +#include +#include + +static ssize_t display_show_name(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct display_device *dsp = dev_get_drvdata(dev); + return snprintf(buf, PAGE_SIZE, "%s\n", dsp->name); +} + +static ssize_t display_show_type(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct display_device *dsp = dev_get_drvdata(dev); + return snprintf(buf, PAGE_SIZE, "%s\n", dsp->type); +} + +static ssize_t display_show_contrast(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct display_device *dsp = dev_get_drvdata(dev); + 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 device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct display_device *dsp = dev_get_drvdata(dev); + 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 device *dev, + struct device_attribute *attr, + char *buf) +{ + struct display_device *dsp = dev_get_drvdata(dev); + 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 struct device_attribute display_attrs[] = { + __ATTR(name, S_IRUGO, display_show_name, NULL), + __ATTR(type, S_IRUGO, display_show_type, NULL), + __ATTR(contrast, S_IRUGO | S_IWUSR, display_show_contrast, display_store_contrast), + __ATTR(max_contrast, S_IRUGO, display_show_max_contrast, NULL), +}; + +static int display_suspend(struct device *dev, pm_message_t state) +{ + struct display_device *dsp = dev_get_drvdata(dev); + + mutex_lock(&dsp->lock); + if (likely(dsp->driver->suspend)) + dsp->driver->suspend(dsp, state); + mutex_unlock(&dsp->lock); + return 0; +}; + +static int display_resume(struct device *dev) +{ + struct display_device *dsp = dev_get_drvdata(dev); + + mutex_lock(&dsp->lock); + if (likely(dsp->driver->resume)) + dsp->driver->resume(dsp); + mutex_unlock(
Re: [PATCH] display class
> this stuff looks el-crappo in an 80-col display. Cleaning up > This guy can have static scope. > > It's a bit awkward having a fixed size. I think lib/idr.c would suit for > this. > > Also, we do set_bit() and clear_bit() on this, which are needlessly atomic. > If we hold a suitable lock then we can use __set_bit() and __clear_bit(). > > And we do need a suitable lock. Fixing... Never knew about idr. Hum nice :-) I will have a new patch your way. I just need to do some teste first. > Why all the ifdeffery? I think plain old > > > static void __exit display_class_exit(void) > { > class_destroy(display_class); > } > module_exit(display_class_exit); > module_init(display_class_init); That is from the hunting of the bug with the broken symlinks in /sys. Will go back to sanity. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] display class
> On Sun, 25 Feb 2007 18:39:57 + (GMT) James Simmons <[EMAIL PROTECTED]> > wrote: > > Here is the second round to the new display class. This is meant to unite > the various solutions to display units ie acpi output device, auxdisplay > and the defunct lcd class in the backlight directory. Please apply. > Little things.. > +static ssize_t display_show_name(struct device *dev, struct device_attribute > *attr, > +static ssize_t display_show_type(struct device *dev, struct device_attribute > *attr, > +static ssize_t display_show_contrast(struct device *dev, struct > device_attribute *attr, this stuff looks el-crappo in an 80-col display. > +DECLARE_BITMAP(inuse, NUM_OF_DISPLAYS); This guy can have static scope. It's a bit awkward having a fixed size. I think lib/idr.c would suit for this. Also, we do set_bit() and clear_bit() on this, which are needlessly atomic. If we hold a suitable lock then we can use __set_bit() and __clear_bit(). And we do need a suitable lock. > + > +struct display_device *display_device_register(struct display_driver *driver, > + struct device *parent, void > *devdata) > +{ > + int idx = find_first_zero_bit(inuse, NUM_OF_DISPLAYS); > + struct display_device *new_dev = NULL; > + struct device *display_device = NULL; > + > + if (unlikely(!driver)) > + return ERR_PTR(-EINVAL); > + > + display_device = device_create(display_class, parent, 0, "display%d", > idx); > + if (IS_ERR(display_device)) > + return ERR_PTR(-ENOMEM); > + > + new_dev = kzalloc(sizeof(struct display_device), GFP_KERNEL); > + if (likely(new_dev) && unlikely(driver->probe(new_dev, devdata))) { > + dev_set_drvdata(display_device, new_dev); > + new_dev->dev = display_device; > + new_dev->parent = parent; > + new_dev->driver = driver; > + mutex_init(&new_dev->lock); > + set_bit(idx, inuse); > + new_dev->idx = idx; > + } else > + device_unregister(display_device); > + return new_dev; > +} > +EXPORT_SYMBOL(display_device_register); Because I think this function is racy in its handling of inuse? > +static int __init display_class_init(void) > +{ > + display_class = class_create(THIS_MODULE, "display"); > + if (IS_ERR(display_class)) { > + printk(KERN_ERR "Failed to create display class\n"); > + display_class = NULL; > + return -EINVAL; > + } > + display_class->dev_attrs = display_attrs; > + display_class->suspend = display_suspend; > + display_class->resume = display_resume; > + bitmap_zero(inuse, NUM_OF_DISPLAYS); > + return 0; > +} > + > +#ifdef MODULE > +module_init(display_class_init); > + > +static void __exit display_class_exit(void) > +{ > + class_destroy(display_class); > +} > +module_exit(display_class_exit); > +#else > +subsys_initcall(display_class_init); > +#endif Why all the ifdeffery? I think plain old static void __exit display_class_exit(void) { class_destroy(display_class); } module_exit(display_class_exit); module_init(display_class_init); would suit here. Unless there's some special reason why we need subsys_initcall() here if it's linked into vmlinux. If so, please add a comment so the next reader doesn't get all confused too. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] display class
Here is the second round to the new display class. This is meant to unite the various solutions to display units ie acpi output device, auxdisplay and the defunct lcd class in the backlight directory. Please apply. Signed-off-by: James Simmons <[EMAIL PROTECTED]> 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.0 -0500 +++ fbdev-2.6/drivers/video/display/display-sysfs.c 2007-02-24 16:20:42.0 -0500 @@ -0,0 +1,203 @@ +/* + * display-sysfs.c - Display output driver sysfs interface + * + * 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 +#include +#include +#include +#include + +static ssize_t display_show_name(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct display_device *dsp = dev_get_drvdata(dev); + return snprintf(buf, PAGE_SIZE, "%s\n", dsp->name); +} + +static ssize_t display_show_type(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct display_device *dsp = dev_get_drvdata(dev); + return snprintf(buf, PAGE_SIZE, "%s\n", dsp->type); +} + +static ssize_t display_show_contrast(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct display_device *dsp = dev_get_drvdata(dev); + 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 device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct display_device *dsp = dev_get_drvdata(dev); + 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 device *dev, struct device_attribute *attr, + char *buf) +{ + struct display_device *dsp = dev_get_drvdata(dev); + 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 struct device_attribute display_attrs[] = { + __ATTR(name, S_IRUGO, display_show_name, NULL), + __ATTR(type, S_IRUGO, display_show_type, NULL), + __ATTR(contrast, S_IRUGO | S_IWUSR, display_show_contrast, display_store_contrast), + __ATTR(max_contrast, S_IRUGO, display_show_max_contrast, NULL), +}; + +static int display_suspend(struct device *dev, pm_message_t state) +{ + struct display_device *dsp = dev_get_drvdata(dev); + + mutex_lock(&dsp->lock); + if (likely(dsp->driver->suspend)) + dsp->driver->suspend(dsp, state); + mutex_unlock(&dsp->lock); + return 0; +}; + +static int display_resume(struct device *dev) +{ + struct display_device *dsp = dev_get_drvdata(dev); + + mutex_lock(&dsp->lock); + if (likely(dsp->driver->resume)) +
Re: Display class
On Sat, Jan 13, 2007 at 10:40:55PM +, 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Display class
On Sat, Jan 13, 2007 at 10:40:55PM +, 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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.c1969-12-31 > 19:00:00.0 -0500 > +++ linux-2.6.20-rc3-display/drivers/video/display/display-sysfs.c > 2007-01-13 16:22:54.0 -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 > +#include > +#include > +#include > + > +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; > + > + mute
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.0 -0500 +++ linux-2.6.20-rc3-display/drivers/video/display/display-sysfs.c 2007-01-13 16:22:54.0 -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 +#include +#include +#include + +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) +{ + stru
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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: [EMAIL PROTECTED]:/usr/src/git/linux-2.6.19-git12# dmesg [ 1593.770774] BUG: unable to handle kernel paging request at virtual address ff fc [ 1593.770780] printing eip: [ 1593.770782] c02e3f3f [ 1593.770784] *pde = 3067 [ 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:[]Not tainted VLI [ 1593.770831] EFLAGS: 00010282 (2.6.19-git12 #1) [ 1593.770838] EIP is at mutex_lock+0x0/0xb [ 1593.770841] eax: fffc ebx: fff4 ecx: 0002 edx: f9955f00 [ 1593.770844] esi: edi: fffc 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 0003 f9955066 c01374a1 6761 6663 36383231 [ 1593.770860]62666234 b7eec000 f7d68780 c014f67b b7eed000 f7d6 8784 e2616aac [ 1593.770868]b7eed000 e2616ab8 e2616aac f7d68780 00d687b4 f9955e00 0880 e2ae7fa8 [ 1593.770877] Call Trace: [ 1593.770879] [] display_device_unregister+0x13/0x51 [display] [ 1593.770888] [] cfag12864bfb_exit+0xa/0x23 [cfag12864bfb] [ 1593.770893] [] sys_delete_module+0x11d/0x189 [ 1593.770902] [] do_munmap+0x177/0x1d0 [ 1593.770913] [] sysenter_past_esp+0x5f/0x85 [ 1593.770922] [] 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 ff 08 79 05 e8 1b 01 00 00 c3 f0 ff 00 7f 05 e8 34 00 00 00 [ 1593.770973] EIP: [] mutex_lock+0x0/0xb SS:ESP 0068:e2ae7f48 [ 1593.770979] -- Miguel Ojeda http://maxextreme.googlepages.com/index.htm - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Display class
On Wed, 6 Dec 2006 15:10:44 + (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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Display class
On Wed, 6 Dec 2006 18:24:08 + (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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 000..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] EA 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 repr
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 > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Display class
On Wed, 6 Dec 2006 15:10:44 + (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_LEVELS128 #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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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.0 -0500 +++ fbdev-2.6/drivers/acpi/video.c 2006-12-06 05:01:22.0 -0500 @@ -30,6 +30,7 @@ #include #include #include +#include #include @@ -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.0 -0500 +++ fbdev-2.6/drivers/macintosh/Kconfig 2006-12-06 05:01:22.0 -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.0 -0500 +++ fbdev-2.6/drivers/video/display/display-ddc.c 2006-12-06 04:31:24.0 -0500 @@ -0,0 +1,49 @@ +/* + * display-ddc.c - EDID parsing code + * + * Copyright (C) 2006 James Simmons <[EMAIL PROTECTED]> + * + * ~~ + * + * This program is
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.0 -0500 +++ fbdev-2.6/drivers/acpi/video.c 2006-12-04 10:40:48.0 -0500 @@ -30,6 +30,9 @@ #include #include #include +#if defined(CONFIG_DISPLAY_DDC) +#include +#endif #include @@ -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.0 -0500 +++ fbdev-2.6/drivers/video/display/display-ddc.c 2006-12-03 05:27:00.0 -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 Sof
Re: Display class
On Tue, 5 Dec 2006 18:03:39 + (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.c2006-11-07 05:38:34.0 -0500 > +++ fbdev-2.6/drivers/acpi/video.c2006-12-04 10:40:48.0 -0500 > @@ -30,6 +30,9 @@ > #include > #include > #include > +#if defined(CONFIG_DISPLAY_DDC) > +#include > +#endif > > #include > > @@ -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.0 -0500 > +++ fbdev-2.6/drivers/video/display/display-ddc.c 2006-12-03 > 05:27:00.0 -0500 > @@ -0,0 +1,45 @@ > +/* > + * display-ddc.c - EDID paring code typo: parsing > +#include > +#include > +#include > +#include > + > +#include > + > +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.0 -0500 > +++ fbdev-2.6/drivers/video/display/display-sysfs.c 2006-12-04 > 07:48:34.0 -0500 > @@ -0,0 +1,164 @@ > +#include > +#include > +#include > +#include > + > +struct display_device *display_device_register(struct display_driver *driver, > + struct device *dev, void > *devdata) >
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.0 -0500 +++ fbdev-2.6/drivers/acpi/video.c 2006-12-04 10:40:48.0 -0500 @@ -30,6 +30,9 @@ #include #include #include +#if defined(CONFIG_DISPLAY_DDC) +#include +#endif #include @@ -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.0 -0500 +++ fbdev-2.6/drivers/video/display/display-ddc.c 2006-12-03 05:27:00.0 -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 Foundatio