[Intel-gfx] [PATCH] drm/edid: Fix DDC probe for passive DP dongles

[Jani Nikula]

On Thu, 21 May 2015, Todd Previte <tprevite at gmail.com> wrote:
> Passive DP->DVI/HDMI dongles show up to the system as HDMI devices, as they
> do not have a sink device in them to respond to any AUX traffic. When
> probing these dongles over the DDC, sometimes they will NAK the first attempt
> even though the transaction is valid and they support the DDC protocol. The
> retry loop inside of drm_do_probe_ddc_edid() would normally catch this case
> and try the transaction again, resulting in success.
>
> That, however, was thwarted by the fix for fdo.org bug #41059. The patch is:
> 	commit 9292f37e1f5c79400254dca46f83313488093825
> 	Author: Eugeni Dodonov <eugeni.dodonov at intel.com>
> 	Date:   Thu Jan 5 09:34:28 2012 -0200
>
> 	drm: give up on edid retries when i2c bus is not responding

Some extra background:

That commit refers to the i2c bit banging code, while i915 now prefers
gmbus, and only falls back to big banging on certain failures. (See
gmbux_xfer() in i915/intel_i2c.c). This means that in most cases i915 is
no longer susceptible to the 5*3 timeout loops, but it also means we
don't have the i2c bit banging retry at all on -ENXIO, like Todd notes.

The questions are, is one retry after -ENXIO in drm_do_probe_ddc_edid
enough now? Should we revert the original commit instead since the
underlying algorithm has changed? Or should we return something other
than -ENXIO from our gmbus code to not hit this exit with no retries
path?

> This added code to exit immediately if the return code from the
> i2c_transfer function was -ENXIO in order to reduce the amount of time spent
> in waiting for unresponsive or disconnected devices. For the DP dongles,
> this means that the second retry never happens which results in a failed
> EDID probe and a black screen.
>
> To work around this problem without undoing the fix for bug #41059, the
> number of retries is checked along with the return code. This allows for a
> device to NAK once and still continue operations. A second NAK will result
> in breaking the loop as it would have before and stopping the DDC probe.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85924

Maybe throw this at other dongle bugs you can find too?

We're going to need Tested-bys though.

BR,
Jani.


> Signed-off-by: Todd Previte <tprevite at gmail.com>
> Cc: intel-gfx at lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_edid.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 7087da3..e8047bd 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1238,7 +1238,10 @@ drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned int block, size_t len)
>  		 */
>  		ret = i2c_transfer(adapter, &msgs[3 - xfers], xfers);
>  
> -		if (ret == -ENXIO) {
> +		/* Passive DP->DVI/HDMI dongles sometimes NAK the first probe
> +		 * Try to probe again but if it NAKs, stop trying
> +		 */
> +		if (ret == -ENXIO && retries < 5) {
>  			DRM_DEBUG_KMS("drm: skipping non-existent adapter %s\n",
>  					adapter->name);
>  			break;
> -- 
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center