Checksumming was disabled for CEA blocks by
commit 4a638b4e38234233f5c7e6705662fbc0b58d80c2 Author: Adam Jackson ajax@redhat.com Date: Tue May 25 16:33:09 2010 -0400
drm/edid: Allow non-fatal checksum errors in CEA blocks
If only the checksum is wrong, reading twice should result in identical data, whereas a bad transfer will most likely corrupt different bytes. Comparing checksums is not sufficient, as there is a considerable chance of two bad transfers having the same checksum.
Signed-off-by: Stefan Brüns stefan.bruens@rwth-aachen.de --- drivers/gpu/drm/drm_edid.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 3a10f3f..55963d5 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1203,6 +1203,7 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter) { int i, j = 0, valid_extensions = 0; u8 *block, *new; + u8 *saved_block = NULL; bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS);
if ((block = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL) @@ -1233,12 +1234,27 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
for (j = 1; j <= block[0x7e]; j++) { u8 *ext_block = block + (valid_extensions + 1) * EDID_LENGTH; + u8 csum, last_csum = 0; for (i = 0; i < 4; i++) { if (drm_do_probe_ddc_edid(adapter, ext_block, j, EDID_LENGTH)) goto out; - if (drm_edid_block_valid(ext_block, j, print_bad_edid)) { + csum = drm_edid_block_csum(ext_block); + if (!csum) { valid_extensions++; break; + } else if (ext_block[0] == CEA_EXT) { + /* + * Some switches mangle CEA contents without fixing the checksum. + * Accept CEA blocks when two reads return identical data. + */ + if (saved_block && csum == last_csum && + !memcmp(ext_block, saved_block, EDID_LENGTH)) { + valid_extensions++; + break; + } + kfree(saved_block); + saved_block = kmemdup(ext_block, EDID_LENGTH, GFP_KERNEL); + last_csum = csum; } }
@@ -1249,6 +1265,9 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
connector->bad_edid_counter++; } + + kfree(saved_block); + saved_block = NULL; }
if (valid_extensions != block[0x7e]) { @@ -1270,6 +1289,7 @@ carp: connector->bad_edid_counter++;
out: + kfree(saved_block); kfree(block); return NULL; }
On Thu, 20 Nov 2014, Stefan Brüns stefan.bruens@rwth-aachen.de wrote:
Checksumming was disabled for CEA blocks by
commit 4a638b4e38234233f5c7e6705662fbc0b58d80c2 Author: Adam Jackson ajax@redhat.com Date: Tue May 25 16:33:09 2010 -0400
drm/edid: Allow non-fatal checksum errors in CEA blocksIf only the checksum is wrong, reading twice should result in identical data, whereas a bad transfer will most likely corrupt different bytes. Comparing checksums is not sufficient, as there is a considerable chance of two bad transfers having the same checksum.
Signed-off-by: Stefan Brüns stefan.bruens@rwth-aachen.de
drivers/gpu/drm/drm_edid.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 3a10f3f..55963d5 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1203,6 +1203,7 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter) { int i, j = 0, valid_extensions = 0; u8 *block, *new;
u8 *saved_block = NULL; bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS);
if ((block = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
@@ -1233,12 +1234,27 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
for (j = 1; j <= block[0x7e]; j++) { u8 *ext_block = block + (valid_extensions + 1) * EDID_LENGTH;
u8 csum, last_csum = 0;
if (drm_edid_block_valid(ext_block, j, print_bad_edid)) {
csum = drm_edid_block_csum(ext_block);if (!csum) { valid_extensions++; break;} else if (ext_block[0] == CEA_EXT) {/** Some switches mangle CEA contents without fixing the checksum.* Accept CEA blocks when two reads return identical data.*/if (saved_block && csum == last_csum &&!memcmp(ext_block, saved_block, EDID_LENGTH)) {valid_extensions++;break;}kfree(saved_block);saved_block = kmemdup(ext_block, EDID_LENGTH, GFP_KERNEL);last_csum = csum;
I still feel like this should be embedded in drm_edid_block_valid somehow. Who's going to print the bad edid now? Is it simply no longer printed in this loop?
I'll defer to others; any better suggestions?
Jani.
} }@@ -1249,6 +1265,9 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
connector->bad_edid_counter++; }
kfree(saved_block);saved_block = NULL;}
if (valid_extensions != block[0x7e]) {
@@ -1270,6 +1289,7 @@ carp: connector->bad_edid_counter++;
out:
- kfree(saved_block); kfree(block); return NULL;
}
2.1.2
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Friday 21 November 2014 15:39:40 you wrote:
I still feel like this should be embedded in drm_edid_block_valid somehow. Who's going to print the bad edid now? Is it simply no longer printed in this loop?
I'll defer to others; any better suggestions?
Jani.
This can not be embedded in drm_edid_block_valid, as the function is completely "passive", but we must be able to trigger a retransmission of the bad block.
Regarding printing, this should be added.
Thinking about it, maybe factor out the printing routine, and add the zero block handling to it?
Kind regards,
Stefan
dri-devel@lists.freedesktop.org
-
Jani Nikula -
Stefan Bruens -
Stefan Brüns