| Message ID | 20180329155023.26827-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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;