Re: [PATCH 2/2] drm/i915: abstract i2c bit banging fallback in gmbus xfer

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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@intel.com>
> ---
>  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.

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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx