* [PATCH] matroxfb_base.c - not a so little patch
[not found] <144D3761283A@vcnet.vc.cvut.cz>
@ 2002-03-22 11:47 ` Denis Zaitsev
0 siblings, 0 replies; 6+ messages in thread
From: Denis Zaitsev @ 2002-03-22 11:47 UTC (permalink / raw)
To: Petr Vandrovec; +Cc: linux-kernel
So, why I asked all my questions... It's another patch for
matroxfb_decode_var. I've replaced a very long if-else tree with
some, say, "table-driven" code. It's much more compact. The copule
of a remarks are:
a) Knowing you point of view for the #ifdef's - the table <must> be
splitted with #ifdef's, there is no way for compiler to optimize the
table.
b) There is another similar table (named "colors") somewhere in the
code. I've made the separate one to make the patch as <little and
local> as possible. If you approve the patch at whole, I'll merge
them thru some way.
The patch is against 2.5.5. It seems that matroxfb_base.c is still
the same since that time. And I assume matroxfb_base.c with
<if (var->bits_per_pixel == 4)> branch present, i.e. without my
previous little fix.
Petr, please, apply this patch.
--- drivers/video/matrox/matroxfb_base.c.orig Mon Feb 25 21:38:33 2002
+++ drivers/video/matrox/matroxfb_base.c Sat Mar 16 04:42:19 2002
@@ -483,89 +483,52 @@ 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 {
+ unsigned char bpp;
+ struct {
+ unsigned char offset,
+ length;
+ } rgbt[4];
+ 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
+ };
+ const unsigned char *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[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;
+ *visual= *(signed char*)++p;
+
+ 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);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] matroxfb_base.c - not a so little patch
@ 2002-03-22 12:17 Petr Vandrovec
2002-03-23 19:29 ` Denis Zaitsev
2002-03-30 16:40 ` Denis Zaitsev
0 siblings, 2 replies; 6+ messages in thread
From: Petr Vandrovec @ 2002-03-22 12:17 UTC (permalink / raw)
To: Denis Zaitsev; +Cc: linux-kernel
On 22 Mar 02 at 16:47, Denis Zaitsev wrote:
>
> The patch is against 2.5.5. It seems that matroxfb_base.c is still
> the same since that time. And I assume matroxfb_base.c with
> <if (var->bits_per_pixel == 4)> branch present, i.e. without my
> previous little fix.
Did you verified effects on generated code/data size?
> + 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
vandrove@vc.cvut.cz
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] matroxfb_base.c - not a so little patch
2002-03-22 12:17 [PATCH] matroxfb_base.c - not a so little patch Petr Vandrovec
@ 2002-03-23 19:29 ` Denis Zaitsev
2002-03-30 16:40 ` Denis Zaitsev
1 sibling, 0 replies; 6+ messages in thread
From: Denis Zaitsev @ 2002-03-23 19:29 UTC (permalink / raw)
To: Petr Vandrovec; +Cc: linux-kernel
On Fri, Mar 22, 2002 at 01:17:04PM +0100, Petr Vandrovec wrote:
> 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.
> 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.
--- drivers/video/matrox/matroxfb_base.c.orig Mon Feb 25 21:38:33 2002
+++ drivers/video/matrox/matroxfb_base.c Sat Mar 23 23:41:21 2002
@@ -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);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] matroxfb_base.c - not a so little patch
2002-03-22 12:17 [PATCH] matroxfb_base.c - not a so little patch Petr Vandrovec
2002-03-23 19:29 ` Denis Zaitsev
@ 2002-03-30 16:40 ` Denis Zaitsev
2002-03-31 0:46 ` Petr Vandrovec
1 sibling, 1 reply; 6+ messages in thread
From: Denis Zaitsev @ 2002-03-30 16:40 UTC (permalink / raw)
To: Petr Vandrovec; +Cc: linux-kernel
On Fri, Mar 22, 2002 at 01:17:04PM +0100, Petr Vandrovec wrote:
> On 22 Mar 02 at 16:47, Denis Zaitsev wrote:
> >
> > The patch is against 2.5.5. It seems that matroxfb_base.c is still
> > the same since that time. And I assume matroxfb_base.c with
> > <if (var->bits_per_pixel == 4)> branch present, i.e. without my
> > previous little fix.
>
> Did you verified effects on generated code/data size?
>
> > + 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
> vandrove@vc.cvut.cz
>
I'm sorry, Petr, why don't you answer anything to me? I'd sent the
cured patch a week ago...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] matroxfb_base.c - not a so little patch
2002-03-30 16:40 ` Denis Zaitsev
@ 2002-03-31 0:46 ` Petr Vandrovec
2002-03-31 20:51 ` Denis Zaitsev
0 siblings, 1 reply; 6+ messages in thread
From: Petr Vandrovec @ 2002-03-31 0:46 UTC (permalink / raw)
To: Denis Zaitsev; +Cc: linux-kernel
On Sat, Mar 30, 2002 at 09:40:06PM +0500, Denis Zaitsev wrote:
> On Fri, Mar 22, 2002 at 01:17:04PM +0100, Petr Vandrovec wrote:
> > On 22 Mar 02 at 16:47, Denis Zaitsev wrote:
> > >
> > > The patch is against 2.5.5. It seems that matroxfb_base.c is still
> > > the same since that time. And I assume matroxfb_base.c with
> > > <if (var->bits_per_pixel == 4)> branch present, i.e. without my
> > > previous little fix.
> >
> > Did you verified effects on generated code/data size?
> >
> > > + 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
> > vandrove@vc.cvut.cz
> >
>
> 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
vandrove@vc.cvut.cz
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] matroxfb_base.c - not a so little patch
2002-03-31 0:46 ` Petr Vandrovec
@ 2002-03-31 20:51 ` Denis Zaitsev
0 siblings, 0 replies; 6+ messages in thread
From: Denis Zaitsev @ 2002-03-31 20:51 UTC (permalink / raw)
To: Petr Vandrovec; +Cc: linux-kernel
On Sun, Mar 31, 2002 at 01:46:28AM +0100, Petr Vandrovec wrote:
> >
> > 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
> vandrove@vc.cvut.cz
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...
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2002-03-31 20:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2002-03-22 12:17 [PATCH] matroxfb_base.c - not a so little patch Petr Vandrovec
2002-03-23 19:29 ` Denis Zaitsev
2002-03-30 16:40 ` Denis Zaitsev
2002-03-31 0:46 ` Petr Vandrovec
2002-03-31 20:51 ` Denis Zaitsev
[not found] <144D3761283A@vcnet.vc.cvut.cz>
2002-03-22 11:47 ` Denis Zaitsev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox