[PATCH][RFC][0/3] mode attribute in sysfs tree

Hi,
I played with sysfs for a while, this is the result. Note that is a RFC,
the code is not meant for inclusion.

The patch a "mode" attribute to each fb device. This attribute is not
writable (yet). The attribute show the current video mode in the form
<xres>x<yres>-<bpp>@<refresh>.

This interface is simple and usable by the user but it's not possible to
pass a detailed mode using a fb_var_screeninfo (like the ioctl). It may
be worth adding another attribute (binary) - say "mode_detailed" - that
deals with fb_var_screeninfo and not with strings.

This is how sysfs tree looks like:

kronos@notebook:~$ tree /sys/class/graphics
/sys/class/graphics
`-- fb0
    |-- dev
    `-- mode

1 directory, 2 files

And the output is the following:
kronos@notebook:~$ cat /sys/class/graphics/fb0/mode
1024x768-16@58

Note that 58 is really 60Hz, I'm having problems with the approx.
Does anyone have a better method to calculate the refresh rate?

The patch is composed by 2 parts: the first add a class_device field to
the struct fb_info. In this way modules can attach attributes to this
class_device. The problem with the current code is that class_dev is
initialized in register_framebuffer so you can use it only after current
fb is registered. This happens because with the class_simple interface
it's not possibile to split the creation of the class_device and its
insertion into the hierarchy (this is possible with normal class_device
interface though).

Another problem is that the driver's private modedb is not available,
so the code will search only standard modedb (using fb_find_mode).
This issue can be solved adding a "modedb" field to the struct fb_info. 
Of course drivers can ignore my macro and create their own "mode" attribute
where they do whatever they want, but I don't like this solution very much.

The second patch add the function that does the real work (fb_show_mode)
and a helper macro that helps in the creation of the "mode" attribute.
It's similar to other *_ATTR macros in device.h, and you can use like this:

static FB_MODE_ATTR(my_mode_attr);
...
class_device_create_file(my_fb_info->class_dev, &my_mode_attr);

Finally there's a patch that shows the usage the attribute (with sisfb).
Compile and boot tested on my notebook.

I'll send patches in reply to this mail.

Comments?

