From patchwork Thu Nov 22 10:23:04 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Egbert Eich X-Patchwork-Id: 1783311 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork2.kernel.org (Postfix) with ESMTP id A5B38DF24C for ; Thu, 22 Nov 2012 10:58:16 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 90F17E649E for ; Thu, 22 Nov 2012 02:58:16 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from moutng.kundenserver.de (moutng.kundenserver.de [212.227.17.9]) by gabe.freedesktop.org (Postfix) with ESMTP id B2D06E5E07 for ; Thu, 22 Nov 2012 02:51:07 -0800 (PST) Received: from debian (p5DCF0728.dip0.t-ipconnect.de [93.207.7.40]) by mrelayeu.kundenserver.de (node=mreu1) with ESMTP (Nemesis) id 0ME24x-1TTZkj0hyB-00HFJ9; Thu, 22 Nov 2012 11:51:06 +0100 Received: from sles11.fritz.box (sles11.fritz.box [192.168.178.22]) by debian (Postfix) with ESMTP id 601993F364; Thu, 22 Nov 2012 11:51:04 +0100 (CET) From: Egbert Eich To: dri-devel@lists.freedesktop.org Subject: [PATCH v2 14/18] DRM/KMS/EDID: Consolidate EDID Error Handling (v2) Date: Thu, 22 Nov 2012 05:23:04 -0500 Message-Id: <1353579788-30637-15-git-send-email-eich@suse.com> X-Mailer: git-send-email 1.7.7 In-Reply-To: <1353579788-30637-1-git-send-email-eich@suse.com> References: <1353356598-10634-1-git-send-email-eich@suse.de> <1353579788-30637-1-git-send-email-eich@suse.com> X-Provags-ID: V02:K0:ENfGWfcI5GSwN+PzlB6t/0H97LeFVO0Ef7KY/O5BEKg omfmLUZzB5xTHm5Z//ofXnV2SakVpEM8uc6728jw7/v2R8TRZ7 yumhbfoMSJ0xd/KAuoxQypMoR8/WBlIax3cGOQRw7T6jiA5xEj cbdjcwuNN8fO933uvCIyrF6dE3cQX/aHJB3GIqRQKHiR5B832n NjA7U8P8v1TXQ64lydExwn+yTVz7Nn86avZjQQLX+tNR6mEffK vweGB4tX4G/+AbgowK38G+DbTfFvtbvKU0brwxKiBUVeajsu7Q LeemiRk/yQGJ79t02chtKjrCDac12gRBM2BjElnDUaK90n2kUG 4yWyvkTwybFbWhfswM1Q= Cc: Egbert Eich , tiwai@suse.com X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org 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. Signed-off-by: Egbert Eich --- drivers/gpu/drm/drm_edid.c | 115 +++++++++++++++++----------- drivers/gpu/drm/radeon/radeon_connectors.c | 2 +- include/drm/drm_crtc.h | 4 +- include/drm/drm_edid.h | 10 +++ 4 files changed, 81 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 28b877c..7d8363e 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; +} + 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; } 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)) 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; return block; } #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) - return block; + goto done; /* 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)) { valid_extensions++; /* Test if base block announced extension blocks although * display is not EDDC capable. @@ -538,21 +557,22 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter) done_fix_extension_count: fixup_edid(&block, valid_extensions); +done: drm_cache_edid(connector, (valid_extensions > 0) ? (struct edid *)block : NULL); + connector->last_edid_error_flags = 0; return block; error_carp: - if (print_bad_edid) { + if (last_error_flags != result) { dev_warn(connector->dev->dev, "%s: EDID block %d invalid.\n", drm_get_connector_name(connector), j); } - connector->bad_edid_counter++; - error_free: kfree(block); error: drm_cache_edid(connector, NULL); + connector->last_edid_error_flags = result; return NULL; } @@ -569,17 +589,19 @@ int drm_validate_edid_blob(struct drm_connector *connector, u8 **blockp, int len) { int n_blocks = len / EDID_LENGTH; - int valid_extensions = 0, ret = 0; - bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS); + int valid_extensions = 0, ret = -EINVAL; + int last_error_flags = (drm_debug & DRM_UT_KMS) ? 0 : connector->last_edid_error_flags; + unsigned result = EDID_ERR_NO_DATA; - if (!blockp || !*blockp) - ret = -EINVAL; - else if (!n_blocks || !drm_edid_block_valid(*blockp, 0, print_bad_edid)) { - kfree(*blockp); - *blockp = NULL; - ret = -EINVAL; + if (blockp && *blockp) { + if (n_blocks) + result = drm_edid_block_check_error(*blockp, 0, last_error_flags); + if (result) { + kfree(*blockp); + *blockp = NULL; + } } - if (!ret) { + if (!result) { int cnt = 0; n_blocks--; if ((*blockp)[EDID_EXTENSION_FLAG_OFFSET] < n_blocks) @@ -587,8 +609,8 @@ drm_validate_edid_blob(struct drm_connector *connector, u8 **blockp, int len) while (n_blocks--) { cnt++; - if (drm_edid_block_valid(*blockp + cnt * EDID_LENGTH, - valid_extensions + 1, print_bad_edid)) { + if (!drm_edid_block_check_error(*blockp + cnt * EDID_LENGTH, + valid_extensions + 1, last_error_flags)) { valid_extensions++; if (cnt != valid_extensions) memcpy(*blockp + valid_extensions * EDID_LENGTH, @@ -596,8 +618,9 @@ drm_validate_edid_blob(struct drm_connector *connector, u8 **blockp, int len) } } fixup_edid(blockp, valid_extensions); - } else - connector->bad_edid_counter++; + ret = 0; + } + connector->last_edid_error_flags = result; return ret; } diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index b884c36..e80ba63 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -961,7 +961,7 @@ radeon_dvi_detect(struct drm_connector *connector, bool force) drm_get_connector_name(connector)); /* rs690 seems to have a problem with connectors not existing and always * return a block of 0's. If we see this just stop polling on this output */ - if ((rdev->family == CHIP_RS690 || rdev->family == CHIP_RS740) && radeon_connector->base.null_edid_counter) { + if ((rdev->family == CHIP_RS690 || rdev->family == CHIP_RS740) && radeon_connector->base.last_edid_error_flags & EDID_ERR_NULL) { ret = connector_status_disconnected; DRM_ERROR("%s: detected RS690 floating bus bug, stopping ddc detect\n", drm_get_connector_name(connector)); radeon_connector->ddc_bus = NULL; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 6a1054c..d402b3b 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -608,8 +608,7 @@ struct drm_connector { bool latency_present[2]; int video_latency[2]; /* [0]: progressive, [1]: interlaced */ int audio_latency[2]; - int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */ - unsigned bad_edid_counter; + unsigned last_edid_error_flags; }; /** @@ -1056,7 +1055,6 @@ extern int drm_add_modes_noedid(struct drm_connector *connector, int hdisplay, int vdisplay); extern int drm_edid_header_is_valid(const u8 *raw_edid); -extern bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid); extern bool drm_edid_is_valid(struct edid *edid); struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev, int hsize, int vsize, int fresh, diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index c880510..6bcaee5 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -244,6 +244,15 @@ struct edid { #define EDID_PRODUCT_ID(e) ((e)->prod_code[0] | ((e)->prod_code[1] << 8)) +enum edid_error { + EDID_ERR_NO_BLOCK0 = 1 << 0, + EDID_ERR_NULL = 1 << 1, + EDID_ERR_CSUM = 1 << 2, + EDID_ERR_UNSUPPORTED_VERSION = 1 << 3, + EDID_ERR_NO_DATA = 1 << 4, + EDID_ERR_NO_MEM = 1 << 5, +}; + struct drm_encoder; struct drm_connector; struct drm_display_mode; @@ -257,5 +266,6 @@ struct edid *drm_load_edid_firmware(struct drm_connector *connector); #endif int drm_validate_edid_blob(struct drm_connector *connector, u8 **blockp, int len); void drm_cache_edid(struct drm_connector *connector, struct edid *edid); +unsigned drm_edid_block_check_error(u8 *raw_edid, int block, unsigned last_error_flags); #endif /* __DRM_EDID_H__ */