Re: [PATCH] v2 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"

[Lyude Paul <lyude@redhat.com>]

On Mon, 2018-08-06 at 12:25 +0200, Jan-Marek Glogowski wrote:
> This re-applies the workaround for "some DP sinks, [which] are a
> little nuts" from commit 1a36147bb939 ("drm/i915: Perform link
> quality check unconditionally during long pulse").
> It makes the secondary AOC E2460P monitor connected via DP to an
> acer Veriton N4640G usable again.
> 
> This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST
> DP link retraining into the ->post_hotplug() hook")
> 
> Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 46 ++++++++++++++++++++++++------------
> -----
>  1 file changed, 27 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 8e0e14b..7e6f8a5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4333,18 +4333,6 @@ intel_dp_needs_link_retrain(struct intel_dp
> *intel_dp)
>  	return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
>  }
> 
> -/*
> - * If display is now connected check links status,
> - * there has been known issues of link loss triggering
> - * 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.
> - */
>  int intel_dp_retrain_link(struct intel_encoder *encoder,
>  			  struct drm_modeset_acquire_ctx *ctx)
>  {
> @@ -4361,10 +4349,12 @@ int intel_dp_retrain_link(struct intel_encoder
> *encoder,
>  	if (!connector || connector->base.status !=
> connector_status_connected)
>  		return 0;
> 
> -	ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
> -			       ctx);
> -	if (ret)
> -		return ret;
> +	if (ctx) {
> +		ret = drm_modeset_lock
> +			(&dev_priv->drm.mode_config.connection_mutex, ctx);
> +		if (ret)
> +			return ret;
> +	}
...why are we skipping locking anything if there isn't a
drm_modeset_acquire_ctx passed to us? And additionally, why wouldn't there be
an acquisition context passed to us? We always need to be holding the
connection mutex and the respective CRTC lock whenever we're retraining.

> 
>  	conn_state = connector->base.state;
> 
> @@ -4372,9 +4362,11 @@ int intel_dp_retrain_link(struct intel_encoder
> *encoder,
>  	if (!crtc)
>  		return 0;
> 
> -	ret = drm_modeset_lock(&crtc->base.mutex, ctx);
> -	if (ret)
> -		return ret;
> +	if (ctx) {
> +		ret = drm_modeset_lock(&crtc->base.mutex, ctx);
> +		if (ret)
> +			return ret;
> +	}
> 
>  	crtc_state = to_intel_crtc_state(crtc->base.state);
> 
> @@ -4982,6 +4974,22 @@ intel_dp_long_pulse(struct intel_connector
> *connector)
>  		 */
>  		status = connector_status_disconnected;
>  		goto out;
> +	} else {
> +		/*
> +		 * If display is now connected check links status,
> +		 * there has been known issues of link loss triggering
> +		 * 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.
> +		 */
> +		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
> >base;
> +
> +		intel_dp_retrain_link(encoder, NULL);
>  	}
> 
>  	/*
-- 
Cheers,
	Lyude Paul

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx