linux-kernel - [PATCH] drm: Reduce the number of retries whilst reading EDIDs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 23 Feb 2012 19:52:51 +0000
From:	Chris Wilson <chris@...is-wilson.co.uk>
To:	linux-kernel@...r.kernel.org
Cc:	dri-devel@...ts.freedesktop.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Chris Wilson <chris@...is-wilson.co.uk>,
	Eugeni Dodonov <eugeni.dodonov@...el.com>,
	Dave Airlie <airlied@...hat.com>,
	Alex Deucher <alexdeucher@...il.com>
Subject: [PATCH] drm: Reduce the number of retries whilst reading EDIDs

i2c retries if sees an EGAIN, drm_do_probe_ddc_edid retries until it
gets a result and *then* drm_do_get_edid retries until it gets a result
it is happy with. All in all, that is a lot of processor intensive
looping in cases where we do not expect and cannot get valid data - for
example on Intel with disconnected hardware we will busy-spin until we
hit the i2c timeout. This is then repeated for every connector when
querying the current status of outputs.

So to improve the situation, we can trim the number of retries for
reading the base block and to check for a reschedule before proceeding
so that we do not hog the machine whilst probing outputs. (Though since
we will be likely blocking the graphics device, the user is still going
to notice the latency.)

Reported-by: Linus Torvalds <torvalds@...ux-foundation.org>
Reported-by: Stephan Bärwolf <stephan.baerwolf@...ilmenau.de>
Signed-off-by: Chris Wilson <chris@...is-wilson.co.uk>
Cc: Eugeni Dodonov <eugeni.dodonov@...el.com>
Cc: Dave Airlie <airlied@...hat.com>
Cc: Alex Deucher <alexdeucher@...il.com>
---
 drivers/gpu/drm/drm_edid.c |   30 +++++++++++++++++++++++-------
 1 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index ece03fc..4409cd4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -35,6 +35,9 @@
 #include "drm_edid.h"
 #include "drm_edid_modes.h"
 
+#define EDID_PROBE_RETRIES 1
+#define EDID_READ_RETRIES 5
+
 #define version_greater(edid, maj, min) \
 	(((edid)->version > (maj)) || \
 	 ((edid)->version == (maj) && (edid)->revision > (min)))
@@ -239,11 +242,12 @@ EXPORT_SYMBOL(drm_edid_is_valid);
  * Try to fetch EDID information by calling i2c driver function.
  */
 static int
-drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf,
-		      int block, int len)
+drm_get_edid_block(struct i2c_adapter *adapter,
+		   unsigned char *buf, int block, int len,
+		   int retries)
 {
 	unsigned char start = block * EDID_LENGTH;
-	int ret, retries = 5;
+	int ret;
 
 	/* The core i2c driver will automatically retry the transfer if the
 	 * adapter reports EAGAIN. However, we find that bit-banging transfers
@@ -293,7 +297,19 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
 
 	/* base block fetch */
 	for (i = 0; i < 4; i++) {
-		if (drm_do_probe_ddc_edid(adapter, block, 0, EDID_LENGTH))
+		/* EDID transfer may be quite processor intensive and last
+		 * a long time. For example, when waiting for a timeout on
+		 * a non-existent connector whilst using bit-banging. As a
+		 * result we can end up hogging the machine, so give someone
+		 * else the chance to run first. But when we have started a
+		 * transfer don't interrupt until finished.
+		 */
+		if (need_resched())
+			schedule();
+
+		if (drm_get_edid_block(adapter,
+				       block, 0, EDID_LENGTH,
+				       EDID_PROBE_RETRIES))
 			goto out;
 		if (drm_edid_block_valid(block))
 			break;
@@ -316,9 +332,9 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
 
 	for (j = 1; j <= block[0x7e]; j++) {
 		for (i = 0; i < 4; i++) {
-			if (drm_do_probe_ddc_edid(adapter,
+			if (drm_get_edid_block(adapter,
 				  block + (valid_extensions + 1) * EDID_LENGTH,
-				  j, EDID_LENGTH))
+				  j, EDID_LENGTH, EDID_READ_RETRIES))
 				goto out;
 			if (drm_edid_block_valid(block + (valid_extensions + 1) * EDID_LENGTH)) {
 				valid_extensions++;
@@ -362,7 +378,7 @@ drm_probe_ddc(struct i2c_adapter *adapter)
 {
 	unsigned char out;
 
-	return (drm_do_probe_ddc_edid(adapter, &out, 0, 1) == 0);
+	return drm_get_edid_block(adapter, &out, 0, 1, EDID_PROBE_RETRIES) == 0;
 }
 
 /**
-- 
1.7.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/