[PATCH] drm/edid: shorten log output in case of all zeroes edid block

[PATCH] drm/edid: shorten log output in case of all zeroes edid block

[Stefan Brüns]

There is no need to dump the whole EDID block in case it contains no
information. Just print a single line stating the block is empty instead
of 8 lines containing only zeroes.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/gpu/drm/drm_edid.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3bf9991..ec4f91f 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1080,9 +1080,13 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
 
 bad:
 	if (print_bad_edid) {
-		printk(KERN_ERR "Raw EDID:\n");
-		print_hex_dump(KERN_ERR, " \t", DUMP_PREFIX_NONE, 16, 1,
+		if (drm_edid_is_zero(raw_edid, EDID_LENGTH)) {
+			printk(KERN_ERR "EDID block is all zeroes\n");
+		} else {
+			printk(KERN_ERR "Raw EDID:\n");
+			print_hex_dump(KERN_ERR, " \t", DUMP_PREFIX_NONE, 16, 1,
 			       raw_edid, EDID_LENGTH, false);
+		}
 	}
 	return false;
 }
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/edid: shorten log output in case of all zeroes edid block

[Jani Nikula]

On Thu, 20 Nov 2014, Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote:
> There is no need to dump the whole EDID block in case it contains no
> information. Just print a single line stating the block is empty instead
> of 8 lines containing only zeroes.
>
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/drm_edid.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 3bf9991..ec4f91f 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1080,9 +1080,13 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>  
>  bad:
>  	if (print_bad_edid) {
> -		printk(KERN_ERR "Raw EDID:\n");
> -		print_hex_dump(KERN_ERR, " \t", DUMP_PREFIX_NONE, 16, 1,
> +		if (drm_edid_is_zero(raw_edid, EDID_LENGTH)) {
> +			printk(KERN_ERR "EDID block is all zeroes\n");
> +		} else {
> +			printk(KERN_ERR "Raw EDID:\n");
> +			print_hex_dump(KERN_ERR, " \t", DUMP_PREFIX_NONE, 16, 1,
>  			       raw_edid, EDID_LENGTH, false);
> +		}
>  	}
>  	return false;
>  }
> -- 
> 2.1.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm/edid: move drm_edid_is_zero to top, make edid argument const

[Stefan Brüns]

drm_edid_is_zero will be used by drm_edid_block valid, move it up.
raw_edid argument can be const.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/gpu/drm/drm_edid.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3bf9991..f8fb327 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1014,6 +1014,14 @@ module_param_named(edid_fixup, edid_fixup, int, 0400);
 MODULE_PARM_DESC(edid_fixup,
 		 "Minimum number of valid EDID header bytes (0-8, default 6)");
 
