[Intel-gfx] [PATCH] drm/i915: Avoid HPD poll detect triggering a new detect cycle

[Ville Syrjälä]

On Mon, Oct 28, 2019 at 01:00:31PM +0200, Imre Deak wrote:
> For the HPD interrupt functionality the HW depends on power wells in the
> display core domain to be on. Accordingly when enabling these power
> wells the HPD polling logic will force an HPD detection cycle to account
> for hotplug events that may have happened when such a power well was
> off.
> 
> Thus a detect cycle started by polling could start a new detect cycle if
> a power well in the display core domain gets enabled during detect and
> stays enabled after detect completes. That in turn can lead to a
> detection cycle runaway.
> 
> To prevent re-triggering a poll-detect cycle make sure we drop all power
> references we acquired during detect synchronously by the end of detect.
> This will let the poll-detect logic continue with polling (matching the
> off state of the corresponding power wells) instead of scheduling a new
> detection cycle.
> 
> Fixes: 6cfe7ec02e85 ("drm/i915: Remove the unneeded AUX power ref from intel_dp_detect()")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112125
> Reported-by: Val Kulkov <val.kulkov at gmail.com>
> Reported-and-tested-by: wangqr < wqr.prg at gmail.com>
> Cc: Val Kulkov <val.kulkov at gmail.com>
> Cc: wangqr < wqr.prg at gmail.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_crt.c  |  7 +++++++
>  drivers/gpu/drm/i915/display/intel_dp.c   | 24 ++++++++++++++---------
>  drivers/gpu/drm/i915/display/intel_hdmi.c |  6 ++++++
>  3 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> index ff6126ea793c..834bf1d43bb8 100644
> --- a/drivers/gpu/drm/i915/display/intel_crt.c
> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> @@ -864,6 +864,13 @@ intel_crt_detect(struct drm_connector *connector,
>  
>  out:
>  	intel_display_power_put(dev_priv, intel_encoder->power_domain, wakeref);
> +
> +	/*
> +	 * Make sure the refs for power wells enabled during detect are
> +	 * dropped to avoid a new detect cycle triggered by HPD polling.
> +	 */
> +	intel_display_power_flush_work(dev_priv);
> +
>  	return status;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 86989ec25bc6..f4e0ec05d7c9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5600,6 +5600,7 @@ intel_dp_detect(struct drm_connector *connector,
>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>  	struct intel_encoder *encoder = &dig_port->base;
>  	enum drm_connector_status status;
> +	int err = 0;
>  
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>  		      connector->base.id, connector->name);
> @@ -5626,7 +5627,7 @@ intel_dp_detect(struct drm_connector *connector,
>  							intel_dp->is_mst);
>  		}
>  
> -		goto out;
> +		goto out_update_edid;
>  	}
>  
>  	if (intel_dp->reset_link_params) {
> @@ -5654,7 +5655,7 @@ intel_dp_detect(struct drm_connector *connector,
>  		 * with EDID on it
>  		 */
>  		status = connector_status_disconnected;
> -		goto out;
> +		goto out_update_edid;
>  	}
>  
>  	/*
> @@ -5662,11 +5663,9 @@ intel_dp_detect(struct drm_connector *connector,
>  	 * with an IRQ_HPD, so force a link status check.
>  	 */
>  	if (!intel_dp_is_edp(intel_dp)) {
> -		int ret;
> -
> -		ret = intel_dp_retrain_link(encoder, ctx);
> -		if (ret)
> -			return ret;
> +		err = intel_dp_retrain_link(encoder, ctx);
> +		if (err)

This should probably read 
if (err == -EDEADLK)

Also I don't think we need to change this to a goto since it just
means we're going to retry the whole thing again, so the straight
return should be fine.

> +			goto out_sync_power;
>  	}
>  
>  	/*
> @@ -5684,11 +5683,18 @@ intel_dp_detect(struct drm_connector *connector,
>  
>  	intel_dp_check_service_irq(intel_dp);
>  
> -out:
> +out_update_edid:
>  	if (status != connector_status_connected && !intel_dp->is_mst)
>  		intel_dp_unset_edid(intel_dp);
>  
> -	return status;
> +out_sync_power:
> +	/*
> +	 * Make sure the refs for power wells enabled during detect are
> +	 * dropped to avoid a new detect cycle triggered by HPD polling.
> +	 */
> +	intel_display_power_flush_work(dev_priv);
> +
> +	return err ? err : status;
>  }
>  
>  static void
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index b54ccbb5aad5..ff71a4da3d00 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2626,6 +2626,12 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>  	if (status != connector_status_connected)
>  		cec_notifier_phys_addr_invalidate(intel_hdmi->cec_notifier);
>  
> +	/*
> +	 * Make sure the refs for power wells enabled during detect are
> +	 * dropped to avoid a new detect cycle triggered by HPD polling.
> +	 */
> +	intel_display_power_flush_work(dev_priv);
> +
>  	return status;
>  }
>  
> -- 
> 2.17.1

-- 
Ville Syrjälä
Intel