45 messages, 5 authors, 2003-09-15 · open the first message on its own page
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2003-09-07 16:39:09
Hi ! Ok, enough for today, my head hurts, it builds, it detects my PowerBook's internal panel properly, I didn't test anything else yet nor x86, let me know what you think... No dual head nor mirror yet, though I try hard to detect properly both monitors. Ben.
From: Kronos <hidden>
Date: 2003-09-07 17:49:07
Il Sun, Sep 07, 2003 at 06:37:20PM +0200, Benjamin Herrenschmidt ha scritto:
Ok, enough for today, my head hurts, it builds, it detects my PowerBook's internal panel properly, I didn't test anything else yet nor x86, let me know what you think...
+unmap_rom:
+ if (rinfo->mon1_EDID)
+ kfree(rinfo->mon1_EDID);
+ if (rinfo->mon2_EDID)
+ kfree(rinfo->mon2_EDID);
+ if (rinfo->mon1_modedb)
+ fb_destroy_modedb(rinfo->mon1_modedb);
+ radeon_delete_i2c_busses(rinfo);
+ if (rinfo->bios_seg)
+ radeon_unmap_ROM(rinfo, pdev);
+ iounmap ((void*)rinfo->mmio_base);
+release_mmio:
+ release_mem_region (rinfo->mmio_base_phys,
+ pci_resource_len(pdev, 2));
+release_fb:
+ release_mem_region (rinfo->fb_base_phys,
+ pci_resource_len(pdev, 0));
+free_rinfo:
+ kfree (rinfo);
+ return -ENODEV;
+}
+
+
+
+static void __devexit radeonfb_pci_unregister (struct pci_dev *pdev)
+{
+ struct radeonfb_info *rinfo = pci_get_drvdata(pdev);
+
+ if (!rinfo)
+ return;
+
+ /* restore original state
+ *
+ * Doesn't quite work yet, possibly because of the PPC hacking
+ * I do on startup, disable for now. --BenH
+ */
+ radeon_write_mode (rinfo, &rinfo->init_state);
+
+#ifdef CONFIG_MTRR
+ if (rinfo->mtrr_hdl >= 0)
+ mtrr_del(rinfo->mtrr_hdl, 0, 0);
+#endif
+
+ unregister_framebuffer ((struct fb_info *) rinfo);
+
+ iounmap ((void*)rinfo->mmio_base);
+ iounmap ((void*)rinfo->fb_base);
+
+ release_mem_region (rinfo->mmio_base_phys,
+ pci_resource_len(pdev, 2));
+ release_mem_region (rinfo->fb_base_phys,
+ pci_resource_len(pdev, 0));
+
+ if (rinfo->mon1_EDID)
+ kfree(rinfo->mon1_EDID);
+ if (rinfo->mon2_EDID)
+ kfree(rinfo->mon2_EDID);
+ if (rinfo->mon1_modedb)
+ fb_destroy_modedb(rinfo->mon1_modedb);
+ radeon_delete_i2c_busses(rinfo);
+
+ kfree (rinfo);
+}
There's a problem here: sysfs attributes are unregistered after
radeonfb_pci_unregister returns, so we have a small windows where
mon1_EDID and mon2_EDID are invalid and edid1 and edid2 are still
there in sysfs. Ditto for pci_register.
Oh, you didn't update aty/Makefile ;)
obj-$(CONFIG_FB_RADEON) += radeonfb.o
radeonfb-y += radeon_base.o radeon_monitor.o radeon_pm.o
radeonfb-$(CONFIG_FB_RADEON_I2C) += radeon_i2c.o
Luca
--
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
Software is like sex; it's better when it's free.
Linus Torvalds
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2003-09-07 18:24:36
There's a problem here: sysfs attributes are unregistered after radeonfb_pci_unregister returns, so we have a small windows where mon1_EDID and mon2_EDID are invalid and edid1 and edid2 are still there in sysfs. Ditto for pci_register.
Yup, there is much "polish" still needed on the exit/cleanup path, though I'm waiting to see what you come up with putting class_device in there :)
Oh, you didn't update aty/Makefile ;) obj-$(CONFIG_FB_RADEON) += radeonfb.o radeonfb-y += radeon_base.o radeon_monitor.o radeon_pm.o radeonfb-$(CONFIG_FB_RADEON_I2C) += radeon_i2c.o
It's here, not in the patch, sorry. I won't have time to work more on this until later next week, so feel free to send your own patches/fixes/etc... If you feel it's ready to push to the mesa bk tree, let me know. Ben. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-07 18:44:45
Il Sun, Sep 07, 2003 at 08:23:48PM +0200, Benjamin Herrenschmidt ha scritto:
quoted
There's a problem here: sysfs attributes are unregistered after radeonfb_pci_unregister returns, so we have a small windows where mon1_EDID and mon2_EDID are invalid and edid1 and edid2 are still there in sysfs. Ditto for pci_register.Yup, there is much "polish" still needed on the exit/cleanup path, though I'm waiting to see what you come up with putting class_device in there :)
edid1 and edid2 will be attached to the class_device, they will be unregistered before the "release". The problem will go away. Btw, the class_device stuff won't go in, it's too late. I'll maintain it off tree and submit around 2.6.2.
I won't have time to work more on this until later next week, so feel free to send your own patches/fixes/etc... If you feel it's ready to push to the mesa bk tree, let me know.
I'm compiling it right now :) Luca -- Reply-To: kronos@kronoz.cjb.net Home: http://kronoz.cjb.net "Di tutte le perversioni sessuali, la castita` e` la piu` strana". Anatole France ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2003-09-07 18:50:01
On Sun, 2003-09-07 at 20:43, Kronos wrote:
Il Sun, Sep 07, 2003 at 08:23:48PM +0200, Benjamin Herrenschmidt ha scritto:quoted
quoted
There's a problem here: sysfs attributes are unregistered after radeonfb_pci_unregister returns, so we have a small windows where mon1_EDID and mon2_EDID are invalid and edid1 and edid2 are still there in sysfs. Ditto for pci_register.Yup, there is much "polish" still needed on the exit/cleanup path, though I'm waiting to see what you come up with putting class_device in there :)edid1 and edid2 will be attached to the class_device, they will be unregistered before the "release". The problem will go away. Btw, the class_device stuff won't go in, it's too late. I'll maintain it off tree and submit around 2.6.2.
I don't think it's too late... on the other hand, James is still sitting on a bunch of patches (hey James !) that should get in asap now... Ben. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-07 23:04:37
Il Sun, Sep 07, 2003 at 06:37:20PM +0200, Benjamin Herrenschmidt ha scritto:
I didn't test anything else yet nor x86, let me know what you think...
It dies on boot:
ksymoops 2.4.9 on i686 2.6.0-test4-benh. Options used
-V (default)
-k /proc/ksyms (default)
-l /proc/modules (default)
-o /lib/modules/2.6.0-test4-benh/ (default)
-m /home/kronos/c/src/linux-2.5-benh/System.map (specified)
Error (regular_file): read_ksyms stat /proc/ksyms failed
No modules in ksyms, skipping objects
No ksyms, skipping lsmod
Unable to handle kernel NULL pointer dereference at virtual address 00000014
c02e33c7
*pde = 00000000
Oops: 0000 [#1]
CPU: 0
EIP: 0060:[<c02e33c7>] Not tainted
Using defaults from ksymoops -t elf32-i386 -a i386
EFLAGS: 00010202
eax: 00000000 ebx: ee49c010 ecx: 00000001 edx: ef631dd4
esi: ee49c000 edi: 00000000 ebp: ef631d90 esp: ef631d80
ds: 007b es: 007b ss: 0068
Stack: ffffffff ee49c000 ee49c000 ef631dd4 ef631e60 f09442f0 ee49c010 ee49c000
ef631dd4 00000008 ef631de4 ee49c010 00000000 00003a55 00000400 c0399bcc
ef631e0c ef631e14 c012b837 ef630000 fffffaf8 00000286 c046e445 00000280
Call Trace:
[<f09442f0>] radeon_check_modes+0x240/0x390 [radeonfb]
[<c012b837>] printk+0x277/0x400
[<c02e2315>] show_edid+0x145/0x1b0
[<f0943af3>] radeon_probe_screens+0x133/0x5f0 [radeonfb]
[<f0941709>] radeonfb_pci_register+0x419/0x800 [radeonfb]
[<c0216f6b>] pci_device_probe_static+0x4b/0x60
[<c02172d6>] __pci_device_probe+0x36/0x50
[<c021731c>] pci_device_probe+0x2c/0x50
[<c02a2bbd>] bus_match+0x3d/0x70
[<c02a2d10>] driver_attach+0x70/0xb0
[<c02a305e>] bus_add_driver+0xbe/0xd0
[<c02a3568>] driver_register+0x88/0x90
[<c021771e>] pci_register_driver+0x8e/0xb0
[<f094a015>] init_module+0x15/0x40 [radeonfb]
[<c014d0cf>] sys_init_module+0x1ff/0x3e0
[<c010b20f>] syscall_call+0x7/0xb
Code: 8b 50 14 85 d2 75 12 83 63 54 fd 31 c0 85 c9 0f 94 c0 83 c4
quoted
EIP; c02e33c7 <__fb_try_mode+87/c0> <=====
quoted
ebx; ee49c010 <_end+2e010f48/3fb72f38> edx; ef631dd4 <_end+2f1a6d0c/3fb72f38> esi; ee49c000 <_end+2e010f38/3fb72f38> ebp; ef631d90 <_end+2f1a6cc8/3fb72f38> esp; ef631d80 <_end+2f1a6cb8/3fb72f38>
Trace; f09442f0 <_end+304b9228/3fb72f38> Trace; c012b837 <printk+277/400> Trace; c02e2315 <show_edid+145/1b0> Trace; f0943af3 <_end+304b8a2b/3fb72f38> Trace; f0941709 <_end+304b6641/3fb72f38> Trace; c0216f6b <pci_device_probe_static+4b/60> Trace; c02172d6 <__pci_device_probe+36/50> Trace; c021731c <pci_device_probe+2c/50> Trace; c02a2bbd <bus_match+3d/70> Trace; c02a2d10 <driver_attach+70/b0> Trace; c02a305e <bus_add_driver+be/d0> Trace; c02a3568 <driver_register+88/90> Trace; c021771e <pci_register_driver+8e/b0> Trace; f094a015 <_end+304bef4d/3fb72f38> Trace; c014d0cf <sys_init_module+1ff/3e0> Trace; c010b20f <syscall_call+7/b> Code; c02e33c7 <__fb_try_mode+87/c0> 00000000 <_EIP>: Code; c02e33c7 <__fb_try_mode+87/c0> <===== 0: 8b 50 14 mov 0x14(%eax),%edx <===== Code; c02e33ca <__fb_try_mode+8a/c0> 3: 85 d2 test %edx,%edx Code; c02e33cc <__fb_try_mode+8c/c0> 5: 75 12 jne 19 <_EIP+0x19> c02e33e0 <__fb_try_mode+a0/c0> Code; c02e33ce <__fb_try_mode+8e/c0> 7: 83 63 54 fd andl $0xfffffffd,0x54(%ebx) Code; c02e33d2 <__fb_try_mode+92/c0> b: 31 c0 xor %eax,%eax Code; c02e33d4 <__fb_try_mode+94/c0> d: 85 c9 test %ecx,%ecx Code; c02e33d6 <__fb_try_mode+96/c0> f: 0f 94 c0 sete %al Code; c02e33d9 <__fb_try_mode+99/c0> 12: 83 c4 00 add $0x0,%esp 1 error issued. Results may not be reliable. We are in modedb.c:411, and info->fbops is NULL. info->fbops is set in radeon_set_fbinfo, which is called after radeon_probe_screens. Moving radeon_set_fbinfo up should fix this: ===== drivers/video/aty/radeon_base.c 1.2 vs edited =====
--- 1.2/drivers/video/aty/radeon_base.c Sun Sep 7 23:31:18 2003
+++ edited/drivers/video/aty/radeon_base.c Mon Sep 8 00:55:03 2003@@ -2003,6 +2003,9 @@ radeon_create_i2c_busses(rinfo); #endif + /* set all the vital stuff */ + radeon_set_fbinfo (rinfo); + /* Probe screen types */ radeon_probe_screens(rinfo, monitor_layout, ignore_edid);
@@ -2024,9 +2027,6 @@ /* initialize the engine */ radeon_engine_init (rinfo); } - - /* set all the vital stuff */ - radeon_set_fbinfo (rinfo); pci_set_drvdata(pdev, rinfo); rinfo->next = board_list;
Luca
--
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
"It is more complicated than you think"
-- The Eighth Networking Truth from RFC 1925
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2003-09-08 06:06:55
We are in modedb.c:411, and info->fbops is NULL. info->fbops is set in radeon_set_fbinfo, which is called after radeon_probe_screens. Moving radeon_set_fbinfo up should fix this:
Well... radeon_set_fbinfo relies on the screens beeing probed, at least a couple of lines in it (the depth things) though they may not be that important... (copied from the old driver, I may just rip them off, I need to verify something relative to the call to engine_init, I will probably rip it off too, it has to be done in set_par only). strangely enough it works here, I have to check what's wrong, I suspect fb_find_mode isn't getting called for me but is for you and it's trying to call check_var. I'll fix that and send a new patch. Ben. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-09 02:55:14
Il Mon, Sep 08, 2003 at 08:06:07AM +0200, Benjamin Herrenschmidt ha scritto:
strangely enough it works here, I have to check what's wrong, I suspect fb_find_mode isn't getting called for me but is for you and it's trying to call check_var. I'll fix that and send a new patch.
I pulled you patch. Now it doesn't oops but fb_find_mode is doing something strange. I added some printk: Starting monitor auto detection... radeonfb: I2C (port 2) ... not found radeonfb: I2C (port 4) ... not found radeonfb: I2C (port 3) ... found CRT display radeonfb: Monitor 1 type CRT found radeonfb: EDID probed [EDID stuff] radeonfb: Monitor 2 type no found We are in radeon_check_modes, just before it calls fb_find_mode: radeonfb: mode_option: 1024x768-8@60 radeonfb: has_default_mode = 0 radeonfb: rinfo->mon1_dbsize 20 radeonfb: calling fb_find_mode radeonfb: fb_find_mode -> 1 radeonfb: rinfo->info.var.xres = 640, rinfo->info.var.yres = 480 fb_find_mode returns 1 (which should mean that we are using specified mode_option), but info.var is set to something else... I can't see what's going wrong here. Btw, I see that you switched to module_param. Maybe we should leave the setup function (#ifndef MODULE), otherwise the driver won't recognise option passed via kernel command line (video=....). radeonfb.option=something will work, but it is a bit ugly... Luca -- Reply-To: kronos@kronoz.cjb.net Home: http://kronoz.cjb.net Dicono che il cane sia il miglior amico dell'uomo. Secondo me non e` vero. Quanti dei vostri amici avete fatto castrare, recentemente? ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: James Simmons <hidden>
Date: 2003-09-09 17:18:35
quoted
quoted
Yup, there is much "polish" still needed on the exit/cleanup path, though I'm waiting to see what you come up with putting class_device in there :)edid1 and edid2 will be attached to the class_device, they will be unregistered before the "release". The problem will go away. Btw, the class_device stuff won't go in, it's too late. I'll maintain it off tree and submit around 2.6.2.I don't think it's too late... on the other hand, James is still sitting on a bunch of patches (hey James !) that should get in asap now...
Its not to late. The fbdev layer has bug fixes that need to go in. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-09 17:55:34
Il Tue, Sep 09, 2003 at 06:18:21PM +0100, James Simmons ha scritto:
quoted
quoted
Btw, the class_device stuff won't go in, it's too late. I'll maintain it off tree and submit around 2.6.2.I don't think it's too late... on the other hand, James is still sitting on a bunch of patches (hey James !) that should get in asap now...Its not to late. The fbdev layer has bug fixes that need to go in.
Ok, but the class_dev stuff isn't a bug fix, it's an API change... If you really want it I can start sending patches against your bk tree. Luca -- Reply-To: kronos@kronoz.cjb.net Home: http://kronoz.cjb.net Windows NT crashed. I'm the Blue Screen of Death. No one hears your screams. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2003-09-09 18:14:05
Ok, but the class_dev stuff isn't a bug fix, it's an API change... If you really want it I can start sending patches against your bk tree.
If it's easy enough to fix existing drivers, we can cope with it, that wouldn't be the first similar breakage we did during 2.6-test and I beleive it's actually bringing sanity in :-) Ben. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-09 19:26:07
Il Tue, Sep 09, 2003 at 08:13:19PM +0200, Benjamin Herrenschmidt ha scritto:
quoted
Ok, but the class_dev stuff isn't a bug fix, it's an API change... If you really want it I can start sending patches against your bk tree.If it's easy enough to fix existing drivers, we can cope with it,
Most of work consists in changing "." into "->" ;) Changes to fbmem.c are very similar to Greg KH's patch: ===== drivers/video/fbmem.c 1.80 vs edited =====
--- 1.80/drivers/video/fbmem.c Mon Sep 1 18:28:09 2003
+++ edited/drivers/video/fbmem.c Sat Sep 6 17:44:25 2003@@ -1199,6 +1199,42 @@ #endif }; +#define to_fb_info(d) container_of(d, struct fb_info, class_dev) +static void release_fb_info(struct class_device *class_dev) { + struct fb_info *info = to_fb_info(class_dev); + + if (info->release) + info->release(info); + + kfree(info); +} + +static struct class fb_class = { + .name = "video", + .release = &release_fb_info, +}; + +static ssize_t show_dev(struct class_device *class_dev, char *buf) { + struct fb_info *info = to_fb_info(class_dev); + return sprintf(buf, "%u:%u\n", FB_MAJOR, info->node); +} + +static CLASS_DEVICE_ATTR(dev, S_IRUGO, show_dev, NULL); + +static void fb_add_class_device(struct fb_info *info) { + info->class_dev.class = &fb_class; + snprintf(info->class_dev.class_id, BUS_ID_SIZE, "fb%d", info->node); + + if (class_device_register(&info->class_dev)) + return; + + class_device_create_file(&info->class_dev, &class_device_attr_dev); +} + +void fb_remove_class_device(struct fb_info *info) { + class_device_unregister(&info->class_dev); +} + /** * register_framebuffer - registers a frame buffer device * @fb_info: frame buffer info structure
@@ -1242,6 +1278,7 @@ devfs_mk_cdev(MKDEV(FB_MAJOR, i), S_IFCHR | S_IRUGO | S_IWUGO, "fb/%d", i); + fb_add_class_device(fb_info); return 0; }
@@ -1270,6 +1307,7 @@ kfree(fb_info->pixmap.addr); registered_fb[i]=NULL; num_registered_fb--; + fb_remove_class_device(fb_info); return 0; }
@@ -1293,6 +1331,8 @@ devfs_mk_dir("fb"); if (register_chrdev(FB_MAJOR,"fb",&fb_fops)) printk("unable to get major %d for fb devs\n", FB_MAJOR); + + class_register(&fb_class); #ifdef CONFIG_FB_OF if (ofonly) {
===== include/linux/fb.h 1.53 vs edited =====
--- 1.53/include/linux/fb.h Thu Apr 24 12:30:41 2003
+++ edited/include/linux/fb.h Tue Sep 9 20:49:12 2003@@ -3,6 +3,7 @@ #include <linux/tty.h> #include <linux/workqueue.h> +#include <linux/device.h> #include <asm/types.h> #include <asm/io.h>
@@ -412,6 +413,8 @@ struct vc_data *display_fg; /* Console visible on this display */ int currcon; /* Current VC. */ void *pseudo_palette; /* Fake palette of 16 colors */ + struct class_device class_dev; + void (*release)(struct fb_info *); /* Do driver specific clean up */ /* From here on everything is device dependent */ void *par; };
And this is radeonfb (the one from mesa3d): ===== drivers/video/radeonfb.c 1.44 vs edited =====
--- 1.44/drivers/video/radeonfb.c Fri Sep 5 17:51:41 2003
+++ edited/drivers/video/radeonfb.c Tue Sep 9 21:06:46 2003@@ -30,6 +30,7 @@ * Other changes (Luca Tettamanti <kronos@kronoz.cjb.net>): * * 2003-08-20 support for DDC2b via I2C + * 2003-09-06 switch to dynamically allocated fb_info * * Special thanks to ATI DevRel team for their hardware donations. *
@@ -344,7 +345,7 @@ struct radeonfb_info { - struct fb_info info; + struct fb_info *info; struct radeon_regs state; struct radeon_regs init_state;
@@ -994,6 +995,7 @@ #endif /* CONFIG_PPC_OF */ +static void release_radeonfb_info(struct fb_info *info); #if defined(__i386__) static void * __init radeon_map_ROM(struct radeonfb_info *rinfo, struct pci_dev *dev)
@@ -1661,7 +1663,7 @@ static int __devinit radeon_init_disp (struct radeonfb_info *rinfo) { - struct fb_info *info = &rinfo->info; + struct fb_info *info = rinfo->info; struct fb_var_screeninfo var; var = radeonfb_default_var;
@@ -1684,7 +1686,7 @@ { #ifndef MODULE if (mode_option) - fb_find_mode (var, &rinfo->info, mode_option, + fb_find_mode (var, rinfo->info, mode_option, rinfo->modedb, rinfo->dbsize, NULL, 8); else #endif
@@ -1693,7 +1695,7 @@ *var = radeonfb_default_var; else - fb_find_mode (var, &rinfo->info, "640x480-8@60", + fb_find_mode (var, rinfo->info, "640x480-8@60", NULL, 0, NULL, 0); if (noaccel)
@@ -1769,7 +1771,7 @@ static int radeonfb_check_var (struct fb_var_screeninfo *var, struct fb_info *info) { - struct radeonfb_info *rinfo = (struct radeonfb_info *) info->par; + struct radeonfb_info *rinfo = info->par; struct fb_var_screeninfo v; int nom, den;
@@ -1877,7 +1879,7 @@ static int radeonfb_pan_display (struct fb_var_screeninfo *var, struct fb_info *info) { - struct radeonfb_info *rinfo = (struct radeonfb_info *) info; + struct radeonfb_info *rinfo = info->par; if ((var->xoffset + var->xres > var->xres_virtual) || (var->yoffset + var->yres > var->yres_virtual))
@@ -1895,7 +1897,7 @@ static int radeonfb_ioctl (struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg, struct fb_info *info) { - struct radeonfb_info *rinfo = (struct radeonfb_info *) info; + struct radeonfb_info *rinfo = info->par; unsigned int tmp; u32 value = 0; int rc;
@@ -1983,7 +1985,7 @@ static int radeonfb_blank (int blank, struct fb_info *info) { - struct radeonfb_info *rinfo = (struct radeonfb_info *) info; + struct radeonfb_info *rinfo = info->par; u32 val = INREG(CRTC_EXT_CNTL); u32 val2 = INREG(LVDS_GEN_CNTL);
@@ -2035,7 +2037,7 @@ static int radeonfb_setcolreg (unsigned regno, unsigned red, unsigned green, unsigned blue, unsigned transp, struct fb_info *info) { - struct radeonfb_info *rinfo = (struct radeonfb_info *) info; + struct radeonfb_info *rinfo = info->par; u32 pindex, vclk_cntl; unsigned int i;
@@ -2607,8 +2609,15 @@ { struct fb_info *info; - info = &rinfo->info; + rinfo->info = kmalloc(sizeof(struct fb_info), GFP_KERNEL); + if (!rinfo->info) + return -ENOMEM; + memset(rinfo->info, 0, sizeof(struct fb_info)); + + info = rinfo->info; + info->class_dev.dev = &rinfo->pdev->dev; + info->release = &release_radeonfb_info; info->currcon = -1; info->par = rinfo; info->pseudo_palette = rinfo->pseudo_palette;
@@ -3429,13 +3438,14 @@ } /* set all the vital stuff */ - radeon_set_fbinfo (rinfo); + if (radeon_set_fbinfo (rinfo)) + goto unmap_rom; pci_set_drvdata(pdev, rinfo); rinfo->next = board_list; board_list = rinfo; - if (register_framebuffer ((struct fb_info *) rinfo) < 0) { + if (register_framebuffer (rinfo->info) < 0) { printk (KERN_ERR "radeonfb: could not register framebuffer\n"); goto unmap_fb; }
@@ -3481,6 +3491,7 @@ unmap_rom: if (rinfo->EDID) kfree(rinfo->EDID); + kfree(rinfo->info); fb_destroy_modedb(rinfo->modedb); radeonfb_i2c_bus_del(); radeon_unmap_ROM(pdev, rinfo->bios_seg);
@@ -3496,15 +3507,10 @@ return -ENODEV; } +static void release_radeonfb_info(struct fb_info *info) { + struct radeonfb_info *rinfo = info->par; + struct pci_dev *pdev = rinfo->pdev; - -static void __devexit radeonfb_pci_unregister (struct pci_dev *pdev) -{ - struct radeonfb_info *rinfo = pci_get_drvdata(pdev); - - if (!rinfo) - return; - /* restore original state * * Doesn't quite work yet, possibly because of the PPC hacking
@@ -3516,9 +3522,6 @@ if (rinfo->mtrr_hdl >= 0) mtrr_del(rinfo->mtrr_hdl, 0, 0); #endif - - unregister_framebuffer ((struct fb_info *) rinfo); - iounmap ((void*)rinfo->mmio_base); iounmap ((void*)rinfo->fb_base);
@@ -3533,7 +3536,19 @@ fb_destroy_modedb(rinfo->modedb); - kfree (rinfo); + kfree(rinfo); +} + +static void __devexit radeonfb_pci_unregister (struct pci_dev *pdev) +{ + struct radeonfb_info *rinfo = pci_get_drvdata(pdev); + + if (!rinfo) + return; + + sysfs_remove_bin_file(&rinfo->pdev->dev.kobj, &edid_attr); + + unregister_framebuffer (rinfo->info); }
sysfs attributes can then be attacched to class_dev, they will show up under /sys/class/video/fbX/. Luca -- Reply-To: kronos@kronoz.cjb.net Home: http://kronoz.cjb.net Recursion n.: See Recursion. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2003-09-09 19:38:36
On Tue, 2003-09-09 at 21:24, Kronos wrote:
Most of work consists in changing "." into "->" ;) Changes to fbmem.c are very similar to Greg KH's patch:
You probably want to fix more drivers , that would help getting it merged. I will take care of all the pmac ones (controlfb, platinumfb, valkyriefb) and I can take care of aty128fb, chipsfb and radeonfb too if necessary. Some comments though:
+static void release_fb_info(struct class_device *class_dev) { + struct fb_info *info = to_fb_info(class_dev); + + if (info->release) + info->release(info); + + kfree(info); +}
Since you provide the implicit kfree(), you should also provide an framebuffer_alloc() some for consistency imho. That would take the size of the private structure like for netdevs and allocate both.
+static struct class fb_class = {
+ .name = "video",I'd rather use "display" here. "video" is too broad and will cause confusion with multimedia stuff. Ben. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-09 20:47:14
Il Tue, Sep 09, 2003 at 09:37:17PM +0200, Benjamin Herrenschmidt ha scritto:
quoted
Most of work consists in changing "." into "->" ;) Changes to fbmem.c are very similar to Greg KH's patch:You probably want to fix more drivers , that would help getting it merged.
I sent radeonfb as an example. I've already fixed chipsfb.c, cyber2000fb.c, hgafb.c, imsttfb.c, neofb.c, tdfxfb.c, tridentfb.c, vesafb.c and i810.
Some comments though:quoted
+static void release_fb_info(struct class_device *class_dev) { + struct fb_info *info = to_fb_info(class_dev); + + if (info->release) + info->release(info); + + kfree(info); +}Since you provide the implicit kfree(),
Implicit? What do you mean?
you should also provide an framebuffer_alloc() some for consistency imho. That would take the size of the private structure like for netdevs and allocate both.
Like this?
struct fb_info *framebuffer_alloc(size_t size, struct device *dev) {
struct fb_info *ret;
ret = kmalloc(sizeof(struct fb_info) + size, GFP_KERNEL);
if (!ret)
return NULL;
memset(ret, 0, sizeof(struct fb_info) + size);
ret->class_dev.dev = dev;
if (size)
ret->par = ret + 1;
return ret;
}
quoted
+static struct class fb_class = { + .name = "video",I'd rather use "display" here. "video" is too broad and will cause confusion with multimedia stuff.
Ok. Luca -- Reply-To: kronos@kronoz.cjb.net Home: http://kronoz.cjb.net K.R.O.N.O.S Kinetic Replicant Optimized for Nocturnal Observation and Sabotage ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2003-09-09 21:02:53
On Tue, 9 Sep 2003, Benjamin Herrenschmidt wrote:
On Tue, 2003-09-09 at 21:24, Kronos wrote:quoted
+static struct class fb_class = { + .name = "video",I'd rather use "display" here. "video" is too broad and will cause confusion with multimedia stuff.
Exactly my comment. I was thinking about `graphics' instead of `video', but
indeed `display' sounds better.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2003-09-09 21:08:13
struct fb_info *framebuffer_alloc(size_t size, struct device *dev) {
struct fb_info *ret;
ret = kmalloc(sizeof(struct fb_info) + size, GFP_KERNEL);
if (!ret)
return NULL;
memset(ret, 0, sizeof(struct fb_info) + size);Except I would also make sure that there is some alignement of the private part, if possible on L1 cache line size Ben. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-09 21:54:14
Il Tue, Sep 09, 2003 at 11:07:31PM +0200, Benjamin Herrenschmidt ha scritto:
quoted
struct fb_info *framebuffer_alloc(size_t size, struct device *dev) { struct fb_info *ret; ret = kmalloc(sizeof(struct fb_info) + size, GFP_KERNEL); if (!ret) return NULL; memset(ret, 0, sizeof(struct fb_info) + size);Except I would also make sure that there is some alignement of the private part, if possible on L1 cache line size
struct fb_info *framebuffer_alloc(size_t size, struct device *dev) {
unsigned char *ret;
struct fb_info *info;
int align;
if (size)
align = L1_CACHE_BYTES - (sizeof(struct fb_info) % L1_CACHE_BYTES);
else
align = 0;
ret = kmalloc(sizeof(struct fb_info) + align + size, GFP_KERNEL);
if (!ret)
return NULL;
memset(ret, 0, sizeof(struct fb_info) + size + align);
info = (struct fb_info *)ret;
info->class_dev.dev = dev;
if (size)
info->par = ret + align;
return info;
}
How does it look? Is there a smarter way to do it?
Luca
--
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
The trouble with computers is that they do what you tell them,
not what you want.
D. Cohen
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-09 22:10:08
Il Tue, Sep 09, 2003 at 11:52:29PM +0200, Kronos ha scritto:
struct fb_info *framebuffer_alloc(size_t size, struct device *dev) {
if (size)
info->par = ret + align;
Ops, this should be:
info->par = ret + sizeof(struct fb_info) + align;
return info;
}
Luca
--
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
Recursion n.:
See Recursion.
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-10 16:42:08
Il Mon, Sep 08, 2003 at 09:05:27PM +0200, Kronos ha scritto:
We are in radeon_check_modes, just before it calls fb_find_mode: radeonfb: mode_option: 1024x768-8@60 radeonfb: has_default_mode = 0 radeonfb: rinfo->mon1_dbsize 20 radeonfb: calling fb_find_mode radeonfb: fb_find_mode -> 1 radeonfb: rinfo->info.var.xres = 640, rinfo->info.var.yres = 480 fb_find_mode returns 1 (which should mean that we are using specified mode_option), but info.var is set to something else... I can't see what's going wrong here.
It fails only if compiled as module. I see that fb_find_mode (and
my_atoi) are marked as __init. Maybe they get discarded after boot and
when radeonfb module tries to call them it ends up in deallocated
memory...
When booting with radeonfb compiled in the screen is shifted on the
left. At 1024x768 it is shifted by 2 characters:
hStart = 1048, hEnd = 1184, hTotal = 1344
vStart = 771, vEnd = 777, vTotal = 806
h_total_disp = 0x7f00a7 hsync_strt_wid = 0x910422
v_total_disp = 0x2ff0325 vsync_strt_wid = 0x860302
pixclock = 15384
freq = 6500
post div = 0x4
fb_div = 0x74
ppll_div_3 = 0x20074
Console: switching to colour frame buffer device 128x48
At 1280x768 (native res of my LCD - now it works) it is shifted by 3
chars:
Detailed Monitor Information
108 MHz 1280 1328 1440 1688 1024 1025 1028 1066 +HSync +VSync
Serial No : HD 001699
Monitor Name : Philips 170S4
HorizSync : 30-82 KHz
VertRefresh : 56-76 Hz
Max Pixelclock: 140 MHz
========================================
ATI Radeon NE 9700 R300 SDR SGRAM 128 MB
hStart = 1359, hEnd = 1496, hTotal = 1713
vStart = 1025, vEnd = 1028, vTotal = 1060
h_total_disp = 0x9f00d5 hsync_strt_wid = 0x910559
v_total_disp = 0x3ff0423 vsync_strt_wid = 0x830400
pixclock = 9179
freq = 10894
post div = 0x2
fb_div = 0x61
ppll_div_3 = 0x10061
Console: switching to colour frame buffer device 160x64
This is very annoying because I must re-center the screen every time I
switch from console to X and viceversa. I had the same issue with the
old driver + i2c code.
That doesn't happen with the standard modedb. At 1024x768 with
ignore_edid it's ok. At 1280x1024 it falls back to 800x600.
Luca
--
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
"I've seen things you people wouldn't believe...
Attack Ships on fire off the shores of Orion.
I've watched C-beams glitter in the dark off of Tanhauser Gate.
All those moments will be lost in time...like tears, in rain.
Time to die." -- Roy Batty (played by Rutger Hauer)
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2003-09-10 17:22:07
It fails only if compiled as module. I see that fb_find_mode (and my_atoi) are marked as __init. Maybe they get discarded after boot and when radeonfb module tries to call them it ends up in deallocated memory...
Exactly, they should definitely not be marked __init
When booting with radeonfb compiled in the screen is shifted on the
left. At 1024x768 it is shifted by 2 characters:
hStart = 1048, hEnd = 1184, hTotal = 1344
vStart = 771, vEnd = 777, vTotal = 806
h_total_disp = 0x7f00a7 hsync_strt_wid = 0x910422
v_total_disp = 0x2ff0325 vsync_strt_wid = 0x860302
pixclock = 15384
freq = 6500
post div = 0x4
fb_div = 0x74
ppll_div_3 = 0x20074
Console: switching to colour frame buffer device 128x48
At 1280x768 (native res of my LCD - now it works) it is shifted by 3
chars:
Detailed Monitor Information
108 MHz 1280 1328 1440 1688 1024 1025 1028 1066 +HSync +VSync
Serial No : HD 001699
Monitor Name : Philips 170S4
HorizSync : 30-82 KHz
VertRefresh : 56-76 Hz
Max Pixelclock: 140 MHz
========================================
ATI Radeon NE 9700 R300 SDR SGRAM 128 MB
hStart = 1359, hEnd = 1496, hTotal = 1713
vStart = 1025, vEnd = 1028, vTotal = 1060
h_total_disp = 0x9f00d5 hsync_strt_wid = 0x910559
v_total_disp = 0x3ff0423 vsync_strt_wid = 0x830400
pixclock = 9179
freq = 10894
post div = 0x2
fb_div = 0x61
ppll_div_3 = 0x10061
Console: switching to colour frame buffer device 160x64
This is very annoying because I must re-center the screen every time I
switch from console to X and viceversa. I had the same issue with the
old driver + i2c code.
That doesn't happen with the standard modedb. At 1024x768 with
ignore_edid it's ok. At 1280x1024 it falls back to 800x600.Can you try tweaking the hSync and vSync polarity ? Ben. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-10 17:48:11
Il Wed, Sep 10, 2003 at 06:48:26PM +0200, Benjamin Herrenschmidt ha scritto:
quoted
This is very annoying because I must re-center the screen every time I switch from console to X and viceversa. I had the same issue with the old driver + i2c code. That doesn't happen with the standard modedb. At 1024x768 with ignore_edid it's ok. At 1280x1024 it falls back to 800x600.Can you try tweaking the hSync and vSync polarity ?
Tried, I doesn't change. I'm looking at radeonfb_set_par. What does hSyncPol and vSyncPol do? They seem useless, they aren't used anywhere. I changed h_sync_pol and v_sync_pol (first one, then the other and finally both). Luca -- Reply-To: kronos@kronoz.cjb.net Home: http://kronoz.cjb.net K.R.O.N.O.S Kinetic Replicant Optimized for Nocturnal Observation and Sabotage ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2003-09-10 17:50:29
On Wed, 2003-09-10 at 19:46, Kronos wrote:
Il Wed, Sep 10, 2003 at 06:48:26PM +0200, Benjamin Herrenschmidt ha scritto:quoted
quoted
This is very annoying because I must re-center the screen every time I switch from console to X and viceversa. I had the same issue with the old driver + i2c code. That doesn't happen with the standard modedb. At 1024x768 with ignore_edid it's ok. At 1280x1024 it falls back to 800x600.Can you try tweaking the hSync and vSync polarity ?Tried, I doesn't change. I'm looking at radeonfb_set_par. What does hSyncPol and vSyncPol do? They seem useless, they aren't used anywhere. I changed h_sync_pol and v_sync_pol (first one, then the other and finally both).
The ones from the panel infos ? we should use them indeed rather than the ones in the mode... Anyway, if your EDID returns incorrect infos, then we are screwed, or maybe we aren't calculating the horizontal margin properly when building the mode. How does XFree works for you ? You can try comparing what XFree programs in registers with what we do. Ben. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-10 18:42:25
Il Wed, Sep 10, 2003 at 07:50:10PM +0200, Benjamin Herrenschmidt ha scritto:
On Wed, 2003-09-10 at 19:46, Kronos wrote:quoted
Il Wed, Sep 10, 2003 at 06:48:26PM +0200, Benjamin Herrenschmidt ha scritto:quoted
quoted
This is very annoying because I must re-center the screen every time I switch from console to X and viceversa. I had the same issue with the old driver + i2c code. That doesn't happen with the standard modedb. At 1024x768 with ignore_edid it's ok. At 1280x1024 it falls back to 800x600.Can you try tweaking the hSync and vSync polarity ?Tried, I doesn't change. I'm looking at radeonfb_set_par. What does hSyncPol and vSyncPol do? They seem useless, they aren't used anywhere. I changed h_sync_pol and v_sync_pol (first one, then the other and finally both).The ones from the panel infos ? we should use them indeed rather than the ones in the mode...
My LCD is using the VGA port, panel_info isn't used.
Anyway, if your EDID returns incorrect infos, then we are screwed, or maybe we aren't calculating the horizontal margin properly when building the mode. How does XFree works for you ?
XFree is ok, Windows idem.
You can try comparing what XFree programs in registers with what we do.
I'm doing this right now. Luca -- Reply-To: kronos@kronoz.cjb.net Home: http://kronoz.cjb.net Una donna sposa un uomo sperando che cambi, e lui non cambiera`. Un uomo sposa una donna sperando che non cambi, e lei cambiera`. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-10 21:27:57
Il Wed, Sep 10, 2003 at 07:50:10PM +0200, Benjamin Herrenschmidt ha scritto:
Anyway, if your EDID returns incorrect infos, then we are screwed, or maybe we aren't calculating the horizontal margin properly when building the mode.
This is the modedb created from my edid: [cut] mode 18 mode->xres = 1280, mode->right_margin = 79 mode->hsync_len = 137, mode->left_margin = 217 mode->vres = 1024, mode->lower_margin = 1 mode->vsync_len = 3, mode->upper_margin = 32 mode->refresh = 60 mode 19 mode->xres = 1280, mode->right_margin = 48 mode->hsync_len = 112, mode->left_margin = 248 mode->vres = 1024, mode->lower_margin = 1 mode->vsync_len = 3, mode->upper_margin = 38 mode->refresh = 60 mode 19 comes from get_detailed_timing, while 18 comes from get_std_timing and timings are calculated by calc_mode_timings (which in turn calls fb_get_mode). Any chance that there is some error in fb_get_mode? I'll check XFree ASAP. Luca -- Reply-To: kronos@kronoz.cjb.net Home: http://kronoz.cjb.net Al termine di un pranzo di nozze mi hanno dato un amaro alle erbe cosi' schifoso che perfino sull'etichetta c'era un frate che vomitava. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2003-09-10 21:33:46
On Wed, 2003-09-10 at 23:27, Kronos wrote:
This is the modedb created from my edid: [cut] mode 18 mode->xres = 1280, mode->right_margin = 79 mode->hsync_len = 137, mode->left_margin = 217 mode->vres = 1024, mode->lower_margin = 1 mode->vsync_len = 3, mode->upper_margin = 32 mode->refresh = 60 mode 19 mode->xres = 1280, mode->right_margin = 48 mode->hsync_len = 112, mode->left_margin = 248 mode->vres = 1024, mode->lower_margin = 1 mode->vsync_len = 3, mode->upper_margin = 38 mode->refresh = 60 mode 19 comes from get_detailed_timing, while 18 comes from get_std_timing and timings are calculated by calc_mode_timings (which in turn calls fb_get_mode). Any chance that there is some error in fb_get_mode?
I don't know the details about mode calculation and I never had any VESA spec, ask James here... On the other hand, we really want to use the detailed timings in preferrence to the calculated modes. James didn't reply for that, but I think I'll add a flag to the modes that tell how they were calculated. I need that for flat panels too so I don't have to parse twice and I can be smarter at picking the best LCD panel mode Ben. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2003-09-11 06:05:25
On Wed, 10 Sep 2003, Benjamin Herrenschmidt wrote:
quoted
It fails only if compiled as module. I see that fb_find_mode (and my_atoi) are marked as __init. Maybe they get discarded after boot and when radeonfb module tries to call them it ends up in deallocated memory...Exactly, they should definitely not be marked __init
That doesn't harm. If MODULE is defined, fb_find_mode() is an inline function
that knows about 640x400@70 only, cfr. <linux/fb.h>.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2003-09-11 06:39:19
On Thu, 2003-09-11 at 08:03, Geert Uytterhoeven wrote:
On Wed, 10 Sep 2003, Benjamin Herrenschmidt wrote:quoted
quoted
It fails only if compiled as module. I see that fb_find_mode (and my_atoi) are marked as __init. Maybe they get discarded after boot and when radeonfb module tries to call them it ends up in deallocated memory...Exactly, they should definitely not be marked __initThat doesn't harm. If MODULE is defined, fb_find_mode() is an inline function that knows about 640x400@70 only, cfr. <linux/fb.h>.
But that's wrong ! Why would you want to prevent a modular driver to use fb_find_mode ? Especially in that case where I'm looking for a mode in the modedb built from EDID list... On the other hand, if I add the flags for "detailed timings" vs "calculated" in the modedb list obtained from EDID, I will have to parse the list manually or change fb_find_mode prototype, to get to the detailed timing... Another possibility is to change my algorithm to first use the mode returned by parse_edid (which is from the detailed timings), then only fallback to fb_find_mode Ben. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2003-09-11 06:56:46
On Thu, 11 Sep 2003, Benjamin Herrenschmidt wrote:
On Thu, 2003-09-11 at 08:03, Geert Uytterhoeven wrote:quoted
On Wed, 10 Sep 2003, Benjamin Herrenschmidt wrote:quoted
quoted
It fails only if compiled as module. I see that fb_find_mode (and my_atoi) are marked as __init. Maybe they get discarded after boot and when radeonfb module tries to call them it ends up in deallocated memory...Exactly, they should definitely not be marked __initThat doesn't harm. If MODULE is defined, fb_find_mode() is an inline function that knows about 640x400@70 only, cfr. <linux/fb.h>.But that's wrong ! Why would you want to prevent a modular driver to use fb_find_mode ? Especially in that case where I'm looking for a mode in the modedb built from EDID list...
Because the modedb itself is __init. Once you have modules, you have userspace
and fbset, too.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2003-09-11 07:05:56
Because the modedb itself is __init. Once you have modules, you have userspace and fbset, too.
But the modedb I'm building from EDID isn't __init ;) Ben. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: James Simmons <hidden>
Date: 2003-09-12 17:26:07
quoted
Because the modedb itself is __init. Once you have modules, you have userspace and fbset, too.But the modedb I'm building from EDID isn't __init ;)
Cool.It would be nice to be able to modedb for modular fbdev drivers. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: James Simmons <hidden>
Date: 2003-09-12 17:43:27
quoted
mode 19 comes from get_detailed_timing, while 18 comes from get_std_timing and timings are calculated by calc_mode_timings (which in turn calls fb_get_mode). Any chance that there is some error in fb_get_mode?I don't know the details about mode calculation and I never had any VESA spec, ask James here...
I might be missing something jumping in the middle of the discussing but there is a GTF paper on creating VESA modes including from stratch without edid. The modes generated are pretty close to the standard VESA modes. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: James Simmons <hidden>
Date: 2003-09-12 17:44:36
Yucky!!! On Wed, 10 Sep 2003, Kronos wrote:
Il Tue, Sep 09, 2003 at 11:52:29PM +0200, Kronos ha scritto:quoted
struct fb_info *framebuffer_alloc(size_t size, struct device *dev) { if (size) info->par = ret + align;Ops, this should be: info->par = ret + sizeof(struct fb_info) + align;quoted
return info; }Luca
------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: James Simmons <hidden>
Date: 2003-09-12 17:46:56
On Tue, 9 Sep 2003, Benjamin Herrenschmidt wrote:quoted
On Tue, 2003-09-09 at 21:24, Kronos wrote:quoted
+static struct class fb_class = { + .name = "video",I'd rather use "display" here. "video" is too broad and will cause confusion with multimedia stuff.Exactly my comment. I was thinking about `graphics' instead of `video', but indeed `display' sounds better. Gr{oetje,eeting}s,
I prefere graphics myself. Display sounds to generic. That is what video and graphics output is piped to. Since fbdev doesn't handle video ouput normally this is kind of fuzzy sounding. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: James Simmons <hidden>
Date: 2003-09-12 17:48:36
quoted
quoted
Most of work consists in changing "." into "->" ;) Changes to fbmem.c are very similar to Greg KH's patch:You probably want to fix more drivers , that would help getting it merged.I sent radeonfb as an example. I've already fixed chipsfb.c, cyber2000fb.c, hgafb.c, imsttfb.c, neofb.c, tdfxfb.c, tridentfb.c, vesafb.c and i810.
Patches welcomed. I already have the class_device stuff in my tree. Take a look at the fbdev BK. Please make a patch aginat that tree. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-12 19:37:38
Il Fri, Sep 12, 2003 at 06:43:19PM +0100, James Simmons ha scritto:
quoted
quoted
mode 19 comes from get_detailed_timing, while 18 comes from get_std_timing and timings are calculated by calc_mode_timings (which in turn calls fb_get_mode). Any chance that there is some error in fb_get_mode?I don't know the details about mode calculation and I never had any VESA spec, ask James here...I might be missing something jumping in the middle of the discussing but there is a GTF paper on creating VESA modes including from stratch without edid. The modes generated are pretty close to the standard VESA modes.
The problem is that mode calculated by fb_get_mode is different from the
one that comes from get_detailed_timing.
Also, modes from fb_get_mode are different from XFree modes so every
time I switch from console to X I have to re-center the screen.
This happens only when modedb is created from edid, not when using the
standard modedb.
Currently create_modedb is unused (the new radeon driver is the first
user of edid stuff) so we may be hitting a real bug.
Luca
--
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
#include <stdio.h>
int main(void) {printf("\t\t\b\b\b\b\b\b");
printf("\t\t\b\b\b\b\b\b");return 0;}
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
From: James Simmons <hidden>
Date: 2003-09-12 22:20:20
The problem is that mode calculated by fb_get_mode is different from the one that comes from get_detailed_timing.
:-(
Also, modes from fb_get_mode are different from XFree modes so every time I switch from console to X I have to re-center the screen. This happens only when modedb is created from edid, not when using the standard modedb. Currently create_modedb is unused (the new radeon driver is the first user of edid stuff) so we may be hitting a real bug.
Sounds like bugs in the code. The code was never tested before. BTW where can I find the latest Radeon code. I like to test it myself. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-12 22:46:47
Il Fri, Sep 12, 2003 at 11:20:13PM +0100, James Simmons ha scritto:
quoted
Also, modes from fb_get_mode are different from XFree modes so every time I switch from console to X I have to re-center the screen. This happens only when modedb is created from edid, not when using the standard modedb. Currently create_modedb is unused (the new radeon driver is the first user of edid stuff) so we may be hitting a real bug.Sounds like bugs in the code. The code was never tested before. BTW where can I find the latest Radeon code. I like to test it myself.
You can find it in Ben's tree: bk://ppc.bkbits.net/linuxppc-2.5-benh It's easier to test it userspace, I've adapted fbmon.c and modedb.c to work in userspace (trivial) so that I can fopen the dump of edid block and pass it to create_modedb. I'll send you my edid block if you need it. Luca -- Reply-To: kronos@kronoz.cjb.net Home: http://kronoz.cjb.net E' stato a causa di una donna che ho cominciato a bere e non ho mai avuto la cortesia di ringraziarla. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-12 22:48:45
Il Fri, Sep 12, 2003 at 06:48:30PM +0100, James Simmons ha scritto:
quoted
quoted
quoted
Most of work consists in changing "." into "->" ;) Changes to fbmem.c are very similar to Greg KH's patch:You probably want to fix more drivers , that would help getting it merged.I sent radeonfb as an example. I've already fixed chipsfb.c, cyber2000fb.c, hgafb.c, imsttfb.c, neofb.c, tdfxfb.c, tridentfb.c, vesafb.c and i810.Patches welcomed. I already have the class_device stuff in my tree. Take a look at the fbdev BK. Please make a patch aginat that tree.
I've just finished ;) I want to double-check everything, I'll send patches ASAP. Luca -- Reply-To: kronos@kronoz.cjb.net Home: http://kronoz.cjb.net Let me make your mind, leave yourself behind Be not afraid ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-14 17:33:10
Il Fri, Sep 12, 2003 at 11:20:13PM +0100, James Simmons ha scritto:
quoted
Also, modes from fb_get_mode are different from XFree modes so every time I switch from console to X I have to re-center the screen. This happens only when modedb is created from edid, not when using the standard modedb. Currently create_modedb is unused (the new radeon driver is the first user of edid stuff) so we may be hitting a real bug.Sounds like bugs in the code. The code was never tested before.
Look at this:
bc 1.05
Copyright 1991, 1992, 1993, 1994, 1997, 1998 Free Software Foundation,
Inc.
This is free software with ABSOLUTELY NO WARRANTY.
For details type `warranty'.
xres=1280
yres=1024
refresh=60
hactive=xres
vactive=yres
vfreq=refresh
divisor = (1000000 - (vfreq * 550))/1000
hfreq = (yres + 1) * vfreq * 1000;
hfreq=hfreq/divisor
c_val = (((40-20) * 128)/256 + 20)*1000
m_val = (128 * 600)/256
m_val = (m_val * 1000000)/hfreq;
duty_cycle = c_val - m_val;
hblank = (xres * duty_cycle)/(100000 - duty_cycle);
htotal = hactive+hblank
hsync_len = (htotal * 8)/100
hsync_len
137
hblank
433
duty_cycle
25283
hsync_len should be 112, so:
htotal = (112 * 100)/8
htotal
1400
hblank = htotal - hactive
hblank
120
duty_cycle = (hblank*100000 - xres)/(hblank + 1)
duty_cycle
99162
duty_cycle calculated by from fb_get_hblank_by_hfreq) is 25283
duty_cycle reversed from EDID detailed timing is 99162
I just downloaded GTF_V1R1.xls from vesa.org, now I'm downloading
gnumeric...
Until we find the bug we can change calc_mode_timings in this way:
void calc_mode_timings(int xres, int yres, int refresh, struct fb_videomode *mode)
{
int dbsize = sizeof(modedb)/sizeof(modedb[0]);
struct fb_videomode *cur;
int i;
for (i = 0; i < dbsize; i++) {
cur = &modedb[i];
if (cur->xres == xres && cur->yres == yres &&
cur->refreh == refresh) {
/* Yeah */
mode->xres = xres;
mode->yres = yres;
mode->pixclock = cur->pixclock;
mode->refresh = refresh;
mode->left_margin = cur->left_margin;
mode->right_margin = cur->right_margin;
mode->upper_margin = cur->upper_margin;
mode->lower_margin = cur->lower_margin;
mode->hsync_len = cur->hsync_len;
mode->vsync_len = cur->vsync_len;
mode->vmode = cur->vmode;
mode->sync = cur->sync;
return;
}
}
BUG();
}
Luca
--
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
Not an editor command: Wq
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2003-09-14 17:48:13
BUG();
Wow ! Poor user... Ben. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-14 19:00:33
Il Sun, Sep 14, 2003 at 07:46:36PM +0200, Benjamin Herrenschmidt ha scritto:
quoted
BUG();Wow ! Poor user...
eheh :) It was just an example. Of course it better to print an error message and fall back to some sane default. Luca -- Reply-To: kronos@kronoz.cjb.net Home: http://kronoz.cjb.net Io mi ricordo sempre il compleanno della mia ragazza: cade il giorno dopo di quando lei me lo ricorda ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Tony <hidden>
Date: 2003-09-14 23:49:36
Il Wed, Sep 10, 2003 at 07:50:10PM +0200, Benjamin Herrenschmidt ha scritto:quoted
Anyway, if your EDID returns incorrect infos, then we arescrewed, orquoted
maybe we aren't calculating the horizontal margin properlywhen buildingquoted
the mode.This is the modedb created from my edid: [cut] mode 18 mode->xres = 1280, mode->right_margin = 79 mode->hsync_len = 137, mode->left_margin = 217 mode->vres = 1024, mode->lower_margin = 1 mode->vsync_len = 3, mode->upper_margin = 32 mode->refresh = 60 mode 19 mode->xres = 1280, mode->right_margin = 48 mode->hsync_len = 112, mode->left_margin = 248 mode->vres = 1024, mode->lower_margin = 1 mode->vsync_len = 3, mode->upper_margin = 38 mode->refresh = 60 mode 19 comes from get_detailed_timing, while 18 comes from get_std_timing and timings are calculated by calc_mode_timings (which in turn calls fb_get_mode). Any chance that there is some error in fb_get_mode?
fb_get_mode() uses GTF calculations to estimate the timings. It will surely be different from VESA standard timings and manufacturer's timings (detailed). Most, if not all, modern CRT , monitors should be able to handle GTF calculated timings -- at least it will display, but the frame may be wide/narrow, tall/short, or shifted left/right/up/down. This is when you need to use fbset to fine tune the timings. However GTF calculations should be treated as last resort (manufacturer's detailed timings, VESA discrete timings, GTF -- decreasing priority). One thing I forgot to comment in fbmon.c is that the GTF horizontal timing results are not rounded off. It may be necessary to round it off to the nearest character clock (usually 8 pixels) such as by doing a left_margin = (left_margin + 4) & ~7 . I agree with Ben that modedb should have extra flags to determine which of the modes came from where. One can then check an extra flag in EDID (First detailed timing is preferred) to find what is the best mode for the display. (The print out of show_edid() should tell you this information). I was also planning to extend info->monspecs to include most of the information from EDID, but I still have no time. Regarding your problem about re-centering the screen, why not check the modeline used by XFree86 and by radeonfb? In order to prevent re-centering when switching to/from X, the timings should be the same and if not, either create a custom Xfree86 modeline from fb or a custom timing in /etc/fb.modes taken from your current Xfree86 timings. BTW, i810fb largely uses timings generated from fb_get_modes() and I haven't heard anyone report a display problem yet. The EDID parser though is largely untested, and I've already seen a few bugs. Tony ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Tony <hidden>
Date: 2003-09-15 00:46:17
void calc_mode_timings(int xres, int yres, int refresh,
struct fb_videomode *mode)
{
int dbsize = sizeof(modedb)/sizeof(modedb[0]);
struct fb_videomode *cur;
int i;
for (i = 0; i < dbsize; i++) {
cur = &modedb[i];
if (cur->xres == xres && cur->yres == yres &&
cur->refreh == refresh) {
/* Yeah */
mode->xres = xres;
mode->yres = yres;
mode->pixclock = cur->pixclock;
mode->refresh = refresh;
mode->left_margin = cur->left_margin;
mode->right_margin = cur->right_margin;
mode->upper_margin = cur->upper_margin;
mode->lower_margin = cur->lower_margin;
mode->hsync_len = cur->hsync_len;
mode->vsync_len = cur->vsync_len;
mode->vmode = cur->vmode;
mode->sync = cur->sync;
return;
}
}
BUG();
}
The bug may actually be in the following code:
static int get_std_timing(unsigned char *block, struct fb_videomode
*mode)
{
int xres, yres = 0, refresh, ratio, i;
xres = (block[0] + 31) * 8;
if (xres <= 256)
return 0;
ratio = (block[1] & 0xc0) >> 6;
switch (ratio) {
case 0:
yres = xres;
break;
case 1:
yres = (xres * 3)/4;
break;
case 2:
yres = (xres * 4)/5;
break;j
case 3:
yres = (xres * 9)/16;
break;
}
refresh = (block[1] & 0x3f) + 60;
for (i = 0; i < VESA_MODEDB_SIZE; i++) {
if (vesa_modes[i].xres == xres &&
vesa_modes[i].yres == yres &&
vesa_modes[i].refresh == refresh) {
*mode = vesa_modes[i];
break;
} else {
calc_mode_timings(xres, yres, refresh, mode);
break;
}
}
return 1;
}
----------------------------------
Maybe you can replace it with this:
static int get_std_timing(unsigned char *block, struct fb_videomode
*mode)
{
int xres, yres = 0, refresh, ratio, i, j = 0;
xres = (block[0] + 31) * 8;
if (xres <= 256)
return 0;
ratio = (block[1] & 0xc0) >> 6;
switch (ratio) {
case 0:
yres = xres;
break;
case 1:
yres = (xres * 3)/4;
break;
case 2:
yres = (xres * 4)/5;
break;j
case 3:
yres = (xres * 9)/16;
break;
}
refresh = (block[1] & 0x3f) + 60;
/* First find standard mode from the table of VESA modes */
for (i = 0; i < VESA_MODEDB_SIZE; i++) {
if (vesa_modes[i].xres == xres &&
vesa_modes[i].yres == yres &&
vesa_modes[i].refresh == refresh) {
*mode = vesa_modes[i];
j = 1;
break;
}
}
/* If mode is not found in table, calculate using GTF */
if (!j)
calc_mode_timings(xres, yres, refresh, mode);
return 1;
}
Tony
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-15 15:57:14
Il Mon, Sep 15, 2003 at 07:59:37AM +0800, Tony ha scritto:
Regarding your problem about re-centering the screen, why not check the modeline used by XFree86 and by radeonfb?
This is what I did. The problem is not if modelines are different (yes, they are) but _why_ they differ. Both radeonfb and Xfree are using the same EDID block and have the same VESA table... Luca -- Reply-To: kronos@kronoz.cjb.net Home: http://kronoz.cjb.net La vasca da bagno fu inventata nel 1850, il telefono nel 1875. Se fossi vissuto nel 1850, avrei potuto restare in vasca per 25 anni senza sentir squillare il telefono ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf
From: Kronos <hidden>
Date: 2003-09-15 16:02:12
Il Mon, Sep 15, 2003 at 08:56:07AM +0800, Tony ha scritto:
The bug may actually be in the following code:
static int get_std_timing(unsigned char *block, struct fb_videomode
*mode)
{[cut]
for (i = 0; i < VESA_MODEDB_SIZE; i++) {
if (vesa_modes[i].xres == xres &&
vesa_modes[i].yres == yres &&
vesa_modes[i].refresh == refresh) {
*mode = vesa_modes[i];
break;
} else {
calc_mode_timings(xres, yres, refresh, mode);
break;
}
}
return 1;
}Ah, good catch. I really missed that. This would explain why XFree and radeonfb use different modelines. James, can you apply this patch: ===== drivers/video/fbmon.c 1.9 vs edited =====
--- 1.9/drivers/video/fbmon.c Wed Jun 4 07:55:19 2003
+++ edited/drivers/video/fbmon.c Mon Sep 15 17:59:28 2003@@ -566,17 +566,19 @@ } refresh = (block[1] & 0x3f) + 60; + /* First find standard mode from the table of VESA modes */ for (i = 0; i < VESA_MODEDB_SIZE; i++) { if (vesa_modes[i].xres == xres && vesa_modes[i].yres == yres && vesa_modes[i].refresh == refresh) { *mode = vesa_modes[i]; - break; - } else { - calc_mode_timings(xres, yres, refresh, mode); - break; + return 1; } } + + /* If mode is not found in table, calculate using GTF */ + calc_mode_timings(xres, yres, refresh, mode); + return 1; }
Luca -- Reply-To: kronos@kronoz.cjb.net Home: http://kronoz.cjb.net Il dottore mi ha detto di smettere di fare cene intime per quattro. A meno che non ci siamo altre tre persone. ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf