[PATCH] fbdev sysfs imrovements

Subsystems: framebuffer layer, the rest

STALE7135d

8 messages, 4 authors, 2007-04-05 · open the first message on its own page

[PATCH] fbdev sysfs imrovements

From: James Simmons <hidden>
Date: 2007-03-20 14:25:54

This patch does several things to allow the underlying hardware to be 
shared amount many devices. The most important thing is the use of
the created device via device_create instead of the hardware device. No 
longer should fbdev drivers use the xxx_set_drvdata with the parent
bus device. The second change is having a bus independent power management
for the framebuffer driver. The final change is using the release method 
to cleanup the device. The reason again is to make the fbdev driver 
independent of the bus parent device. Feedback is welcomed.
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 2822526..2cff39c 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1268,8 +1268,6 @@ static const struct file_operations fb_fops = {
 #endif
 };
 
-struct class *fb_class;
-EXPORT_SYMBOL(fb_class);
 /**
  *	register_framebuffer - registers a frame buffer device
  *	@fb_info: frame buffer info structure
@@ -1295,14 +1293,9 @@ register_framebuffer(struct fb_info *fb_info)
 			break;
 	fb_info->node = i;
 
-	fb_info->dev = device_create(fb_class, fb_info->device,
-				     MKDEV(FB_MAJOR, i), "fb%d", i);
-	if (IS_ERR(fb_info->dev)) {
-		/* Not fatal */
+	/* Not fatal */
+	if (fb_init_device(fb_info))
 		printk(KERN_WARNING "Unable to create device for framebuffer %d; errno = %ld\n", i, PTR_ERR(fb_info->dev));
-		fb_info->dev = NULL;
-	} else
-		fb_init_device(fb_info);
 
 	if (fb_info->pixmap.addr == NULL) {
 		fb_info->pixmap.addr = kmalloc(FBPIXMAPSIZE, GFP_KERNEL);
@@ -1356,7 +1349,6 @@ unregister_framebuffer(struct fb_info *fb_info)
 	registered_fb[i]=NULL;
 	num_registered_fb--;
 	fb_cleanup_device(fb_info);
-	device_destroy(fb_class, MKDEV(FB_MAJOR, i));
 	event.info = fb_info;
 	fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
 	return 0;
@@ -1402,11 +1394,7 @@ fbmem_init(void)
 	if (register_chrdev(FB_MAJOR,"fb",&fb_fops))
 		printk("unable to get major %d for fb devs\n", FB_MAJOR);
 
-	fb_class = class_create(THIS_MODULE, "graphics");
-	if (IS_ERR(fb_class)) {
-		printk(KERN_WARNING "Unable to create fb class; errno = %ld\n", PTR_ERR(fb_class));
-		fb_class = NULL;
-	}
+	fb_create_class();
 	return 0;
 }
 
@@ -1415,8 +1403,8 @@ module_init(fbmem_init);
 static void __exit
 fbmem_exit(void)
 {
-	class_destroy(fb_class);
 	unregister_chrdev(FB_MAJOR, "fb");
+	fb_destroy_class();
 }
 
 module_exit(fbmem_exit);
diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
index 40c80c8..d3caba6 100644
--- a/drivers/video/fbsysfs.c
+++ b/drivers/video/fbsysfs.c
@@ -505,42 +505,99 @@ static struct device_attribute device_attrs[] = {
 #endif
 };
 
-int fb_init_device(struct fb_info *fb_info)
+struct class *fb_class;
+EXPORT_SYMBOL(fb_class);
+
+static void fb_device_release(struct device *dev)
 {
-	int i, error = 0;
+	struct fb_info *fb_info = dev_get_drvdata(dev);
 
-	dev_set_drvdata(fb_info->dev, fb_info);
+	if (fb_info) {
+		acquire_console_sem();
 
-	fb_info->class_flag |= FB_SYSFS_FLAG_ATTR;
+		// shutdown the hardware.
+		if (fb_info->fbops->fb_release)
+			fb_info->fbops->fb_release(fb_info, 0);
 
-	for (i = 0; i < ARRAY_SIZE(device_attrs); i++) {
-		error = device_create_file(fb_info->dev, &device_attrs[i]);
+		fb_dealloc_cmap(&fb_info->cmap);
+		unregister_framebuffer(fb_info);
 
-		if (error)
-			break;
+		//framebuffer_release(info); Not every driver does this yet
+		release_console_sem();
 	}
+}
 
-	if (error) {
-		while (--i >= 0)
-			device_remove_file(fb_info->dev, &device_attrs[i]);
+int fb_init_device(struct fb_info *fb_info)
+{
+	int error = 0;
+
+	fb_info->dev = device_create(fb_class, fb_info->device,
+					MKDEV(FB_MAJOR, fb_info->node),
+					"fb%d", fb_info->node);
+	if (IS_ERR(fb_info->dev)) {
 		fb_info->class_flag &= ~FB_SYSFS_FLAG_ATTR;
+		fb_info->dev = NULL;
+		error = -EINVAL;
+	} else {
+		dev_set_drvdata(fb_info->dev, fb_info);
+		fb_info->dev->release = fb_device_release;
+		fb_info->class_flag |= FB_SYSFS_FLAG_ATTR;
 	}
-
-	return 0;
+	return error;
 }
 
 void fb_cleanup_device(struct fb_info *fb_info)
 {
-	unsigned int i;
+	fb_info->class_flag &= ~FB_SYSFS_FLAG_ATTR;
+	dev_set_drvdata(fb_info->dev, NULL);
+	device_destroy(fb_class, MKDEV(FB_MAJOR, fb_info->node));
+}
 
-	if (fb_info->class_flag & FB_SYSFS_FLAG_ATTR) {
-		for (i = 0; i < ARRAY_SIZE(device_attrs); i++)
-			device_remove_file(fb_info->dev, &device_attrs[i]);
+static int fb_class_suspend(struct device *dev, pm_message_t state)
+{
+	struct fb_info *fb_info = dev_get_drvdata(dev);
+	int ret = 0;
 
-		fb_info->class_flag &= ~FB_SYSFS_FLAG_ATTR;
+	acquire_console_sem();
+	fb_set_suspend(fb_info, 1);
+	if (fb_info->fbops->fb_power)
+		ret = fb_info->fbops->fb_power(fb_info, state);
+	release_console_sem();
+	return ret;
+}
+
+static int fb_class_resume(struct device *dev)
+{
+	struct fb_info *fb_info = dev_get_drvdata(dev);
+	int ret = 0;
+
+	acquire_console_sem();
+	fb_set_suspend(fb_info, 0);
+	if (fb_info->fbops->fb_power)
+		ret = fb_info->fbops->fb_power(fb_info, PMSG_ON);
+	release_console_sem();
+	return ret;
+}
+
+void fb_create_class()
+{
+	fb_class = class_create(THIS_MODULE, "graphics");
+	if (IS_ERR(fb_class)) {
+		printk(KERN_WARNING "Unable to create fb class; errno = %ld\n", PTR_ERR(fb_class));
+		fb_class = NULL;
+	} else {
+		fb_class->suspend = fb_class_suspend;
+		fb_class->resume = fb_class_resume;
+		fb_class->dev_attrs = device_attrs;
 	}
 }
 
+void fb_destroy_class()
+{
+	if (fb_class)
+		class_destroy(fb_class);
+}
+
 #ifdef CONFIG_FB_BACKLIGHT
 /* This function generates a linear backlight curve
  *
diff --git a/include/linux/fb.h b/include/linux/fb.h
index be913ec..ab01e0e 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -629,6 +629,9 @@ struct fb_ops {
 	/* perform fb specific mmap */
 	int (*fb_mmap)(struct fb_info *info, struct vm_area_struct *vma);
 
+	/* Power management */
+	int (*fb_power)(struct fb_info *info, pm_message_t state);
+
 	/* save current hardware state */
 	void (*fb_save_state)(struct fb_info *info);
 
@@ -919,6 +922,8 @@ extern void framebuffer_release(struct fb_info *info);
 extern int fb_init_device(struct fb_info *fb_info);
 extern void fb_cleanup_device(struct fb_info *head);
 extern void fb_bl_default_curve(struct fb_info *fb_info, u8 off, u8 min, u8 max);
+extern void fb_destroy_class(void);
+extern void fb_create_class(void);
 
 /* drivers/video/fbmon.c */
 #define FB_MAXTIMINGS		0

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: [PATCH] fbdev sysfs imrovements

From: Greg KH <hidden>
Date: 2007-03-21 00:16:40

On Tue, Mar 20, 2007 at 02:25:49PM +0000, James Simmons wrote:
This patch does several things to allow the underlying hardware to be 
shared amount many devices. The most important thing is the use of
the created device via device_create instead of the hardware device. No 
longer should fbdev drivers use the xxx_set_drvdata with the parent
bus device. The second change is having a bus independent power management
for the framebuffer driver. The final change is using the release method 
to cleanup the device. The reason again is to make the fbdev driver 
independent of the bus parent device. Feedback is welcomed.
Looks good to me.

thanks,

greg k-h

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: [PATCH] fbdev sysfs imrovements

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2007-03-27 03:58:49

On Tue, 20 Mar 2007 14:25:49 +0000 (GMT) James Simmons [off-list ref] wrote:
This patch does several things to allow the underlying hardware to be 
shared amount many devices. The most important thing is the use of
the created device via device_create instead of the hardware device. No 
longer should fbdev drivers use the xxx_set_drvdata with the parent
bus device. The second change is having a bus independent power management
for the framebuffer driver. The final change is using the release method 
to cleanup the device. The reason again is to make the fbdev driver 
independent of the bus parent device. Feedback is welcomed.
Causes an early crash on the powermac G5.

http://userweb.kernel.org/~akpm/s5000489.jpg (the oops is the usual powerpc
mess)

config at http://userweb.kernel.org/~akpm/config-g5.txt

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: [PATCH] fbdev sysfs imrovements

From: Paul Mackerras <hidden>
Date: 2007-03-27 06:14:31

Andrew Morton writes:
http://userweb.kernel.org/~akpm/s5000489.jpg (the oops is the usual powerpc
mess)
Why do you have xmon enabled if you don't have any way to talk to it?

Paul.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: [PATCH] fbdev sysfs imrovements

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2007-03-27 06:20:53

On Tue, 27 Mar 2007 16:14:21 +1000 Paul Mackerras [off-list ref] wrote:
Andrew Morton writes:
quoted
http://userweb.kernel.org/~akpm/s5000489.jpg (the oops is the usual powerpc
mess)
Why do you have xmon enabled if you don't have any way to talk to it?
gawd knows - I've been dragging that config along for over a year.

