matroxfb_base.c - not a so little patch
Quoted 5 lines · Denis Zaitsev
The patch is against 2.5.5. It seems that
matroxfb_base.cis still the same since that time. And I assumematroxfb_base.cwith <if (var->bits_per_pixel== 4)> branch present, i.e. without my previous little fix.
Did you verified effects on generated code/data size?
Review
Quoted 5 lines
(quoted diff) +5
-0+ for (p= &table[0].bpp; *p < bpp; p+= sizeof table[0]); + var->red .offset= *++p; var->red .length= *++p; + var->green .offset= *++p; var->green .length= *++p; + var->blue .offset= *++p; var->blue .length= *++p; + var->transp.offset= *++p; var->transp.length= *++p;
Please no. Access fields by their names, and do not assume anything about padding.
Petr Vandrovec [email protected]
Quoted 1 line · Petr Vandrovec
Did you verified effects on generated code/data size?
text data bss dec hex filename 15191 1840 516 17547 448b matroxfb_base.o 15431 1840 516 17787 457b matroxfb_base.o.orig
I.e. 240 bytes.
Quoted 2 lines
Please no. Access fields by their names, and do not assume anything about padding.
Oh, yes... You are right. So, the cured patch is below.
video/matrox/matroxfb_base.c renamed from video/matrox/matroxfb_base.c.orig +51 -81
@@ -483,89 +483,59 @@ static int matroxfb_decode_var(CPMINFO s var->xoffset = var->xres_virtual - var->xres;if (var->yoffset + var->yres > var->yres_virtual) var->yoffset = var->yres_virtual - var->yres; +{ +struct RGBT { +unsigned char bpp; +struct { +unsigned char offset, + length; + } red, + green, + blue, + transp; +signed char visual; + } +static const table[]= { +#if defined FBCON_HAS_VGATEXT + {0 ,{0 ,6 },{0 ,6 },{0 ,6 },{0 ,0 },MX_VISUAL_PSEUDOCOLOR }, +#endif +#if defined FBCON_HAS_CFB4\ + || defined FBCON_HAS_CFB8 + {8 ,{0 ,8 },{0 ,8 },{0 ,8 },{0 ,0 },MX_VISUAL_PSEUDOCOLOR }, +#endif +#if defined FBCON_HAS_CFB16 + {15 ,{10 ,5 },{5 ,5 },{0 ,5 },{15 ,1 },MX_VISUAL_DIRECTCOLOR }, + {16 ,{11 ,5 },{5 ,6 },{0 ,5 },{0 ,0 },MX_VISUAL_DIRECTCOLOR }, +#endif +#if defined FBCON_HAS_CFB24 + {24 ,{16 ,8 },{8 ,8 },{0 ,8 },{0 ,0 },MX_VISUAL_DIRECTCOLOR }, +#endif +#if defined FBCON_HAS_CFB32 + {32 ,{16 ,8 },{8 ,8 },{0 ,8 },{24 ,8 },MX_VISUAL_DIRECTCOLOR } +#endif + }; +struct RGBT const *p; + +unsigned char bpp= var->bits_per_pixel; +if (bpp ==16 && var->green.length ==5 ) bpp--;/* an artifical value - 15 */ -if (var->bits_per_pixel ==0 ) { - var->red.offset =0 ; - var->red.length =6 ; - var->green.offset =0 ; - var->green.length =6 ; - var->blue.offset =0 ; - var->blue.length =6 ; - var->transp.offset =0 ; - var->transp.length =0 ; - *visual =MX_VISUAL_PSEUDOCOLOR ; - }else if (var->bits_per_pixel ==4 ) { - var->red.offset =0 ; - var->red.length =8 ; - var->green.offset =0 ; - var->green.length =8 ; - var->blue.offset =0 ; - var->blue.length =8 ; - var->transp.offset =0 ; - var->transp.length =0 ; - *visual =MX_VISUAL_PSEUDOCOLOR ; - }else if (var->bits_per_pixel <=8 ) { - var->red.offset =0 ; - var->red.length =8 ; - var->green.offset =0 ; - var->green.length =8 ; - var->blue.offset =0 ; - var->blue.length =8 ; - var->transp.offset =0 ; - var->transp.length =0 ; - *visual =MX_VISUAL_PSEUDOCOLOR ; - }else { -if (var->bits_per_pixel <=16 ) { -if (var->green.length ==5 ) { - var->red.offset =10 ; - var->red.length =5 ; - var->green.offset =5 ; - var->green.length =5 ; - var->blue.offset =0 ; - var->blue.length =5 ; - var->transp.offset =15 ; - var->transp.length =1 ; - }else { - var->red.offset =11 ; - var->red.length =5 ; - var->green.offset =5 ; - var->green.length =6 ; - var->blue.offset =0 ; - var->blue.length =5 ; - var->transp.offset =0 ; - var->transp.length =0 ; - } - }else if (var->bits_per_pixel <=24 ) { - var->red.offset =16 ; - var->red.length =8 ; - var->green.offset =8 ; - var->green.length =8 ; - var->blue.offset =0 ; - var->blue.length =8 ; - var->transp.offset =0 ; - var->transp.length =0 ; - }else { - var->red.offset =16 ; - var->red.length =8 ; - var->green.offset =8 ; - var->green.length =8 ; - var->blue.offset =0 ; - var->blue.length =8 ; - var->transp.offset =24 ; - var->transp.length =8 ; - } +for (p= table; p->bpp < bpp; p++); +#define SETCLR (clr)\ + var->clr.offset= p->clr.offset;\ + var->clr.length= p->clr.length +SETCLR (red); +SETCLR (green); +SETCLR (blue); +SETCLR (transp); +#undef SETCLR + *visual= p->visual; + +if (bpp >8 )dprintk ("matroxfb: truecolor: " -"size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n" , - var->transp.length, - var->red.length, - var->green.length, - var->blue.length, - var->transp.offset, - var->red.offset, - var->green.offset, - var->blue.offset); - *visual =MX_VISUAL_DIRECTCOLOR ; - } +"size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n" , + var->transp.length, var->red.length, var->green.length, var->blue.length, + var->transp.offset, var->red.offset, var->green.offset, var->blue.offset); +} *video_cmap_len =matroxfb_get_cmap_len (var);dprintk (KERN_INFO "requested %d*%d/%dbpp (%d*%d)\n" , var->xres, var->yres, var->bits_per_pixel, var->xres_virtual, var->yres_virtual);
Quoted 15 lines · Petr Vandrovec
Quoted 5 lines · Denis Zaitsev
The patch is against 2.5.5. It seems that
matroxfb_base.cis still the same since that time. And I assumematroxfb_base.cwith <if (var->bits_per_pixel== 4)> branch present, i.e. without my previous little fix.Did you verified effects on generated code/data size?
Review
Quoted 5 lines
(quoted diff) +5
-0+ for (p= &table[0].bpp; *p < bpp; p+= sizeof table[0]); + var->red .offset= *++p; var->red .length= *++p; + var->green .offset= *++p; var->green .length= *++p; + var->blue .offset= *++p; var->blue .length= *++p; + var->transp.offset= *++p; var->transp.length= *++p;
Quoted 5 lines
Please no. Access fields by their names, and do not assume anything about padding.
Petr Vandrovec [email protected]
I'm sorry, Petr, why don't you answer anything to me? I'd sent the cured patch a week ago...
Quoted 16 lines · Denis Zaitsev
Quoted 15 lines · Petr Vandrovec
Quoted 5 lines · Denis Zaitsev
The patch is against 2.5.5. It seems that
matroxfb_base.cis still the same since that time. And I assumematroxfb_base.cwith <if (var->bits_per_pixel== 4)> branch present, i.e. without my previous little fix.Did you verified effects on generated code/data size?
Quoted 5 lines
(quoted diff) +5
-0+ for (p= &table[0].bpp; *p < bpp; p+= sizeof table[0]); + var->red .offset= *++p; var->red .length= *++p; + var->green .offset= *++p; var->green .length= *++p; + var->blue .offset= *++p; var->blue .length= *++p; + var->transp.offset= *++p; var->transp.length= *++p;
Quoted 8 lines
Quoted 5 lines
Please no. Access fields by their names, and do not assume anything about padding.
Petr Vandrovec [email protected]I'm sorry, Petr, why don't you answer anything to me? I'd sent the cured patch a week ago...
What I should answer? Linus is not here, AFAIK, so patch sits on my harddrive.
Petr Vandrovec [email protected]
Quoted 8 lines · Petr Vandrovec
Quoted 3 lines
I'm sorry, Petr, why don't you answer anything to me? I'd sent the cured patch a week ago...
What I should answer? Linus is not here, AFAIK, so patch sits on my harddrive.
Petr Vandrovec [email protected]
I'm sorry again, I was not going to sad you! I was just thinking that may be something had been happened with my mail and you missed it...