Re: [Intel-gfx] [PATCH] drm/i915: Check DP link status on long hpd too

[Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>]

On 8/20/2015 10:07 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We are no longer checkling the DP link status on long hpd. We used to do
> that from the .hot_plug() handler, but it was removed when MST got
> introduced.
>
> If there's no userspace we now fail to retrain the link if the sink
> power is toggled (or cable yanked and replugged), meaning the user is
> left staring at a blank screen. With the retraining put back that should
> be fixed.
>
> Also remove the leftover comment that referred to the old retraining
> from .hot_plug().
>
> Fixes a regression introduced in:
> commit 0e32b39ceed665bfa4a77a4bc307b6652b991632
> Author: Dave Airlie <airlied@redhat.com>
> Date:   Fri May 2 14:02:48 2014 +1000
>
>      drm/i915: add DP 1.2 MST support (v0.7)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89453
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91407
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89461
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89594
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85641
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d32ce48..b014158 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5024,9 +5024,12 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>   
>   		intel_dp_probe_oui(intel_dp);
>   
> -		if (!intel_dp_probe_mst(intel_dp))
> +		if (!intel_dp_probe_mst(intel_dp)) {
> +			drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> +			intel_dp_check_link_status(intel_dp);
> +			drm_modeset_unlock(&dev->mode_config.connection_mutex);
couple of queries for my understanding.
 > why should we check for link status for long pulse, which is supposed 
to be for plug in or plug out ?
 > goto mst_fail will indicate we are falling back to intel_dp_detect, 
isnt this better suited there ?

Also the following bug indicates failure in mst panel, but the changes 
here are in non-mst path
      https://bugs.freedesktop.org/show_bug.cgi?id=89453, how is this 
patch helping this bug ?
>   			goto mst_fail;
> -
> +		}
>   	} else {
>   		if (intel_dp->is_mst) {
>   			if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
> @@ -5034,10 +5037,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>   		}
>   
>   		if (!intel_dp->is_mst) {
> -			/*
> -			 * we'll check the link status via the normal hot plug path later -
> -			 * but for short hpds we should check it now
> -			 */
>   			drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>   			intel_dp_check_link_status(intel_dp);
>   			drm_modeset_unlock(&dev->mode_config.connection_mutex);

-- 
regards,
Sivakumar