Thread: [Linux-fbdev-devel] [PATCH] fbgen & multiple RGBA

SourceForge Headquarters 1320 Columbia Street Suite 310 San Diego, CA 92101 +1 (858) 422-6466

Hello,

the attached patch fix a problem with fbgen when changing the
RGBA components but not the depth ; fbgen would not change
the colormap in this case, where it should.

-- 
romain

> the attached patch fix a problem with fbgen when changing the
> RGBA components but not the depth ; fbgen would not change
> the colormap in this case, where it should.

It would be much easier to use a memcmp.




James Simmons wrote:
> 
> > the attached patch fix a problem with fbgen when changing the
> > RGBA components but not the depth ; fbgen would not change
> > the colormap in this case, where it should.
> 
> It would be much easier to use a memcmp.

For the color component, yes, but you can't use a memcmp
on the 'fb_var_screeninfo', as some member of the struct
are irrelevant to colormap switching (you don't want
to reinstall the colormap if only the refresh rate changed,
for instance).

-- 
DOLBEAU Romain               | l'histoire est entierement vraie, puisque
ENS Cachan / Ker Lann        |     je l'ai imaginee d'un bout a l'autre
do...@ir...             |           -- Boris Vian



> For the color component, yes, but you can't use a memcmp
> on the 'fb_var_screeninfo', as some member of the struct
> are irrelevant to colormap switching (you don't want
> to reinstall the colormap if only the refresh rate changed,
> for instance).

But it does. If you look at the console code it always calls set_palette
which in turn calls fbcon_set_palette which in turn calls fb_set_cmap. 
This happens every time you VC switch. A few driver writers noticed this
and don't bother with calling fb_Set_var in con_switch but instead a few
pieces of the function. But because of the way the current console system
is designed the colormap will always be set on VC switches.




> This happens every time you VC switch.
[snip]
> But because of the way the current console system
> is designed the colormap will always be set on VC switches.

The fix wasn't intended for VC switch, but for any change of
fb_var_screeninfo parameter. Those can happen without VC switching,
that's exactly what 'fbset' is for.

If on your console you do a 'fbset -depth 16 -rgba 5,6,5,0' followed by
a 'fbset -depth 16 -rgba 5,5,5,1' [1], any driver using fbdev will end
up with a crazy colormap because it hasn't been reinstalled after the
RGBA change.

Hence the need for the fix.

[1] yes, in fbset, "-depth" really mean "-bpp" ...

-- 
DOLBEAU Romain               |
ENS Cachan / Ker Lann        | l'histoire est entierement vraie, puisque
Thesard IRISA / CAPS         |     je l'ai imaginee d'un bout a l'autre
dol...@cl...    |           -- Boris Vian



Romain Dolbeau wrote:

> If on your console you do a 'fbset -depth 16 -rgba 5,6,5,0' followed by
> a 'fbset -depth 16 -rgba 5,5,5,1' [1], any driver using fbdev will end
                                                         ^^^^^^^^
That should have been 'fbgen', sorry for the momentary lapse of reason
and the waste of bandwith.

> up with a crazy colormap because it hasn't been reinstalled after the
> RGBA change.

-- 
DOLBEAU Romain               | The Gods made Heavy Metal
ENS Cachan / Ker Lann        |     and it's never gonna die
Thesard IRISA / CAPS         |           -- Manowar
dol...@cl...    | in 'The Gods made Heavy Metal'



> The fix wasn't intended for VC switch, but for any change of
> fb_var_screeninfo parameter. Those can happen without VC switching,
> that's exactly what 'fbset' is for.

This I know. I just want to make sure the fix works for both cases.  

> If on your console you do a 'fbset -depth 16 -rgba 5,6,5,0' followed by
> a 'fbset -depth 16 -rgba 5,5,5,1' [1], any driver using fbdev will end
> up with a crazy colormap because it hasn't been reinstalled after the
> RGBA change.
> 
> Hence the need for the fix.

I will intergrate your changes into my fbgen 2.





James Simmons wrote:

> I will intergrate your changes into my fbgen 2.

Guess that means it's OK to ask for integration.
I repost it with proper inlining (sorry about that)

Description of the patch:

> the attached patch fix a problem with `fbgen' when changing the
> RGBA components but not the depth ; `fbgen' would not change
> the colormap in this case, where it should.
> This patch is for kernel 2.4.x, but can also
> be applied to kernel 2.2.x (same bug, same fix).

#####

--- linux/drivers/video/fbgen.c.ORIG	Thu May 17 14:34:54 2001
+++ linux/drivers/video/fbgen.c	Tue Jun 26 10:26:23 2001
@@ -106,6 +106,7 @@
     struct fb_info_gen *info2 = (struct fb_info_gen *)info;
     int err;
     int oldxres, oldyres, oldbpp, oldxres_virtual, oldyres_virtual,
oldyoffset;
+    struct fb_bitfield oldred, oldgreen, oldblue;
 
     if ((err = fbgen_do_set_var(var, con == currcon, info2)))
 	return err;
@@ -115,12 +116,18 @@
 	oldxres_virtual = fb_display[con].var.xres_virtual;
 	oldyres_virtual = fb_display[con].var.yres_virtual;
 	oldbpp = fb_display[con].var.bits_per_pixel;
+	oldred = fb_display[con].var.red;
+	oldgreen = fb_display[con].var.green;
+	oldblue = fb_display[con].var.blue;
 	oldyoffset = fb_display[con].var.yoffset;
 	fb_display[con].var = *var;
 	if (oldxres != var->xres || oldyres != var->yres ||
 	    oldxres_virtual != var->xres_virtual ||
 	    oldyres_virtual != var->yres_virtual ||
 	    oldbpp != var->bits_per_pixel ||
+	    (!(memcmp(&oldred, &(var->red), sizeof(struct fb_bitfield)))) || 
+	    (!(memcmp(&oldgreen, &(var->green), sizeof(struct fb_bitfield))))
||
+	    (!(memcmp(&oldblue, &(var->blue), sizeof(struct fb_bitfield)))) ||
 	    oldyoffset != var->yoffset) {
 	    fbgen_set_disp(con, info2);
 	    if (info->changevar)
#####


-- 
DOLBEAU Romain               | l'histoire est entierement vraie, puisque
ENS Cachan / Ker Lann        |     je l'ai imaginee d'un bout a l'autre
do...@ir...             |           -- Boris Vian



Romain Dolbeau wrote:

> the attached patch fix a problem with fbgen when changing the
> RGBA components but not the depth ; fbgen would not change
> the colormap in this case, where it should.

This is the same patch but using memcmp() on the 3 color
components.

-- 
DOLBEAU Romain               | l'histoire est entierement vraie, puisque
ENS Cachan / Ker Lann        |     je l'ai imaginee d'un bout a l'autre
do...@ir...             |           -- Boris Vian

Thread: [Linux-fbdev-devel] [PATCH] fbgen & multiple RGBA

linux-fbdev-devel