On Tue, Jun 02, 2015 at 01:42:52PM +0300, Jani Nikula wrote: > Passive DP->DVI/HDMI dongles on DP++ ports show up to the system as HDMI > devices, as they do not have a sink device in them to respond to any AUX > traffic. When probing these dongles over the DDC, sometimes they will > NAK the first attempt even though the transaction is valid and they > support the DDC protocol. The retry loop inside of > drm_do_probe_ddc_edid() would normally catch this case and try the > transaction again, resulting in success. > > That, however, was thwarted by the fix for [1]: > > commit 9292f37e1f5c79400254dca46f83313488093825 > Author: Eugeni Dodonov <eugeni.dodonov@xxxxxxxxx> > Date: Thu Jan 5 09:34:28 2012 -0200 > > drm: give up on edid retries when i2c bus is not responding > > This added code to exit immediately if the return code from the > i2c_transfer function was -ENXIO in order to reduce the amount of time > spent in waiting for unresponsive or disconnected devices. That was > possible because the underlying i2c bit banging algorithm had retries of > its own (which, of course, were part of the reason for the bug the > commit fixes). > > Since its introduction in > > commit f899fc64cda8569d0529452aafc0da31c042df2e > Author: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Date: Tue Jul 20 15:44:45 2010 -0700 > > drm/i915: use GMBUS to manage i2c links > > we've been flipping back and forth enabling the GMBUS transfers, but > we've settled since then. The GMBUS implementation does not do any > retries, however, bailing out of the drm_do_probe_ddc_edid() retry loop > on first encounter of -ENXIO. This, combined with Eugeni's commit, broke > the retry on -ENXIO. > > Retry GMBUS once on -ENXIO on first message to mitigate the issues with > passive adapters. > > This patch is based on the work, and commit message, by Todd Previte > <tprevite@xxxxxxxxx>. > > [1] https://bugs.freedesktop.org/show_bug.cgi?id=41059 > > v2: Don't retry if using bit banging. > > v3: Move retry within gmbux_xfer, retry only on first message. > > v4: Initialize GMBUS0 on retry (Ville). > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85924 > Cc: Todd Previte <tprevite@xxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_i2c.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c > index 92072f56e418..5f2e88d39baf 100644 > --- a/drivers/gpu/drm/i915/intel_i2c.c > +++ b/drivers/gpu/drm/i915/intel_i2c.c > @@ -486,7 +486,7 @@ gmbus_xfer(struct i2c_adapter *adapter, > struct intel_gmbus, > adapter); > struct drm_i915_private *dev_priv = bus->dev_priv; > - int i, reg_offset; > + int i = 0, try = 0, reg_offset; > int ret = 0; > > intel_aux_display_runtime_get(dev_priv); > @@ -499,9 +499,10 @@ gmbus_xfer(struct i2c_adapter *adapter, > > reg_offset = dev_priv->gpio_mmio_base; > > +retry: > I915_WRITE(GMBUS0 + reg_offset, bus->reg0); > > - for (i = 0; i < num; i++) { > + for (; i < num; i++) { > if (gmbus_is_index_read(msgs, i, num)) { > ret = gmbus_xfer_index_read(dev_priv, &msgs[i]); > i += 1; /* set i to the index of the read xfer */ This increment means we'll never retry if we decice to use gmbus_xfer_index_read() for the first two messages. AFAICS that should always be the case for EDID reads where we don't send the segment address. > @@ -576,6 +577,18 @@ clear_err: > adapter->name, msgs[i].addr, > (msgs[i].flags & I2C_M_RD) ? 'r' : 'w', msgs[i].len); > > + /* > + * Passive adapters sometimes NAK the first probe. Retry once on -ENXIO > + * for GMBUS transfers; the bit banging algorithm has retries > + * internally. See also the retry loop in drm_do_probe_ddc_edid, which > + * bails out on the first -ENXIO. > + */ > + if (ret == -ENXIO && i == 0 && try++ == 0) { > + DRM_DEBUG_KMS("GMBUS [%s] NAK on first message, retry\n", > + adapter->name); > + goto retry; > + } > + > goto out; > > timeout: > -- > 2.1.4 -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html