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/
This archive was generated by hypermail 2b29 : Wed Mar 15 2000 - 21:00:24 EST