On Tue, 02 Jun 2015, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > On Tue, Jun 02, 2015 at 07:21:15PM +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). >> >> v5: Take index reads into account (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> > > OK, I think I'm done shooting any more holes into this one: > Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Hopefully it works too. I don't have very good intuition into the gmbus > hardware. I think one day I need to set up a bit-banged i2c slave and > play around with it just for fun. Pushed to drm-intel-fixes, with Tested-by from Jim Bride. Thanks for the review. BR, Jani. > >> --- >> drivers/gpu/drm/i915/intel_i2c.c | 20 +++++++++++++++++--- >> 1 file changed, 17 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c >> index 92072f56e418..a64f26c670af 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, inc, try = 0, reg_offset; >> int ret = 0; >> >> intel_aux_display_runtime_get(dev_priv); >> @@ -499,12 +499,14 @@ 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 += inc) { >> + inc = 1; >> 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 */ >> + inc = 2; /* an index read is two msgs */ >> } else if (msgs[i].flags & I2C_M_RD) { >> ret = gmbus_xfer_read(dev_priv, &msgs[i], 0); >> } else { >> @@ -576,6 +578,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 the first >> + * message 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 -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
