[PATCH v2 14/18] DRM/KMS/EDID: Consolidate EDID Error Handling (v2)

Egbert Eich eich at suse.com
Thu Nov 22 02:23:04 PST 2012


Consolidate the null_edid_counter and the bad_edid_counter
into EDID error state flags which for the last EDID read
are accessible from user.
Errors are looged it the same error has not been present
in the previous read of the EDID. This will reset the
EDID error status for example when the monitor is changed
but still prevents permanent EDID errors from piling up
the the kernel logs.

v2: Fixed conflits due to reordering of commits.
    Set error state where missing.

Signed-off-by: Egbert Eich <eich at suse.com>
---
 drivers/gpu/drm/drm_edid.c                 |  115 +++++++++++++++++-----------
 drivers/gpu/drm/radeon/radeon_connectors.c |    2 +-
 include/drm/drm_crtc.h                     |    4 +-
 include/drm/drm_edid.h                     |   10 +++
 4 files changed, 81 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 28b877c..7d8363e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -157,6 +157,17 @@ int drm_edid_header_is_valid(const u8 *raw_edid)
 }
 EXPORT_SYMBOL(drm_edid_header_is_valid);
 
+static bool drm_edid_is_zero(u8 *in_edid, int length)
+{
+	int i;
+	u32 *raw_edid = (u32 *)in_edid;
+
+	for (i = 0; i < length / 4; i++)
+		if (*(raw_edid + i) != 0)
+			return false;
+	return true;
+}
+
 static int edid_fixup __read_mostly = 6;
 module_param_named(edid_fixup, edid_fixup, int, 0400);
 MODULE_PARM_DESC(edid_fixup,
@@ -166,11 +177,13 @@ MODULE_PARM_DESC(edid_fixup,
  * Sanity check the EDID block (base or extension).  Return 0 if the block
  * doesn't check out, or 1 if it's valid.
  */
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
+unsigned
+drm_edid_block_check_error(u8 *raw_edid, int block, unsigned last_error_flags)
 {
 	int i;
 	u8 csum = 0;
 	struct edid *edid = (struct edid *)raw_edid;
+	unsigned result = 0;
 
 	if (edid_fixup > 8 || edid_fixup < 0)
 		edid_fixup = 6;
@@ -182,27 +195,33 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
 			DRM_DEBUG("Fixing EDID header, your hardware may be failing\n");
 			memcpy(raw_edid, edid_header, sizeof(edid_header));
 		} else {
-			goto bad;
+			result |= EDID_ERR_NO_BLOCK0;
+			if (drm_edid_is_zero(raw_edid, EDID_LENGTH)) {
+				result |= EDID_ERR_NULL;
+				goto bad;
+			}
 		}
 	}
 
 	for (i = 0; i < EDID_LENGTH; i++)
 		csum += raw_edid[i];
 	if (csum) {
-		if (print_bad_edid) {
+		if ((last_error_flags & EDID_ERR_CSUM) == 0)
 			DRM_ERROR("EDID checksum is invalid, remainder is %d\n", csum);
-		}
 
 		/* allow CEA to slide through, switches mangle this */
 		if (raw_edid[0] != 0x02)
-			goto bad;
+			result |= EDID_ERR_CSUM;
 	}
+	if (result)
+		goto bad;
 
 	/* per-block-type checks */
 	switch (raw_edid[0]) {
 	case 0: /* base */
 		if (edid->version != 1) {
 			DRM_ERROR("EDID has major version %d, instead of 1\n", edid->version);
+			result |= EDID_ERR_UNSUPPORTED_VERSION;
 			goto bad;
 		}
 
@@ -214,15 +233,23 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid)
 		break;
 	}
 
-	return 1;
+	return 0;
 
 bad:
-	if (raw_edid && print_bad_edid) {
+	if (raw_edid && last_error_flags != result) {
 		printk(KERN_ERR "Raw EDID:\n");
 		print_hex_dump(KERN_ERR, " \t", DUMP_PREFIX_NONE, 16, 1,
 			       raw_edid, EDID_LENGTH, false);
 	}
-	return 0;
+	return result;
+}
+
+bool
+drm_edid_block_valid(u8 *raw_edid, int block, unsigned last_error_flags)
+{
+	if (!drm_edid_block_check_error(raw_edid, block, last_error_flags))
+		return true;
+	return false;
 }
 EXPORT_SYMBOL(drm_edid_block_valid);
 
@@ -241,7 +268,7 @@ bool drm_edid_is_valid(struct edid *edid)
 		return false;
 
 	for (i = 0; i <= edid->extensions; i++)
-		if (!drm_edid_block_valid(raw + i * EDID_LENGTH, i, true))
+		if (drm_edid_block_check_error(raw + i * EDID_LENGTH, i, true))
 			return false;
 
 	return true;
@@ -310,17 +337,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)
-{
-	int i;
-	u32 *raw_edid = (u32 *)in_edid;
-
-	for (i = 0; i < length / 4; i++)
-		if (*(raw_edid + i) != 0)
-			return false;
-	return true;
-}
-
 static void
 fix_map(u8 *block, int cnt)
 {
@@ -456,37 +472,40 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
 {
 	int i, j = 0, valid_extensions = 0;
 	u8 *block, *new;
-	bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS);
+	int last_error_flags = (drm_debug & DRM_UT_KMS) ? 0 : connector->last_edid_error_flags;
+	unsigned result = EDID_ERR_NO_DATA;
 
 #ifdef CONFIG_DRM_LOAD_EDID_FIRMWARE
 	/* check if the user has specified a 'firmware' EDID file */
 	block = (u8 *)drm_load_edid_firmware(connector);
 	if (block) {
 		drm_cache_edid(connector, NULL);
+		connector->last_edid_error_flags = 0;
 		return block;
 	}
 #endif
-
-	if ((block = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
+	block = kmalloc(EDID_LENGTH, GFP_KERNEL);
+	if (block == NULL) {
+		result = EDID_ERR_NO_MEM;
 		goto error;
+	}
 
 	/* base block fetch */
 	for (i = 0; i < 4; i++) {
 		if (drm_do_probe_ddc_edid(adapter, block, 0, EDID_LENGTH))
 			goto error_free;
-		if (drm_edid_block_valid(block, 0, print_bad_edid))
+		result = drm_edid_block_check_error(block, 0, last_error_flags);
+		if (!result)
 			break;
-		if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
-			connector->null_edid_counter++;
+		if (i == 0 && result & EDID_ERR_NULL)
 			goto error_carp;
-		}
 	}
 	if (i == 4)
 		goto error_carp;
 
 	/* if there are no extensions, we're done - don't bother caching */
 	if (block[EDID_EXTENSION_FLAG_OFFSET] == 0)
-		return block;
+		goto done;
 
 	/* don't expect extension blocks in EDID Versions < 1.3: return base block with correct extension flag */
 	if (block[EDID_VERSION_MINOR_OFFSET] < 3)
@@ -494,7 +513,7 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
 
 	/* see if EDID is in the cache - no need to read all extension blocks */
 	if (compare_get_edid_from_cache(connector, (struct edid **)&block))
-		return block;
+		goto done;
 
 	new = krealloc(block, (block[EDID_EXTENSION_FLAG_OFFSET] + 1) * EDID_LENGTH, GFP_KERNEL);
 	if (!new) {
@@ -512,7 +531,7 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
 				valid_extensions = 0;
 				goto done_fix_extension_count;
 			}
-			if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH, j, print_bad_edid)) {
+			if (!drm_edid_block_check_error(block + (valid_extensions + 1) * EDID_LENGTH, j, last_error_flags)) {
 				valid_extensions++;
 				/* Test if base block announced extension blocks although
 				 * display is not EDDC capable.
@@ -538,21 +557,22 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
 
 done_fix_extension_count:
 	fixup_edid(&block, valid_extensions);
+done:
 	drm_cache_edid(connector, (valid_extensions > 0) ? (struct edid *)block : NULL);
+	connector->last_edid_error_flags = 0;
 
 	return block;
 
 error_carp:
-	if (print_bad_edid) {
+	if (last_error_flags != result) {
 		dev_warn(connector->dev->dev, "%s: EDID block %d invalid.\n",
 			 drm_get_connector_name(connector), j);
 	}
-	connector->bad_edid_counter++;
-
 error_free:
 	kfree(block);
 error:
 	drm_cache_edid(connector, NULL);
+	connector->last_edid_error_flags = result;
 	return NULL;
 }
 
@@ -569,17 +589,19 @@ int
 drm_validate_edid_blob(struct drm_connector *connector, u8 **blockp, int len)
 {
 	int n_blocks = len / EDID_LENGTH;
-	int valid_extensions = 0, ret = 0;
-	bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS);
+	int valid_extensions = 0, ret = -EINVAL;
+	int last_error_flags = (drm_debug & DRM_UT_KMS) ? 0 : connector->last_edid_error_flags;
+	unsigned result = EDID_ERR_NO_DATA;
 
-	if (!blockp || !*blockp)
-		ret = -EINVAL;
-	else if (!n_blocks || !drm_edid_block_valid(*blockp, 0, print_bad_edid)) {
-		kfree(*blockp);
-		*blockp = NULL;
-		ret = -EINVAL;
+	if (blockp && *blockp) {
+		if (n_blocks)
+			result = drm_edid_block_check_error(*blockp, 0, last_error_flags);
+		if (result) {
+			kfree(*blockp);
+			*blockp = NULL;
+		}
 	}
-	if (!ret) {
+	if (!result) {
 		int cnt = 0;
 		n_blocks--;
 		if ((*blockp)[EDID_EXTENSION_FLAG_OFFSET] < n_blocks)
@@ -587,8 +609,8 @@ drm_validate_edid_blob(struct drm_connector *connector, u8 **blockp, int len)
 
 		while (n_blocks--) {
 			cnt++;
-			if (drm_edid_block_valid(*blockp + cnt * EDID_LENGTH,
-						 valid_extensions + 1, print_bad_edid)) {
+			if (!drm_edid_block_check_error(*blockp + cnt * EDID_LENGTH,
+							valid_extensions + 1, last_error_flags)) {
 				valid_extensions++;
 				if (cnt != valid_extensions)
 					memcpy(*blockp + valid_extensions * EDID_LENGTH,
@@ -596,8 +618,9 @@ drm_validate_edid_blob(struct drm_connector *connector, u8 **blockp, int len)
 			}
 		}
 		fixup_edid(blockp, valid_extensions);
-	} else
-		connector->bad_edid_counter++;
+		ret = 0;
+	}
+	connector->last_edid_error_flags = result;
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index b884c36..e80ba63 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -961,7 +961,7 @@ radeon_dvi_detect(struct drm_connector *connector, bool force)
 					drm_get_connector_name(connector));
 			/* rs690 seems to have a problem with connectors not existing and always
 			 * return a block of 0's. If we see this just stop polling on this output */
-			if ((rdev->family == CHIP_RS690 || rdev->family == CHIP_RS740) && radeon_connector->base.null_edid_counter) {
+			if ((rdev->family == CHIP_RS690 || rdev->family == CHIP_RS740) && radeon_connector->base.last_edid_error_flags & EDID_ERR_NULL) {
 				ret = connector_status_disconnected;
 				DRM_ERROR("%s: detected RS690 floating bus bug, stopping ddc detect\n", drm_get_connector_name(connector));
 				radeon_connector->ddc_bus = NULL;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 6a1054c..d402b3b 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -608,8 +608,7 @@ struct drm_connector {
 	bool latency_present[2];
 	int video_latency[2];	/* [0]: progressive, [1]: interlaced */
 	int audio_latency[2];
-	int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
-	unsigned bad_edid_counter;
+	unsigned last_edid_error_flags;
 };
 
 /**
@@ -1056,7 +1055,6 @@ extern int drm_add_modes_noedid(struct drm_connector *connector,
 				int hdisplay, int vdisplay);
 
 extern int drm_edid_header_is_valid(const u8 *raw_edid);
-extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid);
 extern bool drm_edid_is_valid(struct edid *edid);
 struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,
 					   int hsize, int vsize, int fresh,
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index c880510..6bcaee5 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -244,6 +244,15 @@ struct edid {
 
 #define EDID_PRODUCT_ID(e) ((e)->prod_code[0] | ((e)->prod_code[1] << 8))
 
+enum edid_error {
+	EDID_ERR_NO_BLOCK0 = 1 << 0,
+	EDID_ERR_NULL      = 1 << 1,
+	EDID_ERR_CSUM      = 1 << 2,
+	EDID_ERR_UNSUPPORTED_VERSION  = 1 << 3,
+	EDID_ERR_NO_DATA   = 1 << 4,
+	EDID_ERR_NO_MEM    = 1 << 5,
+};
+
 struct drm_encoder;
 struct drm_connector;
 struct drm_display_mode;
@@ -257,5 +266,6 @@ struct edid *drm_load_edid_firmware(struct drm_connector *connector);
 #endif
 int drm_validate_edid_blob(struct drm_connector *connector, u8 **blockp, int len);
 void drm_cache_edid(struct drm_connector *connector, struct edid *edid);
+unsigned drm_edid_block_check_error(u8 *raw_edid, int block, unsigned last_error_flags);
 
 #endif /* __DRM_EDID_H__ */
-- 
1.7.7



More information about the dri-devel mailing list