On Mon, Oct 17, 2016 at 09:35:14AM +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 at chris-wilson.co.uk>
> ---
> drivers/gpu/drm/drm_edid.c | 42 +++++++++++++++++++++++++-----------------
> 1 file changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 95de47ba1e77..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
> @@ -1283,7 +1303,6 @@ struct edid *drm_do_get_edid(struct drm_connector
> *connector,
> {
> int i, j = 0, valid_extensions = 0;
> u8 *edid, *new;
> - bool print_bad_edid = !connector->bad_edid_counter || (drm_debug &
> DRM_UT_KMS);
>
> if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
> return NULL;
> @@ -1292,7 +1311,7 @@ struct edid *drm_do_get_edid(struct drm_connector
> *connector,
> for (i = 0; i < 4; i++) {
> if (get_edid_block(data, edid, 0, EDID_LENGTH))
> goto out;
> - if (drm_edid_block_valid(edid, 0, print_bad_edid,
> + if (drm_edid_block_valid(edid, 0, false,
> &connector->edid_corrupt))
> break;
> if (i == 0 && drm_edid_is_zero(edid, EDID_LENGTH)) {
> @@ -1318,20 +1337,14 @@ struct edid *drm_do_get_edid(struct drm_connector
> *connector,
> for (i = 0; i < 4; i++) {
> if (get_edid_block(data, block, j, EDID_LENGTH))
> goto out;
> - if (drm_edid_block_valid(block, 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);
Hmm. So this will only print the first bad block we find. Should we
perhaps just dump out the entire EDID at the end, with the bad blocks
clearly marked as such?
> }
>
> if (valid_extensions != edid[0x7e]) {
> @@ -1346,12 +1359,7 @@ struct edid *drm_do_get_edid(struct drm_connector
> *connector,
> 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(edid);
> return NULL;
> --
> 2.9.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
