Re: [PATCH 4/4] drm/i915: Retry for live status

Daniel Vetter
2015-08-26T09:40:01Z

On Tue, Aug 25, 2015 at 05:31:33PM +0530, Sonika Jindal wrote:
> Some monitors take time in setting the live status.
> So retry for few times if this is a connect HPD
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>

Why was this bugfix not part of the original series? Now I have to retest
on my ivb to figure out whether maybe this one here is the issue ...

Also how exactly does this work? I thought the hpd bits control whether we
get an interrupt, not the other way round? Why exactly does this help?
Definitely needs a lot more explanation.

Also this seems to break bisect, since before the preceeding patch to
check hpd status we just retried edid reading for a while.

This kind of hacking doesn't really convince me that hpd status is
working, just that our own testing isn't good enough to catch all
real-world issues.
-Daniel

> ---
> drivers/gpu/drm/i915/intel_hdmi.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 59518b4..239d70d 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1415,6 +1415,7 @@ void intel_hdmi_probe(struct intel_encoder *intel_encoder)
> struct intel_connector *intel_connector =
> intel_hdmi->attached_connector;
> bool live_status = false;
> + unsigned int retry = 3;
>
> /*
> * Sometimes DDI ports are enumerated as DP as well as HDMI and
> @@ -1425,6 +1426,20 @@ void intel_hdmi_probe(struct intel_encoder *intel_encoder)
> return;
>
> live_status = intel_hdmi_live_status(hdmi_to_dig_port(intel_hdmi));
> + if (!intel_connector->detect_edid && live_status == false) {
> + /*
> + * Hotplug had occurred and old status was disconnected,
> + * so it might be possible that live status is not set,
> + * so retry for few times
> + */
> + do {
> + mdelay(10);
> + live_status = intel_hdmi_live_status(hdmi_to_dig_port(intel_hdmi));
> + if (live_status)
> + break;
> + } while (retry--);
> + }
> +
> /*
> * We are here, means there is a hotplug or a force
> * detection. Clear the cached EDID and probe the
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation