From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org
Subject: Re: [Intel-gfx] [PATCH v3] drm/i915: Fix DDC probe for passive adapters
Date: Tue, 2 Jun 2015 13:35:14 +0300 [thread overview]
Message-ID: <20150602103514.GU5176@intel.com> (raw)
In-Reply-To: <1433233763-7027-1-git-send-email-jani.nikula@intel.com>
On Tue, Jun 02, 2015 at 11:29:23AM +0300, Jani Nikula wrote:
> Passive DP->DVI/HDMI dongles on DP++ ports 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 [1]:
>
> commit 9292f37e1f5c79400254dca46f83313488093825
> Author: Eugeni Dodonov <eugeni.dodonov@intel.com>
> Date: Thu Jan 5 09:34:28 2012 -0200
>
> drm: give up on edid retries when i2c bus is not responding
>
> 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. That was
> possible because the underlying i2c bit banging algorithm had retries of
> its own (which, of course, were part of the reason for the bug the
> commit fixes).
>
> Since its introduction in
>
> commit f899fc64cda8569d0529452aafc0da31c042df2e
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date: Tue Jul 20 15:44:45 2010 -0700
>
> drm/i915: use GMBUS to manage i2c links
>
> we've been flipping back and forth enabling the GMBUS transfers, but
> we've settled since then. The GMBUS implementation does not do any
> retries, however, bailing out of the drm_do_probe_ddc_edid() retry loop
> on first encounter of -ENXIO. This, combined with Eugeni's commit, broke
> the retry on -ENXIO.
>
> Retry GMBUS once on -ENXIO on first message to mitigate the issues with
> passive adapters.
>
> This patch is based on the work, and commit message, by Todd Previte
> <tprevite@gmail.com>.
>
> [1] https://bugs.freedesktop.org/show_bug.cgi?id=41059
>
> v2: Don't retry if using bit banging.
>
> v3: Move retry within gmbux_xfer, retry only on first message.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85924
> Cc: Todd Previte <tprevite@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_i2c.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index 92072f56e418..5519ad3a9574 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -486,7 +486,7 @@ gmbus_xfer(struct i2c_adapter *adapter,
> struct intel_gmbus,
> adapter);
> struct drm_i915_private *dev_priv = bus->dev_priv;
> - int i, reg_offset;
> + int i, try = 0, reg_offset;
> int ret = 0;
>
> intel_aux_display_runtime_get(dev_priv);
> @@ -502,6 +502,7 @@ gmbus_xfer(struct i2c_adapter *adapter,
> I915_WRITE(GMBUS0 + reg_offset, bus->reg0);
>
> for (i = 0; i < num; i++) {
> +retry:
> if (gmbus_is_index_read(msgs, i, num)) {
> ret = gmbus_xfer_index_read(dev_priv, &msgs[i]);
> i += 1; /* set i to the index of the read xfer */
> @@ -576,6 +577,18 @@ clear_err:
> adapter->name, msgs[i].addr,
> (msgs[i].flags & I2C_M_RD) ? 'r' : 'w', msgs[i].len);
>
> + /*
> + * Passive adapters sometimes NAK the first probe. Retry once on -ENXIO
> + * for GMBUS transfers; the bit banging algorithm has retries
> + * internally. See also the retry loop in drm_do_probe_ddc_edid, which
> + * bails out on the first -ENXIO.
> + */
> + if (ret == -ENXIO && i == 0 && try++ == 0) {
> + DRM_DEBUG_KMS("GMBUS [%s] NAK on first message, retry\n",
> + adapter->name);
> + goto retry;
> + }
> +
This would leave GMBUS0 set to 0 for the retry.
> goto out;
>
> timeout:
> --
> 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
next prev parent reply other threads:[~2015-06-02 10:35 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-28 8:51 [PATCH] drm/i915: Fix DDC probe for passive adapters Jani Nikula
2015-05-28 10:05 ` [PATCH v2] " Jani Nikula
2015-05-28 10:48 ` [Intel-gfx] " Ville Syrjälä
2015-05-28 11:36 ` Jani Nikula
2015-05-28 12:26 ` Ville Syrjälä
2015-05-28 12:28 ` Daniel Vetter
2015-05-28 12:34 ` Jani Nikula
2015-06-02 8:29 ` [PATCH v3] " Jani Nikula
2015-06-02 10:35 ` Ville Syrjälä [this message]
2015-06-02 10:42 ` [PATCH v4] " Jani Nikula
2015-06-02 13:09 ` Ville Syrjälä
2015-06-02 16:21 ` [PATCH v5] " Jani Nikula
2015-06-02 16:40 ` Ville Syrjälä
2015-06-02 16:43 ` Jani Nikula
2015-06-09 7:44 ` Jani Nikula
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150602103514.GU5176@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@intel.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).