Luca
-- 
Home: http://kronoz.cjb.net
Il piu` bel momento dell'amore e` quando ci si illude che duri per 
sempre; il piu` brutto, quando ci si accorge che dura da troppo.


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

On Wednesday 16 June 2004 01:21, Kronos wrote:

I still haven't seen the rest of your patch.  I assume you have 3 more pending.

Hi,
I played with sysfs for a while, this is the result. Note that is a RFC,
the code is not meant for inclusion.

The patch a "mode" attribute to each fb device. This attribute is not
writable (yet). The attribute show the current video mode in the form
<xres>x<yres>-<bpp>@<refresh>.

This interface is simple and usable by the user but it's not possible to
pass a detailed mode using a fb_var_screeninfo (like the ioctl). It may
be worth adding another attribute (binary) - say "mode_detailed" - that
deals with fb_var_screeninfo and not with strings.

This is how sysfs tree looks like:

kronos@notebook:~$ tree /sys/class/graphics
/sys/class/graphics
`-- fb0

    |-- dev

    `-- mode

1 directory, 2 files

And the output is the following:
kronos@notebook:~$ cat /sys/class/graphics/fb0/mode
1024x768-16@58

Note that 58 is really 60Hz, I'm having problems with the approx.
Does anyone have a better method to calculate the refresh rate?

refresh = hfreq/vtotal

where:
	hfreq = pixclock/htotal
	pixclock = in Hz
	htotal = xres + left_margin + right_margin + hsync_len
	vtotal = yres + upper_margin + lower_margin + vsync_len

The above is not an approximation, but should give the actual refresh rate, 
assuming the driver properly normalized all variables in fb_var_screeninfo.

The patch is composed by 2 parts: the first add a class_device field to
the struct fb_info. In this way modules can attach attributes to this
class_device. The problem with the current code is that class_dev is
initialized in register_framebuffer so you can use it only after current
fb is registered. This happens because with the class_simple interface
it's not possibile to split the creation of the class_device and its
insertion into the hierarchy (this is possible with normal class_device
interface though).

Another problem is that the driver's private modedb is not available,
so the code will search only standard modedb (using fb_find_mode).
This issue can be solved adding a "modedb" field to the struct fb_info.

We already have info->monspecs.modedb. If driver has DDC/EDID support, it
can run fb_edid_to_monspecs() in fbmon.c.  Of course, info->monspecs.edid 
is valid only for the monitor.  This can still be further filtered against the 
capability of graphics card.
 
Tony




-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

First patch, with basic infrastructure:

diff -Nru -X dontdiff linux-2.6-vanilla/drivers/video/fbmem.c linux-2.6/drivers/video/fbmem.c

--- linux-2.6-vanilla/drivers/video/fbmem.c	2004-06-10 15:51:31.000000000 +0200
+++ linux-2.6/drivers/video/fbmem.c	2004-06-12 21:19:06.000000000 +0200

@@ -1294,6 +1294,8 @@
 		/* Not fatal */
 		printk(KERN_WARNING "Unable to create class_device for framebuffer %d; errno = %ld\n", i, PTR_ERR(c));
 	}
+	fb_info->class_dev = c;
+	class_set_devdata(fb_info->class_dev, fb_info);
 	
 	if (fb_info->pixmap.addr == NULL) {
 		fb_info->pixmap.addr = kmalloc(FBPIXMAPSIZE, GFP_KERNEL);

diff -Nru -X dontdiff linux-2.6-vanilla/include/linux/fb.h linux-2.6/include/linux/fb.h

--- linux-2.6-vanilla/include/linux/fb.h	2004-06-10 15:51:46.000000000 +0200
+++ linux-2.6/include/linux/fb.h	2004-06-14 17:34:28.000000000 +0200

@@ -389,6 +389,7 @@
 #include <linux/workqueue.h>
 #include <linux/devfs_fs_kernel.h>
 #include <linux/notifier.h>
+#include <linux/device.h>
 #include <asm/io.h>
 
 struct vm_area_struct;

@@ -521,6 +522,7 @@
 #define FBINFO_STATE_RUNNING	0
 #define FBINFO_STATE_SUSPENDED	1
 	u32 state;			/* Hardware state i.e suspend */
+	struct class_device *class_dev;
 
 	/* From here on everything is device dependent */
 	void *par;	

Luca
-- 
Home: http://kronoz.cjb.net
Se non puoi convincerli, confondili.


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

fb_show_mode function and FB_MODE_ATTR macro.

diff -Nru -X dontdiff linux-2.6-vanilla/drivers/video/fbsysfs.c linux-2.6/drivers/video/fbsysfs.c

--- linux-2.6-vanilla/drivers/video/fbsysfs.c	2004-02-28 13:32:58.000000000 +0100
+++ linux-2.6/drivers/video/fbsysfs.c	2004-06-14 17:34:15.000000000 +0200

@@ -70,5 +70,32 @@
 	kfree(info);
 }
 
+ssize_t fb_show_mode(struct class_device *class_dev, char * buf) {
+	struct fb_info *info = class_get_devdata(class_dev);
+	struct fb_var_screeninfo *var = &info->var;
+	
+	u32 htotal = var->xres + var->upper_margin + var->lower_margin + var->hsync_len;
+	u32 vtotal = var->yres + var->left_margin + var->right_margin + var->vsync_len;
+
+	if ((var->vmode & FB_VMODE_MASK) == FB_VMODE_DOUBLE)
+		vtotal = vtotal << 2;
+	else if ((var->vmode & FB_VMODE_MASK) == FB_VMODE_NONINTERLACED)
+		vtotal = vtotal << 1;
+
+	/* 
+	 * This should be:
+	 *                     1000000000000
+	 *            ------------------------------- * 2
+	 *            var->pixclock * htotal * vtotal
+	 *            
+	 * we divide numerator and pixclock by 1000 so the expression does not overflow.
+	 */
+	u32 refresh = 1000000000UL / ((var->pixclock / 1000) * htotal * vtotal) * 2;
+	
+	return snprintf(buf, PAGE_SIZE, "%ux%u-%u@%u\n", var->xres, var->yres,
+			var->bits_per_pixel, refresh);
+}
+
+EXPORT_SYMBOL(fb_show_mode);
 EXPORT_SYMBOL(framebuffer_release);
 EXPORT_SYMBOL(framebuffer_alloc);

diff -Nru -X dontdiff linux-2.6-vanilla/include/linux/fb.h linux-2.6/include/linux/fb.h

--- linux-2.6-vanilla/include/linux/fb.h	2004-06-15 15:38:40.000000000 +0200
+++ linux-2.6/include/linux/fb.h	2004-06-14 17:34:28.000000000 +0200

@@ -611,6 +611,15 @@
 /* drivers/video/fbsysfs.c */
 extern struct fb_info *framebuffer_alloc(size_t size, struct device *dev);
 extern void framebuffer_release(struct fb_info *info);
+extern ssize_t fb_show_mode(struct class_device *class_dev, char * buf);
+
+#define FB_MODE_ATTR(_name)			\
+struct class_device_attribute _name = {		\
+	.attr = {.name = "mode", .mode = 0444, .owner = THIS_MODULE },	\
+	.show = fb_show_mode,			\
+	.store = NULL,				\
+};
+
 
 /* drivers/video/fbmon.c */
 #define FB_MAXTIMINGS		0

Luca
-- 
Home: http://kronoz.cjb.net
Not an editor command: Wq


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND

Simple patch to show usage of the attribute with sisfb.

diff -Nru -X dontdiff linux-2.6-vanilla/drivers/video/sis/sis_main.c linux-2.6/drivers/video/sis/sis_main.c

--- linux-2.6-vanilla/drivers/video/sis/sis_main.c	2004-02-28 13:32:58.000000000 +0100
+++ linux-2.6/drivers/video/sis/sis_main.c	2004-06-12 19:21:06.000000000 +0200

@@ -4102,7 +4102,9 @@
         return NULL;
 }
 
-
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,7)
+static FB_MODE_ATTR(sis_mode_attr);
+#endif
 
 int __init sisfb_init(void)
 {

@@ -4954,6 +4956,9 @@
 			kfree(sis_fb_info);
 			return -EINVAL;
 		}
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,7)
+		class_device_create_file(sis_fb_info->class_dev, &sis_mode_attr);
+#endif
 
 		sisfb_registered = 1;			
 

@@ -5266,6 +5271,9 @@
 	/* Unregister the framebuffer */
 	if(sisfb_registered) {
 		unregister_framebuffer(sis_fb_info);
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,7)
+		class_device_remove_file(sis_fb_info->class_dev, &sis_mode_attr);
+#endif
 #if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)) && (defined(NEWFBDEV))
 		framebuffer_release(sis_fb_info);
 #else

Luca
-- 
Home: http://kronoz.cjb.net
Collect some stars to shine for you
And start today 'cause there's only a few
A sign of times my friend


-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND