[PATCH 23/24] matroxfb: Color setting fixes

Antonino A. Daplas

author

2007-06-02 10:23 UTC #0 lore raw

PATCH 23/24 matroxfb: Color setting fixes

- the pseudo_palette is only 16 elements long.
- do not write to the pseudo_palette if regno (array index) is more than 15

Signed-off-by Antonino Daplas <[email protected]>

---

 drivers/video/matrox/matroxfb_base.c  |    4 ++++
 drivers/video/matrox/matroxfb_base.h  |    2 +-
 drivers/video/matrox/matroxfb_crtc2.h |    2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

drivers/video/matrox/matroxfb_base.c +4 -0

index c8559a7..7ebb562 100644
@@ -679,6 +679,8 @@ #endif
 		mga_outb(M_DAC_VAL, blue);
 		break;
 	case 16:
+		if (regno >= 16)
+			break;
 		{
 			u_int16_t col =
 				(red << ACCESS_FBINFO(fbcon).var.red.offset)     |

@@ -690,6 +692,8 @@ #endif
 		break;
 	case 24:
 	case 32:
+		if (regno >= 16)
+			break;
 		ACCESS_FBINFO(cmap[regno]) =
 			(red   << ACCESS_FBINFO(fbcon).var.red.offset)   |
 			(green << ACCESS_FBINFO(fbcon).var.green.offset) |

drivers/video/matrox/matroxfb_base.h +1 -1

index 9c25c2f..d59577c 100644
@@ -518,7 +518,7 @@ #endif
 					dll:1;
 				      } memory;
 			      } values;
-	u_int32_t cmap[17];
+	u_int32_t cmap[16];
 };
 
 #define info2minfo(info) container_of(info, struct matrox_fb_info, fbcon)

drivers/video/matrox/matroxfb_crtc2.h +1 -1

index 608e40b..24450df 100644
@@ -30,7 +30,7 @@ struct matroxfb_dh_fb_info {
 
 	unsigned int		interlaced:1;
 
-	u_int32_t cmap[17];
+	u_int32_t cmap[16];
 };
 
 #endif /* __MATROXFB_CRTC2_H__ */

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Petr Vandrovec

2007-06-03 05:35 UTC #1 lore raw

Quoted 4 lines · Antonino A. Daplas

- the pseudo_palette is only 16 elements long.
- do not write to the pseudo_palette if regno (array index) is more than 15
Signed-off-by Antonino Daplas <[email protected]>

NACK. If you want to remove entry #16, you need to also fix all uses of it - matroxfb_crtc2 uses it, matroxfb_accel as well. Before hardware cursor was removed from fbcon it was used by hardware cursor - now it is only set, but never used. Removing entry without removing code to set entry #16 memory corruption will occur.
Petr

Quoted 49 lines

---

 drivers/video/matrox/matroxfb_base.c  |    4 ++++
 drivers/video/matrox/matroxfb_base.h  |    2 +-
 drivers/video/matrox/matroxfb_crtc2.h |    2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

drivers/video/matrox/matroxfb_base.c +4 -0

index c8559a7..7ebb562 100644
@@ -679,6 +679,8 @@ #endif
 		mga_outb(M_DAC_VAL, blue);
 		break;
 	case 16:
+		if (regno >= 16)
+			break;
 		{
 			u_int16_t col =
 				(red << ACCESS_FBINFO(fbcon).var.red.offset)     |

@@ -690,6 +692,8 @@ #endif
 		break;
 	case 24:
 	case 32:
+		if (regno >= 16)
+			break;
 		ACCESS_FBINFO(cmap[regno]) =
 			(red   << ACCESS_FBINFO(fbcon).var.red.offset)   |
 			(green << ACCESS_FBINFO(fbcon).var.green.offset) |

drivers/video/matrox/matroxfb_base.h +1 -1

index 9c25c2f..d59577c 100644
@@ -518,7 +518,7 @@ #endif
 					dll:1;
 				      } memory;
 			      } values;
-	u_int32_t cmap[17];
+	u_int32_t cmap[16];
 };
 
 #define info2minfo(info) container_of(info, struct matrox_fb_info, fbcon)

drivers/video/matrox/matroxfb_crtc2.h +0 -0

index 608e40b..24450df 100644

drivers/video/matrox/matroxfb_crtc2.h struct matroxfb_dh_fb_info { · line 36

Quoted 8 lines

drivers/video/matrox/matroxfb_crtc2.h +1 -1

@@ -30,7 +30,7 @@ struct matroxfb_dh_fb_info {
 
 	unsigned int		interlaced:1;
 
-	u_int32_t cmap[17];
+	u_int32_t cmap[16];
 };
 
 #endif /* __MATROXFB_CRTC2_H__ */

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Antonino Daplas

author

2007-06-03 09:59 UTC #2 lore raw

Quoted 4 lines · Petr Vandrovec

Quoted 3 lines · Antonino A. Daplas
- the pseudo_palette is only 16 elements long.
- do not write to the pseudo_palette if regno (array index) is more than 15

Quoted 8 lines

Quoted 1 line
Signed-off-by Antonino Daplas <[email protected]>
NACK. If you want to remove entry #16, you need to also fix all uses of it - matroxfb_crtc2 uses it, matroxfb_accel as well. Before hardware cursor was removed from fbcon it was used by hardware cursor - now it is only set, but never used. Removing entry without removing code to set entry #16 memory corruption will occur.
Petr

Okay, I see it now. Andrew, drop the patch for matroxfb.

Petr,

Do you have plans for the 17th entry?

Tony

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Petr Vandrovec

2007-06-03 10:36 UTC #3 lore raw

Quoted 15 lines · Antonino Daplas

Quoted 5 lines · Petr Vandrovec
Quoted 2 lines · Antonino A. Daplas
- the pseudo_palette is only 16 elements long.
- do not write to the pseudo_palette if regno (array index) is more
than 15
Quoted 1 line
Quoted 8 lines
Quoted 1 line
Signed-off-by Antonino Daplas <[email protected]>
NACK. If you want to remove entry #16, you need to also fix all uses of it - matroxfb_crtc2 uses it, matroxfb_accel as well. Before hardware cursor was removed from fbcon it was used by hardware cursor - now it is only set, but never used. Removing entry without removing code to set entry #16 memory corruption will occur.
Petr

Quoted 5 lines

Okay, I see it now. Andrew, drop the patch for matroxfb.
Petr,
Do you have plans for the 17th entry?

No. It was used in the past when hardware XOR cursor was used (so palette 4bpp/8bpp needed XOR mask 0x0F, while truecolor modes needed 0x7FFF for 15bpp/0xFFFF for 16bpp/0xFFFFFF for 32bpp). Now matroxfb uses software cursor (which just repaints complete character) and I have no plans to resurrect it.

So feel free to remove code which sets that entry. I believe that passing matroxfb sources through "grep -v 'cmap\[16\]\|pal\[.*F'" "fixes" it...

petr@gwy:/usr/src/linus/linux-2.6.22-rc3/drivers/video/matrox$ grep 
'cmap\[16\]\|pal\[.*F' *
matroxfb_accel.c:       pal[i] = 0xFFFFFFFF;
matroxfb_accel.c:       pal[i] = 0x0F0F0F0F;
matroxfb_accel.c: 
ACCESS_FBINFO(cmap[16]) = 0x7FFF7FFF;
matroxfb_accel.c: 
ACCESS_FBINFO(cmap[16]) = 0xFFFFFFFF;
matroxfb_accel.c:                               ACCESS_FBINFO(cmap[16]) 
= 0xFFFFFFFF;
matroxfb_accel.c:                               ACCESS_FBINFO(cmap[16]) 
= 0xFFFFFFFF;
matroxfb_crtc2.c:       m2info->cmap[16] = 0xFFFFFFFF;

Petr

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/