On Tue, 01 Dec 2015, Ville Syrjälä <[email protected]> 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 <[email protected]>
>> ---
>>  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
>> [email protected]
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to