On Thu, Oct 13, 2016 at 08:43:55PM +0100, Chris Wilson wrote: > Currently, if drm.debug is enabled, we get a DRM_ERROR message on the > intermediate edid reads. This causes transient failures in CI which > flags up the sporadic EDID read failures, which are recovered by > rereading the EDID automatically. This patch combines the reporting done > by drm_do_get_edid() itself with the bad block printing from > get_edid_block(), into a single warning associated with the connector > once all attempts to retrieve the EDID fail. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=98228 > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> Can I haz a version of this patch without the s/block/edid/? It confuses me this early. If you want it, please split out. -Daniel > --- > drivers/gpu/drm/drm_edid.c | 82 +++++++++++++++++++++++++--------------------- > 1 file changed, 44 insertions(+), 38 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index ec77bd3e1f08..51dd10c65b53 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -1260,6 +1260,26 @@ drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned int block, size_t len) > return ret == xfers ? 0 : -1; > } > > +static void connector_add_bad_edid(struct drm_connector *connector, > + u8 *block, int num) > +{ > + if (connector->bad_edid_counter++ && !(drm_debug & DRM_UT_KMS)) > + return; > + > + if (drm_edid_is_zero(block, EDID_LENGTH)) { > + dev_warn(connector->dev->dev, > + "%s: EDID block %d is all zeroes.\n", > + connector->name, num); > + } else { > + dev_warn(connector->dev->dev, > + "%s: EDID block %d invalid:\n", > + connector->name, num); > + print_hex_dump(KERN_WARNING, > + " \t", DUMP_PREFIX_NONE, 16, 1, > + block, EDID_LENGTH, false); > + } > +} > + > /** > * drm_do_get_edid - get EDID data using a custom EDID block read function > * @connector: connector we're probing > @@ -1282,20 +1302,19 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, > void *data) > { > int i, j = 0, valid_extensions = 0; > - u8 *block, *new; > - bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS); > + u8 *edid, *new; > > - if ((block = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL) > + if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL) > return NULL; > > /* base block fetch */ > for (i = 0; i < 4; i++) { > - if (get_edid_block(data, block, 0, EDID_LENGTH)) > + if (get_edid_block(data, edid, 0, EDID_LENGTH)) > goto out; > - if (drm_edid_block_valid(block, 0, print_bad_edid, > + if (drm_edid_block_valid(edid, 0, false, > &connector->edid_corrupt)) > break; > - if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) { > + if (i == 0 && drm_edid_is_zero(edid, EDID_LENGTH)) { > connector->null_edid_counter++; > goto carp; > } > @@ -1304,58 +1323,45 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, > goto carp; > > /* if there's no extensions, we're done */ > - if (block[0x7e] == 0) > - return (struct edid *)block; > + if (edid[0x7e] == 0) > + return (struct edid *)edid; > > - new = krealloc(block, (block[0x7e] + 1) * EDID_LENGTH, GFP_KERNEL); > + new = krealloc(edid, (edid[0x7e] + 1) * EDID_LENGTH, GFP_KERNEL); > if (!new) > goto out; > - block = new; > + edid = new; > + > + for (j = 1; j <= edid[0x7e]; j++) { > + u8 *block = edid + (valid_extensions + 1) * EDID_LENGTH; > > - for (j = 1; j <= block[0x7e]; j++) { > for (i = 0; i < 4; i++) { > - if (get_edid_block(data, > - block + (valid_extensions + 1) * EDID_LENGTH, > - j, EDID_LENGTH)) > + if (get_edid_block(data, block, j, EDID_LENGTH)) > goto out; > - if (drm_edid_block_valid(block + (valid_extensions + 1) > - * EDID_LENGTH, j, > - print_bad_edid, > - NULL)) { > + if (drm_edid_block_valid(block, j, false, NULL)) { > valid_extensions++; > break; > } > } > > - if (i == 4 && print_bad_edid) { > - dev_warn(connector->dev->dev, > - "%s: Ignoring invalid EDID block %d.\n", > - connector->name, j); > - > - connector->bad_edid_counter++; > - } > + if (i == 4) > + connector_add_bad_edid(connector, block, j); > } > > - if (valid_extensions != block[0x7e]) { > - block[EDID_LENGTH-1] += block[0x7e] - valid_extensions; > - block[0x7e] = valid_extensions; > - new = krealloc(block, (valid_extensions + 1) * EDID_LENGTH, GFP_KERNEL); > + if (valid_extensions != edid[0x7e]) { > + edid[EDID_LENGTH-1] += edid[0x7e] - valid_extensions; > + edid[0x7e] = valid_extensions; > + new = krealloc(edid, (valid_extensions + 1) * EDID_LENGTH, GFP_KERNEL); > if (!new) > goto out; > - block = new; > + edid = new; > } > > - return (struct edid *)block; > + return (struct edid *)edid; > > carp: > - if (print_bad_edid) { > - dev_warn(connector->dev->dev, "%s: EDID block %d invalid.\n", > - connector->name, j); > - } > - connector->bad_edid_counter++; > - > + connector_add_bad_edid(connector, edid, 0); > out: > - kfree(block); > + kfree(edid); > return NULL; > } > EXPORT_SYMBOL_GPL(drm_do_get_edid); > -- > 2.9.3 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