Maybe I subconsciously like seeing the mess it makes?

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: [PATCH] fbdev sysfs imrovements

From: James Simmons <hidden>
Date: 2007-04-01 00:56:39

I can't seem to duplicate this error. Do you have any patches applied to 
the nvidia driver?

On Mon, 26 Mar 2007, Andrew Morton wrote:
On Tue, 20 Mar 2007 14:25:49 +0000 (GMT) James Simmons [off-list ref] wrote:
quoted
This patch does several things to allow the underlying hardware to be 
shared amount many devices. The most important thing is the use of
the created device via device_create instead of the hardware device. No 
longer should fbdev drivers use the xxx_set_drvdata with the parent
bus device. The second change is having a bus independent power management
for the framebuffer driver. The final change is using the release method 
to cleanup the device. The reason again is to make the fbdev driver 
independent of the bus parent device. Feedback is welcomed.
Causes an early crash on the powermac G5.

http://userweb.kernel.org/~akpm/s5000489.jpg (the oops is the usual powerpc
mess)

config at http://userweb.kernel.org/~akpm/config-g5.txt

Re: [PATCH] fbdev sysfs imrovements

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2007-04-01 01:24:24

On Sun, 1 Apr 2007 01:56:28 +0100 (BST) James Simmons [off-list ref] wrote:

I can't seem to duplicate this error.
OK, I'll poke at it some more.
 Do you have any patches applied to 
the nvidia driver?
I don't think so - whatever's in -mm.
On Mon, 26 Mar 2007, Andrew Morton wrote:
quoted
On Tue, 20 Mar 2007 14:25:49 +0000 (GMT) James Simmons [off-list ref] wrote:
quoted
This patch does several things to allow the underlying hardware to be 
shared amount many devices. The most important thing is the use of
the created device via device_create instead of the hardware device. No 
longer should fbdev drivers use the xxx_set_drvdata with the parent
bus device. The second change is having a bus independent power management
for the framebuffer driver. The final change is using the release method 
to cleanup the device. The reason again is to make the fbdev driver 
independent of the bus parent device. Feedback is welcomed.
Causes an early crash on the powermac G5.

http://userweb.kernel.org/~akpm/s5000489.jpg (the oops is the usual powerpc
mess)

config at http://userweb.kernel.org/~akpm/config-g5.txt
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: [PATCH] fbdev sysfs imrovements

From: James Simmons <hidden>
Date: 2007-04-05 13:46:27

quoted
On Sun, 1 Apr 2007 01:56:28 +0100 (BST) James Simmons [off-list ref] wrote:

I can't seem to duplicate this error.
OK, I'll poke at it some more.
Any news?
 
I don't think so - whatever's in -mm.
I have been running this patch again mainline. I will give it try again 
-mm over the weekend.


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help