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

Jindal, Sonika
2015-08-26T10:05:13Z

HPD bits control the interrupt but the live status (with some monitors) takes time to get set.
We had experienced this with VLV and CHV with few monitors.
So Android code always has this retry for live status.

Yes, this was not added in the previous series because we planned to add the next set of optimization a little while later.
But this seems to be an important one.

It will be great if you can try it with your ivb. But for that you would need to first change the gen check and add a call to check live status for ivb.

Regards,
Sonika
-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Wednesday, August 26, 2015 3:10 PM
To: Jindal, Sonika
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 4/4] drm/i915: Retry for live status

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