Getting the EDID succeeds most of the time on my system. However, every now and then, especially when the machine is highly loaded, i2c communication and thus drm_get_edid() fails. My guess is that some kind of electrical noise and/or misdesign in the used components is the root cause of this.
The display with the issues is an Apple Cinema connected via DVI. Another LG display is connected through VGA to the other connector of the ATI Radeon HD 5450. (xrandr is used)
This is what kernel log shows on errors:
[ 9509.250829] i2c i2c-3: sendbytes: NAK bailout. [ 9519.655190] i2c i2c-3: readbytes: ack/nak timeout [ 9519.655258] [drm:radeon_dvi_detect] *ERROR* DVI-I-1: probed a monitor but no|invalid EDID [ 9550.716078] [drm:radeon_dvi_detect] *ERROR* DVI-I-1: probed a monitor but no|invalid EDID [ 9560.738835] i2c i2c-3: sendbytes: NAK bailout. [ 9612.778073] [drm:radeon_dvi_detect] *ERROR* DVI-I-1: probed a monitor but no|invalid EDID
The display flickers for a few seconds and then continues to work correctly.
So I applied the following patch to the kernel to workaround the issue:
Index: linux-2.6.37/drivers/gpu/drm/drm_edid.c =================================================================== --- linux-2.6.37.orig/drivers/gpu/drm/drm_edid.c 2011-03-05 13:09:17.001960834 +0100 +++ linux-2.6.37/drivers/gpu/drm/drm_edid.c 2011-03-05 13:11:34.974194354 +0100 @@ -343,9 +343,16 @@ struct edid *drm_get_edid(struct drm_con struct i2c_adapter *adapter) { struct edid *edid = NULL; + unsigned int tries;
- if (drm_probe_ddc(adapter)) - edid = (struct edid *)drm_do_get_edid(connector, adapter); + for (tries = 0; tries < 20; tries++) { + if (drm_probe_ddc(adapter)) + edid = (struct edid *)drm_do_get_edid(connector, adapter); + if (edid) + break; + } + if (tries > 0 && tries < 20) + printk(KERN_INFO "drm_get_edid(): Succeed after %u tries\n", tries);
connector->display_info.raw_edid = (char *)edid;
This seems to help. I get messages like:
[ 6506.029922] i2c i2c-3: sendbytes: NAK bailout. [ 6506.030828] i2c i2c-3: sendbytes: NAK bailout. [ 6506.137459] drm_get_edid(): Succeed after 3 tries [ 8647.073894] i2c i2c-3: sendbytes: NAK bailout. [ 8647.196343] drm_get_edid(): Succeed after 2 tries [ 8749.381956] drm_get_edid(): Succeed after 1 tries [ 8759.523868] i2c i2c-3: readbytes: ack/nak timeout [ 8759.668134] drm_get_edid(): Succeed after 1 tries
I was wondering if a real fix or workaround could be found for mainline inclusion. Would it be acceptable to have a retry loop similar to the above patch? Some suggestions?
Usually EDID retrieval is fine. However, sometimes, especially when the machine is loaded, it fails, but succeeds after a few retries.
Based on a patch by Michael Buesch.
Reported-by: Michael Buesch mb@bu3sch.de Signed-off-by: Chris Wilson chris@chris-wilson.co.uk ---
I was going to suggest tuning the i2c_adapter.retries of the affected components, but that only covers EGAIN and not EREMOTEIO/ETIMEDOUT that afflicts us. With the alteration to only retry the transfer of the affected block, this looks to be a reasonable idea and complements the logic to retry corrupted transfers. -Chris
--- drivers/gpu/drm/drm_edid.c | 44 ++++++++++++++++++++++++++------------------ 1 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index a245d17..ddc3da9 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -230,24 +230,32 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf, int block, int len) { unsigned char start = block * EDID_LENGTH; - struct i2c_msg msgs[] = { - { - .addr = DDC_ADDR, - .flags = 0, - .len = 1, - .buf = &start, - }, { - .addr = DDC_ADDR, - .flags = I2C_M_RD, - .len = len, - .buf = buf, - } - }; + int ret, retries = 5;
- if (i2c_transfer(adapter, msgs, 2) == 2) - return 0; + /* The core i2c driver will automatically retry the transfer if the + * adapter reports EAGAIN. However, we find that bit-banging transfers + * are susceptible to errors under a heavily loaded machine and + * generate spurious NAKs and timeouts. Retrying the transfer + * of the individual block a few times seems to overcome this. + */ + do { + struct i2c_msg msgs[] = { + { + .addr = DDC_ADDR, + .flags = 0, + .len = 1, + .buf = &start, + }, { + .addr = DDC_ADDR, + .flags = I2C_M_RD, + .len = len, + .buf = buf, + } + }; + ret = i2c_transfer(adapter, msgs, 2); + } while (ret != 2 && --retries);
- return -1; + return ret == 2 ? 0 : -1; }
static u8 *
On Tue, Mar 15, 2011 at 7:04 AM, Chris Wilson chris@chris-wilson.co.uk wrote:
Usually EDID retrieval is fine. However, sometimes, especially when the machine is loaded, it fails, but succeeds after a few retries.
Based on a patch by Michael Buesch.
Reported-by: Michael Buesch mb@bu3sch.de Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
Looks good.
Reviewed-by: Alex Deucher alexdeucher@gmail.com
I was going to suggest tuning the i2c_adapter.retries of the affected components, but that only covers EGAIN and not EREMOTEIO/ETIMEDOUT that afflicts us. With the alteration to only retry the transfer of the affected block, this looks to be a reasonable idea and complements the logic to retry corrupted transfers. -Chris
drivers/gpu/drm/drm_edid.c | 44 ++++++++++++++++++++++++++------------------ 1 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index a245d17..ddc3da9 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -230,24 +230,32 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf, int block, int len) { unsigned char start = block * EDID_LENGTH;
- struct i2c_msg msgs[] = {
- {
- .addr = DDC_ADDR,
- .flags = 0,
- .len = 1,
- .buf = &start,
- }, {
- .addr = DDC_ADDR,
- .flags = I2C_M_RD,
- .len = len,
- .buf = buf,
- }
- };
- int ret, retries = 5;
- if (i2c_transfer(adapter, msgs, 2) == 2)
- return 0;
- /* The core i2c driver will automatically retry the transfer if the
- * adapter reports EAGAIN. However, we find that bit-banging transfers
- * are susceptible to errors under a heavily loaded machine and
- * generate spurious NAKs and timeouts. Retrying the transfer
- * of the individual block a few times seems to overcome this.
- */
- do {
- struct i2c_msg msgs[] = {
- {
- .addr = DDC_ADDR,
- .flags = 0,
- .len = 1,
- .buf = &start,
- }, {
- .addr = DDC_ADDR,
- .flags = I2C_M_RD,
- .len = len,
- .buf = buf,
- }
- };
- ret = i2c_transfer(adapter, msgs, 2);
- } while (ret != 2 && --retries);
- return -1;
- return ret == 2 ? 0 : -1;
}
static u8 *
1.7.2.3
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, 2011-03-15 at 11:04 +0000, Chris Wilson wrote:
Usually EDID retrieval is fine. However, sometimes, especially when the machine is loaded, it fails, but succeeds after a few retries.
Based on a patch by Michael Buesch.
Reported-by: Michael Buesch mb@bu3sch.de Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
I'm currently testing this. It boots, but to see if it fixes the issue, I'll have to run some more tests.
Note that this patch is subject to .37-table and probably earlier releases, too.
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Chris Wilson -
Michael Buesch