From: Ville Syrjälä ville.syrjala@linux.intel.com
Having the EDID available is often very beneficial for bug analysis, even when the EDID itself is valid and not the direct cause of the bug. So let's dump the EDID to dmesg even when it's valid. This should also give us a better historical record of EDIDs for later analysis.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 134069f36482..1153b2f74c58 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1517,17 +1517,27 @@ drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned int block, size_t len) return ret == xfers ? 0 : -1; }
-static void connector_bad_edid(struct drm_connector *connector, - u8 *edid, int num_blocks) +static void connector_dump_edid(struct drm_connector *connector, + u8 *edid, int num_blocks, + bool valid) { int i;
- if (connector->bad_edid_counter++ && !(drm_debug & DRM_UT_KMS)) - return; + if (valid) { + if (!(drm_debug & DRM_UT_KMS)) + return; + + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] EDID is valid:\n", + connector->base.id, connector->name); + } else { + if (connector->bad_edid_counter++ && !(drm_debug & DRM_UT_KMS)) + return; + + dev_warn(connector->dev->dev, + "[CONNECTOR:%d:%s] EDID is invalid:\n", + connector->base.id, connector->name); + }
- dev_warn(connector->dev->dev, - "%s: EDID is invalid:\n", - connector->name); for (i = 0; i < num_blocks; i++) { u8 *block = edid + i * EDID_LENGTH; char prefix[20]; @@ -1539,7 +1549,7 @@ static void connector_bad_edid(struct drm_connector *connector, else sprintf(prefix, "\t[%02x] GOOD ", i);
- print_hex_dump(KERN_WARNING, + print_hex_dump(valid ? KERN_DEBUG : KERN_WARNING, prefix, DUMP_PREFIX_NONE, 16, 1, block, EDID_LENGTH, false); } @@ -1580,8 +1590,10 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, if (!override) override = drm_load_edid_firmware(connector);
- if (!IS_ERR_OR_NULL(override)) - return override; + if (!IS_ERR_OR_NULL(override)) { + edid = (u8 *)override; + goto done; + }
if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL) return NULL; @@ -1628,7 +1640,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, if (valid_extensions != edid[0x7e]) { u8 *base;
- connector_bad_edid(connector, edid, edid[0x7e] + 1); + connector_dump_edid(connector, edid, edid[0x7e] + 1, false);
edid[EDID_LENGTH-1] += edid[0x7e] - valid_extensions; edid[0x7e] = valid_extensions; @@ -1652,10 +1664,13 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, edid = new; }
+done: + connector_dump_edid(connector, edid, edid[0x7e] + 1, true); + return (struct edid *)edid;
carp: - connector_bad_edid(connector, edid, 1); + connector_dump_edid(connector, edid, 1, false); out: kfree(edid); return NULL;
Quoting Ville Syrjala (2018-03-29 16:50:23)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Having the EDID available is often very beneficial for bug analysis, even when the EDID itself is valid and not the direct cause of the bug. So let's dump the EDID to dmesg even when it's valid. This should also give us a better historical record of EDIDs for later analysis.
Isn't this a bit frequent for a largely unchanging blob? -Chris
On Thu, Mar 29, 2018 at 05:01:13PM +0100, Chris Wilson wrote:
Quoting Ville Syrjala (2018-03-29 16:50:23)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Having the EDID available is often very beneficial for bug analysis, even when the EDID itself is valid and not the direct cause of the bug. So let's dump the EDID to dmesg even when it's valid. This should also give us a better historical record of EDIDs for later analysis.
Isn't this a bit frequent for a largely unchanging blob?
Perhaps. Though ideally we shouldn't go re-reading it all the time. But I guess that's wisful thinking.
Not sure we have a good place where we could memcmp() the new EDID against the old one and only print if it changed.
Quoting Ville Syrjälä (2018-03-29 17:14:05)
On Thu, Mar 29, 2018 at 05:01:13PM +0100, Chris Wilson wrote:
Quoting Ville Syrjala (2018-03-29 16:50:23)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Having the EDID available is often very beneficial for bug analysis, even when the EDID itself is valid and not the direct cause of the bug. So let's dump the EDID to dmesg even when it's valid. This should also give us a better historical record of EDIDs for later analysis.
Isn't this a bit frequent for a largely unchanging blob?
Perhaps. Though ideally we shouldn't go re-reading it all the time. But I guess that's wisful thinking.
Ok, that was far less frequent that I expected for an igt run, though it does look like Maarten has a few more probes to kill. -Chris
dri-devel@lists.freedesktop.org