<div class="gmail_quote">On Thu, Oct 20, 2011 at 10:33, Jean Delvare <<a href="mailto:khali@linux-fr.org">khali@linux-fr.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"> <div><div></div>Just to clarify: by "connectivity is setup", do you mean code in the </div> driver setting the GPIO pin direction etc., or a display being connected to the graphics card? I admit I am a little surprised. I2C buses should have their lines up by default, thanks to pull-up resistors, and masters and slaves should merely drive the lines low as needed. The absence of slaves should have no impact on the line behavior. If the Intel graphics chips (or the driver) rely on the display to hold the lines high, I'd say this is a seriously broken design. As a side note, I thought that HDMI had the capability of presence detection, so there may be a better way to know if a display is connected or not, and this information could used to dynamically instantiate and delete the I2C buses? That way we could skip probing for EDID when there is no chance of success. </blockquote><div> Yes, I think so too. I admit I haven't got to the root of the problem here. My test was really simple, borrowed from the test_bus() at i2c-algo-bit.c - without HDMI cable plugged in, I was getting the "SDA stuck high" messages; with the cable plugged in, I wasn't getting any of those. But in any case, I haven't investigated it deeper in the hardware direction after figuring out that drm_get_edid would retry its attempts for retreiving the edid for 15 times, getting the -ENXIO error for all of them. </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> Well, you could always do manual line testing of the I2C bus _before_ calling drm_do_probe_ddc_edid()? And skip the call if the test fails (i.e. the bus isn't ready for use.) As said before, I am willing to export bit_test if it helps any. I've attached a patch doing exactly this. Let me know if you want me to commit it. </blockquote><div> Yes, surely, I can do this. I did a similar test in the i915-specific patch, checking if we can talk to i2c adapter pior to call drm_do_probe_ddc_edid, but I thought that perhaps it would be easier for all the cards relying on drm_get_edid to have a faster return path in case of problems. I am fine with any solution, if this problem is happening to appear on i915 cards only, we could do this in our driver. It is that 15 attempts looks a bit overkill. </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> Your proposed patch is better than I first thought. I had forgotten that i2c-algo-bit tries up to 3 times to get a first ack from a slave. So if i2c_transfer returns -ENXIO, this means that i2c-algo-bit has already attempted 3 times to contact the slave, with no reply, so there's probably no point going on. A communication problem with a present slave would have returned a different error code. Without your patch, a missing slave would cause 3 * 5 = 15 retries, which is definitely too much. That being said, even then the whole probe sequence shouldn't exceed 10 ms, which I wouldn't expect a user to notice. The user-reported 4 second delay when running xrandr can't be caused by this. 4 seconds for 15 attempts is 250 ms per attempt, this looks like a timeout due to non-functional bus, not a nack. Not that 250 ms is 1 jiffy for HZ=250, which I guess was what the reporting user was running. So even with your patch, there's still 750 ms to wait, I don't think this is acceptable. You really have to fix that I2C bus or skip probing it. </blockquote><div> Yep, true. I've followed the easiest route so far - find out where the initial problem happens, and attempt at solving it. This change in drm_get_edid solves the delay at its origin, but we certainly could have additional delays propagated somewhere else. I couldn't reproduce the original reporter's huge delay, so I looked at what was within my reach. </div></div> In any case - looking at a faster way to leave the drm_do_probe_ddc_edid, while also allowing a way for having a more detailed probing - would it be an acceptable solution to add a: MODULE_PARM(skip_unresponsive_edid, "Ignore outputs which do not provide edid on first attempt"); and update the patch to use this option? -- Eugeni Dodonov<a href="http://eugeni.dodonov.net/" target="_blank"> </a>