Hi Dave, Eugeni, Alex, On Tue, 18 Oct 2011 11:02:00 +0100, Dave Airlie wrote: > > This allows to avoid talking to a non-existent bus repeatedly until we > > finally timeout. The non-existent bus is signalled by -ENXIO error, > > provided by i2c_algo_bit:bit_doAddress call. > > > > As the advantage of such change, all the other routines which use > > drm_get_edid would benefit for this timeout. > > > > This change should fix > > https://bugs.freedesktop.org/show_bug.cgi?id=41059 and improve overall > > edid detection timing by 10-30% in most cases, and by a much larger margin > > in case of phantom outputs. > > Jean, Alex, > > I'm thinking of thowing this into -next, any objections? This seems to be the wrong place for the test. The code below is really testing for non-responsive (possibly not present) EDID, NOT "non-existent adapter". Non-existent adapter should be checked in the firmware if possible, or failing that, by testing the bus lines at bus creation time, as i2c_algo_bit.bit_test=1 is doing. While this setting has been known to cause trouble recently (not per se but because it was triggering a bug somewhere else in the radeon driver), it might still have value, and could be changed to a per-adapter setting by exporting the test function (I have a patch ready doing exactly this) and letting video drivers test their I2C buses and discard the non-working ones if they want. I am worried that the patch below will cause regressions on other machines. There's a comment right before the loop which reads: /* 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. */ So the retries are there for a reason, and -ENXIO is exactly what you get on spurious NAKs. One thing which may make sense would be to make the retry count a module parameter, instead of a hard-coded value, so that users can lower the value if they care more about boot time than reliability. But again, ideally problematic buses would not have been created in the first place. > > > > Signed-off-by: Eugeni Dodonov <eugeni.dodonov@xxxxxxxxx> > > --- > > drivers/gpu/drm/drm_edid.c | 5 +++++ > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > index 7425e5c..1bca6d7 100644 > > --- a/drivers/gpu/drm/drm_edid.c > > +++ b/drivers/gpu/drm/drm_edid.c > > @@ -265,6 +265,11 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf, > > } > > }; > > ret = i2c_transfer(adapter, msgs, 2); > > + if (ret == -ENXIO) { > > + DRM_DEBUG_KMS("drm: skipping non-existent adapter %s\n", > > + adapter->name); > > + break; > > + } > > } while (ret != 2 && --retries); > > > > return ret == 2 ? 0 : -1; -- Jean Delvare _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
