On Tue, 01 Dec 2015, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Tue, Dec 01, 2015 at 05:38:30PM +0200, Jani Nikula wrote:
>> 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.
>
> Hmm. I was thinking it's shorter than that, but I guess I was mixing it
> up with the i2c-bit-algo timeout which is 2.2ms (per some DDC spec
> IIRC).
>
> 1 second seems plenty enough to me.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Both pushed to drm-intel-next-queued, thanks for the review.
BR,
Jani.
>
>>
>> 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
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
