Re: PATCH: fixup EDID for slightly broken monitors

Could the fbdev people please review this?

Brian, in future, kernel patches should be in `patch -p1' form.  And your
mailer replaces tabs with spaces.  And your signature gives `patch' a heart
attack.  And we always place the body of a `for' loop on a separate line.

Thanks.



From: "Brian S. Julin" <redacted>

The following patch will allow monitor EDID blocks some leeway in
conformance to the DDC specifications.

My monitor (DEC PCXAV-YZ) seems to not emit a legal EDID header block, as
the first two bytes are consistantly 0x23, 0x81 instead of the expected
0x00, 0xff.  I have logged the raw i2c bit transfers and found that, at
least as far as my limited knowlege of DDC goes, there seems to be no error
in the use of the protocol, and these values are in fact what is being
returned by the monitor.  Plus there haven't been large scale complaints
that DDC fails so I assume it is working for most other users.

The EDID checksum fails when these values are left as is (and the header
verification would also fail even were this not the case) but the EDID data
seems to be valid otherwise, and the checksum succeeds if a normal header
is reconstructed.  Doing so only slightly increases the probability of a
false-ok on the checksum/verification, so I figured perhaps this was the
best workaround.

The given method may seem quirky but it should also work for the
alternative EDID header I saw mention of (sorry I lost the URL to that
reference) if that ever needs to be supported.

My work was done with RadeonFB on a Radeon QD.  X also has trouble with DDC
on this monitor.

I'd be interested to know if anyone else has noticed problems with these
monitors.  

Signed-off-by: Andrew Morton <redacted>
---

 25-akpm/drivers/video/fbmon.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff -puN drivers/video/fbmon.c~fixup-edid-for-slightly-broken-monitors drivers/video/fbmon.c

--- 25/drivers/video/fbmon.c~fixup-edid-for-slightly-broken-monitors	2004-07-26 15:23:39.890721168 -0700
+++ 25-akpm/drivers/video/fbmon.c	2004-07-26 15:23:54.888441168 -0700

@@ -72,13 +72,18 @@ static int edid_checksum(unsigned char *
 	for (i = 0; i < EDID_LENGTH; i++)
 		csum += edid[i];
 
-	if (csum == 0x00) {
-		/* checksum passed, everything's good */
+	if (csum == 0x00) /* checksum passed, everything's good */
 		return 1;
-	} else {
-		printk("EDID checksum failed, aborting\n");
-		return 0;
-	}
+	printk("EDID checksum failed, trying a header reconstruct\n");
+	for (i = 0; i < 4; i++)
+		edid[i] = edid[7-i];
+	csum = 0;
+	for (i = 0; i < EDID_LENGTH; i++)
+		csum += edid[i];
+	if (csum == 0x00) /* checksum passed, everything's good */
+		return 1;
+	printk("Reconstructed EDID checksum failed\n");
+	return 0;
 }
 
 static int edid_check_header(unsigned char *edid)

_



-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=4721&alloc_id=10040&op=click

On Tuesday 27 July 2004 06:25, Andrew Morton wrote:

Could the fbdev people please review this?

Brian, in future, kernel patches should be in `patch -p1' form.  And your
mailer replaces tabs with spaces.  And your signature gives `patch' a heart
attack.  And we always place the body of a `for' loop on a separate line.

Thanks.

Instead of blindly doing a header reconstruct, why not check against a known
database of broken displays?

One of the plans was to do that, so this is a good time to start as any. If 
in the future the database becomes too heavy, we can always config this out.  

Brian, I used "DEC" for the manufacturer string, and 0x073a for the model id.
Is this correct?

Tony

1. Created a database of broken displays and their associated fixes. 
Original patch from Brian S. Julin [off-list ref].  Only the
monitor manufacturer and model is checked, perhaps we can also check
for the serial number?

2. Added an all_null check in edid_checksum since the checksum will also be 
zero if the entire block is zeroed.

Signed-off-by: Antonino Daplas <redacted>
---
diff -uprN linux-2.6.8-rc1-mm1-orig/drivers/video/fbmon.c linux-2.6.8-rc1-mm1/drivers/video/fbmon.c

