Attached is a diff file that contains some fixes related to EDID in the Common Accelerant code. The fixes/changes are:
1) The checksum of the EDID info is computed and the EDID version and revision are tested to see if the EDID info is valid. Some TRACE statements were also added to identify these errors in the syslog.
2) 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.
3) 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.
4) Function copy_str() in file decode_edid.c had a compiler warning message (when built outside of the Haiku build system) that indicated that the second for statement would never terminate under some conditions. I also found that the function did not remove any trailing spaces which it was intended to do. This function has been fixed.
Best regards, Gerald
Index: src/add-ons/accelerants/common/create_display_modes.cpp
===================================================================
--- src/add-ons/accelerants/common/create_display_modes.cpp (revision 24892)
+++ src/add-ons/accelerants/common/create_display_modes.cpp (working copy)
@@ -112,10 +112,8 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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;
}
}
}
Index: src/add-ons/accelerants/common/decode_edid.c
===================================================================
--- src/add-ons/accelerants/common/decode_edid.c (revision 24892)
+++ src/add-ons/accelerants/common/decode_edid.c (working copy)
@@ -155,7 +155,7 @@
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 @@
}
// remove trailing spaces
- for (i = i - 1; i >= 0; --i) {
- if (*dest-- != ' ')
+ while (i-- > 0) {
+ if (*(dest - 1) != ' ')
break;
+
+ dest--;
}
- *++dest = 0;
+ *dest = '\0';
}
Index: src/add-ons/accelerants/common/ddc.c
===================================================================
--- src/add-ons/accelerants/common/ddc.c (revision 24892)
+++ src/add-ons/accelerants/common/ddc.c (working copy)
@@ -18,13 +18,20 @@
#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)
{
@@ -38,18 +45,17 @@
}
if (all_or == 0) {
- SHOW_ERROR0(2, "DDC information contains zeros only");
+ 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,8 +72,12 @@
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*/)
+
+ if (status != B_OK) {
+ TRACE("ddc2_read() DDC information read failure\n");
+ }
+
+ if (status == B_OK && verify_checksum(buffer, length) == B_OK)
break;
status = B_ERROR;
@@ -142,9 +152,16 @@
{
edid1_raw raw;
status_t status = ddc2_read(bus, 0, (uint8 *)&raw, sizeof(raw));
- if (status != B_OK)
+ if (status != B_OK) {
+ TRACE("ddc2_read_edid1() EDID read failure or bad checksum\n");
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)
