On Thu, Nov 22, 2012 at 09:44:42AM -0500, Egbert Eich wrote:
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. v3: Don't update cache when returning block from cache.
Signed-off-by: Egbert Eich eich@suse.com
drivers/gpu/drm/drm_edid.c | 117 +++++++++++++++++----------- drivers/gpu/drm/radeon/radeon_connectors.c | 2 +- include/drm/drm_crtc.h | 4 +- include/drm/drm_edid.h | 10 +++ 4 files changed, 82 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index dd0df60..aa9b34d 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;
You could use memchr_inv() here. But the compiler can't optimize it since it's not inline, so I suppose it might make it slower.
+}
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;
return !drm_edid_block_check_error();
} 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))
^^^^
That looks wrong. Also the 's/!drm_edid_block_valid/drm_edid_block_check_error' change seems superfluous since you're not using the more detailed return value from drm_edid_block_check_error().
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;#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)
goto done;
goto done_update_cache;/* 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)) {
Again the change of function seems superfluous.