--- linux-2.6.8-rc1-mm1-orig/drivers/video/fbmon.c	2004-07-27 01:43:36.000000000 +0000
+++ linux-2.6.8-rc1-mm1/drivers/video/fbmon.c	2004-07-27 02:04:55.477448456 +0000

@@ -49,6 +49,21 @@
 #define DPRINTK(fmt, args...)
 #endif
 
+#define FBMON_FIX_HEADER 1
+
+struct broken_edid {
+	u8  manufacturer[4];
+	u32 model;
+	u32 fix;
+};
+
+static struct broken_edid brokendb[] = {
+	/* DEC FR-PCXAV-YZ */
+	{ .manufacturer = "DEC",
+	  .model        = 0x073a,
+	  .fix          = FBMON_FIX_HEADER,
+	},
+};
 
 const unsigned char edid_v1_header[] = { 0x00, 0xff, 0xff, 0xff,
 	0xff, 0xff, 0xff, 0x00

@@ -65,30 +80,77 @@ static void copy_string(unsigned char *c
   while (i-- && (*--s == 0x20)) *s = 0;
 }
 
-static int edid_checksum(unsigned char *edid)
+static void fix_broken_edid(unsigned char *edid)
 {
-	unsigned char i, csum = 0;
+	unsigned char *block = edid + ID_MANUFACTURER_NAME, manufacturer[4];
+	u32 model, i;
+
+	manufacturer[0] = ((block[0] & 0x7c) >> 2) + '@';
+	manufacturer[1] = ((block[0] & 0x03) << 3) +
+		((block[1] & 0xe0) >> 5) + '@';
+	manufacturer[2] = (block[1] & 0x1f) + '@';
+	manufacturer[3] = 0;
+	model = block[2] + (block[3] << 8);
+	
+	for (i = 0; i < ARRAY_SIZE(brokendb); i++) {
+		if (!strncmp(manufacturer, brokendb[i].manufacturer, 4) &&
+			brokendb[i].model == model) {
+			switch (brokendb[i].fix) {
+			case FBMON_FIX_HEADER:
+				printk("fbmon: The EDID header of "
+				       "Manufacturer: %s Model: 0x%x is "
+				       "known to be broken,\n"
+				       "fbmon: trying a header "
+				       "reconstruct\n", manufacturer, model);
+				memcpy(edid, edid_v1_header, 8);
+				break;
+			}
+		}
+	}
+}
 
-	for (i = 0; i < EDID_LENGTH; i++)
+static int edid_checksum(unsigned char *edid)
+{
+	unsigned char i, csum = 0, all_null = 0;
+	
+	for (i = 0; i < EDID_LENGTH; i++) {
 		csum += edid[i];
+		all_null |= edid[i];
+	}
 
-	if (csum == 0x00) {
+	if (csum == 0x00 && all_null) {
 		/* checksum passed, everything's good */
 		return 1;
-	} else {
+	}
+
+	fix_broken_edid(edid);
+	csum = all_null = 0;
+	for (i = 0; i < EDID_LENGTH; i++) {
+		csum += edid[i];
+		all_null |= edid[i];
+	}
+	if (csum != 0x00 || !all_null) {
 		printk("EDID checksum failed, aborting\n");
 		return 0;
 	}
+	return 1;
 }
 
 static int edid_check_header(unsigned char *edid)
 {
-	if ((edid[0] != 0x00) || (edid[1] != 0xff) || (edid[2] != 0xff) ||
-	    (edid[3] != 0xff) || (edid[4] != 0xff) || (edid[5] != 0xff) ||
-	    (edid[6] != 0xff)) {
-		printk
-		    ("EDID header doesn't match EDID v1 header, aborting\n");
-		return 0;
+	int i, fix = 0;
+
+	for (i = 0; i < 8; i++) {
+		if (edid[i] != edid_v1_header[i])
+			fix = 1;
+	}
+	if (!fix) 
+		return 1;
+
+	fix_broken_edid(edid);
+	for (i = 0; i < 8; i++) {
+		if (edid[i] != edid_v1_header[i])
+			return 0;
 	}
 	return 1;
 }




-------------------------------------------------------

This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=4721&alloc_id=10040&op=click