Committed patch by Gerald Zajac, thanks!:

* Enabled testing the checksum of the EDID info.
* Added a version check of the EDID info.
* Added more debug output.
* In the ModeList class, changes were made to how the refresh rate is 
  computed and used.  Previously, some of the basic VESA modes were not 
  added to the mode list because the computed and specified refresh rates 
  did not exactly match.  Now if the computed refresh rate is within 1.2% 
  of the specified rate, the mode is added.  With this change, all basic 
  VESA modes selected by the EDID info are now added to the mode list.
* The "additional video modes" shown in the EDID dump are now added to 
  the mode list.  Previously, this mode data was setup but not added to 
  the mode list.  Code was also added here to set the vertical & 
  horizontal sync polarity according the EDID info.  The sync polarities 
  are set according to a VESA document that I have.
* Fixed copy_str() warning, broken removal of trailing spaces, and null
  termination.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@24959 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/add-ons/accelerants/common/create_display_modes.cpp

@@ -112,10 +112,8 @@ using namespace BPrivate;
 static float
 get_refresh_rate(const display_mode& mode)
 {
-	// we have to be catious as refresh rate cannot be controlled directly,
-	// so it suffers under rounding errors and hardware restrictions
-	return rint(10 * float(mode.timing.pixel_clock * 1000) / 
-		float(mode.timing.h_total * mode.timing.v_total)) / 10.0;
+	return float(mode.timing.pixel_clock * 1000) / 
+		float(mode.timing.h_total * mode.timing.v_total);
 }
 
 
@@ -140,8 +138,7 @@ compare_mode(const void* _mode1, const void* _mode2)
 	if (mode1->space != mode2->space)
 		return mode1->space - mode2->space;
 
-	return (int)(10 * get_refresh_rate(*mode1)
-		-  10 * get_refresh_rate(*mode2));
+	return (int)(100 * (get_refresh_rate(*mode1) - get_refresh_rate(*mode2)));
 }
 
 
@@ -220,9 +217,13 @@ ModeList::AddModes(edid1_info* info)
 		if (info->detailed_monitor[i].monitor_desc_type != EDID1_IS_DETAILED_TIMING)
 			continue;
 
-		// TODO: handle sync and flags correctly!
+		// TODO: handle flags correctly!
 		const edid1_detailed_timing& timing = info->detailed_monitor[i].data.detailed_timing;
 		display_mode mode;
+		
+		if (timing.pixel_clock <= 0 || timing.sync != 3)
+			continue;
+			
 		mode.timing.pixel_clock = timing.pixel_clock * 10;
 		mode.timing.h_display = timing.h_active;
 		mode.timing.h_sync_start = timing.h_active + timing.h_sync_off;
@@ -232,7 +233,13 @@ ModeList::AddModes(edid1_info* info)
 		mode.timing.v_sync_start = timing.v_active + timing.v_sync_off;
 		mode.timing.v_sync_end = mode.timing.v_sync_start + timing.v_sync_width;
 		mode.timing.v_total = timing.v_active + timing.v_blank;
-		mode.timing.flags = POSITIVE_SYNC;
+		mode.timing.flags = 0;
+		if (timing.sync == 3) {
+			if (timing.misc & 1)
+				mode.timing.flags |= B_POSITIVE_HSYNC;
+			if (timing.misc & 2)
+				mode.timing.flags |= B_POSITIVE_VSYNC;
+		}
 		if (timing.interlaced)
 			mode.timing.flags |= B_TIMING_INTERLACED;
 		mode.space = B_RGB32;
@@ -241,6 +248,8 @@ ModeList::AddModes(edid1_info* info)
 		mode.h_display_start = 0;
 		mode.v_display_start = 0;
 		mode.flags = MODE_FLAGS;
+		
+		_AddMode(&mode);
 	}
 	
 	// TODO: add other modes from the base list that satisfy the display's
@@ -315,10 +324,15 @@ ModeList::_AddBaseMode(uint16 width, uint16 height, uint32 refresh)
 	for (uint32 i = 0; i < kNumBaseModes; i++) {
 		const display_mode& mode = kBaseModeList[i];
 
+		// Add mode if width and height match, and the computed refresh rate of
+		// the mode is within 1.2 percent of the refresh rate specified by the
+		// caller.  Note that refresh rates computed from mode parameters is
+		// not exact;  thus, the tolerance of 1.2% was obtained by testing the
+		// various established modes that can be selected by the EDID info.
+		
 		if (mode.timing.h_display == width && mode.timing.v_display == height
-			&& get_refresh_rate(mode) == refresh) {
+			&& fabs(get_refresh_rate(mode) - refresh) < refresh * 0.012) {
 			_AddMode(&mode);
-			return;
 		}
 	}
 }

