Messages in this thread | | | From | "Petr Vandrovec" <> | Date | Mon, 13 Mar 2000 11:55:46 MET-1 | Subject | Re: [linux-fbdev] fbdev 2.4.0 cleanups |
| |
On 13 Mar 00 at 11:35, Geert Uytterhoeven wrote: > > > the top of vfb that _new_ drivers not complying to the new rules will not > > > be accepted (and I'll enforce this by asking Linus to remove any _new_ > > > drivers that don't). > > If you think. I still do not agree with these ideas, but you are two and > > I'm one... > What's wrong with removing the `con' argument from the get/set functions and > moving the VC dependant part to fbcon? Nothing. But there are no such checks in fbmem/fbcon just now. Although for example for FBIOGET_VSCREENINFO it is trivial. But this abstraction (as I think it should be done) is going just in opposite way than current. I think that it should look (fbmem.c:fb_ioctl): i = PROC_CONSOLE(info); ... case FBIOGET_VSCREENINFO: struct fb_var_screeninfo *pvar;
if (con2fb_map[i] != fbidx) return -EXDEV; if (i < 0) { pvar = &info->disp->var; } else { pvar = fb_display[i].var; } return copy_to_user((void *)arg, pvar, sizeof(*pvar)) ? -EFAULT : 0; See? You got rid of get_var AT ALL. Or fbcon.c:fbcon_set_palette. There is no check whether unit is visble or not - call to fb_set_cmap is unconditional. Although every function looks (real code taken from sbusfb.c) if (con >= 0) disp = &fb_display[con]; else disp = info->disp; if (!disp->cmap.len) { if ((err = fb_alloc_cmap(&disp->cmap, 1 << disp->var.bits_per_pixel, 0))) return err; } if (con == currcon) { /* hardware dependent */ err = fb_set_cmap() ... return err; } else { fb_copy_cmap(cmap, &disp->cmap, kspc ? 0 : 1); } return 0; Except that sbusfb implementation is buggy, it tries to allocate 64K entries palette for 16bpp, 16M for 24bpp and 4G for 32bpp. It should use '1 << bits_per_pixel' only for pseudocolor modes. For other modes it should be either 16 (only console palette) or 1 << max3(red.length, green.length, blue.length) for directcolor... > > > - Updated other fbdev (cfr. vfb), if possible > > If it makes code smaller. It does not with currently done changes. > You think it doesn't make the code smaller? A lot of comments were added to > vfb.c. Yes, but last time I checked vfb, I started adding if (currcon == con) at the start of each function. Then I gave up... So it does not bring nothing, except that instead of managing fb_display[i] structure I have to copy it from fb_display[] to fbinfo->var/fix on each switch, copy it back on each switch, add special code for handling this instead of simple example of FBIOGET_VSCREENINFO above (you must do: if (con < 0) use info->disp->var else if (con == currcon) use info->var else use fb_display[i].var It is not shorter in any way I can see. It is longer, it adds dependency of currcon on place where it is not needed.) Best regards, Petr Vandrovec vandrove@vc.cvut.cz
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |