Skip to contentSkip to conversation

matroxfb_base.c - not a so little patch

PATCH Petr Vandrovec 2 participants 5 messages Mar 22, 2002 – Mar 31, 2002

lore
Share & tools
lore
#0 lore raw
Quoted 5 lines · Denis Zaitsev

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?

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]

#1 lore raw
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);
#2 lore raw
In reply to Petr Vandrovec
Quoted 15 lines · Petr Vandrovec
Quoted 5 lines · Denis Zaitsev

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?

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...

#3 lore raw
In reply to Petr Vandrovec
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.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?

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]

#4 lore raw
In reply to Petr Vandrovec
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...