src/add-ons/accelerants/common/ddc.c

@@ -3,9 +3,7 @@
  * Distributed under the terms of the MIT License.
  */
 
-/*!
-	DDC communication
-*/
+/*!	DDC communication */
 
 
 #include "ddc_int.h"
@@ -18,38 +16,44 @@
 #include <stdlib.h>
 
 
-#define READ_RETRIES 4
-	// number of retries to read ddc data
+#define READ_RETRIES 4		// number of retries to read ddc data
 
-#if 0
-/*!	Verify checksum of ddc data
-	(some monitors have a broken checksum - bad luck for them)
-*/
+
+#define TRACE_DDC
+#ifdef TRACE_DDC
+extern void _sPrintf(const char* format, ...);
+#	define TRACE(x...) _sPrintf("DDC: " x)
+#else
+#	define TRACE(x...) ;
+#endif
+
+
+
+//! Verify checksum of DDC data.
 static status_t
 verify_checksum(const uint8 *data, size_t len)
 {
 	uint32 index;
 	uint8 sum = 0;
-	uint8 all_or = 0;
+	uint8 allOr = 0;
 
 	for (index = 0; index < len; ++index, ++data) {
 		sum += *data;
-		all_or |= *data;
+		allOr |= *data;
 	}
 
-	if (all_or == 0) {
-		SHOW_ERROR0(2, "DDC information contains zeros only");
+	if (allOr == 0) {
+		TRACE("verify_checksum() DDC information contains zeros only\n");
 		return B_ERROR;
 	}
 
 	if (sum != 0) {
-		SHOW_ERROR0(2, "Checksum error of DDC information");
+		TRACE("verify_checksum() Checksum error in DDC information\n");
 		return B_IO_ERROR;
 	}
 
 	return B_OK;
 }
-#endif
 
 
 //!	Read ddc2 data from monitor
@@ -66,11 +70,17 @@ ddc2_read(const i2c_bus *bus, int start, uint8 *buffer, size_t length)
 	for (i = 0; i < READ_RETRIES; ++i) {
 		status = i2c_send_receive(bus, 0xa0, writeBuffer,
 			start < 0x100 ? 1 : 2, buffer, length);
-		// don't verify checksum - it's often broken
-		if (status == B_OK /*&& verify_checksum( buffer, len ) == B_OK*/)
-			break;
 
-		status = B_ERROR;
+		if (status != B_OK)
+			TRACE("ddc2_read(): DDC information read failure\n");
+
+		if (status == B_OK) {
+			status = verify_checksum(buffer, length);
+			if (status == B_OK)
+				break;
+
+			dprintf("DDC checksum incorrect!\n");
+		}
 	}
 
 	return status;
@@ -121,6 +131,9 @@ ddc2_read_vdif(const i2c_bus *bus, int start,
 #endif
 
 
+//	#pragma mark -
+
+
 void
 ddc2_init_timing(i2c_bus *bus)
 {
@@ -145,6 +158,11 @@ ddc2_read_edid1(const i2c_bus *bus, edid1_info *edid,
 	if (status != B_OK)
 		return status;
 
+	if (raw.version.version != 1 || raw.version.revision > 4) {
+		TRACE("ddc2_read_edid1() EDID version or revision out of range\n");
+		return B_ERROR;
+	}
+
 	edid_decode(edid, &raw);
 
 	if (vdif != NULL)

src/add-ons/accelerants/common/decode_edid.c

@@ -155,7 +155,7 @@ copy_str(char *dest, const uint8 *src, size_t len)
 	uint32 i;
 
 	// copy until 0xa
-	for (i = 0; i < len; ++i) {
+	for (i = 0; i < len; i++) {
 		if (*src == 0xa)
 			break;
 
@@ -163,12 +163,14 @@ copy_str(char *dest, const uint8 *src, size_t len)
 	}
 
 	// remove trailing spaces
-	for (i = i - 1; i >= 0; --i) {
-		if (*dest-- != ' ')
+	while (i-- > 0) {
+		if (*(dest - 1) != ' ')
 			break;
+
+		dest--;
 	}
 
-	*++dest = 0;
+	*dest = '\0';
 }