Re: [Intel-gfx] [PATCH 2/2] Check if the bus is valid prior to discovering edid.

[Ben Widawsky <ben@bwidawsk.net>]

On Fri, Oct 07, 2011 at 07:00:42PM -0300, Eugeni Dodonov wrote:
> This adds a new function intel_drm_get_valid_edid, which is used instead
> of drm_get_edid within the i915 driver.
> 
> It does a similar check to the one in previous patch, but it is limited to
> i915 driver.
> 
> The check is similar to the first part of EDID discovery performed by the
> drm_do_probe_ddc_edid. In case the i2c_transfer fails with the -ENXIO
> result, we know that the i2c_algo_bit was unable to locate the hardware,
> so we give up on further edid discovery.
> 
> They should also fix https://bugs.freedesktop.org/show_bug.cgi?id=41059
> 
> v2: change printk level to KERN_DEBUG to avoid filling up dmesg
> 
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
> ---
> 
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index d98cee6..77115b9 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -470,3 +470,36 @@ void intel_teardown_gmbus(struct drm_device *dev)
>  	kfree(dev_priv->gmbus);
>  	dev_priv->gmbus = NULL;
>  }
> +
> +/**
> + * intel_drm_get_valid_edid - gets edid from existent adapters only
> + * @connector: DRM connector device to use
> + * @adapter: i2c adapter
> + *
> + * Verifies if the i2c adapter is responding to our queries before
> + * attempting to do proper communication with it. If it does,
> + * retreive the EDID with help of drm_get_edid
> + */
> +struct edid *
> +intel_drm_get_valid_edid(struct drm_connector *connector,
> +		struct i2c_adapter *adapter)
> +{
> +	unsigned char out;
> +	int ret;
> +	struct i2c_msg msgs[] = {
> +		{
> +			.addr	= 0x50,
> +			.flags	= 0,
> +			.len	= 1,
> +			.buf	= &out,
> +		}
> +	};
> +	/* Let's try one edid transfer */
> +	ret = i2c_transfer(adapter, msgs, 1);
> +	if (ret == -ENXIO) {
> +		printk(KERN_DEBUG "i915: adapter %s not responding: %d\n",
> +				adapter->name, ret);
> +		return NULL;
> +	}
> +	return drm_get_edid(connector, adapter);
> +}

I think you may as well optimistically try to get the edid_data here.
The problem is, in the success case you add ~10 i2c clocks because you
next call drm_get_edid. If you optimistacally try to do both you should
receive the -ENXIO after the slaves ignore the address byte, and not
lose time. (So win on exists case, lose a *slight* amount of CPU time in
fail case).

Now if you do that I think most of the code can be taken from
intel_ddc_probe. Just modify that function to take the args you need
(dev_priv, and adatper I am thinking). I only see one caller of that
function, which can easily be modified.

Ben