Re: [PATCH v3] drm/i915: Perform link quality check unconditionally during long pulse

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Apr 12, 2017 at 10:30:17PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> 
> Apparently some DP sinks are a little nuts and cause HPD to drop
> intermittently during modesets. This happens eg. on an ASUS PB287Q.
> In oder to recover from this we can't really use the previous
> connector status to determine if the link needs retraining, so let's
> just ignore that piece of information and do the retrain
> unconditionally. We do of course still check whether the link is
> supposed to be running or not.
> 
> To actually get read out the EDID and update things properly we
> also need to nuke the goto out added by commit 7d23e3c37bb3
> ("drm/i915: Cleaning up intel_dp_hpd_pulse"). I'm actually not sure
> why that was there. Perhaps to avoid an EDID read if the connector
> status didn't appear to change, but that sort of thing is quite racy
> and would have failed anyway if we failed to keep up with the
> hotplugs (if we missed the HPD down in between two HPD ups). And
> now that we take this codepath unconditionally we definitely need
> to drop the goto as otherwise we would never do the EDID read.
> 
> v2: Drop the goto that made us skip EDID reads entirely. Doh!
> v3: Rebase due to locking changes
>     s/apparely/apparently/ in the comment (Chris)
> 
> Cc: stable@vger.kernel.org
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Reported-by: Palmer Dabbelt <palmer@dabbelt.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99766
> References: https://lists.freedesktop.org/archives/intel-gfx/2017-February/119779.html
> Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

And pushed to dinq. Thanks for the review.

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 16b7bf7af537..09601a22d3cc 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4685,9 +4685,20 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>  		 */
>  		status = connector_status_disconnected;
>  		goto out;
> -	} else if (connector->status == connector_status_connected) {
> +	} else {
> +		/*
> +		 * If display is now connected check links status,
> +		 * there has been known issues of link loss triggerring
> +		 * long pulse.
> +		 *
> +		 * Some sinks (eg. ASUS PB287Q) seem to perform some
> +		 * weird HPD ping pong during modesets. So we can apparently
> +		 * end up with HPD going low during a modeset, and then
> +		 * going back up soon after. And once that happens we must
> +		 * retrain the link to get a picture. That's in case no
> +		 * userspace component reacted to intermittent HPD dip.
> +		 */
>  		intel_dp_check_link_status(intel_dp);
> -		goto out;
>  	}
>  
>  	/*
> -- 
> 2.10.2

-- 
Ville Syrj�l�
Intel OTC