+static bool drm_edid_is_zero(const u8 *in_edid, int length)
+{
+	if (memchr_inv(in_edid, 0, length))
+		return false;
+
+	return true;
+}
+
 /**
  * drm_edid_block_valid - Sanity check the EDID block (base or extension)
  * @raw_edid: pointer to raw EDID block
@@ -1176,14 +1184,6 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf,
 	return ret == xfers ? 0 : -1;
 }
 
-static bool drm_edid_is_zero(u8 *in_edid, int length)
-{
-	if (memchr_inv(in_edid, 0, length))
-		return false;
-
-	return true;
-}
-
 static u8 *
 drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
 {
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/edid: shorten log output in case of all zeroes edid block V2

[Stefan Brüns]

There is no need to dump the whole EDID block in case it contains no
information. Just print a single line stating the block is empty instead
of 8 lines containing only zeroes.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/gpu/drm/drm_edid.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index f8fb327..a71ed93 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1088,9 +1088,13 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
 
 bad:
 	if (print_bad_edid) {
-		printk(KERN_ERR "Raw EDID:\n");
-		print_hex_dump(KERN_ERR, " \t", DUMP_PREFIX_NONE, 16, 1,
+		if (drm_edid_is_zero(raw_edid, EDID_LENGTH)) {
+			printk(KERN_ERR "EDID block is all zeroes\n");
+		} else {
+			printk(KERN_ERR "Raw EDID:\n");
+			print_hex_dump(KERN_ERR, " \t", DUMP_PREFIX_NONE, 16, 1,
 			       raw_edid, EDID_LENGTH, false);
+		}
 	}
 	return false;
 }
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/3] drm/edid: new drm_edid_block_checksum helper function

[Stefan Brüns]

The function will also be used by a later patch, so factor it out.

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/gpu/drm/drm_edid.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index ec4f91f..b072041 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1048,8 +1048,7 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
 		}
 	}
 
-	for (i = 0; i < EDID_LENGTH; i++)
-		csum += raw_edid[i];
+	csum = drm_edid_block_checksum(raw_edid);
 	if (csum) {
 		if (print_bad_edid) {
 			DRM_ERROR("EDID checksum is invalid, remainder is %d\n", csum);
@@ -1188,6 +1187,17 @@ static bool drm_edid_is_zero(u8 *in_edid, int length)
 	return true;
 }
 
+static u8
+drm_edid_block_checksum(u8 *raw_edid)
+{
+	int i;
+	u8 csum = 0;
+	for (i = 0; i < EDID_LENGTH; i++)
+		csum += raw_edid[i];
+
+	return csum;
+}
+
 static u8 *
 drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
 {
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/edid: new drm_edid_block_checksum helper function

[Jani Nikula]

On Thu, 20 Nov 2014, Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote:
> The function will also be used by a later patch, so factor it out.
>
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> ---
>  drivers/gpu/drm/drm_edid.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index ec4f91f..b072041 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1048,8 +1048,7 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>  		}
>  	}
>  
> -	for (i = 0; i < EDID_LENGTH; i++)
> -		csum += raw_edid[i];
> +	csum = drm_edid_block_checksum(raw_edid);

With this change, it's no longer necessary to initialize csum to 0 in
drm_edid_block_valid.

>  	if (csum) {
>  		if (print_bad_edid) {
>  			DRM_ERROR("EDID checksum is invalid, remainder is %d\n", csum);
> @@ -1188,6 +1187,17 @@ static bool drm_edid_is_zero(u8 *in_edid, int length)
>  	return true;
>  }
>  
> +static u8
> +drm_edid_block_checksum(u8 *raw_edid)

const u8 *raw_edid, please.

Those fixed,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> +{
> +	int i;
> +	u8 csum = 0;
> +	for (i = 0; i < EDID_LENGTH; i++)
> +		csum += raw_edid[i];
> +
> +	return csum;
> +}
> +
>  static u8 *
>  drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
>  {
> -- 
> 2.1.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/3] drm/edid: new drm_edid_block_checksum helper function V2

[Stefan Brüns]

The function will also be used by a later patch, so factor it out.

V2: make raw_edid const, define/declare before first use

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a71ed93..f6c4a1d 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1022,6 +1022,16 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
 	return true;
 }
 
+static u8 drm_edid_block_checksum(const u8 *raw_edid)
+{
+	int i;
+	u8 csum = 0;
+	for (i = 0; i < EDID_LENGTH; i++)
+		csum += raw_edid[i];
+
+	return csum;
+}
+
 /**
  * drm_edid_block_valid - Sanity check the EDID block (base or extension)
  * @raw_edid: pointer to raw EDID block
@@ -1036,7 +1046,6 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
 bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
 {
 	int i;
-	u8 csum = 0;
 	struct edid *edid = (struct edid *)raw_edid;
 
 	if (WARN_ON(!raw_edid))
@@ -1056,9 +1065,7 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
 		}
 	}
 
-	for (i = 0; i < EDID_LENGTH; i++)
-		csum += raw_edid[i];
-	if (csum) {
+	if (drm_edid_block_checksum(raw_edid)) {
 		if (print_bad_edid) {
 			DRM_ERROR("EDID checksum is invalid, remainder is %d\n", csum);
 		}
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/edid: new drm_edid_block_checksum helper function V2

[Jani Nikula]

On Sun, 23 Nov 2014, Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote:
> The function will also be used by a later patch, so factor it out.
>
> V2: make raw_edid const, define/declare before first use
>
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index a71ed93..f6c4a1d 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1022,6 +1022,16 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
>  	return true;
>  }
>  
> +static u8 drm_edid_block_checksum(const u8 *raw_edid)
> +{
> +	int i;
> +	u8 csum = 0;
> +	for (i = 0; i < EDID_LENGTH; i++)
> +		csum += raw_edid[i];
> +
> +	return csum;
> +}
> +
>  /**
>   * drm_edid_block_valid - Sanity check the EDID block (base or extension)
>   * @raw_edid: pointer to raw EDID block
> @@ -1036,7 +1046,6 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length)
>  bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>  {
>  	int i;
> -	u8 csum = 0;

I meant it's no longer necessary to initialize this to 0. You still need
the variable. Please try compiling the code you send and you'll see
why. ;)

BR,
Jani.

>  	struct edid *edid = (struct edid *)raw_edid;
>  
>  	if (WARN_ON(!raw_edid))
> @@ -1056,9 +1065,7 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
>  		}
>  	}
>  
> -	for (i = 0; i < EDID_LENGTH; i++)
> -		csum += raw_edid[i];
> -	if (csum) {
> +	if (drm_edid_block_checksum(raw_edid)) {
>  		if (print_bad_edid) {
>  			DRM_ERROR("EDID checksum is invalid, remainder is %d\n", csum);
>  		}
> -- 
> 2.1.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/edid: calculate address of current extension block only once

[Stefan Brüns]

Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 drivers/gpu/drm/drm_edid.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index b072041..3a10f3f 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1232,12 +1232,11 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
 	block = new;
 
 	for (j = 1; j <= block[0x7e]; j++) {
+		u8 *ext_block = block + (valid_extensions + 1) * EDID_LENGTH;
 		for (i = 0; i < 4; i++) {
-			if (drm_do_probe_ddc_edid(adapter,
-				  block + (valid_extensions + 1) * EDID_LENGTH,
-				  j, EDID_LENGTH))
+			if (drm_do_probe_ddc_edid(adapter, ext_block, j, EDID_LENGTH))
 				goto out;
-			if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
+			if (drm_edid_block_valid(ext_block, j, print_bad_edid)) {
 				valid_extensions++;
 				break;
 			}
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/edid: calculate address of current extension block only once

[Jani Nikula]

On Thu, 20 Nov 2014, Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote:
> Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/drm_edid.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index b072041..3a10f3f 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1232,12 +1232,11 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
>  	block = new;
>  
>  	for (j = 1; j <= block[0x7e]; j++) {
> +		u8 *ext_block = block + (valid_extensions + 1) * EDID_LENGTH;
>  		for (i = 0; i < 4; i++) {
> -			if (drm_do_probe_ddc_edid(adapter,
> -				  block + (valid_extensions + 1) * EDID_LENGTH,
> -				  j, EDID_LENGTH))
> +			if (drm_do_probe_ddc_edid(adapter, ext_block, j, EDID_LENGTH))
>  				goto out;
> -			if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
> +			if (drm_edid_block_valid(ext_block, j, print_bad_edid)) {
>  				valid_extensions++;
>  				break;
>  			}
> -- 
> 2.1.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/3] drm/edid: Tighten checksum conditions for CEA blocks

[Stefan Brüns]

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;
 }
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/edid: Tighten checksum conditions for CEA blocks

[Jani Nikula]

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 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;

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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/edid: Tighten checksum conditions for CEA blocks

[Stefan Bruens]

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 mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel