On Tue, 01 Dec 2015, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > On Tue, Dec 01, 2015 at 04:29:26PM +0200, Jani Nikula wrote: >> Choose between i2c bit banging and gmbus in a new higher level function, >> and let the i2c core retry the first time we fall back to bit banging. >> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/intel_i2c.c | 39 +++++++++++++++++++++++++-------------- >> 1 file changed, 25 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c >> index ccb522c176bd..e26e22a72e3b 100644 >> --- a/drivers/gpu/drm/i915/intel_i2c.c >> +++ b/drivers/gpu/drm/i915/intel_i2c.c >> @@ -472,9 +472,7 @@ gmbus_xfer_index_read(struct drm_i915_private *dev_priv, struct i2c_msg *msgs) >> } >> >> static int >> -gmbus_xfer(struct i2c_adapter *adapter, >> - struct i2c_msg *msgs, >> - int num) >> +do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) >> { >> struct intel_gmbus *bus = container_of(adapter, >> struct intel_gmbus, >> @@ -483,14 +481,6 @@ gmbus_xfer(struct i2c_adapter *adapter, >> int i = 0, inc, try = 0; >> int ret = 0; >> >> - intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); >> - mutex_lock(&dev_priv->gmbus_mutex); >> - >> - if (bus->force_bit) { >> - ret = i2c_bit_algo.master_xfer(adapter, msgs, num); >> - goto out; >> - } >> - >> retry: >> I915_WRITE(GMBUS0, bus->reg0); >> >> @@ -585,13 +575,34 @@ timeout: >> bus->adapter.name, bus->reg0 & 0xff); >> I915_WRITE(GMBUS0, 0); >> >> - /* Hardware may not support GMBUS over these pins? Try GPIO bitbanging instead. */ >> + /* >> + * Hardware may not support GMBUS over these pins? Try GPIO bitbanging >> + * instead. Use EAGAIN to have i2c core retry. >> + */ >> bus->force_bit = 1; >> - ret = i2c_bit_algo.master_xfer(adapter, msgs, num); >> + ret = -EAGAIN; > > This does mean we're left to the mercy of the timeout handling in the > i2c core. That is if our gmbus attempt takes too long, we may never > get a chance to retry with bit banging. Not sure if that's a real > concern or not. IIUC the longest it can take to transfer 4 bytes without timing out is just under 50 ms. The i2c core use a default timeout of 1000 ms. So if we first succesfully, but slowly manage to transfer quite a bit (well, at least 80 bytes, almost but not quite timing out), and then time out, i2c core won't try again. I do not think this is a real concern, but if it is, we can increase the adapter->timeout in our gmbus setup code. BR, Jani. > > Otherwise lgtm. > >> >> out: >> - mutex_unlock(&dev_priv->gmbus_mutex); >> + return ret; >> +} >> + >> +static int >> +gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) >> +{ >> + struct intel_gmbus *bus = container_of(adapter, struct intel_gmbus, >> + adapter); >> + struct drm_i915_private *dev_priv = bus->dev_priv; >> + int ret; >> + >> + intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); >> + mutex_lock(&dev_priv->gmbus_mutex); >> + >> + if (bus->force_bit) >> + ret = i2c_bit_algo.master_xfer(adapter, msgs, num); >> + else >> + ret = do_gmbus_xfer(adapter, msgs, num); >> >> + mutex_unlock(&dev_priv->gmbus_mutex); >> intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS); >> >> return ret; >> -- >> 2.1.4 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx